From ef48699880d7b07c9369aa9fd0a7bde995206f64 Mon Sep 17 00:00:00 2001 From: Dor Askayo Date: Sat, 29 Aug 2020 17:45:51 +0300 Subject: [PATCH 1/2] clutter/frame-clock: Allow update dispatch while presentation is pending Instead of inhibiting the frame clock while presentation is pending, only inhibit the creation of the frame. In case an update is dispatched while presentation is pending, skip the frame callback and reschedule another update at the next frame interval. Timeline updates are considered part of the frame and therefore skipped as well. Since updates are still scheduled to occur not more than once in every refresh period, and only when an update is due, the update frequency in idle and light rendering workloads remains the same and isn't affected by this change. However, in scenarios where the rendering workload is more intense and Mutter is expected to redraw at a rate which is lower than the maximum refresh rate, this change guarantees that updates would still occur at their scheduled time. The result of this is smooth hardware cursor movement even under load, and maximum input event latency of a single refresh period between Mutter and Wayland clients. This change does not affect the rate of updates in idle and light rendering workloads, it should provide a good balance between latency and power usage. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1241 --- clutter/clutter/clutter-frame-clock.c | 46 ++++++++++++++++----------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/clutter/clutter/clutter-frame-clock.c b/clutter/clutter/clutter-frame-clock.c index 1fea057bd2f..ece090c22f1 100644 --- a/clutter/clutter/clutter-frame-clock.c +++ b/clutter/clutter/clutter-frame-clock.c @@ -55,7 +55,6 @@ typedef enum _ClutterFrameClockState CLUTTER_FRAME_CLOCK_STATE_IDLE, CLUTTER_FRAME_CLOCK_STATE_SCHEDULED, CLUTTER_FRAME_CLOCK_STATE_DISPATCHING, - CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED, } ClutterFrameClockState; struct _ClutterFrameClock @@ -78,6 +77,8 @@ struct _ClutterFrameClock gboolean pending_reschedule; gboolean pending_reschedule_now; + gboolean pending_presented; + int inhibit_count; GList *timelines; @@ -197,15 +198,17 @@ clutter_frame_clock_notify_presented (ClutterFrameClock *frame_clock, frame_clock->last_presentation_time_us = g_get_monotonic_time (); } + frame_clock->pending_presented = FALSE; + switch (frame_clock->state) { case CLUTTER_FRAME_CLOCK_STATE_INIT: + g_warn_if_reached (); + break; case CLUTTER_FRAME_CLOCK_STATE_IDLE: case CLUTTER_FRAME_CLOCK_STATE_SCHEDULED: - g_warn_if_reached (); break; case CLUTTER_FRAME_CLOCK_STATE_DISPATCHING: - case CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED: frame_clock->state = CLUTTER_FRAME_CLOCK_STATE_IDLE; maybe_reschedule_update (frame_clock); break; @@ -293,7 +296,6 @@ clutter_frame_clock_inhibit (ClutterFrameClock *frame_clock) frame_clock->state = CLUTTER_FRAME_CLOCK_STATE_IDLE; break; case CLUTTER_FRAME_CLOCK_STATE_DISPATCHING: - case CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED: break; } @@ -333,7 +335,6 @@ clutter_frame_clock_schedule_update_now (ClutterFrameClock *frame_clock) case CLUTTER_FRAME_CLOCK_STATE_SCHEDULED: return; case CLUTTER_FRAME_CLOCK_STATE_DISPATCHING: - case CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED: frame_clock->pending_reschedule = TRUE; frame_clock->pending_reschedule_now = TRUE; return; @@ -371,7 +372,6 @@ clutter_frame_clock_schedule_update (ClutterFrameClock *frame_clock) case CLUTTER_FRAME_CLOCK_STATE_SCHEDULED: return; case CLUTTER_FRAME_CLOCK_STATE_DISPATCHING: - case CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED: frame_clock->pending_reschedule = TRUE; return; } @@ -406,21 +406,28 @@ clutter_frame_clock_dispatch (ClutterFrameClock *frame_clock, } COGL_TRACE_END (ClutterFrameClockEvents); - COGL_TRACE_BEGIN (ClutterFrameClockTimelines, "Frame Clock (timelines)"); - advance_timelines (frame_clock, time_us); - COGL_TRACE_END (ClutterFrameClockTimelines); + if (!frame_clock->pending_presented) + { + COGL_TRACE_BEGIN (ClutterFrameClockTimelines, "Frame Clock (timelines)"); + advance_timelines (frame_clock, time_us); + COGL_TRACE_END (ClutterFrameClockTimelines); - COGL_TRACE_BEGIN (ClutterFrameClockFrame, "Frame Clock (frame)"); - result = frame_clock->listener.iface->frame (frame_clock, - frame_count, - time_us, - frame_clock->listener.user_data); - COGL_TRACE_END (ClutterFrameClockFrame); + COGL_TRACE_BEGIN (ClutterFrameClockFrame, "Frame Clock (frame)"); + result = frame_clock->listener.iface->frame (frame_clock, + frame_count, + time_us, + frame_clock->listener.user_data); + COGL_TRACE_END (ClutterFrameClockFrame); + } + else + { + result = CLUTTER_FRAME_RESULT_IDLE; + frame_clock->pending_reschedule = TRUE; + } switch (frame_clock->state) { case CLUTTER_FRAME_CLOCK_STATE_INIT: - case CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED: g_warn_if_reached (); break; case CLUTTER_FRAME_CLOCK_STATE_IDLE: @@ -430,13 +437,14 @@ clutter_frame_clock_dispatch (ClutterFrameClock *frame_clock, switch (result) { case CLUTTER_FRAME_RESULT_PENDING_PRESENTED: - frame_clock->state = CLUTTER_FRAME_CLOCK_STATE_PENDING_PRESENTED; + frame_clock->pending_presented = TRUE; break; case CLUTTER_FRAME_RESULT_IDLE: - frame_clock->state = CLUTTER_FRAME_CLOCK_STATE_IDLE; - maybe_reschedule_update (frame_clock); break; } + + frame_clock->state = CLUTTER_FRAME_CLOCK_STATE_IDLE; + maybe_reschedule_update (frame_clock); break; } } -- GitLab From eeb3c814777a26aa98d5d887a976e5c1c92255d5 Mon Sep 17 00:00:00 2001 From: Dor Askayo Date: Sun, 20 Sep 2020 14:55:58 +0300 Subject: [PATCH 2/2] clutter/frame-clock: Rename "before_frame" to "update" This callback now represents the update stage of a potential frame, so name it appropriately. This callback allows users the opportunity to perform some operatons at regular frame timing interval without being delayed by GPU load, number of available frame buffers or pending presentation feedback. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1241 --- clutter/clutter/clutter-frame-clock.c | 12 ++++++------ clutter/clutter/clutter-frame-clock.h | 6 +++--- clutter/clutter/clutter-stage-view.c | 8 ++++---- src/tests/clutter/conform/frame-clock.c | 26 ++++++++++++------------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/clutter/clutter/clutter-frame-clock.c b/clutter/clutter/clutter-frame-clock.c index ece090c22f1..71f2ec896ed 100644 --- a/clutter/clutter/clutter-frame-clock.c +++ b/clutter/clutter/clutter-frame-clock.c @@ -397,14 +397,14 @@ clutter_frame_clock_dispatch (ClutterFrameClock *frame_clock, frame_count = frame_clock->frame_count++; - COGL_TRACE_BEGIN (ClutterFrameClockEvents, "Frame Clock (before frame)"); - if (frame_clock->listener.iface->before_frame) + COGL_TRACE_BEGIN (ClutterFrameClockUpdate, "Frame Clock (update)"); + if (frame_clock->listener.iface->update) { - frame_clock->listener.iface->before_frame (frame_clock, - frame_count, - frame_clock->listener.user_data); + frame_clock->listener.iface->update (frame_clock, + frame_count, + frame_clock->listener.user_data); } - COGL_TRACE_END (ClutterFrameClockEvents); + COGL_TRACE_END (ClutterFrameClockUpdate); if (!frame_clock->pending_presented) { diff --git a/clutter/clutter/clutter-frame-clock.h b/clutter/clutter/clutter-frame-clock.h index 3c9ab7b5ad2..894d218298e 100644 --- a/clutter/clutter/clutter-frame-clock.h +++ b/clutter/clutter/clutter-frame-clock.h @@ -42,9 +42,9 @@ G_DECLARE_FINAL_TYPE (ClutterFrameClock, clutter_frame_clock, typedef struct _ClutterFrameListenerIface { - void (* before_frame) (ClutterFrameClock *frame_clock, - int64_t frame_count, - gpointer user_data); + void (* update) (ClutterFrameClock *frame_clock, + int64_t frame_count, + gpointer user_data); ClutterFrameResult (* frame) (ClutterFrameClock *frame_clock, int64_t frame_count, int64_t time_us, diff --git a/clutter/clutter/clutter-stage-view.c b/clutter/clutter/clutter-stage-view.c index 00a4d5ad146..17bd61846fc 100644 --- a/clutter/clutter/clutter-stage-view.c +++ b/clutter/clutter/clutter-stage-view.c @@ -1052,9 +1052,9 @@ clutter_stage_view_get_frame_clock (ClutterStageView *view) } static void -handle_frame_clock_before_frame (ClutterFrameClock *frame_clock, - int64_t frame_count, - gpointer user_data) +handle_frame_clock_update (ClutterFrameClock *frame_clock, + int64_t frame_count, + gpointer user_data) { ClutterStageView *view = user_data; ClutterStageViewPrivate *priv = @@ -1123,7 +1123,7 @@ handle_frame_clock_frame (ClutterFrameClock *frame_clock, } static const ClutterFrameListenerIface frame_clock_listener_iface = { - .before_frame = handle_frame_clock_before_frame, + .update = handle_frame_clock_update, .frame = handle_frame_clock_frame, }; diff --git a/src/tests/clutter/conform/frame-clock.c b/src/tests/clutter/conform/frame-clock.c index 62a4176722b..655ba1a8c52 100644 --- a/src/tests/clutter/conform/frame-clock.c +++ b/src/tests/clutter/conform/frame-clock.c @@ -478,9 +478,9 @@ frame_clock_schedule_update_now (void) } static void -before_frame_frame_clock_before_frame (ClutterFrameClock *frame_clock, - int64_t frame_count, - gpointer user_data) +update_frame_clock_update (ClutterFrameClock *frame_clock, + int64_t frame_count, + gpointer user_data) { int64_t *expected_frame_count = user_data; @@ -488,10 +488,10 @@ before_frame_frame_clock_before_frame (ClutterFrameClock *frame_clock, } static ClutterFrameResult -before_frame_frame_clock_frame (ClutterFrameClock *frame_clock, - int64_t frame_count, - int64_t time_us, - gpointer user_data) +update_frame_clock_frame (ClutterFrameClock *frame_clock, + int64_t frame_count, + int64_t time_us, + gpointer user_data) { int64_t *expected_frame_count = user_data; ClutterFrameInfo frame_info; @@ -507,9 +507,9 @@ before_frame_frame_clock_frame (ClutterFrameClock *frame_clock, return CLUTTER_FRAME_RESULT_PENDING_PRESENTED; } -static const ClutterFrameListenerIface before_frame_frame_listener_iface = { - .before_frame = before_frame_frame_clock_before_frame, - .frame = before_frame_frame_clock_frame, +static const ClutterFrameListenerIface update_frame_listener_iface = { + .update = update_frame_clock_update, + .frame = update_frame_clock_frame, }; static gboolean @@ -523,7 +523,7 @@ quit_main_loop_timeout (gpointer user_data) } static void -frame_clock_before_frame (void) +frame_clock_update (void) { GMainLoop *main_loop; ClutterFrameClock *frame_clock; @@ -532,7 +532,7 @@ frame_clock_before_frame (void) main_loop = g_main_loop_new (NULL, FALSE); frame_clock = clutter_frame_clock_new (refresh_rate, - &before_frame_frame_listener_iface, + &update_frame_listener_iface, &expected_frame_count); clutter_frame_clock_schedule_update (frame_clock); @@ -753,7 +753,7 @@ CLUTTER_TEST_SUITE ( CLUTTER_TEST_UNIT ("/frame-clock/delayed-damage", frame_clock_delayed_damage) CLUTTER_TEST_UNIT ("/frame-clock/no-damage", frame_clock_no_damage) CLUTTER_TEST_UNIT ("/frame-clock/schedule-update-now", frame_clock_schedule_update_now) - CLUTTER_TEST_UNIT ("/frame-clock/before-frame", frame_clock_before_frame) + CLUTTER_TEST_UNIT ("/frame-clock/update", frame_clock_update) CLUTTER_TEST_UNIT ("/frame-clock/inhibit", frame_clock_inhibit) CLUTTER_TEST_UNIT ("/frame-clock/reschedule-on-idle", frame_clock_reschedule_on_idle) CLUTTER_TEST_UNIT ("/frame-clock/destroy-signal", frame_clock_destroy_signal) -- GitLab