Commit 7cf776c9 authored by Alexandru Băluț's avatar Alexandru Băluț Committed by Thibault Saunier

timeline: Fix layers priorities updating when a layer is removed

Until now, when the timeline detected a layer removal, it was updating
the priorities of the rest of the layers itself. This was not good
because when the removal is done when undoing a layer-add operation, the
rest of the layers have their priorities decreased twice: once by the
timeline and once by the LayerMoved actions recorded when the original
undoable action has been performed.

Now the priorities of the remaining layers are decreased at the same
place where the removal is performed: `LayerControls.__delete_layer_cb`.
The Timeline is responsible only for updating the position of the layer
widgets, done through `__update_layers`.

Fixes https://phabricator.freedesktop.org/T7700Reviewed-by: Thibault Saunier's avatarThibault Saunier <tsaunier@gnome.org>
Differential Revision: https://phabricator.freedesktop.org/D1660
parent 9b2cc124
......@@ -150,7 +150,7 @@ class LayerControls(Gtk.EventBox, Loggable):
last = priority == layers_count - 1
self.__move_layer_down_action.props.enabled = not last
self.__move_layer_bottom_action.props.enabled = not last
self.__delete_layer_action.props.enabled = layers_count > 1
self.delete_layer_action.props.enabled = layers_count > 1
def __updateName(self):
self.name_entry.set_text(self.ges_layer.ui.getName())
......@@ -183,8 +183,8 @@ class LayerControls(Gtk.EventBox, Loggable):
action_group.add_action(action)
menu_model.append(_("Move layer to bottom"), "layer.%s" % action.get_name().replace(" ", "."))
self.__delete_layer_action = Gio.SimpleAction.new("delete-layer", None)
action = self.__delete_layer_action
self.delete_layer_action = Gio.SimpleAction.new("delete-layer", None)
action = self.delete_layer_action
action.connect("activate", self.__delete_layer_cb)
action_group.add_action(action)
menu_model.append(_("Delete layer"), "layer.%s" % action.get_name())
......@@ -196,6 +196,10 @@ class LayerControls(Gtk.EventBox, Loggable):
with self.app.action_log.started("delete layer",
CommitTimelineFinalizingAction(pipeline)):
self.ges_timeline.remove_layer(self.ges_layer)
removed_priority = self.ges_layer.props.priority
for ges_layer in self.ges_timeline.get_layers():
if ges_layer.props.priority > removed_priority:
ges_layer.props.priority -= 1
def __move_layer_cb(self, unused_simple_action, unused_parameter, step):
index = self.ges_layer.get_priority()
......
......@@ -443,11 +443,11 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
self.ges_timeline.disconnect_by_func(self._durationChangedCb)
self.ges_timeline.disconnect_by_func(self._layer_added_cb)
self.ges_timeline.disconnect_by_func(self._layerRemovedCb)
self.ges_timeline.disconnect_by_func(self._layer_removed_cb)
self.ges_timeline.disconnect_by_func(self._snapCb)
self.ges_timeline.disconnect_by_func(self._snapEndedCb)
for ges_layer in self.ges_timeline.get_layers():
self._removeLayer(ges_layer)
self._remove_layer(ges_layer)
self.ges_timeline.ui = None
self.ges_timeline = None
......@@ -467,7 +467,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
self.ges_timeline.connect("notify::duration", self._durationChangedCb)
self.ges_timeline.connect("layer-added", self._layer_added_cb)
self.ges_timeline.connect("layer-removed", self._layerRemovedCb)
self.ges_timeline.connect("layer-removed", self._layer_removed_cb)
self.ges_timeline.connect("snapping-started", self._snapCb)
self.ges_timeline.connect("snapping-ended", self._snapEndedCb)
......@@ -998,10 +998,10 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
self.debug("Layers still being shuffled, not updating widgets: %s", priorities)
return
self.debug("Updating layers widgets positions")
for i, ges_layer in enumerate(self.ges_timeline.get_layers()):
for ges_layer in self.ges_timeline.get_layers():
self.__update_layer(ges_layer)
def _removeLayer(self, ges_layer):
def _remove_layer(self, ges_layer):
self.info("Removing layer: %s", ges_layer.props.priority)
self.layout.layers_vbox.remove(ges_layer.ui)
self._layers_controls_vbox.remove(ges_layer.control_ui)
......@@ -1016,12 +1016,9 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
ges_layer.ui = None
ges_layer.control_ui = None
def _layerRemovedCb(self, ges_timeline, ges_layer):
self._removeLayer(ges_layer)
removed_priority = ges_layer.props.priority
for priority, ges_layer in enumerate(ges_timeline.get_layers()):
if priority >= removed_priority:
ges_layer.props.priority -= 1
def _layer_removed_cb(self, unused_ges_timeline, ges_layer):
self._remove_layer(ges_layer)
self.__update_layers()
def separator_priority(self, separator):
position = self.layout.layers_vbox.child_get_property(separator, "position")
......
......@@ -478,6 +478,9 @@ class LayerRemoved(UndoableAction):
self.ges_timeline = ges_timeline
self.ges_layer = ges_layer
def __repr__(self):
return "<LayerRemoved %s>" % self.ges_layer
def do(self):
self.ges_timeline.remove_layer(self.ges_layer)
self.ges_timeline.get_asset().pipeline.commit_timeline()
......@@ -499,6 +502,11 @@ class LayerMoved(UndoableAction):
self.old_priority = old_priority
self.priority = priority
def __repr__(self):
return "<LayerMoved %s: %s -> %s>" % (self.ges_layer,
self.old_priority,
self.priority)
def do(self):
self.ges_layer.props.priority = self.priority
......
......@@ -202,27 +202,29 @@ class TestLayers(BaseTestTimeline):
self.assertListEqual(positions, expected_positions)
def test_remove_layer(self):
self.check_remove_layer([0, 0, 0, 0])
self.check_remove_layer([0, 0, 1, 0])
self.check_remove_layer([0, 1, 0, 0])
self.check_remove_layer([0, 2, 1, 0])
self.check_remove_layer([1, 0, 1, 0])
self.check_remove_layer([2, 2, 0, 0])
self.check_remove_layer([3, 2, 1, 0])
self.check_remove_layer([0, 0, 0])
self.check_remove_layer([0, 0, 1])
self.check_remove_layer([0, 1, 0])
self.check_remove_layer([0, 2, 1])
self.check_remove_layer([1, 0, 1])
self.check_remove_layer([2, 2, 0])
self.check_remove_layer([3, 2, 1])
def check_remove_layer(self, removal_order):
timeline = create_timeline_container().timeline
# Add layers to remove them later.
ges_layers = []
for priority in range(len(removal_order)):
# Pitivi doesn't support removing the last remaining layer,
# that's why we create an extra layer.
for priority in range(len(removal_order) + 1):
ges_layer = timeline.createLayer(priority)
ges_layers.append(ges_layer)
# Remove the layers in the specified order.
# Remove layers one by one in the specified order.
for priority in removal_order:
ges_layer = ges_layers[priority]
self.assertTrue(timeline.ges_timeline.remove_layer(ges_layer))
ges_layer.control_ui.delete_layer_action.activate(None)
ges_layers.remove(ges_layer)
self.check_priorities_and_positions(timeline, ges_layers, list(range(len(ges_layers))))
......
......@@ -112,12 +112,14 @@ class TestTimelineObserver(BaseTestUndoTimeline):
self.check_removal(self.timeline.get_layers())
def check_removal(self, ges_layers):
if len(ges_layers) == 1:
# We don't support removing the last remaining layer.
return
for ges_layer in ges_layers:
remaining_layers = list(ges_layers)
remaining_layers.remove(ges_layer)
with self.action_log.started("layer removed"):
self.timeline.remove_layer(ges_layer)
ges_layer.control_ui.delete_layer_action.activate(None)
self.check_layers(remaining_layers)
self.action_log.undo()
......@@ -847,6 +849,7 @@ class TestDragDropUndo(BaseTestUndoTimeline):
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 2)
self.assertEqual(layers[0], self.layer)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [])
self.assertEqual(layers[1].get_clips(), [clip])
......@@ -854,12 +857,14 @@ class TestDragDropUndo(BaseTestUndoTimeline):
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 1)
self.assertEqual(layers[0], self.layer)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [clip])
self.action_log.redo()
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 2)
self.assertEqual(layers[0], self.layer)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [])
self.assertEqual(layers[1].get_clips(), [clip])
......@@ -897,18 +902,21 @@ class TestDragDropUndo(BaseTestUndoTimeline):
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 2)
self.assertEqual(layers[1], self.layer)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [clip])
self.assertEqual(layers[1].get_clips(), [])
self.action_log.undo()
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 1)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [clip])
self.action_log.redo()
layers = self.timeline.get_layers()
self.assertEqual(len(layers), 2)
self.assertEqual(layers[0], self.layer)
self.check_layers(layers)
self.assertEqual(layers[0].get_clips(), [clip])
self.assertEqual(layers[1].get_clips(), [])
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment