[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

@@ -430,22 +430,76 @@ clutter_timeline_init (ClutterTimeline *self)
   priv->elapsed_time = 0;
 }
 
-static void
+static gboolean
-check_if_marker_hit (const gchar *name,
+have_passed_time (ClutterTimeline *timeline,
-                     TimelineMarker *marker,
+                  gint new_time,
-                     ClutterTimeline *timeline)
+                  gint msecs)
 {
   ClutterTimelinePrivate *priv = timeline->priv;
 
-  if (priv->direction == CLUTTER_TIMELINE_FORWARD
+  /* Ignore markers that are outside the duration of the timeline */
-      ? (marker->msecs > priv->elapsed_time - priv->msecs_delta
+  if (msecs < 0 || msecs > priv->duration)
-         && marker->msecs <= priv->elapsed_time)
+    return FALSE;
-      : (marker->msecs >= priv->elapsed_time
+
-         && marker->msecs < priv->elapsed_time + priv->msecs_delta))
+  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
+       * want to take a peek */
-       * to correpondingly reduce msecs_delta to reflect the correct
+      if (priv->loop)
-       * range of times */
+        {
+          /* We try and interpolate smoothly around a loop */
           if (priv->direction == CLUTTER_TIMELINE_FORWARD)
-	{
+            priv->elapsed_time = priv->elapsed_time % priv->duration;
-	  priv->msecs_delta -= (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;
-	}
 
       end_msecs = priv->elapsed_time;
 
-      /* Emit the signal */
+      /* Check if the markers have been hit using the old value of
-      emit_frame_signal (timeline);
+         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)
+      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;
-
-          /* 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;
+      return priv->loop;
-        }
     }
 }
 

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
+   * the next timeout will happen. We then wrap expected_frames
-   * to TEST_TIMELINE_DURATION since clutter-timeline
+   * around TEST_TIMELINE_DURATION */
-   * 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)