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
(cherry picked from commit 239280f855db26fec93646af51bbc772478ef698)
Signed-off-by: Emmanuele Bassi <ebassi@gnome.org>

2 files changed, 54 insertions, 59 deletions

diff --git a/clutter/cogl/clutter-stage-cogl.c b/clutter/cogl/clutter-stage-cogl.c
index 882189d46..25e95f0ee 100644
--- a/clutter/cogl/clutter-stage-cogl.c
+++ b/clutter/cogl/clutter-stage-cogl.c
@@ -393,6 +393,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)
@@ -469,65 +478,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);
@@ -704,7 +695,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;
@@ -712,7 +703,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 dd34e387c..e13e58d7e 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