From 73cf36fa25a79278d401deccfacc9624e5a4904a Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Tue, 12 Jan 2016 14:51:55 +0000 Subject: [PATCH] timeline: reimplement snap_to_position a bit more appropriately. It could yet be made be simpler, but it would require touching the rest of the timeline editing code. Fixes https://phabricator.freedesktop.org/T7587 Reviewed-by: Thibault Saunier Differential Revision: https://phabricator.freedesktop.org/D657 --- ges/ges-timeline.c | 96 +++++++++++-------------------------- tests/check/ges/timelineedition.c | 11 ++--- tests/check/python/test_clip.py | 2 - tests/check/python/test_timeline.py | 31 ++++++++++-- 4 files changed, 59 insertions(+), 81 deletions(-) diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c index aa6d9f3..cd32a37 100644 --- a/ges/ges-timeline.c +++ b/ges/ges-timeline.c @@ -1271,85 +1271,46 @@ ges_timeline_emit_snappig (GESTimeline * timeline, GESTrackElement * obj1, } } -static guint64 * +static GstClockTime * ges_timeline_snap_position (GESTimeline * timeline, - GESTrackElement * trackelement, guint64 * current, guint64 timecode, - gboolean emit) + GESTrackElement * trackelement, GstClockTime * current, + GstClockTime timecode, gboolean emit) { GESTimelinePrivate *priv = timeline->priv; - GSequenceIter *iter, *prev_iter, *nxt_iter; - GESTrackElement *tmp_trackelement; - GESContainer *tmp_container, *container; - - GstClockTime *last_snap_ts = priv->movecontext.last_snap_ts; - guint64 snap_distance = timeline->priv->snapping_distance; - guint64 *prev_tc, *next_tc, *ret = NULL, off = G_MAXUINT64, off1 = - G_MAXUINT64; - - /* Avoid useless calculations */ - if (snap_distance == 0) - return NULL; - - /* If we can just resnap as last snap... do it */ - if (last_snap_ts) { - off = timecode > *last_snap_ts ? - timecode - *last_snap_ts : *last_snap_ts - timecode; - if (off <= snap_distance) { - ret = last_snap_ts; - goto done; - } - } - - container = get_toplevel_container (trackelement); - - iter = g_sequence_search (priv->starts_ends, &timecode, + GSequenceIter *iter, *end_iter; + GESContainer *container = get_toplevel_container (trackelement); + GstClockTime *ret = NULL; + GstClockTime smallest_offset = G_MAXUINT64; + GstClockTime tmp_pos; + + tmp_pos = timecode - priv->snapping_distance; + /* Rippling, not snapping with previous elements */ + if (priv->movecontext.moving_trackelements) + tmp_pos = timecode; + iter = g_sequence_search (priv->starts_ends, &tmp_pos, (GCompareDataFunc) compare_uint64, NULL); - /* Getting the next/previous values, and use the closest one if any "respects" - * the snap_distance value */ - nxt_iter = iter; - while (!g_sequence_iter_is_end (nxt_iter)) { - next_tc = g_sequence_get (iter); - tmp_trackelement = g_hash_table_lookup (timeline->priv->by_object, next_tc); - tmp_container = get_toplevel_container (tmp_trackelement); - - off = timecode > *next_tc ? timecode - *next_tc : *next_tc - timecode; - if (next_tc != current && off <= snap_distance - && container != tmp_container) { - - ret = next_tc; - break; - } - - nxt_iter = g_sequence_iter_next (nxt_iter); - } - - if (ret == NULL) - off = G_MAXUINT64; - - if (priv->movecontext.moving_trackelements) { - GST_INFO_OBJECT (timeline, "Rippling, no way we snap end"); - goto done; - } + tmp_pos = timecode + priv->snapping_distance; + end_iter = g_sequence_search (priv->starts_ends, &tmp_pos, + (GCompareDataFunc) compare_uint64, NULL); - prev_iter = g_sequence_iter_prev (iter); - while (!g_sequence_iter_is_begin (prev_iter)) { - prev_tc = g_sequence_get (prev_iter); - tmp_trackelement = g_hash_table_lookup (timeline->priv->by_object, prev_tc); - tmp_container = get_toplevel_container (tmp_trackelement); + for (; iter != end_iter && !g_sequence_iter_is_end (iter); + iter = g_sequence_iter_next (iter)) { + GstClockTime *iter_tc = g_sequence_get (iter); + GESTrackElement *tmp_trackelement = + g_hash_table_lookup (priv->by_object, iter_tc); + GESContainer *tmp_container = get_toplevel_container (tmp_trackelement); - off1 = timecode > *prev_tc ? timecode - *prev_tc : *prev_tc - timecode; - if (prev_tc != current && off1 < off && off1 <= snap_distance && - container != tmp_container) { - ret = prev_tc; + if (tmp_container == container) + continue; + if (ABS (timecode - *iter_tc) > smallest_offset) break; - } - prev_iter = g_sequence_iter_prev (prev_iter); + smallest_offset = ABS (timecode - *iter_tc); + ret = iter_tc; } -done: /* We emit the snapping signal only if we snapped with a different value * than the current one */ if (emit) { @@ -1413,7 +1374,6 @@ ges_move_context_set_objects (GESTimeline * timeline, GESTrackElement * obj, GSequenceIter *iter, *trackelement_iter; MoveContext *mv_ctx = &timeline->priv->movecontext; - iters = g_hash_table_lookup (timeline->priv->obj_iters, obj); trackelement_iter = iters->iter_obj; switch (edge) { diff --git a/tests/check/ges/timelineedition.c b/tests/check/ges/timelineedition.c index 68d0e21..c827afc 100644 --- a/tests/check/ges/timelineedition.c +++ b/tests/check/ges/timelineedition.c @@ -393,9 +393,8 @@ GST_START_TEST (test_snapping) * time 26-------- 62 --------122 */ ges_timeline_element_set_start (GES_TIMELINE_ELEMENT (clip1), 26); - ges_timeline_element_set_duration (GES_TIMELINE_ELEMENT (clip1), 37); CHECK_OBJECT_PROPS (trackelement, 25, 0, 37); - CHECK_OBJECT_PROPS (trackelement1, 26, 5, 36); + CHECK_OBJECT_PROPS (trackelement1, 26, 5, 37); CHECK_OBJECT_PROPS (trackelement2, 62, 0, 60); /** @@ -409,7 +408,7 @@ GST_START_TEST (test_snapping) fail_unless (ges_timeline_element_ripple (GES_TIMELINE_ELEMENT (clip1), 58) == TRUE); CHECK_OBJECT_PROPS (trackelement, 25, 0, 37); - CHECK_OBJECT_PROPS (trackelement1, 62, 5, 36); + CHECK_OBJECT_PROPS (trackelement1, 62, 5, 37); CHECK_OBJECT_PROPS (trackelement2, 98, 0, 60); /** @@ -419,7 +418,7 @@ GST_START_TEST (test_snapping) */ ges_timeline_element_set_start (GES_TIMELINE_ELEMENT (clip2), 110); CHECK_OBJECT_PROPS (trackelement, 25, 0, 37); - CHECK_OBJECT_PROPS (trackelement1, 62, 5, 36); + CHECK_OBJECT_PROPS (trackelement1, 62, 5, 37); CHECK_OBJECT_PROPS (trackelement2, 110, 0, 60); /** @@ -430,7 +429,7 @@ GST_START_TEST (test_snapping) fail_unless (ges_container_edit (clip1, NULL, -1, GES_EDIT_MODE_NORMAL, GES_EDGE_NONE, 72) == TRUE); CHECK_OBJECT_PROPS (trackelement, 25, 0, 37); - CHECK_OBJECT_PROPS (trackelement1, 73, 5, 36); + CHECK_OBJECT_PROPS (trackelement1, 73, 5, 37); CHECK_OBJECT_PROPS (trackelement2, 110, 0, 60); /** @@ -441,7 +440,7 @@ GST_START_TEST (test_snapping) fail_unless (ges_container_edit (clip1, NULL, -1, GES_EDIT_MODE_NORMAL, GES_EDGE_NONE, 58) == TRUE); CHECK_OBJECT_PROPS (trackelement, 25, 0, 37); - CHECK_OBJECT_PROPS (trackelement1, 62, 5, 36); + CHECK_OBJECT_PROPS (trackelement1, 62, 5, 37); CHECK_OBJECT_PROPS (trackelement2, 110, 0, 60); diff --git a/tests/check/python/test_clip.py b/tests/check/python/test_clip.py index fd57f76..efc3c2c 100644 --- a/tests/check/python/test_clip.py +++ b/tests/check/python/test_clip.py @@ -25,8 +25,6 @@ gi.require_version("GES", "1.0") from gi.repository import Gst # noqa from gi.repository import GES # noqa -from .common import GESSimpleTimelineTest # noqa - import unittest # noqa diff --git a/tests/check/python/test_timeline.py b/tests/check/python/test_timeline.py index 54d9a5c..b10948c 100644 --- a/tests/check/python/test_timeline.py +++ b/tests/check/python/test_timeline.py @@ -27,7 +27,9 @@ from gi.repository import GES # noqa import unittest # noqa from unittest import mock -from . import common +from .common import create_main_loop +from .common import create_project +from .common import GESSimpleTimelineTest # noqa Gst.init(None) GES.init() @@ -36,8 +38,8 @@ GES.init() class TestTimeline(unittest.TestCase): def test_signals_not_emitted_when_loading(self): - mainloop = common.create_main_loop() - timeline = common.create_project(with_group=True, saved=True) + mainloop = create_main_loop() + timeline = create_project(with_group=True, saved=True) # Reload the project, check the group. project = GES.Project.new(uri=timeline.get_asset().props.uri) @@ -52,7 +54,6 @@ class TestTimeline(unittest.TestCase): timeline = project.extract() signals = ["layer-added", "group-added", "track-added"] - called = [] handle = mock.Mock() for signal in signals: timeline.connect(signal, handle) @@ -61,7 +62,7 @@ class TestTimeline(unittest.TestCase): self.assertTrue(loaded_called) handle.assert_not_called() -class TestEditing(common.GESSimpleTimelineTest): +class TestEditing(GESSimpleTimelineTest): def test_transition_disappears_when_moving_to_another_layer(self): self.timeline.props.auto_transition = True @@ -111,3 +112,23 @@ class TestEditing(common.GESSimpleTimelineTest): self.assertEquals(len(self.layer.get_clips()), 4) clip1.edit([], self.layer.get_priority(), GES.EditMode.EDIT_RIPPLE, GES.Edge.EDGE_NONE, 35 * Gst.SECOND) self.assertEquals(len(self.layer.get_clips()), 4) + + +class TestSnapping(GESSimpleTimelineTest): + + def test_snapping(self): + self.timeline.props.auto_transition = True + self.timeline.set_snapping_distance(1) + clip1 = self.add_clip(0, 0, 100) + + # Split clip1. + split_position = 50 + clip2 = clip1.split(split_position) + self.assertEquals(len(self.layer.get_clips()), 2) + self.assertEqual(clip1.props.duration, split_position) + self.assertEqual(clip2.props.start, split_position) + + # Make sure snapping prevents clip2 to be moved to the left. + clip2.edit([], self.layer.get_priority(), GES.EditMode.EDIT_NORMAL, GES.Edge.EDGE_NONE, + clip2.props.start - 1) + self.assertEqual(clip2.props.start, split_position) -- 2.7.4