From 6705ce6c6a8c7b490714a15c697c1ad24c9bf8b0 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 8 Jun 2009 13:00:09 -0400 Subject: [PATCH] Move elapsed-time calculations into ClutterTimeline Instead of calculating a delta in the master clock, and passing that into each timeline, make each timeline individually responsible for remembering the last time and computing the delta. This: - Fixes a problem where we could spin infinitely processing timeline-only frames with < 1msec differences. - Makes timelines consistently start timing on the first frame; instead of doing different things for the first started timeline and other timelines. - Improves accuracy of elapsed time computations by avoiding accumulating microsecond => millisecond truncation errors. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 37 ++-------------- clutter/clutter-timeline.c | 53 +++++++++++++++++------ clutter/clutter-timeline.h | 4 +- tests/conform/test-timeline-interpolate.c | 17 +------- tests/conform/test-timeline-rewind.c | 17 +------- tests/conform/test-timeline.c | 15 +------ 6 files changed, 48 insertions(+), 95 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 8e6d569b1..08a5c744f 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -66,8 +66,6 @@ struct _ClutterMasterClock * a redraw on the stage and drive the animations */ GSource *source; - - guint timelines_running : 1; }; struct _ClutterMasterClockClass @@ -364,8 +362,6 @@ _clutter_master_clock_remove_timeline (ClutterMasterClock *master_clock, { master_clock->timelines = g_slist_remove (master_clock->timelines, timeline); - if (master_clock->timelines == NULL) - master_clock->timelines_running = FALSE; } /* @@ -395,39 +391,14 @@ _clutter_master_clock_start_running (ClutterMasterClock *master_clock) void _clutter_master_clock_advance (ClutterMasterClock *master_clock) { - gulong msecs; - GSList *l; + GSList *l, *next; g_return_if_fail (CLUTTER_IS_MASTER_CLOCK (master_clock)); - if (master_clock->timelines == NULL) - return; - - if (!master_clock->timelines_running) + for (l = master_clock->timelines; l != NULL; l = next) { - /* When we start running, we count the first frame as time 0, - * so we don't need an advance. (And master_clock->prev_tick - * doesn't meaningfully relate to these timelines.) - */ - master_clock->timelines_running = TRUE; - return; - } + next = l->next; - msecs = (master_clock->cur_tick.tv_sec - master_clock->prev_tick.tv_sec) * 1000 - + (master_clock->cur_tick.tv_usec - master_clock->prev_tick.tv_usec) / 1000; - - if (msecs == 0) - return; - - CLUTTER_NOTE (SCHEDULER, "Advancing %d timelines by %lu milliseconds", - g_slist_length (master_clock->timelines), - msecs); - - for (l = master_clock->timelines; l != NULL; l = l->next) - { - ClutterTimeline *timeline = l->data; - - if (clutter_timeline_is_playing (timeline)) - clutter_timeline_advance_delta (timeline, msecs); + clutter_timeline_do_tick (l->data, &master_clock->cur_tick); } } diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index 45093d404..2d68bc5d1 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -62,8 +62,13 @@ struct _ClutterTimelinePrivate GHashTable *markers_by_name; + /* Time we last advanced the elapsed time and showed a frame */ + GTimeVal last_frame_time; + guint loop : 1; guint is_playing : 1; + /* If we've just started playing and haven't yet gotten a tick from the master clock */ + guint waiting_first_tick : 1; }; typedef struct { @@ -489,13 +494,18 @@ set_is_playing (ClutterTimeline *timeline, priv->is_playing = is_playing; master_clock = _clutter_master_clock_get_default (); if (priv->is_playing) - _clutter_master_clock_add_timeline (master_clock, timeline); + { + _clutter_master_clock_add_timeline (master_clock, timeline); + priv->waiting_first_tick = TRUE; + } else - _clutter_master_clock_remove_timeline (master_clock, timeline); + { + _clutter_master_clock_remove_timeline (master_clock, timeline); + } } static gboolean -clutter_timeline_advance_internal (ClutterTimeline *timeline) +clutter_timeline_do_frame (ClutterTimeline *timeline) { ClutterTimelinePrivate *priv; @@ -1135,19 +1145,18 @@ clutter_timeline_get_delta (ClutterTimeline *timeline) } /* - * clutter_timeline_advance_delta: + * clutter_timeline_do_tick * @timeline: a #ClutterTimeline - * @msecs: advance in milliseconds + * @tick_time: time of advance * - * Advances @timeline by @msecs. This function is called by the master - * clock and it is used to advance a timeline by the amount of milliseconds - * elapsed since the last redraw operation. The @timeline will use this - * interval to emit the #ClutterTimeline::new-frame signal and eventually - * skip frames. + * Advances @timeline based on the time passed in @msecs. This + * function is called by the master clock. The @timeline will use this + * interval to emit the #ClutterTimeline::new-frame signal and + * eventually skip frames. */ void -clutter_timeline_advance_delta (ClutterTimeline *timeline, - guint msecs) +clutter_timeline_do_tick (ClutterTimeline *timeline, + GTimeVal *tick_time) { ClutterTimelinePrivate *priv; @@ -1155,9 +1164,25 @@ clutter_timeline_advance_delta (ClutterTimeline *timeline, priv = timeline->priv; - priv->msecs_delta = msecs; + if (priv->waiting_first_tick) + { + priv->last_frame_time = *tick_time; + priv->waiting_first_tick = FALSE; + } + else + { + gint msecs = + (tick_time->tv_sec - priv->last_frame_time.tv_sec) * 1000 + + (tick_time->tv_usec - priv->last_frame_time.tv_usec) / 1000; - clutter_timeline_advance_internal (timeline); + if (msecs != 0) + { + /* Avoid accumulating error */ + g_time_val_add (&priv->last_frame_time, msecs * 1000L); + priv->msecs_delta = msecs; + clutter_timeline_do_frame (timeline); + } + } } static inline void diff --git a/clutter/clutter-timeline.h b/clutter/clutter-timeline.h index 3b372408f..2cb83e7d5 100644 --- a/clutter/clutter-timeline.h +++ b/clutter/clutter-timeline.h @@ -155,8 +155,8 @@ void clutter_timeline_advance_to_marker (ClutterTimeline *timeli const gchar *marker_name); /*< private >*/ -void clutter_timeline_advance_delta (ClutterTimeline *timeline, - guint msecs); +void clutter_timeline_do_tick (ClutterTimeline *timeline, + GTimeVal *tick_time); G_END_DECLS diff --git a/tests/conform/test-timeline-interpolate.c b/tests/conform/test-timeline-interpolate.c index c4837a5ec..29fe9263f 100644 --- a/tests/conform/test-timeline-interpolate.c +++ b/tests/conform/test-timeline-interpolate.c @@ -24,8 +24,6 @@ typedef struct _TestState gint completion_count; gboolean passed; guint source_id; - GTimeVal prev_tick; - gulong msecs_delta; } TestState; @@ -139,21 +137,11 @@ frame_tick (gpointer data) { TestState *state = data; GTimeVal cur_tick = { 0, }; - gulong msecs; g_get_current_time (&cur_tick); - if (state->prev_tick.tv_sec == 0) - state->prev_tick = cur_tick; - - msecs = (cur_tick.tv_sec - state->prev_tick.tv_sec) * 1000 - + (cur_tick.tv_usec - state->prev_tick.tv_usec) / 1000; - if (clutter_timeline_is_playing (state->timeline)) - clutter_timeline_advance_delta (state->timeline, msecs); - - state->msecs_delta = msecs; - state->prev_tick = cur_tick; + clutter_timeline_do_tick (state->timeline, &cur_tick); return TRUE; } @@ -180,9 +168,6 @@ test_timeline_interpolate (TestConformSimpleFixture *fixture, state.new_frame_counter = 0; state.passed = TRUE; state.expected_frame = 0; - state.prev_tick.tv_sec = 0; - state.prev_tick.tv_usec = 0; - state.msecs_delta = 0; state.source_id = clutter_threads_add_frame_source (60, frame_tick, &state); diff --git a/tests/conform/test-timeline-rewind.c b/tests/conform/test-timeline-rewind.c index b1cfa13c0..67668c55f 100644 --- a/tests/conform/test-timeline-rewind.c +++ b/tests/conform/test-timeline-rewind.c @@ -12,8 +12,6 @@ typedef struct _TestState ClutterTimeline *timeline; gint rewind_count; guint source_id; - GTimeVal prev_tick; - gulong msecs_delta; } TestState; static gboolean @@ -74,21 +72,11 @@ frame_tick (gpointer data) { TestState *state = data; GTimeVal cur_tick = { 0, }; - gulong msecs; g_get_current_time (&cur_tick); - if (state->prev_tick.tv_sec == 0) - state->prev_tick = cur_tick; - - msecs = (cur_tick.tv_sec - state->prev_tick.tv_sec) * 1000 - + (cur_tick.tv_usec - state->prev_tick.tv_usec) / 1000; - if (clutter_timeline_is_playing (state->timeline)) - clutter_timeline_advance_delta (state->timeline, msecs); - - state->msecs_delta = msecs; - state->prev_tick = cur_tick; + clutter_timeline_do_tick (state->timeline, &cur_tick); return TRUE; } @@ -111,9 +99,6 @@ test_timeline_rewind (TestConformSimpleFixture *fixture, (GSourceFunc)watchdog_timeout, &state); state.rewind_count = 0; - state.prev_tick.tv_sec = 0; - state.prev_tick.tv_usec = 0; - state.msecs_delta = 0; state.source_id = clutter_threads_add_frame_source (60, frame_tick, &state); diff --git a/tests/conform/test-timeline.c b/tests/conform/test-timeline.c index 94568095e..0a1038e54 100644 --- a/tests/conform/test-timeline.c +++ b/tests/conform/test-timeline.c @@ -185,9 +185,6 @@ typedef struct _FrameCounter FrameCounter; struct _FrameCounter { - GTimeVal prev_tick; - gulong msecs_delta; - GSList *timelines; }; @@ -197,27 +194,17 @@ frame_tick (gpointer data) FrameCounter *counter = data; GTimeVal cur_tick = { 0, }; GSList *l; - gulong msecs; g_get_current_time (&cur_tick); - if (counter->prev_tick.tv_sec == 0) - counter->prev_tick = cur_tick; - - msecs = (cur_tick.tv_sec - counter->prev_tick.tv_sec) * 1000 - + (cur_tick.tv_usec - counter->prev_tick.tv_usec) / 1000; - for (l = counter->timelines; l != NULL; l = l->next) { ClutterTimeline *timeline = l->data; if (clutter_timeline_is_playing (timeline)) - clutter_timeline_advance_delta (timeline, msecs); + clutter_timeline_do_tick (timeline, &cur_tick); } - counter->msecs_delta = msecs; - counter->prev_tick = cur_tick; - return TRUE; }