[pitivi/1.0] effects: Handle dependent property update

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

commit 6e0bcf194b5f6be243e6b7a977dbf92be4f689c9
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Fri Jan 25 17:54:39 2019 +0100

    effects: Handle dependent property update
    
    When EffectsPropertiesManager._on_widget_value_changed_cb sets the
    effect property and this updates a dependent property, its widget is
    updated and we should ignore that.
    
    Fixes #2274
    
    The check in EffectsPropertiesManager._on_widget_value_changed_cb
    whether the value actually changed has been removed as per
    00ab81d3e7bb5bea03a09b91ceb010e92c19d619

 pitivi/effects.py       | 32 +++++++++++++++++---------------
 pitivi/utils/widgets.py |  6 ++++--
 tests/test_effects.py   | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 17 deletions(-)
---
diff --git a/pitivi/effects.py b/pitivi/effects.py
index 6a5d20e8..c3e642ae 100644
--- a/pitivi/effects.py
+++ b/pitivi/effects.py
@@ -649,27 +649,29 @@ class EffectsPropertiesManager:
                 if isinstance(widget, FractionWidget):
                     widget.addPresets(["4:3", "5:4", "9:3", "16:9", "16:10"])
 
-    def _connectAllWidgetCallbacks(self, effect_settings_widget, effect):
-        for prop, widget in effect_settings_widget.properties.items():
-            widget.connectValueChanged(self._onValueChangedCb, widget, prop, effect)
-
-    def _onSetDefaultCb(self, unused_widget, effect_widget):
-        effect_widget.setWidgetToDefault()
+    def _connectAllWidgetCallbacks(self, effect_widget, effect):
+        for prop, widget in effect_widget.properties.items():
+            widget.connectValueChanged(self._on_widget_value_changed_cb, widget, prop, effect, effect_widget)
+
+    def _on_widget_value_changed_cb(self, unused_widget, prop_widget, prop, effect, effect_widget):
+        if effect_widget.updating_property:
+            # The widget is updated as a side-effect of setting one of its
+            # properties. Ignore.
+            return
 
-    def _onValueChangedCb(self, unused_widget, effect_widget, prop, effect):
-        value = effect_widget.getWidgetValue()
+        effect_widget.updating_property = True
+        try:
+            value = prop_widget.getWidgetValue()
 
-        # FIXME Workaround in order to make aspectratiocrop working
-        if isinstance(value, Gst.Fraction):
-            value = Gst.Fraction(int(value.num), int(value.denom))
+            # FIXME Workaround in order to make aspectratiocrop working
+            if isinstance(value, Gst.Fraction):
+                value = Gst.Fraction(int(value.num), int(value.denom))
 
-        res, current_value = effect.get_child_property(prop.name)
-        assert res
-        if value != current_value:
             from pitivi.undo.timeline import CommitTimelineFinalizingAction
-
             pipeline = self.app.project_manager.current_project.pipeline
             with self.app.action_log.started("Effect property change",
                                              finalizing_action=CommitTimelineFinalizingAction(pipeline),
                                              toplevel=True):
                 effect.set_child_property(prop.name, value)
+        finally:
+            effect_widget.updating_property = False
diff --git a/pitivi/utils/widgets.py b/pitivi/utils/widgets.py
index d6fa7fbb..4cf38136 100644
--- a/pitivi/utils/widgets.py
+++ b/pitivi/utils/widgets.py
@@ -417,8 +417,9 @@ class FractionWidget(TextWidget, DynamicWidget):
             return self._parseText(self.last_valid)
         return Gst.Fraction(1, 1)
 
-    def _parseText(self, text):
-        match = self.fraction_regex.match(text)
+    @classmethod
+    def _parseText(cls, text):
+        match = cls.fraction_regex.match(text)
         groups = match.groups()
         num = 1.0
         denom = 1.0
@@ -663,6 +664,7 @@ class GstElementSettingsWidget(Gtk.Box, Loggable):
         self.caps_widgets = {}
         self.__controllable = controllable
         self.set_orientation(Gtk.Orientation.VERTICAL)
+        self.updating_property = False
 
     def deactivate_keyframe_toggle_buttons(self):
         """Makes sure the keyframe togglebuttons are deactivated."""
diff --git a/tests/test_effects.py b/tests/test_effects.py
index bf995625..f3160ed6 100644
--- a/tests/test_effects.py
+++ b/tests/test_effects.py
@@ -18,9 +18,11 @@
 # Boston, MA 02110-1301, USA.
 """Tests for the effects module."""
 from gi.repository import GES
+from gi.repository import Gst
 
 from pitivi.effects import AUDIO_EFFECT
 from pitivi.effects import EffectInfo
+from pitivi.effects import EffectsPropertiesManager
 from pitivi.effects import VIDEO_EFFECT
 from tests import common
 
@@ -64,3 +66,43 @@ class EffectInfoTest(common.TestCase):
         effect_info = EffectInfo(None, VIDEO_EFFECT, None, None, None)
         self.assertFalse(effect_info.good_for_track_element(audio_track_element))
         self.assertTrue(effect_info.good_for_track_element(video_track_element))
+
+
+class EffectsPropertiesManagerTest(common.TestCase):
+    """Tests for the EffectsPropertiesManager class."""
+
+    def test_dependent_properties(self):
+        """Checks dependent properties updating is handled correctly."""
+        mainloop = common.create_main_loop()
+        app = common.create_pitivi()
+        app.project_manager.newBlankProject()
+        manager = EffectsPropertiesManager(app)
+
+        called = False
+
+        def set_child_property(prop_name, value):
+            nonlocal called
+            called = True
+
+            self.assertEqual(prop_name, "aspect-ratio")
+            GES.Effect.set_child_property(effect, prop_name, value)
+
+            # When setting the aspect-ratio property, and the stars align,
+            # the effect also changes the left/right properties.
+            # Here we simulate the updating of the dependent properties.
+            GES.Effect.set_child_property(effect, "left", 100)
+            GES.Effect.set_child_property(effect, "right", 100)
+
+        effect = GES.Effect.new("aspectratiocrop")
+        effect.set_child_property = set_child_property
+
+        effect_widget = manager.getEffectConfigurationUI(effect)
+
+        widgets = {prop.name: widget
+                   for prop, widget in effect_widget.properties.items()}
+        # Simulate the user choosing an aspect-ratio.
+        widgets["aspect-ratio"].setWidgetValue(Gst.Fraction(4, 3))
+
+        mainloop.run(until_empty=True)
+
+        self.assertTrue(called)