From 912a9ecfba5224966d13cd4761d759586b72784f Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Tue, 2 Apr 2019 17:03:11 +0800 Subject: [PATCH 1/7] clutter/stage-cogl: Use G_USEC_PER_SEC instead of hardcoded number One million is the number of microseconds in one second, which is also defined by `G_USEC_PER_SEC`. https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index e8ea80d7f85..469645f048e 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -201,7 +201,7 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, if (refresh_rate == 0.0) refresh_rate = 60.0; - refresh_interval = (gint64) (0.5 + 1000000 / refresh_rate); + refresh_interval = (gint64) (0.5 + G_USEC_PER_SEC / refresh_rate); if (refresh_interval == 0) refresh_interval = 16667; /* 1/60th second */ -- GitLab From ccf27e5f83a93fb18510ed0310161106742ed2da Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Thu, 20 Dec 2018 15:50:54 +0800 Subject: [PATCH 2/7] clutter/stage-cogl: Schedule immediate update on zero refresh interval Instead of crazy refresh rates >1MHz falling back to 60Hz, just honour them by rendering unthrottled (same as `sync_delay < 0`). Although I wouldn't actually expect that path to ever be needed in reality, it just ensures an infinite `while` loop never happens. https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index 469645f048e..aa698ac159e 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -203,7 +203,10 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, refresh_interval = (gint64) (0.5 + G_USEC_PER_SEC / refresh_rate); if (refresh_interval == 0) - refresh_interval = 16667; /* 1/60th second */ + { + stage_cogl->update_time = now; + return; + } min_render_time_allowed = refresh_interval / 2; max_render_time_allowed = refresh_interval - 1000 * sync_delay; -- GitLab From a76762a05eb2f69776eecbe04e69504e0b234ba2 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Tue, 2 Apr 2019 16:55:47 +0800 Subject: [PATCH 3/7] clutter/stage-cogl: Use default frame rate instead of hardcoded 60Hz Instead of 0Hz falling back to 60Hz, use `CLUTTER_DEFAULT_FPS` which is also 60Hz by default. https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index aa698ac159e..1f69b3092f0 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -198,8 +198,8 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, } refresh_rate = stage_cogl->refresh_rate; - if (refresh_rate == 0.0) - refresh_rate = 60.0; + if (refresh_rate <= 0.0) + refresh_rate = clutter_get_default_frame_rate (); refresh_interval = (gint64) (0.5 + G_USEC_PER_SEC / refresh_rate); if (refresh_interval == 0) -- GitLab From 35aa278194ea26aef10928877a0792a371bbdd42 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Thu, 20 Dec 2018 17:24:09 +0800 Subject: [PATCH 4/7] clutter/stage-cogl: Stop `schedule_update` repeatedly returning `now` That could happen if the backend did not provide presentation timestamps, or if the screen was not changing other than the hardware cursor: if (stage_cogl->last_presentation_time == 0|| stage_cogl->last_presentation_time < now - 150000) { stage_cogl->update_time = now; return; } By setting `update_time` to `now`, master_clock_get_swap_wait_time() returns 0: gint64 now = g_source_get_time (master_clock->source); if (min_update_time < now) { return 0; } else { gint64 delay_us = min_update_time - now; return (delay_us + 999) / 1000; } However, zero is a value unsupported by the default master clock due to: if (swap_delay != 0) return swap_delay; All cases are now handled by extrapolating when the next presentation time would be and calculating an appropriate update time to meet that. We also need to add a check for `update_time == last_update_time`, which is a situation that just became possible since we support old (or zero) values of `last_presentation_time`. This avoids getting more than one stage update per frame interval when input events arrive without triggering a stage redraw (e.g. moving the hardware cursor). https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 16 ++++------------ clutter/clutter/cogl/clutter-stage-cogl.h | 1 + 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index 1f69b3092f0..229015b90b1 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -185,18 +185,6 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, return; } - /* We only extrapolate presentation times for 150ms - this is somewhat - * arbitrary. The reasons it might not be accurate for larger times are - * that the refresh interval might be wrong or the vertical refresh - * might be downclocked if nothing is going on onscreen. - */ - if (stage_cogl->last_presentation_time == 0|| - stage_cogl->last_presentation_time < now - 150000) - { - stage_cogl->update_time = now; - return; - } - refresh_rate = stage_cogl->refresh_rate; if (refresh_rate <= 0.0) refresh_rate = clutter_get_default_frame_rate (); @@ -220,6 +208,9 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, next_presentation_time += refresh_interval; stage_cogl->update_time = next_presentation_time - max_render_time_allowed; + + if (stage_cogl->update_time == stage_cogl->last_update_time) + stage_cogl->update_time = stage_cogl->last_update_time + refresh_interval; } static gint64 @@ -238,6 +229,7 @@ clutter_stage_cogl_clear_update_time (ClutterStageWindow *stage_window) { ClutterStageCogl *stage_cogl = CLUTTER_STAGE_COGL (stage_window); + stage_cogl->last_update_time = stage_cogl->update_time; stage_cogl->update_time = -1; } diff --git a/clutter/clutter/cogl/clutter-stage-cogl.h b/clutter/clutter/cogl/clutter-stage-cogl.h index aead9785e0b..a69c424eb02 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.h +++ b/clutter/clutter/cogl/clutter-stage-cogl.h @@ -53,6 +53,7 @@ struct _ClutterStageCogl gint64 last_presentation_time; gint64 update_time; + int64_t last_update_time; /* We only enable clipped redraws after 2 frames, since we've seen * a lot of drivers can struggle to get going and may output some -- GitLab From 67a3715ded0f36fd03193adafb72e079460b7167 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Thu, 6 Jun 2019 15:39:26 +0800 Subject: [PATCH 5/7] clutter/stage-cogl: Reduce while loop iterations If `last_presentation_time` is zero (unsupported) or just very old (system was idle) then we would like to avoid that triggering a large number of loop interations. This will get us closer to the right answer without iterating. https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index 229015b90b1..03f06726714 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -204,6 +204,19 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, next_presentation_time = stage_cogl->last_presentation_time + refresh_interval; + /* Get next_presentation_time closer to its final value, to reduce + * the number of while iterations below. + */ + if (next_presentation_time < now) + { + int64_t last_virtual_presentation_time = now - now % refresh_interval; + int64_t hardware_clock_phase = + stage_cogl->last_presentation_time % refresh_interval; + + next_presentation_time = + last_virtual_presentation_time + hardware_clock_phase; + } + while (next_presentation_time < now + min_render_time_allowed) next_presentation_time += refresh_interval; -- GitLab From e415cc538ae2ab885314aced4381d09221c17970 Mon Sep 17 00:00:00 2001 From: Daniel van Vugt Date: Thu, 20 Dec 2018 18:18:57 +0800 Subject: [PATCH 6/7] clutter/master-clock: Remove fallback throttles The presentation timing logic (via `master_clock_get_swap_wait_time`) now works unconditionally. By "works" we mean that a result of zero from `master_clock_get_swap_wait_time` actually means zero now. Previously zero could mean either a successful result of zero milliseconds or that the backend couldn't get an answer. And a non-zero result is the same as before. This works even if the screen is "idle" and even if the backend doesn't provide presentation timestamps. So now our two fallback throttling mechanisms of relying on `CLUTTER_FEATURE_SWAP_THROTTLE` and decimating to `clutter_get_default_frame_rate` can be deleted. Closes: https://gitlab.gnome.org/GNOME/mutter/issues/406 and https://bugzilla.gnome.org/show_bug.cgi?id=781835 https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- .../clutter/clutter-master-clock-default.c | 90 +------------------ 1 file changed, 2 insertions(+), 88 deletions(-) diff --git a/clutter/clutter/clutter-master-clock-default.c b/clutter/clutter/clutter-master-clock-default.c index 6779eb70b74..3197a32e3e4 100644 --- a/clutter/clutter/clutter-master-clock-default.c +++ b/clutter/clutter/clutter-master-clock-default.c @@ -64,9 +64,6 @@ struct _ClutterMasterClockDefault /* the current state of the clock, in usecs */ gint64 cur_tick; - /* the previous state of the clock, in usecs, used to compute the delta */ - gint64 prev_tick; - #ifdef CLUTTER_ENABLE_DEBUG gint64 frame_budget; gint64 remaining_budget; @@ -77,12 +74,6 @@ struct _ClutterMasterClockDefault */ GSource *source; - /* If the master clock is idle that means it has - * fallen back to idle polling for timeline - * progressions and it may have been some time since - * the last real stage update. - */ - guint idle : 1; guint ensure_next_iteration : 1; guint paused : 1; @@ -275,78 +266,12 @@ master_clock_reschedule_stage_updates (ClutterMasterClockDefault *master_clock, static gint master_clock_next_frame_delay (ClutterMasterClockDefault *master_clock) { - gint64 now, next; - gint swap_delay; - if (!master_clock_is_running (master_clock)) return -1; /* If all of the stages are busy waiting for a swap-buffers to complete * then we wait for one to be ready.. */ - swap_delay = master_clock_get_swap_wait_time (master_clock); - if (swap_delay != 0) - return swap_delay; - - /* When we have sync-to-vblank, we count on swap-buffer requests (or - * swap-buffer-complete events if supported in the backend) to throttle our - * frame rate so no additional delay is needed to start the next frame. - * - * If the master-clock has become idle due to no timeline progression causing - * redraws then we can no longer rely on vblank synchronization because the - * last real stage update/redraw may have happened a long time ago and so we - * fallback to polling for timeline progressions every 1/frame_rate seconds. - * - * (NB: if there aren't even any timelines running then the master clock will - * be completely stopped in master_clock_is_running()) - */ - if (clutter_feature_available (CLUTTER_FEATURE_SWAP_THROTTLE) && - !master_clock->idle) - { - CLUTTER_NOTE (SCHEDULER, "swap throttling available and updated stages"); - return 0; - } - - if (master_clock->prev_tick == 0) - { - /* If we weren't previously running, then draw the next frame - * immediately - */ - CLUTTER_NOTE (SCHEDULER, "draw the first frame immediately"); - return 0; - } - - /* Otherwise, wait at least 1/frame_rate seconds since we last - * started a frame - */ - now = g_source_get_time (master_clock->source); - - next = master_clock->prev_tick; - - /* If time has gone backwards then there's no way of knowing how - long we should wait so let's just dispatch immediately */ - if (now <= next) - { - CLUTTER_NOTE (SCHEDULER, "Time has gone backwards"); - - return 0; - } - - next += (1000000L / clutter_get_default_frame_rate ()); - - if (next <= now) - { - CLUTTER_NOTE (SCHEDULER, "Less than %lu microsecs", - 1000000L / (gulong) clutter_get_default_frame_rate ()); - - return 0; - } - else - { - CLUTTER_NOTE (SCHEDULER, "Waiting %" G_GINT64_FORMAT " msecs", - (next - now) / 1000); - - return (next - now) / 1000; - } + return master_clock_get_swap_wait_time (master_clock); } static void @@ -530,7 +455,6 @@ clutter_clock_dispatch (GSource *source, { ClutterClockSource *clock_source = (ClutterClockSource *) source; ClutterMasterClockDefault *master_clock = clock_source->master_clock; - gboolean stages_updated = FALSE; GSList *stages; CLUTTER_NOTE (SCHEDULER, "Master clock [tick]"); @@ -550,8 +474,6 @@ clutter_clock_dispatch (GSource *source, */ stages = master_clock_list_ready_stages (master_clock); - master_clock->idle = FALSE; - /* Each frame is split into three separate phases: */ /* 1. process all the events; each stage goes through its events queue @@ -564,19 +486,12 @@ clutter_clock_dispatch (GSource *source, master_clock_advance_timelines (master_clock); /* 3. relayout and redraw the stages */ - stages_updated = master_clock_update_stages (master_clock, stages); - - /* The master clock goes idle if no stages were updated and falls back - * to polling for timeline progressions... */ - if (!stages_updated) - master_clock->idle = TRUE; + master_clock_update_stages (master_clock, stages); master_clock_reschedule_stage_updates (master_clock, stages); g_slist_free_full (stages, g_object_unref); - master_clock->prev_tick = master_clock->cur_tick; - _clutter_threads_release_lock (); return TRUE; @@ -608,7 +523,6 @@ clutter_master_clock_default_init (ClutterMasterClockDefault *self) source = clutter_clock_source_new (self); self->source = source; - self->idle = FALSE; self->ensure_next_iteration = FALSE; self->paused = FALSE; -- GitLab From 1dbf25afa15779d922ed66ddfb8bcda205ae7494 Mon Sep 17 00:00:00 2001 From: Georges Basile Stavracas Neto Date: Fri, 7 Jun 2019 12:12:55 -0300 Subject: [PATCH 7/7] clutter/stage-cogl: Protect against extremely high refresh rates After 4faeb12731b8, the maximum time allowed for an update to happen is calculated as: max_render_time_allowed = refresh_interval - 1000 * sync_delay; However, extremely small refresh intervals -- that come as consequence to extremely high refresh rates -- may fall into an odd numerical range when refresh_interval < 1000 * sync_delay. That would give us a negative time. To be extra cautious about it, add another sanity check for this case. Change suggested by Jasper St. Pierre. https://gitlab.gnome.org/GNOME/mutter/merge_requests/363 --- clutter/clutter/cogl/clutter-stage-cogl.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clutter/clutter/cogl/clutter-stage-cogl.c b/clutter/clutter/cogl/clutter-stage-cogl.c index 03f06726714..5b6eeb8bf45 100644 --- a/clutter/clutter/cogl/clutter-stage-cogl.c +++ b/clutter/clutter/cogl/clutter-stage-cogl.c @@ -199,6 +199,17 @@ clutter_stage_cogl_schedule_update (ClutterStageWindow *stage_window, min_render_time_allowed = refresh_interval / 2; max_render_time_allowed = refresh_interval - 1000 * sync_delay; + /* Be robust in the case of incredibly bogus refresh rate */ + if (max_render_time_allowed <= 0) + { + g_warning ("Unsupported monitor refresh rate detected. " + "(Refresh rate: %.3f, refresh interval: %ld)", + refresh_rate, + refresh_interval); + stage_cogl->update_time = now; + return; + } + if (min_render_time_allowed > max_render_time_allowed) min_render_time_allowed = max_render_time_allowed; -- GitLab