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 <ebassi@linux.intel.com>

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
+ * Checks if @master_clock has any running timeline.
- * is not %NULL then the timeline will be filtered from the
- * list of timelines held by @master_clock
  *
  * Return value: %TRUE if the #ClutterMasterClock has at least
  *   one running timeline
  */
 static gboolean
-has_running_timeline (ClutterMasterClock *master_clock,
+has_running_timeline (ClutterMasterClock *master_clock)
-                      ClutterTimeline    *filter)
 {
-  GSList *l;
+  ClutterStageManager *stage_manager = clutter_stage_manager_get_default ();
+  const GSList *stages, *l;
 
-  if (master_clock->timelines == NULL)
+  if (master_clock->timelines)
-    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;
-    }
 
   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
+ * Adds @timeline to the list of playing timelines held by the master
- * clock. This function should be called during the instance
+ * clock.
- * creation phase of the timeline.
  */
 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),
+  if (is_first)
-                     timeline_weak_ref,
+    {
-                     master_clock);
+      /* Start timing from scratch */
-
+      master_clock->prev_tick.tv_sec = 0;
-  g_signal_connect (timeline, "started",
+    }
-                    G_CALLBACK (on_timeline_started),
-                    master_clock);
 }
 
 /*
@@ -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
+ * Removes @timeline from the list of playing timelines held by the
- * master clock. This function should be called during the
+ * master clock.
- * #ClutterTimeline finalization.
  */
 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;
 }
 
 /*

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);
 
+  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);
 }