[pitivi] undo: Remove obsolete optimization

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

commit 1495203d02efbe8331b18daf4cf646273a4643b9
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sat May 14 00:14:38 2016 +0200

    undo: Remove obsolete optimization
    
    TimelineObserver._clipPropertyChangedCb, the final handler of the prop
    changed events ("monitored-property-changed"), which pushes the
    ClipPropertyChanged actions, was ignoring these events when the clips
    were being dragged around. To be able to record the final changes,
    ClipPropertyChangeTracker had to connect to the timeline's "commited"
    signal and report if changes have been detected.
    
    The goal was to have as few as possible actions in the UndoableActionLog
    because when undoing or redoing, all these actions were committing the
    timeline and this was very expensive. Now the individual actions are not
    committing the timeline anymore, so we can live with a high number of
    actions in the stack.
    
    As a result of the simplification, the TimelineObserver does not need to
    keep track of the prop values itself (by setting attributes on the
    ClipPropertyChangeTracker) and the "monitored-property-changed" signal
    can be removed as now PropertyChangeTracker can push the
    PropertyChangedAction instances itself.
    
    Finally, PropertyChangeTracker can now be used easily to monitor the
    props of other objects besides clips, stay tuned!
    
    Differential Revision: https://phabricator.freedesktop.org/D993

 pitivi/undo/timeline.py     |   67 ++-----------------------------------------
 pitivi/undo/undo.py         |   53 ++++++++++++++++++++-------------
 tests/test_undo_timeline.py |    4 +-
 3 files changed, 37 insertions(+), 87 deletions(-)
---
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index 077dbb7..08725fb 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -218,28 +218,6 @@ class TrackElementRemoved(UndoableAction):
         return st
 
 
-class ClipPropertyChangeTracker(PropertyChangeTracker):
-
-    def __init__(self, ges_clip):
-        PropertyChangeTracker.__init__(self, ges_clip,
-            # No out-point as that's calculated.
-            property_names=["start", "duration", "in-point", "priority"])
-        self.timeline = ges_clip.timeline
-        self.timeline.connect("commited", self._timelineCommitedCb)
-
-    def release(self):
-        self.timeline.disconnect_by_func(self._timelineCommitedCb)
-        PropertyChangeTracker.release(self)
-
-    def _timelineCommitedCb(self, timeline):
-        properties = self._takeCurrentSnapshot(self.gobject)
-        for property_name, property_value in properties.items():
-            old_value = self.properties[property_name]
-            if old_value != property_value:
-                self._propertyChangedCb(
-                    self.gobject, property_value, property_name)
-
-
 class KeyframeChangeTracker(GObject.Object):
 
     __gsignals__ = {
@@ -287,24 +265,6 @@ class KeyframeChangeTracker(GObject.Object):
         return (keyframe.timestamp, keyframe.value)
 
 
-class ClipPropertyChanged(UndoableAction):
-
-    def __init__(self, clip, property_name, old_value, new_value):
-        UndoableAction.__init__(self)
-        self.clip = clip
-        self.property_name = property_name
-        self.old_value = old_value
-        self.new_value = new_value
-
-    def do(self):
-        self.clip.set_property(
-            self.property_name.replace("-", "_"), self.new_value)
-
-    def undo(self):
-        self.clip.set_property(
-            self.property_name.replace("-", "_"), self.old_value)
-
-
 class ClipAdded(UndoableAction):
 
     def __init__(self, layer, clip):
@@ -560,13 +520,9 @@ class TimelineObserver(Loggable):
         ges_layer.disconnect_by_func(self._layer_moved_cb)
 
     def _connectToClip(self, ges_clip):
-        tracker = ClipPropertyChangeTracker(ges_clip)
-        for property_name in tracker.property_names:
-            attr_name = "last-%s" % property_name
-            last_value = ges_clip.get_property(property_name)
-            setattr(tracker, attr_name, last_value)
-        tracker.connect(
-            "monitored-property-changed", self._clipPropertyChangedCb)
+        tracker = PropertyChangeTracker(ges_clip,
+            ["start", "duration", "in-point", "priority"],
+            self.action_log)
         self.clip_property_trackers[ges_clip] = tracker
 
         ges_clip.connect("child-added", self._clipTrackElementAddedCb)
@@ -585,7 +541,6 @@ class TimelineObserver(Loggable):
         clip.disconnect_by_func(self._clipTrackElementRemovedCb)
         tracker = self.clip_property_trackers.pop(clip)
         tracker.release()
-        tracker.disconnect_by_func(self._clipPropertyChangedCb)
 
     def _controlBindingAddedCb(self, track_element, binding):
         self._connectToControlSource(track_element, binding)
@@ -662,22 +617,6 @@ class TimelineObserver(Loggable):
         action = ClipRemoved(layer, clip)
         self.action_log.push(action)
 
-    def _clipPropertyChangedCb(self, tracker, clip,
-                               property_name, old_value, new_value):
-
-        # Do not track clips when we are just moving clips around
-        # while drag and dropping them.
-        if self.app.gui.timeline_ui.timeline.dropping_clips:
-            return
-
-        attr_name = "last-%s" % property_name
-        new_value = clip.get_property(property_name)
-        old_value = getattr(tracker, attr_name)
-        action = ClipPropertyChanged(clip, property_name,
-                                     old_value, new_value)
-        setattr(tracker, attr_name, new_value)
-        self.action_log.push(action)
-
     def _clipTrackElementAddedCb(self, clip, track_element):
         self._connectToTrackElement(track_element)
         if isinstance(track_element, GES.BaseEffect):
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index 8e97f37..8436ce7 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -329,6 +329,22 @@ class MetaContainerObserver(GObject.Object):
         self.action_log.push(action)
 
 
+class PropertyChangedAction(UndoableAction):
+
+    def __init__(self, gobject, field_name, old_value, new_value):
+        UndoableAction.__init__(self)
+        self.gobject = gobject
+        self.field_name = field_name
+        self.old_value = old_value
+        self.new_value = new_value
+
+    def do(self):
+        self.gobject.set_property(self.field_name, self.new_value)
+
+    def undo(self):
+        self.gobject.set_property(self.field_name, self.old_value)
+
+
 class PropertyChangeTracker(GObject.Object):
     """
     Monitors a GObject.Object's props and reports UndoableActions.
@@ -338,34 +354,29 @@ class PropertyChangeTracker(GObject.Object):
         property_names (List[str]): The props to be monitored.
     """
 
-    __gsignals__ = {
-        "monitored-property-changed": (GObject.SIGNAL_RUN_LAST, None, (object, str, object, object)),
-    }
-
-    def __init__(self, gobject, property_names):
+    def __init__(self, gobject, property_names, action_log):
         GObject.Object.__init__(self)
-        self.property_names = property_names
         self.gobject = gobject
-        self.properties = self._takeCurrentSnapshot(gobject)
-        # Connect to obj to keep track when the monitored props change.
-        for property_name in self.property_names:
-            signal_name = "notify::%s" % property_name
-            gobject.connect(signal_name, self._propertyChangedCb, property_name)
+        self.property_names = property_names
+        self.action_log = action_log
 
-    def _takeCurrentSnapshot(self, obj):
-        properties = {}
+        self.properties = {}
         for property_name in self.property_names:
-            properties[property_name] = obj.get_property(
-                property_name.replace("-", "_"))
-
-        return properties
+            field_name = property_name.replace("-", "_")
+            self.properties[property_name] = gobject.get_property(field_name)
+            # Connect to obj to keep track when the monitored props change.
+            signal_name = "notify::%s" % property_name
+            gobject.connect(signal_name, self._property_changed_cb,
+                            property_name, field_name)
 
     def release(self):
-        self.gobject.disconnect_by_func(self._propertyChangedCb)
+        self.gobject.disconnect_by_func(self._property_changed_cb)
         self.gobject = None
 
-    def _propertyChangedCb(self, gobject, property_value, property_name):
+    def _property_changed_cb(self, gobject, pspec, property_name, field_name):
         old_value = self.properties[property_name]
+        property_value = gobject.get_property(field_name)
         self.properties[property_name] = property_value
-        self.emit("monitored-property-changed", gobject,
-                  property_name, old_value, property_value)
+        action = PropertyChangedAction(gobject, field_name,
+                                       old_value, property_value)
+        self.action_log.push(action)
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 243ef42..3b3c167 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -29,10 +29,10 @@ from gi.repository import GstController
 from pitivi.timeline.timeline import Timeline
 from pitivi.undo.project import AssetAddedAction
 from pitivi.undo.timeline import ClipAdded
-from pitivi.undo.timeline import ClipPropertyChanged
 from pitivi.undo.timeline import ClipRemoved
 from pitivi.undo.timeline import TimelineObserver
 from pitivi.undo.timeline import TrackElementAdded
+from pitivi.undo.undo import PropertyChangedAction
 from pitivi.undo.undo import UndoableActionLog
 from tests import common
 
@@ -416,7 +416,7 @@ class TestTimelineUndo(TestCase):
         stack = stacks[0]
         self.assertEqual(1, len(stack.done_actions))
         action = stack.done_actions[0]
-        self.assertTrue(isinstance(action, ClipPropertyChanged))
+        self.assertTrue(isinstance(action, PropertyChangedAction))
         self.assertEqual(10 * Gst.SECOND, clip1.get_start())
 
         self.action_log.undo()