[pitivi/clipprops: 2/14] clipproperties: Fix single-clip selection restoring

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

commit 4488180207f2b8e79077c8a30c0065e4d77d921e
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sun Sep 6 21:13:53 2020 +0200

    clipproperties: Fix single-clip selection restoring
    
    When reopening the project, the selected clip was not being properly
    shown in the middle pane because the TransformationProperties expander
    connects to the new-project-loaded signal after the timeline restores
    the selection.
    
    By refactoring TransformationProperties's mechanism of being notified
    about the selection change, it gets properly notified of the initial
    selection.
    
    This brings TransformationProperties in line with the EffectProperties
    and the other expanders, all of them being notified through
    ClipProperties._selection_changed_cb now.
    
    Fixes #2486

 pitivi/clip_properties/title.py    |  9 ++---
 pitivi/clipproperties.py           | 71 +++++++++++++-------------------------
 pitivi/editorperspective.py        |  7 ++--
 pitivi/viewer/overlay_stack.py     |  6 ++++
 pitivi/viewer/viewer.py            |  9 ++---
 tests/common.py                    |  2 +-
 tests/test_clipproperties_color.py |  3 ++
 tests/test_timeline_elements.py    |  2 +-
 tests/test_viewer_viewer.py        |  4 +--
 9 files changed, 50 insertions(+), 63 deletions(-)
---
diff --git a/pitivi/clip_properties/title.py b/pitivi/clip_properties/title.py
index 9f375ce99..8eecc10d3 100644
--- a/pitivi/clip_properties/title.py
+++ b/pitivi/clip_properties/title.py
@@ -207,15 +207,16 @@ class TitleProperties(Gtk.Expander, Loggable):
         Args:
             source (GES.TitleSource): The source of the clip.
         """
-        self.debug("Source set to %s", source)
-        if self._children_props_handler is not None:
+        self.debug("Setting source to %s", source)
+        if self.source:
             self.source.disconnect(self._children_props_handler)
             self._children_props_handler = None
-        self.source = None
+
+        self.source = source
+
         if source:
             assert isinstance(source, (GES.TextOverlay, GES.TitleSource))
             self._update_from_source(source)
-            self.source = source
             self._children_props_handler = self.source.connect("deep-notify",
                                                                self._source_deep_notify_cb)
         self.set_visible(bool(self.source))
diff --git a/pitivi/clipproperties.py b/pitivi/clipproperties.py
index 186aaf93d..b12d44c7e 100644
--- a/pitivi/clipproperties.py
+++ b/pitivi/clipproperties.py
@@ -107,16 +107,13 @@ class ClipProperties(Gtk.ScrolledWindow, Loggable):
         self.helper_box = self.create_helper_box()
         self.clips_box.pack_start(self.helper_box, False, False, 0)
 
+        self.transformation_expander.set_source(None)
         self.title_expander.set_source(None)
         self.color_expander.set_source(None)
         self.effect_expander.set_clip(None)
 
         self._project = None
         self._selection = None
-        self.app.project_manager.connect_after(
-            "new-project-loaded", self.new_project_loaded_cb)
-        self.app.project_manager.connect_after(
-            "project-closed", self.__project_closed_cb)
 
     def create_helper_box(self):
         """Creates the widgets to display when no clip is selected."""
@@ -174,13 +171,13 @@ class ClipProperties(Gtk.ScrolledWindow, Loggable):
             self.app.gui.editor.timeline_ui.insert_clips_on_first_layer([color_clip])
         self._selection.set_selection([color_clip], SELECT)
 
-    def new_project_loaded_cb(self, unused_project_manager, project):
-        if self._selection is not None:
+    def set_project(self, project, timeline_ui):
+        if self._project:
             self._selection.disconnect_by_func(self._selection_changed_cb)
             self._selection = None
         if project:
-            self._selection = project.ges_timeline.ui.selection
-            self._selection.connect('selection-changed', self._selection_changed_cb)
+            self._selection = timeline_ui.timeline.selection
+            self._selection.connect("selection-changed", self._selection_changed_cb)
         self._project = project
 
     def _selection_changed_cb(self, selection):
@@ -188,23 +185,27 @@ class ClipProperties(Gtk.ScrolledWindow, Loggable):
         single_clip_selected = len(selected_clips) == 1
         self.helper_box.set_visible(not single_clip_selected)
 
+        video_source = None
         title_source = None
         color_clip_source = None
         if single_clip_selected:
             ges_clip = list(selected_clips)[0]
             self.effect_expander.set_clip(ges_clip)
+
             for child in ges_clip.get_children(False):
+                if isinstance(child, GES.VideoSource):
+                    video_source = child
+
                 if isinstance(child, GES.TitleSource):
                     title_source = child
-                    break
-                if isinstance(child, GES.VideoTestSource):
+                elif isinstance(child, GES.VideoTestSource):
                     color_clip_source = child
-                    break
+
+        self.transformation_expander.set_source(video_source)
         self.title_expander.set_source(title_source)
         self.color_expander.set_source(color_clip_source)
 
-    def __project_closed_cb(self, unused_project_manager, unused_project):
-        self._project = None
+        self.app.gui.editor.viewer.overlay_stack.select(video_source)
 
 
 class EffectProperties(Gtk.Expander, Loggable):
@@ -546,9 +547,7 @@ class TransformationProperties(Gtk.Expander, Loggable):
         Loggable.__init__(self)
         self.app = app
         self._project = None
-        self._selection = None
         self.source = None
-        self._selected_clip = None
         self.spin_buttons = {}
         self.spin_buttons_handler_ids = {}
         self.set_label(_("Transformation"))
@@ -578,18 +577,12 @@ class TransformationProperties(Gtk.Expander, Loggable):
         self.app.project_manager.connect_after(
             "project-closed", self.__project_closed_cb)
 
-    def _new_project_loaded_cb(self, unused_app, project):
-        if self._selection is not None:
-            self._selection.disconnect_by_func(self._selection_changed_cb)
-            self._selection = None
+    def _new_project_loaded_cb(self, unused_project_manager, project):
         if self._project:
             self._project.pipeline.disconnect_by_func(self._position_cb)
 
         self._project = project
         if project:
-            self._selection = project.ges_timeline.ui.selection
-            self._selection.connect(
-                'selection-changed', self._selection_changed_cb)
             self._project.pipeline.connect("position", self._position_cb)
 
     def __project_closed_cb(self, unused_project_manager, unused_project):
@@ -787,6 +780,7 @@ class TransformationProperties(Gtk.Expander, Loggable):
     def _position_cb(self, unused_pipeline, unused_position):
         if not self.__source_uses_keyframes():
             return
+
         for prop in ["posx", "posy", "width", "height"]:
             self.__update_spin_btn(prop)
         # Keep the overlay stack in sync with the spin buttons values
@@ -867,16 +861,15 @@ class TransformationProperties(Gtk.Expander, Loggable):
                 self.__set_prop(prop, value)
             self.app.gui.editor.viewer.overlay_stack.update(self.source)
 
-    def __set_source(self, source):
+    def set_source(self, source):
+        self.debug("Setting source to %s", source)
+
         if self.source:
-            try:
-                self.source.disconnect_by_func(
-                    self.__source_property_changed_cb)
-                disconnect_all_by_func(
-                    self.source, self._control_bindings_changed)
-            except TypeError:
-                pass
+            self.source.disconnect_by_func(self.__source_property_changed_cb)
+            disconnect_all_by_func(self.source, self._control_bindings_changed)
+
         self.source = source
+
         if self.source:
             self.__update_control_bindings()
             for prop in self.spin_buttons:
@@ -886,20 +879,4 @@ class TransformationProperties(Gtk.Expander, Loggable):
             self.source.connect("control-binding-added", self._control_bindings_changed)
             self.source.connect("control-binding-removed", self._control_bindings_changed)
 
-    def _selection_changed_cb(self, unused_timeline):
-        if len(self._selection) == 1:
-            clip = list(self._selection)[0]
-            source = clip.find_track_element(None, GES.VideoSource)
-            if source:
-                self._selected_clip = clip
-                self.__set_source(source)
-                self.app.gui.editor.viewer.overlay_stack.select(source)
-                self.show()
-                return
-
-        # Deselect
-        if self._selected_clip:
-            self._selected_clip = None
-            self._project.pipeline.commit_timeline()
-        self.__set_source(None)
-        self.hide()
+        self.set_visible(bool(self.source))
diff --git a/pitivi/editorperspective.py b/pitivi/editorperspective.py
index 0c5790b26..4a88a67a0 100644
--- a/pitivi/editorperspective.py
+++ b/pitivi/editorperspective.py
@@ -476,8 +476,8 @@ class EditorPerspective(Perspective, Loggable):
         self._connect_to_project(project)
         project.pipeline.activate_position_listener()
 
-        self.clipconfig.project = project
-
+        self.viewer.set_project(project)
+        self.clipconfig.set_project(project, self.timeline_ui)
         self.timeline_ui.set_project(project)
 
         # When creating a blank project there's no project URI yet.
@@ -591,7 +591,10 @@ class EditorPerspective(Perspective, Loggable):
         self.info("Project closed")
         if project.loaded:
             self._disconnect_from_project(project)
+
         self.timeline_ui.set_project(None)
+        self.clipconfig.set_project(None, None)
+
         self.render_button.set_sensitive(False)
         return False
 
diff --git a/pitivi/viewer/overlay_stack.py b/pitivi/viewer/overlay_stack.py
index c269107bc..6c4f2f0dc 100644
--- a/pitivi/viewer/overlay_stack.py
+++ b/pitivi/viewer/overlay_stack.py
@@ -138,6 +138,7 @@ class OverlayStack(Gtk.Overlay, Loggable):
         return True
 
     def set_current_sources(self, sources):
+        """Sets the sources at the playhead."""
         self.__visible_overlays = []
         # check if source has instanced viewer
         for source in sources:
@@ -155,6 +156,11 @@ class OverlayStack(Gtk.Overlay, Loggable):
         self.__overlays[source].update_from_source()
 
     def select(self, source):
+        """Specifies the selected source between the sources at the playhead."""
+        if not source:
+            self.selected_overlay = None
+            return
+
         self.selected_overlay = self.__overlay_for_source(source)
         self.selected_overlay.queue_draw()
 
diff --git a/pitivi/viewer/viewer.py b/pitivi/viewer/viewer.py
index 621eaae42..63608e8cb 100644
--- a/pitivi/viewer/viewer.py
+++ b/pitivi/viewer/viewer.py
@@ -102,14 +102,8 @@ class ViewerContainer(Gtk.Box, Loggable):
         self.__translation = None
 
         pm = self.app.project_manager
-        pm.connect("new-project-loaded", self._project_manager_new_project_loaded_cb)
         pm.connect("project-closed", self._project_manager_project_closed_cb)
 
-    def _project_manager_new_project_loaded_cb(self, unused_project_manager, project):
-        project.connect("rendering-settings-changed",
-                        self._project_rendering_settings_changed_cb)
-        self.set_project(project)
-
     def _project_manager_project_closed_cb(self, unused_project_manager, project):
         if self.project == project:
             project.disconnect_by_func(self._project_rendering_settings_changed_cb)
@@ -150,6 +144,9 @@ class ViewerContainer(Gtk.Box, Loggable):
         # appears in a separate window.
         project.pipeline.pause()
 
+        project.connect("rendering-settings-changed",
+                        self._project_rendering_settings_changed_cb)
+
     def __create_new_viewer(self):
         _, sink_widget = self.project.pipeline.create_sink()
 
diff --git a/tests/common.py b/tests/common.py
index 57c301e8b..aadce0839 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -274,7 +274,7 @@ def setup_clipproperties(func):
         app = self.timeline_container.app
 
         self.clipproperties = ClipProperties(app)
-        self.clipproperties.new_project_loaded_cb(None, self.project)
+        self.clipproperties.set_project(self.project, self.timeline_container)
 
         self.transformation_box = self.clipproperties.transformation_expander
         self.transformation_box._new_project_loaded_cb(None, self.project)
diff --git a/tests/test_clipproperties_color.py b/tests/test_clipproperties_color.py
index 8227532f6..d6ecd9def 100644
--- a/tests/test_clipproperties_color.py
+++ b/tests/test_clipproperties_color.py
@@ -51,6 +51,9 @@ class ColorPropertiesTest(common.TestCase):
 
         self.clipproperties.create_color_clip_cb(None)
 
+        mainloop = common.create_main_loop()
+        mainloop.run(until_empty=True)
+
         color_expander = self.clipproperties.color_expander
         color_picker_mock = mock.Mock()
         color_picker_mock.calculate_argb.return_value = 1 << 24 | 2 << 16 | 3 << 8 | 4
diff --git a/tests/test_timeline_elements.py b/tests/test_timeline_elements.py
index 59c2ecbe8..edc8f41d7 100644
--- a/tests/test_timeline_elements.py
+++ b/tests/test_timeline_elements.py
@@ -353,8 +353,8 @@ class TestVideoSource(common.TestCase):
 
         def check_set_pos_and_project_size(new_position, new_project_width,
                                            new_project_height, expected_position):
-
             self.timeline_container.timeline.selection.select([clip])
+            self.transformation_box.set_source(source)
             reset_clip_properties_button.clicked()
 
             assert_child_props(source, {"width": self.project.videowidth, "height": 
self.project.videoheight})
diff --git a/tests/test_viewer_viewer.py b/tests/test_viewer_viewer.py
index 4de424cb6..bf3d7be87 100644
--- a/tests/test_viewer_viewer.py
+++ b/tests/test_viewer_viewer.py
@@ -27,10 +27,10 @@ class TestViewerContainer(common.TestCase):
         """Checks the effect of the toggle_safe_areas_action."""
         app = common.create_pitivi_mock()
         app.project_manager = ProjectManager(app)
-
         viewer_container = ViewerContainer(app)
 
-        app.project_manager.new_blank_project()
+        project = app.project_manager.new_blank_project()
+        viewer_container.set_project(project)
         self.assertFalse(viewer_container.overlay_stack.safe_areas_overlay.get_visible())
 
         viewer_container.toggle_safe_areas_action.activate()