[clutter] stage-cogl: Fix damage tracking with varying buffer ages



commit 239280f855db26fec93646af51bbc772478ef698
Author: Chris Wilson <chris chris-wilson co uk>
Date:   Wed Jan 28 22:08:03 2015 +0000

    stage-cogl: Fix damage tracking with varying buffer ages
    
    With server-side buffer allocation, buffers may be returned out of order
    (e.g. they may be held onto by external references or hardware). As such
    we may see older buffers the frame after we discard the history from
    seeing a very young buffer. To overcome this we want to keep the history
    in a ring so we can keep track of older entries without keeping an
    unbounded list. After converting to a ring, the maximum buffer age
    observed during testing was 5 (expected value of 4), but before we could
    see ages as high as 9 due to the huge latency spikes caused by doing full
    buffer redraws (compounded by external listeners doing readback on the
    damaged areas, for example vnc, drm/udl, prime). For this reason, a
    maximum age of 16 was chosen to be suitably large enough to prevent these
    worst cases from taxing the system.
    
    v2: Fix off-by-one in combining the damage histroy into the clipping
    rectangle, and apply copious whitespace fixes.
    
    Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=745512
    References: https://bugzilla.gnome.org/show_bug.cgi?id=724788
    References: https://bugzilla.gnome.org/show_bug.cgi?id=669122

 clutter/cogl/clutter-stage-cogl.c |  108 +++++++++++++++++--------------------
 clutter/cogl/clutter-stage-cogl.h |    5 ++-
 2 files changed, 54 insertions(+), 59 deletions(-)
---
diff --git a/clutter/cogl/clutter-stage-cogl.c b/clutter/cogl/clutter-stage-cogl.c
index 378797d..5a8c3bd 100644
--- a/clutter/cogl/clutter-stage-cogl.c
+++ b/clutter/cogl/clutter-stage-cogl.c
@@ -392,6 +392,15 @@ clutter_stage_cogl_get_redraw_clip_bounds (ClutterStageWindow    *stage_window,
   return FALSE;
 }
 
+static inline gboolean
+valid_buffer_age (ClutterStageCogl *stage_cogl, int age)
+{
+  if (age <= 0 || stage_cogl->dirty_backbuffer)
+    return FALSE;
+
+  return age < MIN (stage_cogl->damage_index, DAMAGE_HISTORY_MAX);
+}
+
 /* XXX: This is basically identical to clutter_stage_glx_redraw */
 static void
 clutter_stage_cogl_redraw (ClutterStageWindow *stage_window)
@@ -452,65 +461,47 @@ clutter_stage_cogl_redraw (ClutterStageWindow *stage_window)
 
   window_scale = _clutter_stage_window_get_scale_factor (stage_window);
 
-  if (use_clipped_redraw)
+  if (has_buffer_age)
     {
-      if (has_buffer_age)
-      {
-        int age = cogl_onscreen_get_buffer_age (stage_cogl->onscreen);
-        cairo_rectangle_int_t *current_damage;
-
-        current_damage = g_new0 (cairo_rectangle_int_t, 1);
-        current_damage->x = clip_region->x;
-        current_damage->y = clip_region->y;
-        current_damage->width = clip_region->width;
-        current_damage->height = clip_region->height;
-
-        stage_cogl->damage_history = g_slist_prepend (stage_cogl->damage_history, current_damage);
-
-        if (age != 0 && !stage_cogl->dirty_backbuffer && g_slist_length (stage_cogl->damage_history) > age)
-          {
-            int i = 0;
-            GSList *tmp = NULL;
-            /* We skip the first entry because it is the clip_region itself */
-            for (tmp = stage_cogl->damage_history->next; tmp; tmp = tmp->next)
-              {
-                _clutter_util_rectangle_union (clip_region, tmp->data, clip_region);
-                i++;
-                if (i == age)
-                  {
-                    g_slist_free_full (tmp->next, g_free);
-                    tmp->next = NULL;
-                  }
-              }
-
-            force_swap = TRUE;
-
-            CLUTTER_NOTE (CLIPPING, "Reusing back buffer - repairing region: x=%d, y=%d, width=%d, 
height=%d\n",
-                          clip_region->x,
-                          clip_region->y,
-                          clip_region->width,
-                          clip_region->height);
-
-          }
-        else if (age == 0 || stage_cogl->dirty_backbuffer)
-          {
-            CLUTTER_NOTE (CLIPPING, "Invalid back buffer: Resetting damage history list.\n");
-            g_slist_free_full (stage_cogl->damage_history, g_free);
-            stage_cogl->damage_history = NULL;
-          }
-
-      }
-    }
-  else if (has_buffer_age)
-    {
-      CLUTTER_NOTE (CLIPPING, "Unclipped redraw: Resetting damage history list.\n");
-      g_slist_free_full (stage_cogl->damage_history, g_free);
-      stage_cogl->damage_history = NULL;
+      cairo_rectangle_int_t *current_damage =
+       &stage_cogl->damage_history[DAMAGE_HISTORY (stage_cogl->damage_index++)];
+
+      if (use_clipped_redraw)
+       {
+         int age = cogl_onscreen_get_buffer_age (stage_cogl->onscreen), i;
+
+         *current_damage = *clip_region;
+
+         if (valid_buffer_age (stage_cogl, age))
+           {
+             for (i = 1; i <= age; i++)
+               _clutter_util_rectangle_union (clip_region,
+                                              &stage_cogl->damage_history[DAMAGE_HISTORY 
(stage_cogl->damage_index - i - 1)],
+                                              clip_region);
+
+             CLUTTER_NOTE (CLIPPING, "Reusing back buffer(age=%d) - repairing region: x=%d, y=%d, width=%d, 
height=%d\n",
+                           age,
+                           clip_region->x,
+                           clip_region->y,
+                           clip_region->width,
+                           clip_region->height);
+             force_swap = TRUE;
+           }
+         else
+           {
+             CLUTTER_NOTE (CLIPPING, "Invalid back buffer(age=%d): forcing full redraw\n", age);
+             use_clipped_redraw = FALSE;
+           }
+       }
+      else
+       {
+         current_damage->x = 0;
+         current_damage->y = 0;
+         current_damage->width  = geom.width;
+         current_damage->height = geom.height;
+       }
     }
 
-  if (has_buffer_age && !force_swap)
-    use_clipped_redraw = FALSE;
-
   if (use_clipped_redraw)
     {
       CoglFramebuffer *fb = COGL_FRAMEBUFFER (stage_cogl->onscreen);
@@ -678,7 +669,7 @@ clutter_stage_cogl_get_dirty_pixel (ClutterStageWindow *stage_window,
   ClutterStageCogl *stage_cogl = CLUTTER_STAGE_COGL (stage_window);
   gboolean has_buffer_age = cogl_clutter_winsys_has_feature (COGL_WINSYS_FEATURE_BUFFER_AGE);
 
-  if ((stage_cogl->damage_history == NULL && has_buffer_age) || !has_buffer_age)
+  if (!has_buffer_age)
     {
       *x = 0;
       *y = 0;
@@ -686,7 +677,8 @@ clutter_stage_cogl_get_dirty_pixel (ClutterStageWindow *stage_window,
   else
     {
       cairo_rectangle_int_t *rect;
-      rect = (cairo_rectangle_int_t *) (stage_cogl->damage_history->data);
+
+      rect = &stage_cogl->damage_history[DAMAGE_HISTORY (stage_cogl->damage_index-1)];
       *x = rect->x;
       *y = rect->y;
     }
diff --git a/clutter/cogl/clutter-stage-cogl.h b/clutter/cogl/clutter-stage-cogl.h
index dd34e38..e13e58d 100644
--- a/clutter/cogl/clutter-stage-cogl.h
+++ b/clutter/cogl/clutter-stage-cogl.h
@@ -58,7 +58,10 @@ struct _ClutterStageCogl
   guint dirty_backbuffer     : 1;
 
   /* Stores a list of previous damaged areas */
-  GSList *damage_history;
+#define DAMAGE_HISTORY_MAX 16
+#define DAMAGE_HISTORY(x) ((x) & (DAMAGE_HISTORY_MAX - 1))
+  cairo_rectangle_int_t damage_history[DAMAGE_HISTORY_MAX];
+  unsigned damage_index;
 };
 
 struct _ClutterStageCoglClass


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]