Commit 11edd091 authored by Alexandru Băluț's avatar Alexandru Băluț

undo: Fix clip ungroup redo adding extra track element

When a clip is ungrouped, an empty clip is added (by GES) then a video
track element is added to it. When redoing these steps in Pitivi, GES
adds track elements itself because the clip has none, so we end up with
too many.

Added a new ClipAction undoable class to reuse the add-clip-as-is logic
between ClipAdded and ClipRemoved.
Reviewed-by: Thibault Saunier's avatarThibault Saunier <tsaunier@gnome.org>
Differential Revision: https://phabricator.freedesktop.org/D1583
parent eb168b1e
......@@ -22,7 +22,6 @@ from gi.repository import Gst
from pitivi.effects import PROPS_TO_IGNORE
from pitivi.undo.undo import Action
from pitivi.undo.undo import ExpandableUndoableAction
from pitivi.undo.undo import FinalizingAction
from pitivi.undo.undo import GObjectObserver
from pitivi.undo.undo import MetaContainerObserver
......@@ -267,25 +266,42 @@ class ControlSourceObserver(GObject.Object):
self.action_log.push(action)
class ClipAdded(UndoableAction):
class ClipAction(UndoableAction):
def __init__(self, layer, clip):
UndoableAction.__init__(self)
self.layer = layer
self.clip = clip
def __repr__(self):
return "<ClipAdded %s>" % self.clip
def do(self):
def add(self):
self.clip.set_name(None)
children = self.clip.get_children(False)
self.layer.add_clip(self.clip)
# GES adds children if the clip had none. Make sure they are removed.
for child in self.clip.get_children(False):
if child not in children:
self.clip.remove(child)
self.layer.get_timeline().get_asset().pipeline.commit_timeline()
def undo(self):
def _child_added_cb(self, clip, track_element):
clip.remove(track_element)
def remove(self):
self.layer.remove_clip(self.clip)
self.layer.get_timeline().get_asset().pipeline.commit_timeline()
class ClipAdded(ClipAction):
def __repr__(self):
return "<ClipAdded %s>" % self.clip
def do(self):
self.add()
def undo(self):
self.remove()
def asScenarioAction(self):
timeline = self.layer.get_timeline()
if hasattr(self.layer, "splitting_object") and \
......@@ -305,12 +321,10 @@ class ClipAdded(UndoableAction):
return st
class ClipRemoved(ExpandableUndoableAction):
class ClipRemoved(ClipAction):
def __init__(self, layer, clip):
ExpandableUndoableAction.__init__(self)
self.layer = layer
self.clip = clip
ClipAction.__init__(self, layer, clip)
self.transition_removed_actions = []
def __repr__(self):
......@@ -323,13 +337,10 @@ class ClipRemoved(ExpandableUndoableAction):
return True
def do(self):
self.layer.remove_clip(self.clip)
self.layer.get_timeline().get_asset().pipeline.commit_timeline()
self.remove()
def undo(self):
self.clip.set_name(None)
self.layer.add_clip(self.clip)
self.layer.get_timeline().get_asset().pipeline.commit_timeline()
self.add()
# Update the automatically created transitions.
for action in self.transition_removed_actions:
action.undo()
......@@ -727,6 +738,9 @@ class TimelineElementAddedToGroup(UndoableAction):
self.ges_group = ges_group
self.ges_timeline_element = ges_timeline_element
def __repr__(self):
return "<TimelineElementAddedToGroup %s, %s>" % (self.ges_group, self.ges_timeline_element)
def do(self):
self.ges_group.add(self.ges_timeline_element)
......@@ -741,6 +755,9 @@ class TimelineElementRemovedFromGroup(UndoableAction):
self.ges_group = ges_group
self.ges_timeline_element = ges_timeline_element
def __repr__(self):
return "<TimelineElementRemovedFromGroup %s, %s>" % (self.ges_group, self.ges_timeline_element)
def do(self):
self.ges_group.remove(self.ges_timeline_element)
......@@ -819,7 +836,7 @@ class TimelineObserver(Loggable):
def _connect_to_group(self, ges_group):
if not ges_group.props.serialize:
return
return False
# A group is added when it gets its first element, thus
# when undoing/redoing a group can be added multiple times.
......@@ -828,9 +845,12 @@ class TimelineObserver(Loggable):
if ges_group not in self.group_observers:
group_observer = GroupObserver(ges_group, self.action_log)
self.group_observers[ges_group] = group_observer
return True
def __group_added_cb(self, unused_ges_timeline, ges_group):
self._connect_to_group(ges_group)
if not self._connect_to_group(ges_group):
return
# This should be a single clip.
for ges_clip in ges_group.get_children(recursive=False):
action = TimelineElementAddedToGroup(ges_group, ges_clip)
......
......@@ -58,6 +58,18 @@ class UndoableAction(Action):
def undo(self):
raise NotImplementedError()
def expand(self, action):
"""Allows the action to expand by including the specified action.
Args:
action (UndoableAction): The action to include.
Returns:
bool: Whether the action has been included, in which case
it should not be used for anything else.
"""
return False
class UndoableAutomaticObjectAction(UndoableAction):
"""An action on an automatically created object.
......@@ -94,22 +106,6 @@ class UndoableAutomaticObjectAction(UndoableAction):
cls.__updates[other] = new_auto_object
class ExpandableUndoableAction(UndoableAction):
"""An action which can include immediately following actions."""
def expand(self, action):
"""Expands including the specified action.
Args:
action (UndoableAction): The action to include.
Returns:
bool: Whether the action has been included, in which case
it should not be used for anything else.
"""
raise NotImplementedError()
class FinalizingAction:
"""Base class for actions applied when an undo or redo is performed."""
......@@ -140,10 +136,9 @@ class UndoableActionStack(UndoableAction):
def push(self, action):
if self.done_actions:
last_action = self.done_actions[-1]
if isinstance(last_action, ExpandableUndoableAction):
if last_action.expand(action):
# The action has been included in the previous one.
return
if last_action.expand(action):
# The action has been included in the previous one.
return
self.done_actions.append(action)
def _run_action(self, actions, method_name):
......
......@@ -63,7 +63,9 @@ class TestUndoableActionStack(TestCase):
"""
stack = UndoableActionStack("meh")
action1 = mock.Mock(spec=UndoableAction)
action1.expand.return_value = False
action2 = mock.Mock(spec=UndoableAction)
action2.expand.return_value = False
action2.undo.side_effect = UndoError("meh")
action3 = mock.Mock(spec=UndoableAction)
stack.push(action1)
......@@ -284,6 +286,7 @@ class TestUndoableActionLog(TestCase):
# push two actions
action1 = mock.Mock(spec=UndoableAction)
action1.expand.return_value = False
self.log.push(action1)
self.assertEqual(len(self.signals), 2)
name, (stack, signalAction) = self.signals[1]
......@@ -342,8 +345,11 @@ class TestUndoableActionLog(TestCase):
"""
order = mock.Mock()
order.action1 = mock.Mock(spec=UndoableAction)
order.action1.expand.return_value = False
order.action2 = mock.Mock(spec=UndoableAction)
order.action2.expand.return_value = False
order.action3 = mock.Mock(spec=UndoableAction)
order.action3.expand.return_value = False
with self.log.started("meh"):
self.log.push(order.action1)
......
......@@ -187,7 +187,23 @@ class TestTimelineObserver(BaseTestUndoTimeline):
self.assertEqual(len(clips[0].get_children(False)), 1)
self.assertEqual(len(clips[1].get_children(False)), 1)
timeline.selection.select(clips)
timeline.resetSelectionGroup()
for clip in clips:
timeline.current_group.add(clip)
self.timeline_container.group_action.activate(None)
clips = list(self.getTimelineClips())
self.assertEqual(len(clips), 1, clips)
self.assertEqual(len(clips[0].get_children(False)), 2)
for i in range(2):
# Undo grouping.
self.action_log.undo()
clips = list(self.getTimelineClips())
self.assertEqual(len(clips), 2, clips)
self.assertEqual(len(clips[0].get_children(False)), 1)
self.assertEqual(len(clips[1].get_children(False)), 1)
# Undo ungrouping.
self.action_log.undo()
clips = list(self.getTimelineClips())
......@@ -201,6 +217,12 @@ class TestTimelineObserver(BaseTestUndoTimeline):
self.assertEqual(len(clips[0].get_children(False)), 1)
self.assertEqual(len(clips[1].get_children(False)), 1)
# Redo grouping.
self.action_log.redo()
clips = list(self.getTimelineClips())
self.assertEqual(len(clips), 1, clips)
self.assertEqual(len(clips[0].get_children(False)), 2)
class TestLayerObserver(BaseTestUndoTimeline):
......
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