[pitivi] undo: Allow undoing transition changes

[Alexandru Băluț <alexbalut src gnome org>]

commit 33ebe5c861643c082de85576424e6e9d3bf72aec
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Tue May 31 17:07:06 2016 +0200

    undo: Allow undoing transition changes
    
    This simply handles the props of the video element of the transition
    clip.
    
    Differential Revision: https://phabricator.freedesktop.org/D1042

 pitivi/transitions.py       |   66 +++++++++++++++++++++++++++++++++----------
 pitivi/undo/timeline.py     |   35 +++++++++++++++--------
 pitivi/undo/undo.py         |    2 +-
 tests/test_undo_timeline.py |   40 ++++++++++++++++++++++++++
 4 files changed, 115 insertions(+), 28 deletions(-)
---
diff --git a/pitivi/transitions.py b/pitivi/transitions.py
index 8885ede..4ed0a89 100644
--- a/pitivi/transitions.py
+++ b/pitivi/transitions.py
@@ -26,6 +26,7 @@ from gi.repository import Gtk
 
 from pitivi.configure import get_pixmap_dir
 from pitivi.utils.loggable import Loggable
+from pitivi.utils.misc import disconnectAllByFunc
 from pitivi.utils.ui import PADDING
 from pitivi.utils.ui import SPACING
 
@@ -35,11 +36,15 @@ from pitivi.utils.ui import SPACING
  COL_DESC_TEXT,
  COL_ICON) = list(range(4))
 
+BORDER_LOOP_THRESHOLD = 50000
+
 
 class TransitionsListWidget(Gtk.Box, Loggable):
     """Widget for configuring the selected transition.
 
-    @type app: L{Pitivi}
+    Attributes:
+        app (Pitivi): The app.
+        element (GES.VideoTransition): The transition being configured.
     """
 
     def __init__(self, app):
@@ -52,6 +57,8 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         icon_theme = Gtk.IconTheme.get_default()
         self._question_icon = icon_theme.load_icon("dialog-question", 48, 0)
         self.set_orientation(Gtk.Orientation.VERTICAL)
+        # Whether a child widget has the focus.
+        self.container_focused = False
 
         # Tooltip handling
         self._current_transition_name = None
@@ -142,12 +149,34 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         self.props_widgets.hide()
         self.searchbar.hide()
 
+    def do_set_focus_child(self, child):
+        Gtk.Box.do_set_focus_child(self, child)
+        action_log = self.app.action_log
+        if not action_log:
+            # This happens when the user is editing a transition and
+            # suddenly closes the window. Don't bother.
+            return
+        if child:
+            if not self.container_focused:
+                self.container_focused = True
+                action_log.begin("Change transaction")
+        else:
+            if self.container_focused:
+                self.container_focused = False
+                action_log.commit("Change transaction")
+
     def __connectUi(self):
         self.iconview.connect("selection-changed", self._transitionSelectedCb)
         self.border_scale.connect("value-changed", self._borderScaleCb)
         self.invert_checkbox.connect("toggled", self._invertCheckboxCb)
         self.border_mode_normal.connect("released", self._borderTypeChangedCb)
         self.border_mode_loop.connect("released", self._borderTypeChangedCb)
+        self.element.connect("notify::border", self.__updated_cb)
+        self.element.connect("notify::invert", self.__updated_cb)
+        self.element.connect("notify::transition-type", self.__updated_cb)
+
+    def __updated_cb(self, element, unused_param):
+        self._update_ui()
 
     def __disconnectUi(self):
         self.iconview.disconnect_by_func(self._transitionSelectedCb)
@@ -155,6 +184,7 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         self.invert_checkbox.disconnect_by_func(self._invertCheckboxCb)
         self.border_mode_normal.disconnect_by_func(self._borderTypeChangedCb)
         self.border_mode_loop.disconnect_by_func(self._borderTypeChangedCb)
+        disconnectAllByFunc(self.element, self.__updated_cb)
 
 # UI callbacks
 
@@ -172,7 +202,6 @@ class TransitionsListWidget(Gtk.Box, Loggable):
             self.props_widgets.set_sensitive(True)
 
         self.element.get_parent().set_asset(transition_asset)
-        self.app.project_manager.current_project.setModificationState(True)
         self.app.write_action("element-set-asset", {
             "asset-id": transition_asset.get_id(),
             "element-name": self.element.get_name()})
@@ -199,7 +228,7 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         # The "border" property in gstreamer is unlimited, but if you go over
         # 25 thousand it "loops" the transition instead of smoothing it.
         if border is not None:
-            loop = border >= 50000
+            loop = border >= BORDER_LOOP_THRESHOLD
         if loop:
             self.border_scale.set_range(50000, 500000)
             self.border_scale.clear_marks()
@@ -245,30 +274,37 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         if isinstance(element, GES.AudioTransition):
             return
         self.element = element
-        transition_asset = element.get_parent().get_asset()
-        if transition_asset.get_id() == "crossfade":
-            self.props_widgets.set_sensitive(False)
-        else:
-            self.props_widgets.set_sensitive(True)
+        self._update_ui()
         self.iconview.show_all()
         self.props_widgets.show_all()
         self.searchbar.show_all()
-        self.__selectTransition(transition_asset)
-        border = element.get_border()
-        self.__updateBorderScale(border=border)
-        self.border_scale.set_value(border)
-        self.invert_checkbox.set_active(element.is_inverted())
         self.__connectUi()
         # We REALLY want the infobar to be hidden as space is really constrained
         # and yet GTK 3.10 seems to be racy in showing/hiding infobars, so
         # this must happen *after* the tab has been made visible/switched to:
         self.infobar.hide()
 
-    def __selectTransition(self, transition_asset):
+    def _update_ui(self):
+        transition_type = self.element.get_transition_type()
+        self.props_widgets.set_sensitive(
+            transition_type != GES.VideoStandardTransitionType.CROSSFADE)
+        self.__select_transition(transition_type)
+        border = self.element.get_border()
+        self.__updateBorderScale(border=border)
+        self.border_scale.set_value(border)
+        self.invert_checkbox.set_active(self.element.is_inverted())
+        loop = border >= BORDER_LOOP_THRESHOLD
+        if loop:
+            self.border_mode_loop.activate()
+        else:
+            self.border_mode_normal.activate()
+
+    def __select_transition(self, transition_type):
         """Selects the specified transition type in the iconview."""
         model = self.iconview.get_model()
         for row in model:
-            if transition_asset == row[COL_TRANSITION_ASSET]:
+            asset = row[COL_TRANSITION_ASSET]
+            if transition_type.value_nick == asset.get_id():
                 path = model.get_path(row.iter)
                 self.iconview.select_path(path)
                 self.iconview.scroll_to_path(path, False, 0, 0)
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index 1d4e1c5..d3f649b 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -478,25 +478,30 @@ class LayerObserver(MetaContainerObserver, Loggable):
             self._connectToClip(ges_clip)
 
     def _connectToClip(self, ges_clip):
-        props = ["start", "duration", "in-point", "priority"]
-        clip_observer = GObjectObserver(ges_clip, props, self.action_log)
-        self.clip_observers[ges_clip] = clip_observer
-
         ges_clip.connect("child-added", self._clipTrackElementAddedCb)
         ges_clip.connect("child-removed", self._clipTrackElementRemovedCb)
+
         for track_element in ges_clip.get_children(True):
             self._connectToTrackElement(track_element)
 
-    def _disconnectFromClip(self, clip):
-        if isinstance(clip, GES.TransitionClip):
+        if isinstance(ges_clip, GES.TransitionClip):
             return
 
-        for child in clip.get_children(True):
+        props = ["start", "duration", "in-point", "priority"]
+        clip_observer = GObjectObserver(ges_clip, props, self.action_log)
+        self.clip_observers[ges_clip] = clip_observer
+
+    def _disconnectFromClip(self, ges_clip):
+        ges_clip.disconnect_by_func(self._clipTrackElementAddedCb)
+        ges_clip.disconnect_by_func(self._clipTrackElementRemovedCb)
+
+        for child in ges_clip.get_children(True):
             self._disconnectFromTrackElement(child)
 
-        clip.disconnect_by_func(self._clipTrackElementAddedCb)
-        clip.disconnect_by_func(self._clipTrackElementRemovedCb)
-        clip_observer = self.clip_observers.pop(clip)
+        if isinstance(ges_clip, GES.TransitionClip):
+            return
+
+        clip_observer = self.clip_observers.pop(ges_clip)
         clip_observer.release()
 
     def _controlBindingAddedCb(self, track_element, binding):
@@ -507,6 +512,12 @@ class LayerObserver(MetaContainerObserver, Loggable):
         self.action_log.push(action)
 
     def _connectToTrackElement(self, track_element):
+        if isinstance(track_element, GES.VideoTransition):
+            props = ["border", "invert", "transition-type"]
+            observer = GObjectObserver(track_element, props, self.action_log)
+            self.track_element_observers[track_element] = observer
+            return
+
         for prop, binding in track_element.get_all_control_bindings().items():
             self._connectToControlSource(track_element, binding)
         track_element.connect("control-binding-added",
@@ -538,16 +549,16 @@ class LayerObserver(MetaContainerObserver, Loggable):
         observer.release()
 
     def _clipAddedCb(self, layer, clip):
+        self._connectToClip(clip)
         if isinstance(clip, GES.TransitionClip):
             return
-        self._connectToClip(clip)
         action = ClipAdded(layer, clip)
         self.action_log.push(action)
 
     def _clipRemovedCb(self, layer, clip):
+        self._disconnectFromClip(clip)
         if isinstance(clip, GES.TransitionClip):
             return
-        self._disconnectFromClip(clip)
         action = ClipRemoved(layer, clip)
         self.action_log.push(action)
 
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index c021585..f315e1e 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -169,7 +169,7 @@ class UndoableActionLog(GObject.Object, Loggable):
         try:
             stack = self._get_last_stack()
         except UndoWrongStateError as e:
-            self.warning("Ignore push because: %s", e)
+            self.warning("Failed pushing '%s' because: %s", action, e)
             return
         stack.push(action)
         self.debug("push action %s in action group %s",
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index cc38f0f..81580e1 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -343,6 +343,46 @@ class TestLayerObserver(BaseTestUndoTimeline):
         self.assertEqual(clip2.get_start(), 20 * Gst.SECOND)
         self.assertEqual(len(self.layer.get_clips()), 2)
 
+    def test_transitions(self):
+        self._wait_until_project_loaded()
+        uri = common.get_sample_uri("tears_of_steel.webm")
+        asset = GES.UriClipAsset.request_sync(uri)
+
+        clip1 = asset.extract()
+        clip1.set_start(0 * Gst.SECOND)
+        self.layer.add_clip(clip1)
+
+        clip2 = asset.extract()
+        clip2.set_start(clip1.props.duration / 2)
+        clip2.set_duration(10 * Gst.SECOND)
+        self.layer.add_clip(clip2)
+
+        def get_transition_element(ges_layer):
+            for clip in self.layer.get_clips():
+                if isinstance(clip, GES.TransitionClip):
+                    for element in clip.get_children(False):
+                        if isinstance(element, GES.VideoTransition):
+                            return element
+            self.fail("Cannot find video transition")
+
+        transition_element = get_transition_element(self.layer)
+        self.assertEqual(transition_element.get_transition_type(),
+                         GES.VideoStandardTransitionType.CROSSFADE)
+
+        with self.action_log.started("set transition type"):
+            transition_element.set_transition_type(GES.VideoStandardTransitionType.BAR_WIPE_LR)
+        self.assertEqual(transition_element.get_transition_type(),
+                         GES.VideoStandardTransitionType.BAR_WIPE_LR)
+
+        # Try to undo/redo
+        self.action_log.undo()
+        self.assertEqual(transition_element.get_transition_type(),
+                         GES.VideoStandardTransitionType.CROSSFADE)
+
+        self.action_log.redo()
+        self.assertEqual(transition_element.get_transition_type(),
+                         GES.VideoStandardTransitionType.BAR_WIPE_LR)
+
 
 class TestControlSourceObserver(BaseTestUndoTimeline):