From fc8c2c5d9b4fcdb47477df447cc3394bbeac5fb0 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 3 Nov 2008 12:42:17 +0000 Subject: [PATCH] Bug 1207 - Timelines sometime miss markers * clutter/clutter-timeline.c (timeline_timeout_func): Move the code for firing the new-frame and marker-reached signals into a separate static function so that it can also be called when the last frame is reached. Also fix an issue where the frame numbers were changed in the wrong direction when detecting missed markers in a reversed timeline. Based on a patch by Michael Boccara. * tests/test-timeline.c: Now tries to automatically verify whether the test worked by keeping track of all the signal emissions. The timelines are run a second time with an extra timeout that causes delays to simulate skipped frames. --- ChangeLog | 16 +++ clutter/clutter-timeline.c | 77 ++++++----- tests/test-timeline.c | 256 +++++++++++++++++++++++++++++-------- 3 files changed, 267 insertions(+), 82 deletions(-) diff --git a/ChangeLog b/ChangeLog index ea8c981e7..1efaa3302 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2008-11-03 Neil Roberts + + Bug 1207 - Timelines sometime miss markers + + * clutter/clutter-timeline.c (timeline_timeout_func): Move the + code for firing the new-frame and marker-reached signals into a + separate static function so that it can also be called when the + last frame is reached. Also fix an issue where the frame numbers + were changed in the wrong direction when detecting missed markers + in a reversed timeline. Based on a patch by Michael Boccara. + + * tests/test-timeline.c: Now tries to automatically verify whether + the test worked by keeping track of all the signal emissions. The + timelines are run a second time with an extra timeout that causes + delays to simulate skipped frames. + 2008-10-31 Emmanuele Bassi * clutter/pango/cogl-pango-fontmap.c: diff --git a/clutter/clutter-timeline.c b/clutter/clutter-timeline.c index d3e2fe604..e525756ee 100644 --- a/clutter/clutter-timeline.c +++ b/clutter/clutter-timeline.c @@ -575,6 +575,37 @@ clutter_timeline_init (ClutterTimeline *self) timeline_marker_free); } +static void +emit_frame_signal (ClutterTimeline *timeline) +{ + ClutterTimelinePrivate *priv = timeline->priv; + gint i; + + g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, + priv->current_frame_num); + + for (i = priv->skipped_frames; i >= 0; i--) + { + gint frame_num = priv->current_frame_num + + (priv->direction == CLUTTER_TIMELINE_FORWARD ? -i : i); + GSList *markers, *l; + + markers = g_hash_table_lookup (priv->markers_by_frame, + GUINT_TO_POINTER (frame_num)); + for (l = markers; l; l = l->next) + { + TimelineMarker *marker = l->data; + + CLUTTER_NOTE (SCHEDULER, "Marker `%s' reached", marker->name); + + g_signal_emit (timeline, timeline_signals[MARKER_REACHED], + marker->quark, + marker->name, + marker->frame_num); + } + } +} + static gboolean timeline_timeout_func (gpointer data) { @@ -636,31 +667,8 @@ timeline_timeout_func (gpointer data) (priv->current_frame_num <= 0)) )) { - gint i; - - /* Fire off signal */ - g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, - priv->current_frame_num); - - for (i = priv->skipped_frames; i >= 0; i--) - { - gint frame_num = priv->current_frame_num - i; - GSList *markers, *l; - - markers = g_hash_table_lookup (priv->markers_by_frame, - GUINT_TO_POINTER (frame_num)); - for (l = markers; l; l = l->next) - { - TimelineMarker *marker = l->data; - - CLUTTER_NOTE (SCHEDULER, "Marker `%s' reached", marker->name); - - g_signal_emit (timeline, timeline_signals[MARKER_REACHED], - marker->quark, - marker->name, - marker->frame_num); - } - } + /* Emit the signal */ + emit_frame_signal (timeline); /* Signal pauses timeline ? */ if (!priv->timeout_id) @@ -679,17 +687,24 @@ timeline_timeout_func (gpointer data) guint overflow_frame_num = priv->current_frame_num; gint end_frame; - /* In case the signal handlers want to take a peek... */ + /* Update the current frame number in case the signal handlers + want to take a peek. Don't count skipped frames that run past + the end of the timeline */ if (priv->direction == CLUTTER_TIMELINE_FORWARD) - priv->current_frame_num = priv->n_frames; + { + priv->skipped_frames -= priv->current_frame_num - priv->n_frames; + priv->current_frame_num = priv->n_frames; + } else if (priv->direction == CLUTTER_TIMELINE_BACKWARD) - priv->current_frame_num = 0; + { + priv->skipped_frames += priv->current_frame_num; + priv->current_frame_num = 0; + } end_frame = priv->current_frame_num; - /* Fire off signal */ - g_signal_emit (timeline, timeline_signals[NEW_FRAME], 0, - priv->current_frame_num); + /* Emit the signal */ + emit_frame_signal (timeline); /* Did the signal handler modify the current_frame_num */ if (priv->current_frame_num != end_frame) diff --git a/tests/test-timeline.c b/tests/test-timeline.c index 2a7ff3abf..218dcaa57 100644 --- a/tests/test-timeline.c +++ b/tests/test-timeline.c @@ -3,86 +3,193 @@ #include #include -static void -timeline_1_complete (ClutterTimeline *timeline) +/* This test runs three timelines at 30 fps with 10 frames. Some of + the timelines have markers. Once the timelines are run it then + checks that all of the frames were hit, all of the markers were hit + and that the completed signal was fired. The timelines are then run + again but this time with a timeout source that introduces a + delay. This should cause some frames to be skipped. The test is run + again but only the markers and the completed signal is checked + for. */ + +#define FRAME_COUNT 10 + +typedef struct _TimelineData TimelineData; + +struct _TimelineData { - g_debug ("1: Completed"); + int timeline_num; + + guint frame_hit_count[FRAME_COUNT + 1]; + GSList *markers_hit; + guint completed_count; +}; + +static void +timeline_data_init (TimelineData *data, int timeline_num) +{ + memset (data, 0, sizeof (TimelineData)); + data->timeline_num = timeline_num; } static void -timeline_2_complete (ClutterTimeline *timeline) +timeline_data_destroy (TimelineData *data) { - g_debug ("2: Completed"); + g_slist_foreach (data->markers_hit, (GFunc) g_free, NULL); + g_slist_free (data->markers_hit); } static void -timeline_3_complete (ClutterTimeline *timeline) +timeline_complete_cb (ClutterTimeline *timeline, + TimelineData *data) { - g_debug ("3: Completed"); + printf ("%i: Completed\n", data->timeline_num); + data->completed_count++; } static void -timeline_1_new_frame_cb (ClutterTimeline *timeline, gint frame_no) +timeline_new_frame_cb (ClutterTimeline *timeline, + gint frame_no, + TimelineData *data) { - g_debug ("1: Doing frame %d.", frame_no); + printf ("%i: Doing frame %d, delta = %i\n", + data->timeline_num, frame_no, + clutter_timeline_get_delta (timeline, NULL)); + data->frame_hit_count[frame_no]++; } static void -timeline_2_new_frame_cb (ClutterTimeline *timeline, gint frame_no) +timeline_marker_reached_cb (ClutterTimeline *timeline, + const gchar *marker_name, + guint frame_num, + TimelineData *data) { - g_debug ("2: Doing frame %d.", frame_no); + printf ("%i: Marker `%s' (%d) reached, delta = %i\n", + data->timeline_num, marker_name, frame_num, + clutter_timeline_get_delta (timeline, NULL)); + data->markers_hit = g_slist_prepend (data->markers_hit, + g_strdup (marker_name)); } -static void -timeline_3_new_frame_cb (ClutterTimeline *timeline, gint frame_no) +static gboolean +check_timeline (ClutterTimeline *timeline, + TimelineData *data, + gboolean check_missed_frames) { - g_debug ("3: Doing frame %d.", frame_no); + gchar **markers; + gsize n_markers; + guint *marker_reached_count; + gboolean succeeded = TRUE; + GSList *node; + int i; + int missed_frame_count = 0; + int frame_offset; + + if (clutter_timeline_get_direction (timeline) == CLUTTER_TIMELINE_BACKWARD) + frame_offset = 0; + else + frame_offset = 1; + + markers = clutter_timeline_list_markers (timeline, -1, &n_markers); + marker_reached_count = g_new0 (guint, n_markers); + + for (node = data->markers_hit; node; node = node->next) + { + for (i = 0; i < n_markers; i++) + if (!strcmp (node->data, markers[i])) + break; + + if (i < n_markers) + marker_reached_count[i]++; + else + { + printf ("FAIL: unknown marker '%s' hit for %i\n", + (char *) node->data, data->timeline_num); + succeeded = FALSE; + } + } + + for (i = 0; i < n_markers; i++) + if (marker_reached_count[i] != 1) + { + printf ("FAIL: marker '%s' hit %i times for %i\n", + markers[i], marker_reached_count[i], data->timeline_num); + succeeded = FALSE; + } + + if (check_missed_frames) + { + for (i = 0; i < FRAME_COUNT; i++) + if (data->frame_hit_count[i + frame_offset] != 1) + missed_frame_count++; + + if (missed_frame_count) + { + printf ("FAIL: missed %i frame%s for %i\n", + missed_frame_count, missed_frame_count == 1 ? "" : "s", + data->timeline_num); + succeeded = FALSE; + } + } + + if (data->completed_count != 1) + { + printf ("FAIL: timeline %i completed %i times\n", + data->timeline_num, data->completed_count); + succeeded = FALSE; + } + + g_strfreev (markers); + g_free (marker_reached_count); + + return succeeded; } -static void -timeline_1_marker_reached (ClutterTimeline *timeline, - const gchar *marker_name, - guint frame_num) +static gboolean +timeout_cb (gpointer data) { - g_print ("1: Marker `%s' (%d) reached\n", marker_name, frame_num); + clutter_main_quit (); + + return FALSE; } -static void -timeline_2_marker_reached (ClutterTimeline *timeline, - const gchar *marker_name, - guint frame_num) +static gboolean +delay_cb (gpointer data) { - g_print ("2: Marker `%s' (%d) reached\n", marker_name, frame_num); -} + /* Waste a bit of time so that it will skip frames */ + g_usleep (G_USEC_PER_SEC * 66 / 1000); -static void -timeline_3_marker_reached (ClutterTimeline *timeline, - const gchar *marker_name, - guint frame_num) -{ - g_print ("3: Marker `%s' (%d) reached\n", marker_name, frame_num); + return TRUE; } int main (int argc, char **argv) { ClutterTimeline *timeline_1; + TimelineData data_1; ClutterTimeline *timeline_2; + TimelineData data_2; ClutterTimeline *timeline_3; + TimelineData data_3; gchar **markers; gsize n_markers; + int ret = EXIT_SUCCESS; clutter_init (&argc, &argv); - timeline_1 = clutter_timeline_new (10, 120); + timeline_data_init (&data_1, 1); + timeline_1 = clutter_timeline_new (FRAME_COUNT, 30); clutter_timeline_add_marker_at_frame (timeline_1, "foo", 5); clutter_timeline_add_marker_at_frame (timeline_1, "bar", 5); clutter_timeline_add_marker_at_frame (timeline_1, "baz", 5); + clutter_timeline_add_marker_at_frame (timeline_1, "near-end-marker", 9); + clutter_timeline_add_marker_at_frame (timeline_1, "end-marker", 10); markers = clutter_timeline_list_markers (timeline_1, 5, &n_markers); g_assert (markers != NULL); g_assert (n_markers == 3); g_strfreev (markers); + timeline_data_init (&data_2, 2); timeline_2 = clutter_timeline_clone (timeline_1); clutter_timeline_add_marker_at_frame (timeline_2, "bar", 2); markers = clutter_timeline_list_markers (timeline_2, -1, &n_markers); @@ -91,49 +198,96 @@ main (int argc, char **argv) g_assert (strcmp (markers[0], "bar") == 0); g_strfreev (markers); + 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_frame (timeline_3, "foo", 5); clutter_timeline_add_marker_at_frame (timeline_3, "baz", 8); + clutter_timeline_add_marker_at_frame (timeline_3, "near-end-marker", 1); + clutter_timeline_add_marker_at_frame (timeline_3, "end-marker", 0); g_signal_connect (timeline_1, - "marker-reached", G_CALLBACK (timeline_1_marker_reached), - NULL); + "marker-reached", G_CALLBACK (timeline_marker_reached_cb), + &data_1); g_signal_connect (timeline_1, - "new-frame", G_CALLBACK (timeline_1_new_frame_cb), - NULL); + "new-frame", G_CALLBACK (timeline_new_frame_cb), + &data_1); g_signal_connect (timeline_1, - "completed", G_CALLBACK (timeline_1_complete), - NULL); + "completed", G_CALLBACK (timeline_complete_cb), + &data_1); g_signal_connect (timeline_2, - "marker-reached::bar", G_CALLBACK (timeline_2_marker_reached), - NULL); + "marker-reached::bar", + G_CALLBACK (timeline_marker_reached_cb), + &data_2); g_signal_connect (timeline_2, - "new-frame", G_CALLBACK (timeline_2_new_frame_cb), - NULL); + "new-frame", G_CALLBACK (timeline_new_frame_cb), + &data_2); g_signal_connect (timeline_2, - "completed", G_CALLBACK (timeline_2_complete), - NULL); + "completed", G_CALLBACK (timeline_complete_cb), + &data_2); g_signal_connect (timeline_3, - "marker-reached", G_CALLBACK (timeline_3_marker_reached), - NULL); + "marker-reached", G_CALLBACK (timeline_marker_reached_cb), + &data_3); g_signal_connect (timeline_3, - "new-frame", G_CALLBACK (timeline_3_new_frame_cb), - NULL); + "new-frame", G_CALLBACK (timeline_new_frame_cb), + &data_3); g_signal_connect (timeline_3, - "completed", G_CALLBACK (timeline_3_complete), - NULL); + "completed", G_CALLBACK (timeline_complete_cb), + &data_3); + + printf ("Without delay...\n"); clutter_timeline_start (timeline_1); clutter_timeline_start (timeline_2); clutter_timeline_start (timeline_3); + clutter_threads_add_timeout (2000, timeout_cb, NULL); + clutter_main (); + if (!check_timeline (timeline_1, &data_1, TRUE)) + ret = EXIT_FAILURE; + if (!check_timeline (timeline_2, &data_2, TRUE)) + ret = EXIT_FAILURE; + if (!check_timeline (timeline_3, &data_3, TRUE)) + ret = EXIT_FAILURE; + + printf ("With delay...\n"); + + timeline_data_destroy (&data_1); + timeline_data_init (&data_1, 1); + timeline_data_destroy (&data_2); + timeline_data_init (&data_2, 2); + timeline_data_destroy (&data_3); + timeline_data_init (&data_3, 3); + + clutter_timeline_start (timeline_1); + clutter_timeline_start (timeline_2); + clutter_timeline_start (timeline_3); + + clutter_threads_add_timeout (2000, timeout_cb, NULL); + clutter_threads_add_timeout (99, delay_cb, NULL); + + clutter_main (); + + if (!check_timeline (timeline_1, &data_1, FALSE)) + ret = EXIT_FAILURE; + if (!check_timeline (timeline_2, &data_2, FALSE)) + ret = EXIT_FAILURE; + if (!check_timeline (timeline_3, &data_3, FALSE)) + ret = EXIT_FAILURE; + g_object_unref (timeline_1); g_object_unref (timeline_2); g_object_unref (timeline_3); - return EXIT_SUCCESS; + timeline_data_destroy (&data_1); + timeline_data_destroy (&data_2); + timeline_data_destroy (&data_3); + + printf ("Overall result: %s\n", ret == EXIT_SUCCESS ? "PASS" : "FAIL"); + + return ret; }