From 239280f855db26fec93646af51bbc772478ef698 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 28 Jan 2015 22:08:03 +0000 Subject: [PATCH] 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 | 100 ++++++++++++++---------------- clutter/cogl/clutter-stage-cogl.h | 5 +- 2 files changed, 50 insertions(+), 55 deletions(-) diff --git a/clutter/cogl/clutter-stage-cogl.c b/clutter/cogl/clutter-stage-cogl.c index 378797d70..5a8c3bd0b 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,64 +461,46 @@ 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; + cairo_rectangle_int_t *current_damage = + &stage_cogl->damage_history[DAMAGE_HISTORY (stage_cogl->damage_index++)]; - 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; + if (use_clipped_redraw) + { + int age = cogl_onscreen_get_buffer_age (stage_cogl->onscreen), i; - stage_cogl->damage_history = g_slist_prepend (stage_cogl->damage_history, current_damage); + *current_damage = *clip_region; - 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; - } - } + 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); - 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; - } - - } + 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; + } } - 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; - } - - if (has_buffer_age && !force_swap) - use_clipped_redraw = FALSE; if (use_clipped_redraw) { @@ -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 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