From 3ab303b66234f633d03db660e4a780320dbfe423 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 17:54:05 -0400 Subject: [PATCH 01/22] Only advance the master clock before drawing a frame Remove code to advance the master clock after drawing a frame; if there are any running timelines the master clock will do another frame by itself, and the clock will be advanced before running that frame. With this change, there is no point in queueing an extra frame redraw after completing a timeline, since we are always advancing the timeline *before* redrawing, so remove that code as well. (This does mean that calling clutter_timeline_stop() won't implicitly cause the stage to be redrawn; this doesn't seem like something an app should rely on in any case.) http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-main.c | 5 --- clutter/clutter-master-clock.c | 63 ---------------------------------- 2 files changed, 68 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index d15ff0470..f87813ee9 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -208,11 +208,6 @@ _clutter_do_redraw (ClutterStage *stage) */ _clutter_backend_redraw (ctx->backend, stage); - /* prepare for the next frame; if anything queues a redraw as the - * result of a timeline, this will end up redrawing the scene - */ - _clutter_master_clock_advance (master_clock); - /* Complete FPS info */ if (G_UNLIKELY (clutter_get_show_fps ())) { diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index ea3440555..d6ce9bcb1 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -63,11 +63,6 @@ struct _ClutterMasterClock * a redraw on the stage and drive the animations */ GSource *source; - - /* a guard, so that we dispatch the last redraw even - * after the last timeline has been completed - */ - guint last_advance : 1; }; struct _ClutterMasterClockClass @@ -84,10 +79,6 @@ struct _ClutterClockSource static void on_timeline_started (ClutterTimeline *timeline, ClutterMasterClock *master_clock); -static void on_timeline_completed (ClutterTimeline *timeline, - ClutterMasterClock *master_clock); -static void on_timeline_paused (ClutterTimeline *timeline, - ClutterMasterClock *master_clock); static gboolean clutter_clock_prepare (GSource *source, gint *timeout); @@ -125,9 +116,6 @@ has_running_timeline (ClutterMasterClock *master_clock, { GSList *l; - if (master_clock->last_advance) - return TRUE; - if (master_clock->timelines == NULL) return FALSE; @@ -201,8 +189,6 @@ clutter_clock_dispatch (GSource *source, GSourceFunc callback, gpointer user_data) { - ClutterClockSource *clock_source = (ClutterClockSource *) source; - ClutterMasterClock *master_clock = clock_source->master_clock; ClutterStageManager *stage_manager = clutter_stage_manager_get_default (); const GSList *stages, *l; @@ -217,15 +203,6 @@ clutter_clock_dispatch (GSource *source, for (l = stages; l != NULL; l = l->next) clutter_actor_queue_redraw (l->data); - /* if this is the remainder of an advancement, needed for the last - * timeline to finish its run, then we need to reset the prev_tick - */ - if (master_clock->last_advance) - { - master_clock->prev_tick.tv_sec = 0; - master_clock->last_advance = FALSE; - } - return TRUE; } @@ -259,12 +236,6 @@ clutter_master_clock_finalize (GObject *gobject) g_signal_handlers_disconnect_by_func (timeline, G_CALLBACK (on_timeline_started), master_clock); - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_completed), - master_clock); - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_paused), - master_clock); } g_slist_free (master_clock->timelines); @@ -326,28 +297,6 @@ on_timeline_started (ClutterTimeline *timeline, master_clock->prev_tick.tv_sec = 0; } -static void -on_timeline_completed (ClutterTimeline *timeline, - ClutterMasterClock *master_clock) -{ - /* if this is the last timeline we need to turn :last-advance - * on in order to queue the redraw of the scene for the last - * frame; otherwise the ClockSource will fail the prepare and - * check phases and the last frame will not be painted - */ - if (!has_running_timeline (master_clock, NULL)) - master_clock->last_advance = TRUE; -} - -static void -on_timeline_paused (ClutterTimeline *timeline, - ClutterMasterClock *master_clock) -{ - /* see the comment in on_timeline_completed */ - if (!has_running_timeline (master_clock, NULL)) - master_clock->last_advance = TRUE; -} - /* * _clutter_master_clock_add_timeline: * @master_clock: a #ClutterMasterClock @@ -378,12 +327,6 @@ _clutter_master_clock_add_timeline (ClutterMasterClock *master_clock, g_signal_connect (timeline, "started", G_CALLBACK (on_timeline_started), master_clock); - g_signal_connect (timeline, "completed", - G_CALLBACK (on_timeline_completed), - master_clock); - g_signal_connect (timeline, "paused", - G_CALLBACK (on_timeline_paused), - master_clock); } /* @@ -412,12 +355,6 @@ _clutter_master_clock_remove_timeline (ClutterMasterClock *master_clock, g_signal_handlers_disconnect_by_func (timeline, G_CALLBACK (on_timeline_started), master_clock); - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_completed), - master_clock); - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_paused), - master_clock); /* last timeline: unset the prev_tick so that we can start * from scratch when we add a new timeline From ebaec9798ed6790a559c96010e1074b74c1e260b Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 17:44:40 -0400 Subject: [PATCH 02/22] Simplify timeout list handling for the master clock Instead of keeping a list of all timelines, and connecting to signals and weak notifies, simply keep a list of running timelines; this greatly simplifies both the book-keeping, and also determining if there are any running timelines. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 117 ++++++--------------------------- clutter/clutter-timeline.c | 39 ++++++++--- 2 files changed, 48 insertions(+), 108 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index d6ce9bcb1..c8a849b1c 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -77,9 +77,6 @@ struct _ClutterClockSource ClutterMasterClock *master_clock; }; -static void on_timeline_started (ClutterTimeline *timeline, - ClutterMasterClock *master_clock); - static gboolean clutter_clock_prepare (GSource *source, gint *timeout); static gboolean clutter_clock_check (GSource *source); @@ -101,35 +98,20 @@ G_DEFINE_TYPE (ClutterMasterClock, clutter_master_clock, G_TYPE_OBJECT); /* * has_running_timeline: * @master_clock: a #ClutterMasterClock - * @filter: a #ClutterTimeline or %NULL * - * Checks if @master_clock has any running timeline; if @filter - * is not %NULL then the timeline will be filtered from the - * list of timelines held by @master_clock + * Checks if @master_clock has any running timeline. * * Return value: %TRUE if the #ClutterMasterClock has at least * one running timeline */ static gboolean -has_running_timeline (ClutterMasterClock *master_clock, - ClutterTimeline *filter) +has_running_timeline (ClutterMasterClock *master_clock) { - GSList *l; + ClutterStageManager *stage_manager = clutter_stage_manager_get_default (); + const GSList *stages, *l; - if (master_clock->timelines == NULL) - return FALSE; - - for (l = master_clock->timelines; l != NULL; l = l->next) - { - /* if we get a timeline then we should filter it - * from the list of timelines we want to check - */ - if (filter != NULL && filter == l->data) - continue; - - if (clutter_timeline_is_playing (l->data)) - return TRUE; - } + if (master_clock->timelines) + return TRUE; return FALSE; } @@ -167,7 +149,7 @@ clutter_clock_prepare (GSource *source, /* just like an idle source, we are ready if nothing else is */ *timeout = -1; - retval = has_running_timeline (master_clock, NULL); + retval = has_running_timeline (master_clock); return retval; } @@ -179,7 +161,7 @@ clutter_clock_check (GSource *source) ClutterMasterClock *master_clock = clock_source->master_clock; gboolean retval; - retval = has_running_timeline (master_clock, NULL); + retval = has_running_timeline (master_clock); return retval; } @@ -206,37 +188,10 @@ clutter_clock_dispatch (GSource *source, return TRUE; } -static void -timeline_weak_ref (gpointer data, - GObject *object_pointer) -{ - ClutterMasterClock *master_clock = data; - - master_clock->timelines = - g_slist_remove (master_clock->timelines, object_pointer); - - if (master_clock->timelines == NULL) - master_clock->prev_tick.tv_sec = 0; -} - static void clutter_master_clock_finalize (GObject *gobject) { ClutterMasterClock *master_clock = CLUTTER_MASTER_CLOCK (gobject); - GSList *l; - - for (l = master_clock->timelines; l != NULL; l = l->next) - { - ClutterTimeline *timeline = l->data; - - g_object_weak_unref (G_OBJECT (timeline), - timeline_weak_ref, - master_clock); - - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_started), - master_clock); - } g_slist_free (master_clock->timelines); @@ -284,49 +239,33 @@ _clutter_master_clock_get_default (void) return default_clock; } -static void -on_timeline_started (ClutterTimeline *timeline, - ClutterMasterClock *master_clock) -{ - /* we want to reset the prev_tick if this is the first - * timeline; since timeline is playing we need to filter - * it out, otherwise has_running_timeline() will return - * TRUE and prev_tick will not be unset - */ - if (!has_running_timeline (master_clock, timeline)) - master_clock->prev_tick.tv_sec = 0; -} - /* * _clutter_master_clock_add_timeline: * @master_clock: a #ClutterMasterClock * @timeline: a #ClutterTimeline * - * Adds @timeline to the list of timelines held by the master - * clock. This function should be called during the instance - * creation phase of the timeline. + * Adds @timeline to the list of playing timelines held by the master + * clock. */ void _clutter_master_clock_add_timeline (ClutterMasterClock *master_clock, ClutterTimeline *timeline) { - gboolean is_first = FALSE; + gboolean is_first; if (g_slist_find (master_clock->timelines, timeline)) return; - is_first = (master_clock->timelines == NULL) ? TRUE : FALSE; + is_first = master_clock->timelines == NULL; master_clock->timelines = g_slist_prepend (master_clock->timelines, timeline); - g_object_weak_ref (G_OBJECT (timeline), - timeline_weak_ref, - master_clock); - - g_signal_connect (timeline, "started", - G_CALLBACK (on_timeline_started), - master_clock); + if (is_first) + { + /* Start timing from scratch */ + master_clock->prev_tick.tv_sec = 0; + } } /* @@ -334,33 +273,15 @@ _clutter_master_clock_add_timeline (ClutterMasterClock *master_clock, * @master_clock: a #ClutterMasterClock * @timeline: a #ClutterTimeline * - * Removes @timeline from the list of timelines held by the - * master clock. This function should be called during the - * #ClutterTimeline finalization. + * Removes @timeline from the list of playing timelines held by the + * master clock. */ void _clutter_master_clock_remove_timeline (ClutterMasterClock *master_clock, ClutterTimeline *timeline) { - if (!g_slist_find (master_clock->timelines, timeline)) - return; - master_clock->timelines = g_slist_remove (master_clock->timelines, timeline); - - g_object_weak_unref (G_OBJECT (timeline), - timeline_weak_ref, - master_clock); - - g_signal_handlers_disconnect_by_func (timeline, - G_CALLBACK (on_timeline_started), - master_clock); - - /* last timeline: unset the prev_tick so that we can start - * from scratch when we add a new timeline - */ - if (master_clock->timelines == NULL) - master_clock->prev_tick.tv_sec = 0; } /* diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index e76ddf628..45093d404 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -204,8 +204,11 @@ clutter_timeline_finalize (GObject *object) if (priv->markers_by_name) g_hash_table_destroy (priv->markers_by_name); - master_clock = _clutter_master_clock_get_default (); - _clutter_master_clock_remove_timeline (master_clock, self); + if (priv->is_playing) + { + master_clock = _clutter_master_clock_get_default (); + _clutter_master_clock_remove_timeline (master_clock, self); + } G_OBJECT_CLASS (clutter_timeline_parent_class)->finalize (object); } @@ -412,7 +415,6 @@ static void clutter_timeline_init (ClutterTimeline *self) { ClutterTimelinePrivate *priv; - ClutterMasterClock *master_clock; self->priv = priv = G_TYPE_INSTANCE_GET_PRIVATE (self, CLUTTER_TYPE_TIMELINE, @@ -421,9 +423,6 @@ clutter_timeline_init (ClutterTimeline *self) priv->duration = 0; priv->delay = 0; priv->elapsed_time = 0; - - master_clock = _clutter_master_clock_get_default (); - _clutter_master_clock_add_timeline (master_clock, self); } static void @@ -475,6 +474,26 @@ is_complete (ClutterTimeline *timeline) : priv->elapsed_time <= 0); } +static void +set_is_playing (ClutterTimeline *timeline, + gboolean is_playing) +{ + ClutterTimelinePrivate *priv = timeline->priv; + ClutterMasterClock *master_clock; + + is_playing = is_playing != FALSE; + + if (is_playing == priv->is_playing) + return; + + priv->is_playing = is_playing; + master_clock = _clutter_master_clock_get_default (); + if (priv->is_playing) + _clutter_master_clock_add_timeline (master_clock, timeline); + else + _clutter_master_clock_remove_timeline (master_clock, timeline); +} + static gboolean clutter_timeline_advance_internal (ClutterTimeline *timeline) { @@ -561,7 +580,7 @@ clutter_timeline_advance_internal (ClutterTimeline *timeline) * XXX Perhaps we should remove this earlier, and regardless * of priv->loop. Are we limiting the things that could be done in * the above new-frame signal handler */ - priv->is_playing = FALSE; + set_is_playing (timeline, FALSE); } g_signal_emit (timeline, timeline_signals[COMPLETED], 0); @@ -613,7 +632,7 @@ delay_timeout_func (gpointer data) priv->delay_id = 0; priv->msecs_delta = 0; - priv->is_playing = TRUE; + set_is_playing (timeline, TRUE); g_signal_emit (timeline, timeline_signals[STARTED], 0); @@ -648,7 +667,7 @@ clutter_timeline_start (ClutterTimeline *timeline) else { priv->msecs_delta = 0; - priv->is_playing = TRUE; + set_is_playing (timeline, TRUE); g_signal_emit (timeline, timeline_signals[STARTED], 0); } @@ -679,7 +698,7 @@ clutter_timeline_pause (ClutterTimeline *timeline) } priv->msecs_delta = 0; - priv->is_playing = FALSE; + set_is_playing (timeline, FALSE); g_signal_emit (timeline, timeline_signals[PAUSED], 0); } From 4b63f9524e64f2c0a2f7baa4034f2a9657de7ddb Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 18:11:19 -0400 Subject: [PATCH 03/22] Remove unused msecs_delta member msecs_delta member of ClutterMasterClock was set but not used. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index c8a849b1c..3ed350139 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -57,7 +57,6 @@ struct _ClutterMasterClock * the delta */ GTimeVal prev_tick; - gulong msecs_delta; /* an idle source, used by the Master Clock to queue * a redraw on the stage and drive the animations @@ -330,6 +329,5 @@ _clutter_master_clock_advance (ClutterMasterClock *master_clock) /* store the previous state so that we can use * it for the next advancement */ - master_clock->msecs_delta = msecs; master_clock->prev_tick = cur_tick; } From 77cd4e2bc8ec4ad99ab2349fcb10dc5f0b57dca8 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 18:12:26 -0400 Subject: [PATCH 04/22] Call g_main_context_wakeup() when we start running timelines If a timeline is added from a different thread, we need to call g_main_context_wakeup() to wake the main thread up to start updating the timeline. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 3ed350139..fd9ebaa96 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -264,6 +264,11 @@ _clutter_master_clock_add_timeline (ClutterMasterClock *master_clock, { /* Start timing from scratch */ master_clock->prev_tick.tv_sec = 0; + + /* If called from a different thread, we need to wake up the + * main loop to start running the timelines + */ + g_main_context_wakeup (NULL); } } From 89a8fd7755b7a12d579994f7cf0d0e66cf7f99b6 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 18:22:51 -0400 Subject: [PATCH 05/22] Remove stage update idle and do updates from the master clock When a redraw is queued on a stage, simply set a flag; then in the check/prepare functions of the master clock source, check for stages that need redrawing. This avoids the complexity of having multiple competing sources at the same priority and makes the update ordering more reliable and understandable. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 27 +++++++---- clutter/clutter-private.h | 2 + clutter/clutter-stage.c | 82 ++++++++++++++++++---------------- 3 files changed, 63 insertions(+), 48 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index fd9ebaa96..9314e9118 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -95,16 +95,17 @@ static GSourceFuncs clock_funcs = { G_DEFINE_TYPE (ClutterMasterClock, clutter_master_clock, G_TYPE_OBJECT); /* - * has_running_timeline: + * master_clock_is_running: * @master_clock: a #ClutterMasterClock * - * Checks if @master_clock has any running timeline. + * Checks if we should currently be advancing timelines or redrawing + * stages. * * Return value: %TRUE if the #ClutterMasterClock has at least * one running timeline */ static gboolean -has_running_timeline (ClutterMasterClock *master_clock) +master_clock_is_running (ClutterMasterClock *master_clock) { ClutterStageManager *stage_manager = clutter_stage_manager_get_default (); const GSList *stages, *l; @@ -112,6 +113,11 @@ has_running_timeline (ClutterMasterClock *master_clock) if (master_clock->timelines) return TRUE; + stages = clutter_stage_manager_peek_stages (stage_manager); + for (l = stages; l; l = l->next) + if (_clutter_stage_needs_update (l->data)) + return TRUE; + return FALSE; } @@ -148,7 +154,7 @@ clutter_clock_prepare (GSource *source, /* just like an idle source, we are ready if nothing else is */ *timeout = -1; - retval = has_running_timeline (master_clock); + retval = master_clock_is_running (master_clock); return retval; } @@ -160,7 +166,7 @@ clutter_clock_check (GSource *source) ClutterMasterClock *master_clock = clock_source->master_clock; gboolean retval; - retval = has_running_timeline (master_clock); + retval = master_clock_is_running (master_clock); return retval; } @@ -170,6 +176,8 @@ clutter_clock_dispatch (GSource *source, GSourceFunc callback, gpointer user_data) { + ClutterClockSource *clock_source = (ClutterClockSource *) source; + ClutterMasterClock *master_clock = clock_source->master_clock; ClutterStageManager *stage_manager = clutter_stage_manager_get_default (); const GSList *stages, *l; @@ -177,12 +185,13 @@ clutter_clock_dispatch (GSource *source, stages = clutter_stage_manager_peek_stages (stage_manager); - /* queue a redraw for each stage; this will advance each timeline - * held by the master clock of the amount of milliseconds elapsed - * since the last redraw + _clutter_master_clock_advance (master_clock); + + /* Update any stage that needs redraw/relayout after the clock + * is advanced. */ for (l = stages; l != NULL; l = l->next) - clutter_actor_queue_redraw (l->data); + _clutter_stage_do_update (l->data); return TRUE; } diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h index c384323d1..e367f0d73 100644 --- a/clutter/clutter-private.h +++ b/clutter/clutter-private.h @@ -175,6 +175,8 @@ ClutterStageWindow *_clutter_stage_get_window (ClutterStage *sta ClutterStageWindow *_clutter_stage_get_default_window (void); void _clutter_stage_maybe_setup_viewport (ClutterStage *stage); void _clutter_stage_maybe_relayout (ClutterActor *stage); +gboolean _clutter_stage_needs_update (ClutterStage *stage); +void _clutter_stage_do_update (ClutterStage *stage); /* vfuncs implemented by backend */ GType _clutter_backend_impl_get_type (void); diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 63f33d72a..dcb2e8ad6 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -87,8 +87,7 @@ struct _ClutterStagePrivate gchar *title; ClutterActor *key_focused_actor; - guint update_idle; /* repaint idler id */ - + guint redraw_pending : 1; guint is_fullscreen : 1; guint is_offscreen : 1; guint is_cursor_visible : 1; @@ -390,29 +389,43 @@ clutter_stage_real_fullscreen (ClutterStage *stage) CLUTTER_ALLOCATION_NONE); } -static gboolean -redraw_update_idle (gpointer user_data) +/** + * _clutter_stage_needs_update: + * @stage: A #ClutterStage + * + * Determines if _clutter_stage_do_update() needs to be called. + * + * Return value: %TRUE if the stages need layout or painting + */ +gboolean +_clutter_stage_needs_update (ClutterStage *stage) { - ClutterStage *stage = user_data; - ClutterStagePrivate *priv = stage->priv; - ClutterMasterClock *master_clock; + ClutterStagePrivate *priv; - /* before we redraw we advance the master clock of one tick; this means - * that all the timelines that need advancing will be advanced by one - * frame. this will cause multiple redraw requests, so we do this before - * we ask for a relayout and before we do the actual redraw. this ensures - * that we paint the most updated scenegraph state and that all animations - * are in sync with the paint process. - */ - CLUTTER_NOTE (PAINT, "Avdancing master clock"); - master_clock = _clutter_master_clock_get_default (); - _clutter_master_clock_advance (master_clock); + g_return_val_if_fail (CLUTTER_IS_STAGE (stage), FALSE); - /* run the (eventual) repaint functions; since those might end up queuing - * a relayout or a redraw we need to execute them before maybe_relayout() - */ - CLUTTER_NOTE (PAINT, "Repaint functions"); - _clutter_run_repaint_functions (); + priv = stage->priv; + + return priv->redraw_pending; +} + +/** + * _clutter_stage_do_update: + * @stage: A #ClutterStage + * + * Handles per-frame layout and repaint for the stage. + */ +void +_clutter_stage_do_update (ClutterStage *stage) +{ + ClutterStagePrivate *priv; + + g_return_if_fail (CLUTTER_IS_STAGE (stage)); + + priv = stage->priv; + + if (!priv->redraw_pending) + return; /* clutter_do_redraw() will also call maybe_relayout(), but since a relayout * can queue a redraw, we want to do the relayout before we clear the @@ -422,12 +435,11 @@ redraw_update_idle (gpointer user_data) */ _clutter_stage_maybe_relayout (CLUTTER_ACTOR (stage)); - /* redrawing will advance the master clock */ CLUTTER_NOTE (PAINT, "redrawing via idle for stage[%p]", stage); _clutter_do_redraw (stage); /* reset the guard, so that new redraws are possible */ - priv->update_idle = 0; + priv->redraw_pending = FALSE; if (CLUTTER_CONTEXT ()->redraw_count > 0) { @@ -436,8 +448,6 @@ redraw_update_idle (gpointer user_data) CLUTTER_CONTEXT ()->redraw_count = 0; } - - return FALSE; } static void @@ -450,15 +460,15 @@ clutter_stage_real_queue_redraw (ClutterActor *actor, CLUTTER_NOTE (PAINT, "Redraw request number %lu", CLUTTER_CONTEXT ()->redraw_count + 1); - if (priv->update_idle == 0) + if (!priv->redraw_pending) { - CLUTTER_NOTE (PAINT, "Adding idle source for stage: %p", stage); + priv->redraw_pending = TRUE; - priv->update_idle = - clutter_threads_add_idle_full (CLUTTER_PRIORITY_REDRAW, - redraw_update_idle, - stage, - NULL); + /* If called from a thread, we need to wake up the main loop + * out of its sleep so the clock source notices that we have + * a redraw pending + */ + g_main_context_wakeup (NULL); } else CLUTTER_CONTEXT ()->redraw_count += 1; @@ -608,12 +618,6 @@ clutter_stage_dispose (GObject *object) clutter_actor_hide (CLUTTER_ACTOR (object)); - if (priv->update_idle) - { - g_source_remove (priv->update_idle); - priv->update_idle = 0; - } - _clutter_stage_manager_remove_stage (stage_manager, stage); if (priv->impl) From 6e69692e22334d15e2d01d39439362bbf1ba8679 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 19:10:41 -0400 Subject: [PATCH 06/22] Compress events as part of the frame cycle Instead of trying to guess about which motion events are extraneous, queue up all events until we process a frame. This allows us to look ahead and reliably compress consecutive sequence of motion events. clutter-main.c: Feed received events to the stage for queueing. Remove old compression code. Remove clutter_get_motion_events_frequency() clutter_set_motion_events_frequency() clutter-stage.c: Keep a queue of pending events. clutter-master-clock.c: Add processng of queued events to the clock source dispatch function. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-main.c | 129 ++++++--------------------------- clutter/clutter-main.h | 2 - clutter/clutter-master-clock.c | 19 ++++- clutter/clutter-private.h | 10 ++- clutter/clutter-stage.c | 90 ++++++++++++++++++++++- 5 files changed, 136 insertions(+), 114 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index f87813ee9..610900b3c 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -2004,6 +2004,29 @@ generate_enter_leave_events (ClutterEvent *event) */ void clutter_do_event (ClutterEvent *event) +{ + if (!event->any.stage) + return; + + /* Instead of processing events when received, we queue them up to + * handle per-frame before animations, layout, and drawing. + * + * This gives us the chance to reliably compress motion events + * because we've "looked ahead" and know all motion events that + * will occur before drawing the frame. + */ + _clutter_stage_queue_event (event->any.stage, event); +} + +/** + * _clutter_process_event + * @event: a #ClutterEvent. + * + * Does the actual work of processing an event that was queued earlier + * out of clutter_do_event(). + */ +void +_clutter_process_event (ClutterEvent *event) { /* FIXME: This should probably be clutter_cook_event() - it would * take a raw event from the backend and 'cook' it so its more tasty. @@ -2013,8 +2036,6 @@ clutter_do_event (ClutterEvent *event) ClutterBackend *backend; ClutterActor *stage; ClutterInputDevice *device = NULL; - static gint32 motion_last_time = 0L; - gint32 local_motion_time; context = clutter_context_get_default (); backend = context->backend; @@ -2091,52 +2112,6 @@ clutter_do_event (ClutterEvent *event) case CLUTTER_MOTION: device = event->motion.device; - if (device) - local_motion_time = device->motion_last_time; - else - local_motion_time = motion_last_time; - - /* avoid rate throttling for synthetic motion events or if - * the per-actor events are disabled - */ - if (!(event->any.flags & CLUTTER_EVENT_FLAG_SYNTHETIC) || - !context->motion_events_per_actor) - { - gint32 frame_rate, delta; - - /* avoid issuing too many motion events, which leads to many - * redraws in pick mode (performance penalty) - */ - frame_rate = clutter_get_motion_events_frequency (); - delta = 1000 / frame_rate; - - CLUTTER_NOTE (EVENT, - "skip motion event: %s (last:%d, delta:%d, time:%d)", - (event->any.time < (local_motion_time + delta) ? "yes" : "no"), - local_motion_time, - delta, - event->any.time); - - /* we need to guard against roll-overs and the - * case where the time is rolled backwards and - * the backend is not ensuring a monotonic clock - * for the events. - * - * see: - * http://bugzilla.openedhand.com/show_bug.cgi?id=1130 - */ - if (event->any.time >= local_motion_time && - event->any.time < (local_motion_time + delta)) - break; - else - local_motion_time = event->any.time; - } - - if (device) - device->motion_last_time = local_motion_time; - else - motion_last_time = local_motion_time; - /* Only stage gets motion events if clutter_set_motion_events is TRUE, * and the event is not a synthetic event with source set. */ @@ -2321,9 +2296,6 @@ clutter_base_init (void) * Retrieves the default frame rate used when creating #ClutterTimelines. * - * This value is also used to compute the default frequency of motion - * events. - * * Return value: the default frame rate * * Since: 0.6 @@ -2621,61 +2593,6 @@ clutter_get_keyboard_grab (void) return context->keyboard_grab_actor; } -/** - * clutter_get_motion_events_frequency: - * - * Retrieves the number of motion events per second that are delivered - * to the stage. - * - * See clutter_set_motion_events_frequency(). - * - * Return value: the number of motion events per second - * - * Since: 0.6 - */ -guint -clutter_get_motion_events_frequency (void) -{ - ClutterMainContext *context = clutter_context_get_default (); - - if (G_LIKELY (context->motion_frequency == 0)) - { - guint frequency; - - frequency = clutter_default_fps / 4; - frequency = CLAMP (frequency, 20, 45); - - return frequency; - } - else - return context->motion_frequency; -} - -/** - * clutter_set_motion_events_frequency: - * @frequency: the number of motion events per second, or 0 for the - * default value - * - * Sets the motion events frequency. Setting this to a non-zero value - * will override the default setting, so it should be rarely used. - * - * Motion events are delivered from the default backend to the stage - * and are used to generate the enter/leave events pair. This might lead - * to a performance penalty due to the way the actors are identified. - * Using this function is possible to reduce the frequency of the motion - * events delivery to the stage. - * - * Since: 0.6 - */ -void -clutter_set_motion_events_frequency (guint frequency) -{ - ClutterMainContext *context = clutter_context_get_default (); - - /* never allow the motion events to exceed the default frame rate */ - context->motion_frequency = CLAMP (frequency, 1, clutter_default_fps); -} - /** * clutter_clear_glyph_cache: * diff --git a/clutter/clutter-main.h b/clutter/clutter-main.h index d6f1099d0..194faec6f 100644 --- a/clutter/clutter-main.h +++ b/clutter/clutter-main.h @@ -144,8 +144,6 @@ void clutter_threads_remove_repaint_func (guint handle_id void clutter_set_motion_events_enabled (gboolean enable); gboolean clutter_get_motion_events_enabled (void); -void clutter_set_motion_events_frequency (guint frequency); -guint clutter_get_motion_events_frequency (void); void clutter_set_default_frame_rate (guint frames_per_sec); guint clutter_get_default_frame_rate (void); diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 9314e9118..919877051 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -115,7 +115,8 @@ master_clock_is_running (ClutterMasterClock *master_clock) stages = clutter_stage_manager_peek_stages (stage_manager); for (l = stages; l; l = l->next) - if (_clutter_stage_needs_update (l->data)) + if (_clutter_stage_has_queued_events (l->data) || + _clutter_stage_needs_update (l->data)) return TRUE; return FALSE; @@ -179,11 +180,20 @@ clutter_clock_dispatch (GSource *source, ClutterClockSource *clock_source = (ClutterClockSource *) source; ClutterMasterClock *master_clock = clock_source->master_clock; ClutterStageManager *stage_manager = clutter_stage_manager_get_default (); - const GSList *stages, *l; + GSList *stages, *l; CLUTTER_NOTE (SCHEDULER, "Master clock [tick]"); - stages = clutter_stage_manager_peek_stages (stage_manager); + /* We need to protect ourselves against stages being destroyed during + * event handling + */ + stages = clutter_stage_manager_list_stages (stage_manager); + g_slist_foreach (stages, (GFunc)g_object_ref, NULL); + + /* Process queued events + */ + for (l = stages; l != NULL; l = l->next) + _clutter_stage_process_queued_events (l->data); _clutter_master_clock_advance (master_clock); @@ -193,6 +203,9 @@ clutter_clock_dispatch (GSource *source, for (l = stages; l != NULL; l = l->next) _clutter_stage_do_update (l->data); + g_slist_foreach (stages, (GFunc)g_object_unref, NULL); + g_slist_free (stages); + return TRUE; } diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h index e367f0d73..340cc75c6 100644 --- a/clutter/clutter-private.h +++ b/clutter/clutter-private.h @@ -70,7 +70,6 @@ typedef enum { struct _ClutterInputDevice { gint id; - gint32 motion_last_time; ClutterActor *pointer_grab_actor; ClutterActor *motion_last_actor; @@ -99,7 +98,6 @@ struct _ClutterMainContext ClutterPickMode pick_mode; /* Indicates pick render mode */ - guint motion_frequency; /* Motion events per second */ gint num_reactives; /* Num of reactive actors */ ClutterIDPool *id_pool; /* mapping between reused integer ids @@ -178,6 +176,11 @@ void _clutter_stage_maybe_relayout (ClutterActor *sta gboolean _clutter_stage_needs_update (ClutterStage *stage); void _clutter_stage_do_update (ClutterStage *stage); +void _clutter_stage_queue_event (ClutterStage *stage, + ClutterEvent *event); +gboolean _clutter_stage_has_queued_events (ClutterStage *stage); +void _clutter_stage_process_queued_events (ClutterStage *stage); + /* vfuncs implemented by backend */ GType _clutter_backend_impl_get_type (void); @@ -207,6 +210,9 @@ gfloat _clutter_backend_get_units_per_em (ClutterBackend *backend void _clutter_feature_init (void); +/* Reinjecting queued events for processing */ +void _clutter_process_event (ClutterEvent *event); + /* Picking code */ ClutterActor *_clutter_do_pick (ClutterStage *stage, gint x, diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index dcb2e8ad6..4f1826722 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -87,6 +87,8 @@ struct _ClutterStagePrivate gchar *title; ClutterActor *key_focused_actor; + GQueue *event_queue; + guint redraw_pending : 1; guint is_fullscreen : 1; guint is_offscreen : 1; @@ -389,6 +391,86 @@ clutter_stage_real_fullscreen (ClutterStage *stage) CLUTTER_ALLOCATION_NONE); } +void +_clutter_stage_queue_event (ClutterStage *stage, + ClutterEvent *event) +{ + ClutterStagePrivate *priv; + + g_return_if_fail (CLUTTER_IS_STAGE (stage)); + + priv = stage->priv; + + g_queue_push_tail (priv->event_queue, + clutter_event_copy (event)); +} + +gboolean +_clutter_stage_has_queued_events (ClutterStage *stage) +{ + ClutterStagePrivate *priv; + + g_return_val_if_fail (CLUTTER_IS_STAGE (stage), FALSE); + + priv = stage->priv; + + return priv->event_queue->length > 0; +} + +void +_clutter_stage_process_queued_events (ClutterStage *stage) +{ + ClutterStagePrivate *priv; + GList *events, *l;; + + g_return_if_fail (CLUTTER_IS_STAGE (stage)); + + priv = stage->priv; + + if (priv->event_queue->length == 0) + return; + + /* In case the stage gets destroyed during event processing */ + g_object_ref (stage); + + /* Steal events before starting processing to avoid reentrancy + * issues */ + events = priv->event_queue->head; + priv->event_queue->head = NULL; + priv->event_queue->tail = NULL; + priv->event_queue->length = 0; + + for (l = events; l; l = l->next) + { + ClutterEvent *event; + ClutterEvent *next_event; + + event = l->data; + next_event = l->next ? l->next->data : NULL; + + /* Skip consecutive motion events */ + if (next_event && + event->type == CLUTTER_MOTION && + (next_event->type == CLUTTER_MOTION || + next_event->type == CLUTTER_LEAVE)) + { + CLUTTER_NOTE (EVENT, + "Omitting motion event at %.2f, %.2f", + event->motion.x, event->motion.y); + goto next_event; + } + + _clutter_process_event (event); + + next_event: + clutter_event_free (event); + } + + g_list_free (events); + + g_object_unref (stage); +} + /** * _clutter_stage_needs_update: * @stage: A #ClutterStage @@ -635,9 +717,13 @@ static void clutter_stage_finalize (GObject *object) { ClutterStage *stage = CLUTTER_STAGE (object); + ClutterStagePrivate *priv = stage->priv; + + g_queue_foreach (priv->event_queue, (GFunc)clutter_event_free, NULL); + g_queue_free (priv->event_queue); g_free (stage->priv->title); - + G_OBJECT_CLASS (clutter_stage_parent_class)->finalize (object); } @@ -908,6 +994,8 @@ clutter_stage_init (ClutterStage *self) /* make sure that the implementation is considered a top level */ CLUTTER_SET_PRIVATE_FLAGS (priv->impl, CLUTTER_ACTOR_IS_TOPLEVEL); + priv->event_queue = g_queue_new (); + priv->is_offscreen = FALSE; priv->is_fullscreen = FALSE; priv->is_user_resizable = FALSE; From fc83e364779431dd3a89733ad9e762440de8a001 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 22:55:34 -0400 Subject: [PATCH 07/22] Avoid motion-compression in test-picking test Using clutter_stage_get_actor_at_pos() rather than synthesizing events; the synthesized events were being compressed, so we were only tesitng one pick per frame. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- tests/micro-bench/test-picking.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tests/micro-bench/test-picking.c b/tests/micro-bench/test-picking.c index 71b08472d..3e867610d 100644 --- a/tests/micro-bench/test-picking.c +++ b/tests/micro-bench/test-picking.c @@ -36,7 +36,6 @@ do_events (ClutterActor *stage) { glong i; static gdouble angle = 0; - ClutterEvent event; for (i = 0; i < n_events; i++) { @@ -44,18 +43,13 @@ do_events (ClutterActor *stage) while (angle > M_PI * 2.0) angle -= M_PI * 2.0; - event.type = CLUTTER_MOTION; - event.any.stage = CLUTTER_STAGE (stage); - event.any.time = CLUTTER_CURRENT_TIME; - event.motion.flags = 0; - event.motion.source = NULL; - event.motion.x = (gint)(256.0 + 206.0 * cos (angle)); - event.motion.y = (gint)(256.0 + 206.0 * sin (angle)); - event.motion.modifier_state = 0; - event.motion.axes = NULL; - event.motion.device = NULL; - - clutter_event_put (&event); + /* If we synthesized events, they would be motion compressed; + * calling get_actor_at_position() doesn't have that problem + */ + clutter_stage_get_actor_at_pos (CLUTTER_STAGE (stage), + CLUTTER_PICK_REACTIVE, + 256.0 + 206.0 * cos (angle), + 256.0 + 206.0 * sin (angle)); } } From 64bb2e694fde2656c26b3b0bf60edbab544ba7ee Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 19:31:32 -0400 Subject: [PATCH 08/22] Decrease the main-loop priority of the frame cycle Change CLUTTER_PRIORITY_REDRAW to be lower than the GTK+ resize and relayout priorities to avoid starving GTK+ when run in the same process as clutter. Remove the unused CLUTTER_PRIORITY_TIMELINE http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-main.h | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/clutter/clutter-main.h b/clutter/clutter-main.h index 194faec6f..d36f8f16f 100644 --- a/clutter/clutter-main.h +++ b/clutter/clutter-main.h @@ -66,20 +66,15 @@ GQuark clutter_init_error_quark (void); /** * CLUTTER_PRIORITY_REDRAW: * - * Priority of the redraws. + * Priority of the redraws. This is chosen to be lower than the GTK+ + * redraw and resize priorities, because in application with both + * GTK+ and Clutter it's more likely that the Clutter part will be + * continually animating (and thus able to starve GTK+) than + * vice-versa. * * Since: 0.8 */ -#define CLUTTER_PRIORITY_REDRAW (G_PRIORITY_HIGH_IDLE + 20) - -/** - * CLUTTER_PRIORITY_TIMELINE: - * - * Priority of the timelines. - * - * Since: 0.8 - */ -#define CLUTTER_PRIORITY_TIMELINE (G_PRIORITY_DEFAULT + 30) +#define CLUTTER_PRIORITY_REDRAW (G_PRIORITY_HIGH_IDLE + 50) /* Initialisation */ void clutter_base_init (void); From dcd8d2831455f1ae950cbab073f37d0ad36d2295 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Sat, 6 Jun 2009 22:48:15 -0400 Subject: [PATCH 09/22] Limit the frame rate when not syncing to VBLANK clutter-master-clock.c clutter-master-clock.h: When the SYNC_TO_VBLANK feature is not available, wait for 1/frame_rate seconds since the start of the last frame before drawing the next frame. Add _clutter_master_clock_start_running() to abstract the usage of g_main_context_wakeup() clutter-stage.c: Add _clutter_master_clock_start_running() clutter-main.c: Update docs for clutter_set_default_frame_rate() clutter_get_default_frame_rate() to no longer talk about timeline frame rates. test-text-perf.c test-text.c: Set a frame rate of 1000fps so that frame-rate limiting doesn't affect the result. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-main.c | 9 +- clutter/clutter-master-clock.c | 131 ++++++++++++++++++++++------- clutter/clutter-master-clock.h | 2 + clutter/clutter-stage.c | 18 ++-- tests/micro-bench/test-text-perf.c | 1 + tests/micro-bench/test-text.c | 1 + 6 files changed, 124 insertions(+), 38 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index 610900b3c..a11d7666c 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -2293,8 +2293,7 @@ clutter_base_init (void) /** * clutter_get_default_frame_rate: * - * Retrieves the default frame rate used when creating #ClutterTimelines. + * Retrieves the default frame rate. See clutter_set_default_frame_rate(). * * Return value: the default frame rate * @@ -2314,8 +2313,10 @@ clutter_get_default_frame_rate (void) * clutter_set_default_frame_rate: * @frames_per_sec: the new default frame rate * - * Sets the default frame rate to be used when creating #ClutterTimelines + * Sets the default frame rate. This frame rate will be used to limit + * the number of frames drawn if Clutter is not able to synchronize + * with the vertical refresh rate of the display. When synchronization + * is possible, this value is ignored. * * Since: 0.6 */ diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 919877051..8e6d569b1 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -53,6 +53,10 @@ struct _ClutterMasterClock /* the list of timelines handled by the clock */ GSList *timelines; + /* the current state of the clock + */ + GTimeVal cur_tick; + /* the previous state of the clock, used to compute * the delta */ @@ -62,6 +66,8 @@ struct _ClutterMasterClock * a redraw on the stage and drive the animations */ GSource *source; + + guint timelines_running : 1; }; struct _ClutterMasterClockClass @@ -122,6 +128,59 @@ master_clock_is_running (ClutterMasterClock *master_clock) return FALSE; } +/* + * master_clock_next_frame_delay: + * @master_clock: a #ClutterMasterClock + * + * Computes the number of delay before we need to draw the next frame. + * + * Return value: -1 if there is no next frame pending, otherwise the + * number of millseconds before the we need to draw the next frame + */ +static gint +master_clock_next_frame_delay (ClutterMasterClock *master_clock) +{ + GTimeVal now; + GTimeVal next; + + if (!master_clock_is_running (master_clock)) + return -1; + + if (clutter_feature_available (CLUTTER_FEATURE_SYNC_TO_VBLANK)) + { + /* When we have sync-to-vblank, we count on that to throttle + * our frame rate, and otherwise draw frames as fast as possible. + */ + return 0; + } + + if (master_clock->prev_tick.tv_sec == 0) + { + /* If we weren't previously running, then draw the next frame + * immediately + */ + return 0; + } + + /* Otherwise, wait at least 1/frame_rate seconds since we last started a frame */ + + g_source_get_current_time (master_clock->source, &now); + + next = master_clock->prev_tick; + g_time_val_add (&next, 1000000 / clutter_get_default_frame_rate ()); + + if (next.tv_sec < now.tv_sec || + (next.tv_sec == now.tv_sec && next.tv_usec < now.tv_usec)) + { + return 0; + } + else + { + return ((next.tv_sec - now.tv_sec) * 1000 + + (next.tv_usec - now.tv_usec) / 1000); + } +} + /* * clutter_clock_source_new: * @master_clock: a #ClutterMasterClock for the source @@ -150,14 +209,13 @@ clutter_clock_prepare (GSource *source, { ClutterClockSource *clock_source = (ClutterClockSource *) source; ClutterMasterClock *master_clock = clock_source->master_clock; - gboolean retval; + int delay; - /* just like an idle source, we are ready if nothing else is */ - *timeout = -1; + delay = master_clock_next_frame_delay (master_clock); - retval = master_clock_is_running (master_clock); + *timeout = delay; - return retval; + return delay == 0; } static gboolean @@ -165,11 +223,11 @@ clutter_clock_check (GSource *source) { ClutterClockSource *clock_source = (ClutterClockSource *) source; ClutterMasterClock *master_clock = clock_source->master_clock; - gboolean retval; + int delay; - retval = master_clock_is_running (master_clock); + delay = master_clock_next_frame_delay (master_clock); - return retval; + return delay == 0; } static gboolean @@ -184,6 +242,10 @@ clutter_clock_dispatch (GSource *source, CLUTTER_NOTE (SCHEDULER, "Master clock [tick]"); + /* Get the time to use for this frame. + */ + g_source_get_current_time (source, &master_clock->cur_tick); + /* We need to protect ourselves against stages being destroyed during * event handling */ @@ -206,6 +268,8 @@ clutter_clock_dispatch (GSource *source, g_slist_foreach (stages, (GFunc)g_object_unref, NULL); g_slist_free (stages); + master_clock->prev_tick = master_clock->cur_tick; + return TRUE; } @@ -283,15 +347,7 @@ _clutter_master_clock_add_timeline (ClutterMasterClock *master_clock, timeline); if (is_first) - { - /* Start timing from scratch */ - master_clock->prev_tick.tv_sec = 0; - - /* If called from a different thread, we need to wake up the - * main loop to start running the timelines - */ - g_main_context_wakeup (NULL); - } + _clutter_master_clock_start_running (master_clock); } /* @@ -308,6 +364,24 @@ _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; +} + +/* + * _clutter_master_clock_start_running: + * @master_clock: a #ClutterMasterClock + * + * Called when we have events or redraws to process; if the clock + * is stopped, does the processing necessary to wake it up again. + */ +void +_clutter_master_clock_start_running (ClutterMasterClock *master_clock) +{ + /* If called from a different thread, we need to wake up the + * main loop to start running the timelines + */ + g_main_context_wakeup (NULL); } /* @@ -321,7 +395,6 @@ _clutter_master_clock_remove_timeline (ClutterMasterClock *master_clock, void _clutter_master_clock_advance (ClutterMasterClock *master_clock) { - GTimeVal cur_tick = { 0, }; gulong msecs; GSList *l; @@ -330,13 +403,18 @@ _clutter_master_clock_advance (ClutterMasterClock *master_clock) if (master_clock->timelines == NULL) return; - g_get_current_time (&cur_tick); + if (!master_clock->timelines_running) + { + /* 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; + } - if (master_clock->prev_tick.tv_sec == 0) - master_clock->prev_tick = cur_tick; - - msecs = (cur_tick.tv_sec - master_clock->prev_tick.tv_sec) * 1000 - + (cur_tick.tv_usec - master_clock->prev_tick.tv_usec) / 1000; + 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; @@ -352,9 +430,4 @@ _clutter_master_clock_advance (ClutterMasterClock *master_clock) if (clutter_timeline_is_playing (timeline)) clutter_timeline_advance_delta (timeline, msecs); } - - /* store the previous state so that we can use - * it for the next advancement - */ - master_clock->prev_tick = cur_tick; } diff --git a/clutter/clutter-master-clock.h b/clutter/clutter-master-clock.h index 5a3f7cd32..4bccefd8d 100644 --- a/clutter/clutter-master-clock.h +++ b/clutter/clutter-master-clock.h @@ -42,6 +42,8 @@ void _clutter_master_clock_add_timeline (ClutterMasterClock *m void _clutter_master_clock_remove_timeline (ClutterMasterClock *master_clock, ClutterTimeline *timeline); void _clutter_master_clock_advance (ClutterMasterClock *master_clock); +void _clutter_master_clock_start_running (ClutterMasterClock *master_clock); + G_END_DECLS diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 4f1826722..5a6d94c2c 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -396,13 +396,22 @@ _clutter_stage_queue_event (ClutterStage *stage, ClutterEvent *event) { ClutterStagePrivate *priv; + gboolean first_event; g_return_if_fail (CLUTTER_IS_STAGE (stage)); priv = stage->priv; + first_event = priv->event_queue->length == 0; + g_queue_push_tail (priv->event_queue, clutter_event_copy (event)); + + if (first_event) + { + ClutterMasterClock *master_clock = _clutter_master_clock_get_default (); + _clutter_master_clock_start_running (master_clock); + } } gboolean @@ -544,13 +553,12 @@ clutter_stage_real_queue_redraw (ClutterActor *actor, if (!priv->redraw_pending) { + ClutterMasterClock *master_clock; + priv->redraw_pending = TRUE; - /* If called from a thread, we need to wake up the main loop - * out of its sleep so the clock source notices that we have - * a redraw pending - */ - g_main_context_wakeup (NULL); + master_clock = _clutter_master_clock_get_default (); + _clutter_master_clock_start_running (master_clock); } else CLUTTER_CONTEXT ()->redraw_count += 1; diff --git a/tests/micro-bench/test-text-perf.c b/tests/micro-bench/test-text-perf.c index cd8d9056e..2be7f27fd 100644 --- a/tests/micro-bench/test-text-perf.c +++ b/tests/micro-bench/test-text-perf.c @@ -77,6 +77,7 @@ main (int argc, char *argv[]) int row, col; g_setenv ("CLUTTER_VBLANK", "none", FALSE); + g_setenv ("CLUTTER_DEFAULT_FPS", "1000", FALSE); clutter_init (&argc, &argv); diff --git a/tests/micro-bench/test-text.c b/tests/micro-bench/test-text.c index 480f63f3e..da86b2b3b 100644 --- a/tests/micro-bench/test-text.c +++ b/tests/micro-bench/test-text.c @@ -48,6 +48,7 @@ main (int argc, char *argv[]) ClutterActor *group; g_setenv ("CLUTTER_VBLANK", "none", FALSE); + g_setenv ("CLUTTER_DEFAULT_FPS", "1000", FALSE); clutter_init (&argc, &argv); From 6705ce6c6a8c7b490714a15c697c1ad24c9bf8b0 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 8 Jun 2009 13:00:09 -0400 Subject: [PATCH 10/22] 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; } From 287d4f76ecf0ccd2171eaf4a61c79ccea06e5f1d Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 8 Jun 2009 13:13:18 -0400 Subject: [PATCH 11/22] Remove useless manual timeline ticking The master clock now works fine whether or not there are any stages, so in the timeline conformance tests don't need to set up their own times. Set CLUTTER_VBLANK=none for the conformance tests, which in addition to removing an test-environment dependency, will result in the ticking for timeline tests being throttled to the default frame rate. http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- tests/conform/test-conform-main.c | 7 ++++ tests/conform/test-timeline-interpolate.c | 19 ---------- tests/conform/test-timeline-rewind.c | 19 ---------- tests/conform/test-timeline.c | 44 ----------------------- 4 files changed, 7 insertions(+), 82 deletions(-) diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c index b2aa07077..05e5e7b4c 100644 --- a/tests/conform/test-conform-main.c +++ b/tests/conform/test-conform-main.c @@ -72,6 +72,13 @@ main (int argc, char **argv) } #endif + /* Turning of sync-to-vblank removes a dependency on the specifics of the + * test environment. It also means that the timeline-only tests are + * throttled to a reasonable frame rate rather than running in tight + * infinite loop. + */ + g_setenv ("CLUTTER_VBLANK", "none", FALSE); + g_test_init (&argc, &argv, NULL); g_test_bug_base ("http://bugzilla.openedhand.com/show_bug.cgi?id=%s"); diff --git a/tests/conform/test-timeline-interpolate.c b/tests/conform/test-timeline-interpolate.c index 29fe9263f..2c6cc59ba 100644 --- a/tests/conform/test-timeline-interpolate.c +++ b/tests/conform/test-timeline-interpolate.c @@ -23,7 +23,6 @@ typedef struct _TestState gint expected_frame; gint completion_count; gboolean passed; - guint source_id; } TestState; @@ -132,20 +131,6 @@ completed_cb (ClutterTimeline *timeline, } } -static gboolean -frame_tick (gpointer data) -{ - TestState *state = data; - GTimeVal cur_tick = { 0, }; - - g_get_current_time (&cur_tick); - - if (clutter_timeline_is_playing (state->timeline)) - clutter_timeline_do_tick (state->timeline, &cur_tick); - - return TRUE; -} - void test_timeline_interpolate (TestConformSimpleFixture *fixture, gconstpointer data) @@ -169,14 +154,10 @@ test_timeline_interpolate (TestConformSimpleFixture *fixture, state.passed = TRUE; state.expected_frame = 0; - state.source_id = - clutter_threads_add_frame_source (60, frame_tick, &state); - g_get_current_time (&state.start_time); clutter_timeline_start (state.timeline); clutter_main(); - g_source_remove (state.source_id); g_object_unref (state.timeline); } diff --git a/tests/conform/test-timeline-rewind.c b/tests/conform/test-timeline-rewind.c index 67668c55f..b1e0838c5 100644 --- a/tests/conform/test-timeline-rewind.c +++ b/tests/conform/test-timeline-rewind.c @@ -11,7 +11,6 @@ typedef struct _TestState { ClutterTimeline *timeline; gint rewind_count; - guint source_id; } TestState; static gboolean @@ -67,20 +66,6 @@ new_frame_cb (ClutterTimeline *timeline, } } -static gboolean -frame_tick (gpointer data) -{ - TestState *state = data; - GTimeVal cur_tick = { 0, }; - - g_get_current_time (&cur_tick); - - if (clutter_timeline_is_playing (state->timeline)) - clutter_timeline_do_tick (state->timeline, &cur_tick); - - return TRUE; -} - void test_timeline_rewind (TestConformSimpleFixture *fixture, gconstpointer data) @@ -100,13 +85,9 @@ test_timeline_rewind (TestConformSimpleFixture *fixture, &state); state.rewind_count = 0; - state.source_id = - clutter_threads_add_frame_source (60, frame_tick, &state); - clutter_timeline_start (state.timeline); clutter_main(); - g_source_remove (state.source_id); g_object_unref (state.timeline); } diff --git a/tests/conform/test-timeline.c b/tests/conform/test-timeline.c index 0a1038e54..bc53b6b13 100644 --- a/tests/conform/test-timeline.c +++ b/tests/conform/test-timeline.c @@ -181,38 +181,10 @@ delay_cb (gpointer data) return TRUE; } -typedef struct _FrameCounter FrameCounter; - -struct _FrameCounter -{ - GSList *timelines; -}; - -static gboolean -frame_tick (gpointer data) -{ - FrameCounter *counter = data; - GTimeVal cur_tick = { 0, }; - GSList *l; - - g_get_current_time (&cur_tick); - - for (l = counter->timelines; l != NULL; l = l->next) - { - ClutterTimeline *timeline = l->data; - - if (clutter_timeline_is_playing (timeline)) - clutter_timeline_do_tick (timeline, &cur_tick); - } - - return TRUE; -} - void test_timeline (TestConformSimpleFixture *fixture, gconstpointer data) { - FrameCounter *counter; ClutterTimeline *timeline_1; TimelineData data_1; ClutterTimeline *timeline_2; @@ -222,11 +194,6 @@ test_timeline (TestConformSimpleFixture *fixture, gchar **markers; gsize n_markers; guint delay_tag; - guint source_id; - - counter = g_new0 (FrameCounter, 1); - - source_id = clutter_threads_add_frame_source (FPS, frame_tick, counter); timeline_data_init (&data_1, 1); timeline_1 = clutter_timeline_new (FRAME_COUNT * 1000 / FPS); @@ -243,8 +210,6 @@ test_timeline (TestConformSimpleFixture *fixture, g_assert (n_markers == 3); g_strfreev (markers); - counter->timelines = g_slist_prepend (counter->timelines, timeline_1); - timeline_data_init (&data_2, 2); timeline_2 = clutter_timeline_clone (timeline_1); clutter_timeline_add_marker_at_time (timeline_2, "bar", 2 * 1000 / FPS); @@ -254,8 +219,6 @@ test_timeline (TestConformSimpleFixture *fixture, g_assert (strcmp (markers[0], "bar") == 0); g_strfreev (markers); - counter->timelines = g_slist_prepend (counter->timelines, timeline_2); - timeline_data_init (&data_3, 3); timeline_3 = clutter_timeline_clone (timeline_1); clutter_timeline_set_direction (timeline_3, CLUTTER_TIMELINE_BACKWARD); @@ -266,8 +229,6 @@ test_timeline (TestConformSimpleFixture *fixture, clutter_timeline_add_marker_at_time (timeline_3, "end-marker", 0 * 1000 / FPS); - counter->timelines = g_slist_prepend (counter->timelines, timeline_3); - g_signal_connect (timeline_1, "marker-reached", G_CALLBACK (timeline_marker_reached_cb), &data_1); @@ -346,9 +307,4 @@ test_timeline (TestConformSimpleFixture *fixture, timeline_data_destroy (&data_3); g_source_remove (delay_tag); - - g_source_remove (source_id); - - g_slist_free (counter->timelines); - g_free (counter); } From 7099d251c664752ef72c076d2b9cc45b70d58624 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 9 Jun 2009 16:28:25 +0100 Subject: [PATCH 12/22] Run the repaint functions inside the redraw cycle Now that every redraw is performed within the master clock we need to run the repaint functions inside it. --- clutter/clutter-master-clock.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 08a5c744f..661a4987a 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -255,7 +255,8 @@ clutter_clock_dispatch (GSource *source, for (l = stages; l != NULL; l = l->next) _clutter_stage_process_queued_events (l->data); - _clutter_master_clock_advance (master_clock); + _clutter_master_clock_advance (master_clock); + _clutter_run_repaint_functions (); /* Update any stage that needs redraw/relayout after the clock * is advanced. From e1cac4fece275c0e8428942a880ae5e5f4b3b13b Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 9 Jun 2009 16:29:29 +0100 Subject: [PATCH 13/22] Fix clutter_redraw() to match the redraw cycle The clutter_redraw() function is used by embedding toolkits to force a redraw on a stage. Since everything is performed by toggling a flag inside the Stage itself and then letting the master clock advance, we need a ClutterStage method to ensure that we start the master clock and redraw. --- clutter/clutter-main.c | 13 ++--------- clutter/clutter-stage.c | 27 ++++++++++++++++++++++ clutter/clutter-stage.h | 1 + doc/reference/clutter/clutter-sections.txt | 1 + 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index a11d7666c..9fdd59f36 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -236,18 +236,9 @@ _clutter_do_redraw (ClutterStage *stage) void clutter_redraw (ClutterStage *stage) { - ClutterMasterClock *master_clock; + g_return_if_fail (CLUTTER_IS_STAGE (stage)); - /* we need to do this because the clutter_redraw() might be called by - * clutter-gtk, and this will not cause the master clock to advance nor - * the repaint functions to be run - */ - master_clock = _clutter_master_clock_get_default (); - _clutter_master_clock_advance (master_clock); - - _clutter_run_repaint_functions (); - - _clutter_do_redraw (stage); + clutter_stage_ensure_redraw (stage); } /** diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 5a6d94c2c..2627169e1 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -1946,6 +1946,33 @@ clutter_stage_ensure_viewport (ClutterStage *stage) clutter_actor_queue_redraw (CLUTTER_ACTOR (stage)); } +/** + * clutter_stage_ensure_redraw: + * @stage: a #ClutterStage + * + * Ensures that @stage is redrawn + * + * This function should not be called by applications: it is + * used when embedding a #ClutterStage into a toolkit with + * another windowing system, like GTK+. + * + * Since: 1.0 + */ +void +clutter_stage_ensure_redraw (ClutterStage *stage) +{ + ClutterMasterClock *master_clock; + ClutterStagePrivate *priv; + + g_return_if_fail (CLUTTER_IS_STAGE (stage)); + + priv = stage->priv; + priv->redraw_pending = TRUE; + + master_clock = _clutter_master_clock_get_default (); + _clutter_master_clock_start_running (master_clock); +} + /** * clutter_stage_queue_redraw: * @stage: the #ClutterStage diff --git a/clutter/clutter-stage.h b/clutter/clutter-stage.h index 2b7e836bb..380ee85d0 100644 --- a/clutter/clutter-stage.h +++ b/clutter/clutter-stage.h @@ -243,6 +243,7 @@ void clutter_stage_ensure_current (ClutterStage *stage); void clutter_stage_queue_redraw (ClutterStage *stage); gboolean clutter_stage_is_default (ClutterStage *stage); void clutter_stage_ensure_viewport (ClutterStage *stage); +void clutter_stage_ensure_redraw (ClutterStage *stage); /* Commodity macro */ #define clutter_stage_add(stage,actor) G_STMT_START { \ diff --git a/doc/reference/clutter/clutter-sections.txt b/doc/reference/clutter/clutter-sections.txt index 10d971b41..b5f65e08b 100644 --- a/doc/reference/clutter/clutter-sections.txt +++ b/doc/reference/clutter/clutter-sections.txt @@ -485,6 +485,7 @@ ClutterPickMode clutter_stage_get_actor_at_pos clutter_stage_ensure_current clutter_stage_ensure_viewport +clutter_stage_ensure_redraw clutter_stage_queue_redraw clutter_stage_event clutter_stage_set_key_focus From 0f9dea0337aadc5c7218c93a592d83ae4828d6b4 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 10 Jun 2009 14:52:53 +0100 Subject: [PATCH 14/22] Force a paint instead of calling clutter_redraw() We do not need the whole redraw machinery inside clutter_stage_read_pixels(): instead, we want Clutter to drop everything, paint and call glReadPixels() with the current buffer. --- clutter/clutter-stage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 2627169e1..7fdb98164 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -1372,8 +1372,8 @@ clutter_stage_read_pixels (ClutterStage *stage, g_return_val_if_fail (x >= 0 && y >= 0, NULL); /* Force a redraw of the stage before reading back pixels */ - clutter_redraw (stage); clutter_stage_ensure_current (stage); + clutter_actor_paint (CLUTTER_ACTOR (stage)); glGetIntegerv (GL_VIEWPORT, viewport); stage_x = viewport[0]; From 7c08f554bc403938d90363fcba552d5121ad1449 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 10 Jun 2009 14:54:19 +0100 Subject: [PATCH 15/22] [docs] Update Clutter's API reference --- doc/reference/clutter/clutter-sections.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/doc/reference/clutter/clutter-sections.txt b/doc/reference/clutter/clutter-sections.txt index b5f65e08b..c16577e48 100644 --- a/doc/reference/clutter/clutter-sections.txt +++ b/doc/reference/clutter/clutter-sections.txt @@ -577,7 +577,7 @@ CLUTTER_TIMELINE_GET_CLASS ClutterTimelinePrivate clutter_timeline_get_type -clutter_timeline_advance_delta +clutter_timeline_do_tick
@@ -969,7 +969,6 @@ clutter_event_get_type clutter-main General CLUTTER_PRIORITY_REDRAW -CLUTTER_PRIORITY_TIMELINE ClutterInitError @@ -992,8 +991,6 @@ clutter_set_default_frame_rate clutter_get_default_frame_rate clutter_set_motion_events_enabled clutter_get_motion_events_enabled -clutter_set_motion_events_frequency -clutter_get_motion_events_frequency clutter_clear_glyph_cache ClutterFontFlags clutter_set_font_flags From acf7722a41dc5c568f895166cc471f53f3546926 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Wed, 10 Jun 2009 15:50:27 +0100 Subject: [PATCH 16/22] [master-clock] Throttle if no redraw was performed A flag in the master clock is now set whenever the dispatch caused an actual redraw of a stage. If this flag is not set during the prepare and check functions then it will resort to limiting the redraw attempts to the default frame rate as if vblank syncing was disabled. Otherwise if a timeline is running that does not cause the scene to change then it would busy-wait with 100% CPU until the next frame. This fix was suggested by Owen Taylor in: http://bugzilla.openedhand.com/show_bug.cgi?id=1637 Signed-off-by: Emmanuele Bassi --- clutter/clutter-master-clock.c | 11 +++++++++-- clutter/clutter-private.h | 2 +- clutter/clutter-stage.c | 12 ++++++++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c index 661a4987a..2d3c87f73 100644 --- a/clutter/clutter-master-clock.c +++ b/clutter/clutter-master-clock.c @@ -66,6 +66,8 @@ struct _ClutterMasterClock * a redraw on the stage and drive the animations */ GSource *source; + + guint updated_stages : 1; }; struct _ClutterMasterClockClass @@ -144,7 +146,8 @@ master_clock_next_frame_delay (ClutterMasterClock *master_clock) if (!master_clock_is_running (master_clock)) return -1; - if (clutter_feature_available (CLUTTER_FEATURE_SYNC_TO_VBLANK)) + if (clutter_feature_available (CLUTTER_FEATURE_SYNC_TO_VBLANK) && + master_clock->updated_stages) { /* When we have sync-to-vblank, we count on that to throttle * our frame rate, and otherwise draw frames as fast as possible. @@ -250,6 +253,8 @@ clutter_clock_dispatch (GSource *source, stages = clutter_stage_manager_list_stages (stage_manager); g_slist_foreach (stages, (GFunc)g_object_ref, NULL); + master_clock->updated_stages = FALSE; + /* Process queued events */ for (l = stages; l != NULL; l = l->next) @@ -262,7 +267,7 @@ clutter_clock_dispatch (GSource *source, * is advanced. */ for (l = stages; l != NULL; l = l->next) - _clutter_stage_do_update (l->data); + master_clock->updated_stages |= _clutter_stage_do_update (l->data); g_slist_foreach (stages, (GFunc)g_object_unref, NULL); g_slist_free (stages); @@ -298,6 +303,8 @@ clutter_master_clock_init (ClutterMasterClock *self) source = clutter_clock_source_new (self); self->source = source; + self->updated_stages = TRUE; + g_source_set_priority (source, CLUTTER_PRIORITY_REDRAW); g_source_set_can_recurse (source, FALSE); g_source_attach (source, NULL); diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h index 340cc75c6..0b96018b0 100644 --- a/clutter/clutter-private.h +++ b/clutter/clutter-private.h @@ -174,7 +174,7 @@ ClutterStageWindow *_clutter_stage_get_default_window (void); void _clutter_stage_maybe_setup_viewport (ClutterStage *stage); void _clutter_stage_maybe_relayout (ClutterActor *stage); gboolean _clutter_stage_needs_update (ClutterStage *stage); -void _clutter_stage_do_update (ClutterStage *stage); +gboolean _clutter_stage_do_update (ClutterStage *stage); void _clutter_stage_queue_event (ClutterStage *stage, ClutterEvent *event); diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 7fdb98164..746d49468 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -486,7 +486,7 @@ _clutter_stage_process_queued_events (ClutterStage *stage) * * Determines if _clutter_stage_do_update() needs to be called. * - * Return value: %TRUE if the stages need layout or painting + * Return value: %TRUE if the stage need layout or painting */ gboolean _clutter_stage_needs_update (ClutterStage *stage) @@ -505,18 +505,20 @@ _clutter_stage_needs_update (ClutterStage *stage) * @stage: A #ClutterStage * * Handles per-frame layout and repaint for the stage. + * + * Return value: %TRUE if the stage was updated */ -void +gboolean _clutter_stage_do_update (ClutterStage *stage) { ClutterStagePrivate *priv; - g_return_if_fail (CLUTTER_IS_STAGE (stage)); + g_return_val_if_fail (CLUTTER_IS_STAGE (stage), FALSE); priv = stage->priv; if (!priv->redraw_pending) - return; + return FALSE; /* clutter_do_redraw() will also call maybe_relayout(), but since a relayout * can queue a redraw, we want to do the relayout before we clear the @@ -539,6 +541,8 @@ _clutter_stage_do_update (ClutterStage *stage) CLUTTER_CONTEXT ()->redraw_count = 0; } + + return TRUE; } static void From 9c5663d6717722d6d77162060f284355b5081174 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Wed, 10 Jun 2009 17:41:16 +0100 Subject: [PATCH 17/22] [timeline] Don't clamp the elapsed time when a looping tl reaches the end The new-frame signal of a timeline was previously guaranteed to be emitted with the elapsed_time set to the end before it emits the completed signal. This doesn't necessarily make sense for looping timelines because it would cause the elapsed time to be clamped to a slightly off value whenever the timeline restarts. This patch makes it perform the wrap around before emitting the new-frame signal so that the elapsed time always corresponds to the time elapsed since the timeline was started. Additionally it no longer fudges the msecs_delta property to make the marker check work so clutter_timeline_get_delta will always return the wall clock time since the last frame. --- clutter/clutter-timeline.c | 144 ++++++++++++++-------- tests/conform/test-timeline-interpolate.c | 8 +- 2 files changed, 97 insertions(+), 55 deletions(-) diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index 2d68bc5d1..d10e9ec6e 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -430,22 +430,76 @@ clutter_timeline_init (ClutterTimeline *self) priv->elapsed_time = 0; } -static void -check_if_marker_hit (const gchar *name, - TimelineMarker *marker, - ClutterTimeline *timeline) +static gboolean +have_passed_time (ClutterTimeline *timeline, + gint new_time, + gint msecs) { ClutterTimelinePrivate *priv = timeline->priv; - if (priv->direction == CLUTTER_TIMELINE_FORWARD - ? (marker->msecs > priv->elapsed_time - priv->msecs_delta - && marker->msecs <= priv->elapsed_time) - : (marker->msecs >= priv->elapsed_time - && marker->msecs < priv->elapsed_time + priv->msecs_delta)) + /* Ignore markers that are outside the duration of the timeline */ + if (msecs < 0 || msecs > priv->duration) + return FALSE; + + if (priv->direction == CLUTTER_TIMELINE_FORWARD) + { + /* We need to special case when a marker is added at the + beginning of the timeline */ + if (msecs == 0 && + priv->msecs_delta > 0 && + new_time - priv->msecs_delta <= 0) + return TRUE; + + /* If the timeline is looping then we need to check for wrap + around */ + if (priv->loop && new_time >= priv->duration) + return (msecs > new_time - priv->msecs_delta || + msecs <= new_time % priv->duration); + + /* Otherwise it's just a simple test if the time is in range of + the previous time and the new time */ + return (msecs > new_time - priv->msecs_delta + && msecs <= new_time); + } + else + { + /* We need to special case when a marker is added at the + end of the timeline */ + if (msecs == priv->duration && + priv->msecs_delta > 0 && + new_time + priv->msecs_delta >= priv->duration) + return TRUE; + + /* If the timeline is looping then we need to check for wrap + around */ + if (priv->loop && new_time <= 0) + return (msecs < new_time + priv->msecs_delta || + msecs >= (priv->duration - + (-new_time % priv->duration))); + + /* Otherwise it's just a simple test if the time is in range of + the previous time and the new time */ + return (msecs >= new_time + && msecs < new_time + priv->msecs_delta); + } +} + +struct CheckIfMarkerHitClosure +{ + ClutterTimeline *timeline; + gint new_time; +}; + +static void +check_if_marker_hit (const gchar *name, + TimelineMarker *marker, + struct CheckIfMarkerHitClosure *data) +{ + if (have_passed_time (data->timeline, data->new_time, marker->msecs)) { CLUTTER_NOTE (SCHEDULER, "Marker '%s' reached", name); - g_signal_emit (timeline, timeline_signals[MARKER_REACHED], + g_signal_emit (data->timeline, timeline_signals[MARKER_REACHED], marker->quark, name, marker->msecs); @@ -453,10 +507,12 @@ check_if_marker_hit (const gchar *name, } static void -emit_frame_signal (ClutterTimeline *timeline) +emit_frame_signal (ClutterTimeline *timeline, gint new_time) { ClutterTimelinePrivate *priv = timeline->priv; + struct CheckIfMarkerHitClosure data; + /* Emit the signal */ g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, priv->elapsed_time); @@ -464,9 +520,12 @@ emit_frame_signal (ClutterTimeline *timeline) if (priv->markers_by_name == NULL) return; + data.timeline = timeline; + data.new_time = new_time; + g_hash_table_foreach (priv->markers_by_name, (GHFunc) check_if_marker_hit, - timeline); + &data); } static gboolean @@ -527,7 +586,7 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) if (!is_complete (timeline)) { /* Emit the signal */ - emit_frame_signal (timeline); + emit_frame_signal (timeline, priv->elapsed_time); /* Signal pauses timeline ? */ if (!priv->is_playing) @@ -542,29 +601,31 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) else { /* Handle loop or stop */ - ClutterTimelineDirection saved_direction = priv->direction; - guint overflow_msecs = priv->elapsed_time; gint end_msecs; + gint overflow_msecs = priv->elapsed_time; /* Update the current elapsed time in case the signal handlers - * want to take a peek. If we clamp elapsed time, then we need - * to correpondingly reduce msecs_delta to reflect the correct - * range of times */ - if (priv->direction == CLUTTER_TIMELINE_FORWARD) - { - priv->msecs_delta -= (priv->elapsed_time - priv->duration); - priv->elapsed_time = priv->duration; - } + * want to take a peek */ + if (priv->loop) + { + /* We try and interpolate smoothly around a loop */ + if (priv->direction == CLUTTER_TIMELINE_FORWARD) + priv->elapsed_time = priv->elapsed_time % priv->duration; + else + priv->elapsed_time = (priv->duration - + (-priv->elapsed_time % priv->duration)); + } + else if (priv->direction == CLUTTER_TIMELINE_FORWARD) + priv->elapsed_time = priv->duration; else if (priv->direction == CLUTTER_TIMELINE_BACKWARD) - { - priv->msecs_delta -= - priv->elapsed_time; - priv->elapsed_time = 0; - } + priv->elapsed_time = 0; end_msecs = priv->elapsed_time; - /* Emit the signal */ - emit_frame_signal (timeline); + /* Check if the markers have been hit using the old value of + elapsed time so it will use the right value for the previous + elapsed time */ + emit_frame_signal (timeline, overflow_msecs); /* Did the signal handler modify the elapsed time? */ if (priv->elapsed_time != end_msecs) @@ -609,28 +670,11 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) return TRUE; } - if (priv->loop) - { - /* We try and interpolate smoothly around a loop */ - if (saved_direction == CLUTTER_TIMELINE_FORWARD) - priv->elapsed_time = overflow_msecs - priv->duration; - else - priv->elapsed_time = priv->duration + overflow_msecs; + if (!priv->loop) + clutter_timeline_rewind (timeline); - /* Or if the direction changed, we try and bounce */ - if (priv->direction != saved_direction) - priv->elapsed_time = priv->duration - priv->elapsed_time; - - g_object_unref (timeline); - return TRUE; - } - else - { - clutter_timeline_rewind (timeline); - - g_object_unref (timeline); - return FALSE; - } + g_object_unref (timeline); + return priv->loop; } } diff --git a/tests/conform/test-timeline-interpolate.c b/tests/conform/test-timeline-interpolate.c index 2c6cc59ba..3dde67a1b 100644 --- a/tests/conform/test-timeline-interpolate.c +++ b/tests/conform/test-timeline-interpolate.c @@ -46,14 +46,12 @@ new_frame_cb (ClutterTimeline *timeline, /* If we expect to have interpolated past the end of the timeline * we keep track of the overflow so we can determine when - * the next timeout will happen. We then clip expected_frames - * to TEST_TIMELINE_DURATION since clutter-timeline - * semantics guaranty this frame is always signaled before - * looping */ + * the next timeout will happen. We then wrap expected_frames + * around TEST_TIMELINE_DURATION */ if (state->expected_frame > TEST_TIMELINE_DURATION) { loop_overflow = state->expected_frame - TEST_TIMELINE_DURATION; - state->expected_frame = TEST_TIMELINE_DURATION; + state->expected_frame %= TEST_TIMELINE_DURATION; } if (current_frame >= (state->expected_frame-TEST_ERROR_TOLERANCE) From 19cbb307831234e5cfeb473802db77eb56328013 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 11 Jun 2009 11:23:45 +0100 Subject: [PATCH 18/22] [test-timeline] Add a marker at the beginning of the timeline The beginning of the timeline needs special treatment to detect a marker so it should have one in the conformance test. --- tests/conform/test-timeline.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/conform/test-timeline.c b/tests/conform/test-timeline.c index bc53b6b13..6a927cb50 100644 --- a/tests/conform/test-timeline.c +++ b/tests/conform/test-timeline.c @@ -197,6 +197,8 @@ test_timeline (TestConformSimpleFixture *fixture, timeline_data_init (&data_1, 1); timeline_1 = clutter_timeline_new (FRAME_COUNT * 1000 / FPS); + clutter_timeline_add_marker_at_time (timeline_1, "start-marker", + 0 * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_1, "foo", 5 * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_1, "bar", 5 * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_1, "baz", 5 * 1000 / FPS); @@ -222,6 +224,8 @@ test_timeline (TestConformSimpleFixture *fixture, timeline_data_init (&data_3, 3); timeline_3 = clutter_timeline_clone (timeline_1); clutter_timeline_set_direction (timeline_3, CLUTTER_TIMELINE_BACKWARD); + clutter_timeline_add_marker_at_time (timeline_3, "start-marker", + FRAME_COUNT * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_3, "foo", 5 * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_3, "baz", 8 * 1000 / FPS); clutter_timeline_add_marker_at_time (timeline_3, "near-end-marker", From 6bb5b3a13e6dfe1934c9d2476f00ac2be86d33cd Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 11 Jun 2009 11:38:49 +0100 Subject: [PATCH 19/22] [tests] Don't add a newline to the end of g_test_message calls Two of the timeline tests were calling g_test_message and adding a \n so the output looked odd. --- tests/conform/test-timeline-interpolate.c | 14 +++++++------- tests/conform/test-timeline-rewind.c | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/conform/test-timeline-interpolate.c b/tests/conform/test-timeline-interpolate.c index 3dde67a1b..ec2f868b7 100644 --- a/tests/conform/test-timeline-interpolate.c +++ b/tests/conform/test-timeline-interpolate.c @@ -57,16 +57,16 @@ new_frame_cb (ClutterTimeline *timeline, if (current_frame >= (state->expected_frame-TEST_ERROR_TOLERANCE) && current_frame <= (state->expected_frame+TEST_ERROR_TOLERANCE)) { - g_test_message ("\nelapsed milliseconds=%-5li " - "expected frame=%-4i actual frame=%-4i (OK)\n", + g_test_message ("elapsed milliseconds=%-5li " + "expected frame=%-4i actual frame=%-4i (OK)", msec_diff, state->expected_frame, current_frame); } else { - g_test_message ("\nelapsed milliseconds=%-5li " - "expected frame=%-4i actual frame=%-4i (FAILED)\n", + g_test_message ("elapsed milliseconds=%-5li " + "expected frame=%-4i actual frame=%-4i (FAILED)", msec_diff, state->expected_frame, current_frame); @@ -77,7 +77,7 @@ new_frame_cb (ClutterTimeline *timeline, { state->expected_frame = current_frame + (TEST_TIMELINE_FPS / 4); g_test_message ("Sleeping for 250ms " - "so next frame should be (%i + %i) = %i\n", + "so next frame should be (%i + %i) = %i", current_frame, (TEST_TIMELINE_FPS / 4), state->expected_frame); @@ -87,7 +87,7 @@ new_frame_cb (ClutterTimeline *timeline, { state->expected_frame = current_frame + TEST_TIMELINE_FPS; g_test_message ("Sleeping for 1sec " - "so next frame should be (%i + %i) = %i\n", + "so next frame should be (%i + %i) = %i", current_frame, TEST_TIMELINE_FPS, state->expected_frame); @@ -99,7 +99,7 @@ new_frame_cb (ClutterTimeline *timeline, state->expected_frame += loop_overflow; state->expected_frame -= TEST_TIMELINE_DURATION; g_test_message ("End of timeline reached: " - "Wrapping expected frame too %i\n", + "Wrapping expected frame too %i", state->expected_frame); } diff --git a/tests/conform/test-timeline-rewind.c b/tests/conform/test-timeline-rewind.c index b1e0838c5..86b8f1ada 100644 --- a/tests/conform/test-timeline-rewind.c +++ b/tests/conform/test-timeline-rewind.c @@ -16,17 +16,17 @@ typedef struct _TestState static gboolean watchdog_timeout (TestState *state) { - g_test_message ("Watchdog timer kicking in\n"); - g_test_message ("rewind_count=%i\n", state->rewind_count); + g_test_message ("Watchdog timer kicking in"); + g_test_message ("rewind_count=%i", state->rewind_count); if (state->rewind_count <= 3) { /* The test has hung */ - g_test_message ("Failed (This test shouldn't have hung!)\n"); + g_test_message ("Failed (This test shouldn't have hung!)"); exit (EXIT_FAILURE); } else { - g_test_message ("Passed\n"); + g_test_message ("Passed"); clutter_main_quit (); } @@ -42,8 +42,8 @@ new_frame_cb (ClutterTimeline *timeline, if (elapsed_time == TEST_TIMELINE_DURATION) { - g_test_message ("new-frame signal recieved (end of timeline)\n"); - g_test_message ("Rewinding timeline\n"); + g_test_message ("new-frame signal received (end of timeline)"); + g_test_message ("Rewinding timeline"); clutter_timeline_rewind (timeline); state->rewind_count++; } @@ -51,16 +51,16 @@ new_frame_cb (ClutterTimeline *timeline, { if (elapsed_time == 0) { - g_test_message ("new-frame signal recieved (start of timeline)\n"); + g_test_message ("new-frame signal received (start of timeline)"); } else { - g_test_message ("new-frame signal recieved (mid frame)\n"); + g_test_message ("new-frame signal received (mid frame)"); } if (state->rewind_count >= 2) { - g_test_message ("Sleeping for 1 second\n"); + g_test_message ("Sleeping for 1 second"); g_usleep (1000000); } } @@ -79,7 +79,7 @@ test_timeline_rewind (TestConformSimpleFixture *fixture, G_CALLBACK(new_frame_cb), &state); g_test_message ("Installing a watchdog timeout " - "to determine if this test hangs\n"); + "to determine if this test hangs"); g_timeout_add (TEST_WATCHDOG_KICK_IN_SECONDS*1000, (GSourceFunc)watchdog_timeout, &state); From 9021aa2909189becd9809d29ebf8c9a3c362ce5f Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 11 Jun 2009 10:55:52 +0100 Subject: [PATCH 20/22] Revert "[timeline] Don't clamp the elapsed time when a looping tl reaches the end" This reverts commit 9c5663d6717722d6d77162060f284355b5081174. The patch was causing problems for applications that expect the elapsed_time to be at either end of the timeline when the completed signal is fired. For example test-behave swaps the direction of the timeline in the completed handler but if the time has overflowed the end then the timeline would only take a short time to get back the beginning. This caused the animation to just vibrate around the beginning. --- clutter/clutter-timeline.c | 142 ++++++++-------------- tests/conform/test-timeline-interpolate.c | 8 +- 2 files changed, 54 insertions(+), 96 deletions(-) diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index d10e9ec6e..2d68bc5d1 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -430,76 +430,22 @@ clutter_timeline_init (ClutterTimeline *self) priv->elapsed_time = 0; } -static gboolean -have_passed_time (ClutterTimeline *timeline, - gint new_time, - gint msecs) -{ - ClutterTimelinePrivate *priv = timeline->priv; - - /* Ignore markers that are outside the duration of the timeline */ - if (msecs < 0 || msecs > priv->duration) - return FALSE; - - if (priv->direction == CLUTTER_TIMELINE_FORWARD) - { - /* We need to special case when a marker is added at the - beginning of the timeline */ - if (msecs == 0 && - priv->msecs_delta > 0 && - new_time - priv->msecs_delta <= 0) - return TRUE; - - /* If the timeline is looping then we need to check for wrap - around */ - if (priv->loop && new_time >= priv->duration) - return (msecs > new_time - priv->msecs_delta || - msecs <= new_time % priv->duration); - - /* Otherwise it's just a simple test if the time is in range of - the previous time and the new time */ - return (msecs > new_time - priv->msecs_delta - && msecs <= new_time); - } - else - { - /* We need to special case when a marker is added at the - end of the timeline */ - if (msecs == priv->duration && - priv->msecs_delta > 0 && - new_time + priv->msecs_delta >= priv->duration) - return TRUE; - - /* If the timeline is looping then we need to check for wrap - around */ - if (priv->loop && new_time <= 0) - return (msecs < new_time + priv->msecs_delta || - msecs >= (priv->duration - - (-new_time % priv->duration))); - - /* Otherwise it's just a simple test if the time is in range of - the previous time and the new time */ - return (msecs >= new_time - && msecs < new_time + priv->msecs_delta); - } -} - -struct CheckIfMarkerHitClosure -{ - ClutterTimeline *timeline; - gint new_time; -}; - static void check_if_marker_hit (const gchar *name, TimelineMarker *marker, - struct CheckIfMarkerHitClosure *data) + ClutterTimeline *timeline) { - if (have_passed_time (data->timeline, data->new_time, marker->msecs)) + ClutterTimelinePrivate *priv = timeline->priv; + + if (priv->direction == CLUTTER_TIMELINE_FORWARD + ? (marker->msecs > priv->elapsed_time - priv->msecs_delta + && marker->msecs <= priv->elapsed_time) + : (marker->msecs >= priv->elapsed_time + && marker->msecs < priv->elapsed_time + priv->msecs_delta)) { CLUTTER_NOTE (SCHEDULER, "Marker '%s' reached", name); - g_signal_emit (data->timeline, timeline_signals[MARKER_REACHED], + g_signal_emit (timeline, timeline_signals[MARKER_REACHED], marker->quark, name, marker->msecs); @@ -507,12 +453,10 @@ check_if_marker_hit (const gchar *name, } static void -emit_frame_signal (ClutterTimeline *timeline, gint new_time) +emit_frame_signal (ClutterTimeline *timeline) { ClutterTimelinePrivate *priv = timeline->priv; - struct CheckIfMarkerHitClosure data; - /* Emit the signal */ g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, priv->elapsed_time); @@ -520,12 +464,9 @@ emit_frame_signal (ClutterTimeline *timeline, gint new_time) if (priv->markers_by_name == NULL) return; - data.timeline = timeline; - data.new_time = new_time; - g_hash_table_foreach (priv->markers_by_name, (GHFunc) check_if_marker_hit, - &data); + timeline); } static gboolean @@ -586,7 +527,7 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) if (!is_complete (timeline)) { /* Emit the signal */ - emit_frame_signal (timeline, priv->elapsed_time); + emit_frame_signal (timeline); /* Signal pauses timeline ? */ if (!priv->is_playing) @@ -601,31 +542,29 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) else { /* Handle loop or stop */ + ClutterTimelineDirection saved_direction = priv->direction; + guint overflow_msecs = priv->elapsed_time; gint end_msecs; - gint overflow_msecs = priv->elapsed_time; /* Update the current elapsed time in case the signal handlers - * want to take a peek */ - if (priv->loop) - { - /* We try and interpolate smoothly around a loop */ - if (priv->direction == CLUTTER_TIMELINE_FORWARD) - priv->elapsed_time = priv->elapsed_time % priv->duration; - else - priv->elapsed_time = (priv->duration - - (-priv->elapsed_time % priv->duration)); - } - else if (priv->direction == CLUTTER_TIMELINE_FORWARD) - priv->elapsed_time = priv->duration; + * want to take a peek. If we clamp elapsed time, then we need + * to correpondingly reduce msecs_delta to reflect the correct + * range of times */ + if (priv->direction == CLUTTER_TIMELINE_FORWARD) + { + priv->msecs_delta -= (priv->elapsed_time - priv->duration); + priv->elapsed_time = priv->duration; + } else if (priv->direction == CLUTTER_TIMELINE_BACKWARD) - priv->elapsed_time = 0; + { + priv->msecs_delta -= - priv->elapsed_time; + priv->elapsed_time = 0; + } end_msecs = priv->elapsed_time; - /* Check if the markers have been hit using the old value of - elapsed time so it will use the right value for the previous - elapsed time */ - emit_frame_signal (timeline, overflow_msecs); + /* Emit the signal */ + emit_frame_signal (timeline); /* Did the signal handler modify the elapsed time? */ if (priv->elapsed_time != end_msecs) @@ -670,11 +609,28 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) return TRUE; } - if (!priv->loop) - clutter_timeline_rewind (timeline); + if (priv->loop) + { + /* We try and interpolate smoothly around a loop */ + if (saved_direction == CLUTTER_TIMELINE_FORWARD) + priv->elapsed_time = overflow_msecs - priv->duration; + else + priv->elapsed_time = priv->duration + overflow_msecs; - g_object_unref (timeline); - return priv->loop; + /* Or if the direction changed, we try and bounce */ + if (priv->direction != saved_direction) + priv->elapsed_time = priv->duration - priv->elapsed_time; + + g_object_unref (timeline); + return TRUE; + } + else + { + clutter_timeline_rewind (timeline); + + g_object_unref (timeline); + return FALSE; + } } } diff --git a/tests/conform/test-timeline-interpolate.c b/tests/conform/test-timeline-interpolate.c index ec2f868b7..bc01d91b1 100644 --- a/tests/conform/test-timeline-interpolate.c +++ b/tests/conform/test-timeline-interpolate.c @@ -46,12 +46,14 @@ new_frame_cb (ClutterTimeline *timeline, /* If we expect to have interpolated past the end of the timeline * we keep track of the overflow so we can determine when - * the next timeout will happen. We then wrap expected_frames - * around TEST_TIMELINE_DURATION */ + * the next timeout will happen. We then clip expected_frames + * to TEST_TIMELINE_DURATION since clutter-timeline + * semantics guaranty this frame is always signaled before + * looping */ if (state->expected_frame > TEST_TIMELINE_DURATION) { loop_overflow = state->expected_frame - TEST_TIMELINE_DURATION; - state->expected_frame %= TEST_TIMELINE_DURATION; + state->expected_frame = TEST_TIMELINE_DURATION; } if (current_frame >= (state->expected_frame-TEST_ERROR_TOLERANCE) From f1000db3caa450e4917b48d3c4f69bea5a255197 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 11 Jun 2009 12:14:53 +0100 Subject: [PATCH 21/22] [timelines] Improve marker hit check and don't fudge the delta Markers added at the start of the timeline need to be special cased because we effectively check whether the marker time is greater than the old frame time or less than or equal to the new frame time but we never actually emit a frame for the start of the timeline. If the timeline is looping then it adjusts the position to interpolate a wrapped around position. However we do not emit a new frame after setting this position so we need to check for markers again there. clutter_timeline_get_delta should return the actual wall clock time between emissions of the new-frame signal - even if the elapsed time has been fudged at the end of the timeline. To make this work it no longer directly manipulates priv->msecs_delta but instead passes a delta value to check_markers. --- clutter/clutter-timeline.c | 108 ++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 20 deletions(-) diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index 2d68bc5d1..98cff0b43 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -430,28 +430,93 @@ clutter_timeline_init (ClutterTimeline *self) priv->elapsed_time = 0; } +struct CheckIfMarkerHitClosure +{ + ClutterTimeline *timeline; + ClutterTimelineDirection direction; + gint new_time; + gint duration; + gint delta; +}; + +static gboolean +have_passed_time (const struct CheckIfMarkerHitClosure *data, + gint msecs) +{ + /* Ignore markers that are outside the duration of the timeline */ + if (msecs < 0 || msecs > data->duration) + return FALSE; + + if (data->direction == CLUTTER_TIMELINE_FORWARD) + { + /* We need to special case when a marker is added at the + beginning of the timeline */ + if (msecs == 0 && + data->delta > 0 && + data->new_time - data->delta <= 0) + return TRUE; + + /* Otherwise it's just a simple test if the time is in range of + the previous time and the new time */ + return (msecs > data->new_time - data->delta + && msecs <= data->new_time); + } + else + { + /* We need to special case when a marker is added at the + end of the timeline */ + if (msecs == data->duration && + data->delta > 0 && + data->new_time + data->delta >= data->duration) + return TRUE; + + /* Otherwise it's just a simple test if the time is in range of + the previous time and the new time */ + return (msecs >= data->new_time + && msecs < data->new_time + data->delta); + } +} + static void check_if_marker_hit (const gchar *name, TimelineMarker *marker, - ClutterTimeline *timeline) + struct CheckIfMarkerHitClosure *data) { - ClutterTimelinePrivate *priv = timeline->priv; - - if (priv->direction == CLUTTER_TIMELINE_FORWARD - ? (marker->msecs > priv->elapsed_time - priv->msecs_delta - && marker->msecs <= priv->elapsed_time) - : (marker->msecs >= priv->elapsed_time - && marker->msecs < priv->elapsed_time + priv->msecs_delta)) + if (have_passed_time (data, marker->msecs)) { CLUTTER_NOTE (SCHEDULER, "Marker '%s' reached", name); - g_signal_emit (timeline, timeline_signals[MARKER_REACHED], + g_signal_emit (data->timeline, timeline_signals[MARKER_REACHED], marker->quark, name, marker->msecs); } } +static void +check_markers (ClutterTimeline *timeline, + gint delta) +{ + ClutterTimelinePrivate *priv = timeline->priv; + struct CheckIfMarkerHitClosure data; + + /* shortcircuit here if we don't have any marker installed */ + if (priv->markers_by_name == NULL) + return; + + /* store the details of the timeline so that changing them in a + marker signal handler won't affect which markers are hit */ + data.timeline = timeline; + data.direction = priv->direction; + data.new_time = priv->elapsed_time; + data.duration = priv->duration; + data.delta = delta; + + g_hash_table_foreach (priv->markers_by_name, + (GHFunc) check_if_marker_hit, + &data); +} + static void emit_frame_signal (ClutterTimeline *timeline) { @@ -459,14 +524,6 @@ emit_frame_signal (ClutterTimeline *timeline) g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, priv->elapsed_time); - - /* shortcircuit here if we don't have any marker installed */ - if (priv->markers_by_name == NULL) - return; - - g_hash_table_foreach (priv->markers_by_name, - (GHFunc) check_if_marker_hit, - timeline); } static gboolean @@ -528,6 +585,7 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) { /* Emit the signal */ emit_frame_signal (timeline); + check_markers (timeline, priv->msecs_delta); /* Signal pauses timeline ? */ if (!priv->is_playing) @@ -543,21 +601,22 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) { /* Handle loop or stop */ ClutterTimelineDirection saved_direction = priv->direction; + gint elapsed_time_delta = priv->msecs_delta; guint overflow_msecs = priv->elapsed_time; gint end_msecs; /* Update the current elapsed time in case the signal handlers * want to take a peek. If we clamp elapsed time, then we need - * to correpondingly reduce msecs_delta to reflect the correct + * to correpondingly reduce elapsed_time_delta to reflect the correct * range of times */ if (priv->direction == CLUTTER_TIMELINE_FORWARD) { - priv->msecs_delta -= (priv->elapsed_time - priv->duration); + elapsed_time_delta -= (priv->elapsed_time - priv->duration); priv->elapsed_time = priv->duration; } else if (priv->direction == CLUTTER_TIMELINE_BACKWARD) { - priv->msecs_delta -= - priv->elapsed_time; + elapsed_time_delta -= - priv->elapsed_time; priv->elapsed_time = 0; } @@ -565,6 +624,7 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) /* Emit the signal */ emit_frame_signal (timeline); + check_markers (timeline, elapsed_time_delta); /* Did the signal handler modify the elapsed time? */ if (priv->elapsed_time != end_msecs) @@ -621,6 +681,14 @@ clutter_timeline_do_frame (ClutterTimeline *timeline) if (priv->direction != saved_direction) priv->elapsed_time = priv->duration - priv->elapsed_time; + /* If we have overflowed then we are changing the elapsed + time without emitting the new frame signal so we need to + check for markers again */ + check_markers (timeline, + priv->direction == CLUTTER_TIMELINE_FORWARD ? + priv->elapsed_time : + priv->duration - priv->elapsed_time); + g_object_unref (timeline); return TRUE; } From e60584ea6f7db76974b18789c9b90aaeeedf072a Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Wed, 10 Jun 2009 18:10:32 +0100 Subject: [PATCH 22/22] Change the paint forcing on the Text cache text The changes in the master clock and the repaint cycle have been changed, and broke the way the test for the Text actor cache of PangoLayouts forces a redraw. We have to call clutter_actor_paint() on the Stage embedding the Text actor we want to test; this is kinda fugly because if the Layout has changed it will end up causing a reallocation cycle in the middle of the Text actor paint. Since it's a test case, and since forcing redraws is a bit of a hack as well, we can close both our eyes on that. --- tests/conform/test-text-cache.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/conform/test-text-cache.c b/tests/conform/test-text-cache.c index ac650528d..a5425e77e 100644 --- a/tests/conform/test-text-cache.c +++ b/tests/conform/test-text-cache.c @@ -49,7 +49,14 @@ on_paint (ClutterActor *label, CallbackData *data) static void force_redraw (CallbackData *data) { - clutter_redraw (CLUTTER_STAGE (clutter_actor_get_stage (data->label))); + /* XXX - this is fugly; we force a paint on the stage, which + * will then paint the Text actor. inside the Text actor we + * check for a Layout with the allocation size. if the allocation + * has changed it will cause a relayout in the middle of the + * paint, which is expensive and broken. this will ensure that + * the test passes, though + */ + clutter_actor_paint (clutter_actor_get_stage (data->label)); } static gboolean