GNOME.org
 

[pitivi: 6/7] viewer: Make title dragging operation undoable


commit 58b5e3b6cac7aec0c5b31e087c5eaa8c5b98ca28
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sat Nov 20 01:52:18 2021 +0100

    viewer: Make title dragging operation undoable
    
    The title UI in the clip properties also had to be fixed so it does not
    create similar operations when its widgets are updated.
    
    Fixes #2586

 pitivi/application.py             | 22 +++++-----
 pitivi/clip_properties/title.py   | 58 ++++++++++++++++++------
 pitivi/clipproperties.py          | 18 ++++----
 pitivi/viewer/overlay_stack.py    |  2 +-
 pitivi/viewer/title_overlay.py    | 17 ++++++--
 tests/common.py                   |  8 ++--
 tests/test_utils_misc.py          |  2 +-
 tests/test_viewer_titleoverlay.py | 92 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 177 insertions(+), 42 deletions(-)
---
diff --git a/pitivi/application.py b/pitivi/application.py
index 68e7a9a69..dc9f6bc47 100644
--- a/pitivi/application.py
+++ b/pitivi/application.py
@@ -2,7 +2,7 @@
 # Pitivi video editor
 # Copyright (c) 2005-2009 Edward Hervey <bilboed bilboed com>
 # Copyright (c) 2008-2009 Alessandro Decina <alessandro d gmail com>
-# Copyright (c) 2014 Alexandru Băluț <alexandru balut gmail com>
+# Copyright (c) 2014 Alex Băluț <alexandru balut gmail com>
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -19,6 +19,7 @@
 import os
 import time
 from gettext import gettext as _
+from typing import Optional
 
 from gi.repository import Gio
 from gi.repository import GLib
@@ -44,6 +45,7 @@ from pitivi.utils.misc import path_from_uri
 from pitivi.utils.misc import quote_uri
 from pitivi.utils.proxy import ProxyManager
 from pitivi.utils.system import get_system
+from pitivi.utils.system import System
 from pitivi.utils.threads import ThreadMaster
 from pitivi.utils.timeline import Zoomable
 
@@ -58,7 +60,7 @@ class Pitivi(Gtk.Application, Loggable):
         recent_manager (Gtk.RecentManager): Manages recently used projects.
         project_manager (ProjectManager): The holder of the current project.
         settings (GlobalSettings): The application-wide settings.
-        system (pitivi.utils.system.System): The system running the app.
+        system (System): The system running the app.
     """
 
     __gsignals__ = {
@@ -71,17 +73,17 @@ class Pitivi(Gtk.Application, Loggable):
                                  flags=Gio.ApplicationFlags.NON_UNIQUE | Gio.ApplicationFlags.HANDLES_OPEN)
         Loggable.__init__(self)
 
-        self.settings = None
-        self.threads = None
-        self.effects = None
-        self.system = None
+        self.settings: Optional[GlobalSettings] = None
+        self.threads: Optional[ThreadMaster] = None
+        self.effects: Optional[EffectsManager] = None
+        self.system: Optional[System] = None
         self.project_manager = ProjectManager(self)
 
-        self.action_log = None
-        self.project_observer = None
-        self._last_action_time = Gst.util_get_timestamp()
+        self.action_log: Optional[UndoableActionLog] = None
+        self.project_observer: Optional[ProjectObserver] = None
+        self._last_action_time: int = Gst.util_get_timestamp()
 
-        self.gui = None
+        self.gui: Optional[MainWindow] = None
         self.recent_manager = Gtk.RecentManager.get_default()
         self.__inhibit_cookies = {}
 
diff --git a/pitivi/clip_properties/title.py b/pitivi/clip_properties/title.py
index 9bd09a324..04aa0f43b 100644
--- a/pitivi/clip_properties/title.py
+++ b/pitivi/clip_properties/title.py
@@ -18,8 +18,11 @@
 import html
 import os
 from gettext import gettext as _
+from typing import Optional
+from typing import Union
 
 from gi.repository import GES
+from gi.repository import GObject
 from gi.repository import Gtk
 from gi.repository import Pango
 
@@ -60,8 +63,13 @@ class TitleProperties(Gtk.Expander, Loggable):
         self.set_label(_("Title"))
         self.set_expanded(True)
         self.app = app
-        self.source = None
+        self.source: Optional[GES.TitleSource] = None
+        # Whether the source's props are being set as a result of UI
+        # interactions.
         self._setting_props = False
+        # Whether the UI is being updated as a result of the props changes
+        # performed not by this class.
+        self._setting_ui = False
         self._children_props_handler = None
 
         self._create_ui()
@@ -139,8 +147,7 @@ class TitleProperties(Gtk.Expander, Loggable):
                 self._setting_props = False
 
     def _drop_shadow_checkbox_cb(self, checkbox):
-        if not self.source:
-            # Nothing to update.
+        if self._setting_ui:
             return
 
         active = checkbox.get_active()
@@ -148,6 +155,9 @@ class TitleProperties(Gtk.Expander, Loggable):
         self._set_child_property("draw-shadow", active)
 
     def _color_picker_value_changed_cb(self, widget, color_button, color_layer):
+        if self._setting_ui:
+            return
+
         argb = widget.calculate_argb()
         self.debug("Setting text %s to %x", color_layer, argb)
         self._set_child_property(color_layer, argb)
@@ -155,11 +165,17 @@ class TitleProperties(Gtk.Expander, Loggable):
         color_button.set_rgba(rgba)
 
     def _background_color_button_cb(self, widget):
+        if self._setting_ui:
+            return
+
         color = gdk_rgba_to_argb(widget.get_rgba())
         self.debug("Setting title background color to %x", color)
         self._set_child_property("foreground-color", color)
 
     def _front_text_color_button_cb(self, widget):
+        if self._setting_ui:
+            return
+
         color = gdk_rgba_to_argb(widget.get_rgba())
         self.debug("Setting title foreground color to %x", color)
         # TODO: Use set_text_color when we work with TitleSources instead of
@@ -167,16 +183,22 @@ class TitleProperties(Gtk.Expander, Loggable):
         self._set_child_property("color", color)
 
     def _front_text_outline_color_button_cb(self, widget):
+        if self._setting_ui:
+            return
+
         color = gdk_rgba_to_argb(widget.get_rgba())
         self.debug("Setting title outline color to %x", color)
         self._set_child_property("outline-color", color)
 
     def _font_button_cb(self, widget):
+        if self._setting_ui:
+            return
+
         font_desc = widget.get_font_desc().to_string()
         self.debug("Setting font desc to %s", font_desc)
         self._set_child_property("font-desc", font_desc)
 
-    def _update_from_source(self, source):
+    def __update_from_source(self, source):
         res, text = source.get_child_property("text")
         assert res
         self.textbuffer.props.text = html.unescape(text or "")
@@ -224,8 +246,7 @@ class TitleProperties(Gtk.Expander, Loggable):
         self.outline_color_button.set_rgba(color)
 
     def _text_changed_cb(self, unused_text_buffer):
-        if not self.source:
-            # Nothing to update.
+        if self._setting_ui:
             return
 
         escaped_text = html.escape(self.textbuffer.props.text)
@@ -234,8 +255,7 @@ class TitleProperties(Gtk.Expander, Loggable):
 
     def _alignment_changed_cb(self, combo):
         """Handles changes in the h/v alignment widgets."""
-        if not self.source:
-            # Nothing to update.
+        if self._setting_ui:
             return
 
         if combo == self.valignment_combo:
@@ -249,8 +269,7 @@ class TitleProperties(Gtk.Expander, Loggable):
 
     def _absolute_alignment_value_changed_cb(self, spin):
         """Handles changes in the absolute alignment widgets."""
-        if not self.source:
-            # Nothing to update.
+        if self._setting_ui:
             return
 
         if spin == self.x_absolute_spin:
@@ -266,7 +285,7 @@ class TitleProperties(Gtk.Expander, Loggable):
         visible = self.halignment_combo.get_active_id() == "absolute"
         self.x_absolute_spin.set_visible(visible)
 
-    def set_source(self, source):
+    def set_source(self, source: Optional[Union[GES.TextOverlay, GES.TitleSource]]):
         """Sets the clip to be edited with this editor.
 
         Args:
@@ -281,7 +300,11 @@ class TitleProperties(Gtk.Expander, Loggable):
 
         if source:
             assert isinstance(source, (GES.TextOverlay, GES.TitleSource))
-            self._update_from_source(source)
+            self._setting_ui = True
+            try:
+                self.__update_from_source(source)
+            finally:
+                self._setting_ui = False
             self._children_props_handler = source.connect("deep-notify",
                                                           self._source_deep_notify_cb)
             self.source = source
@@ -300,6 +323,15 @@ class TitleProperties(Gtk.Expander, Loggable):
                        pspec.name)
             return
 
+        self._setting_ui = True
+        try:
+            self.__update_ui_for_prop(pspec)
+        finally:
+            self._setting_ui = False
+
+        self.app.project_manager.current_project.pipeline.commit_timeline()
+
+    def __update_ui_for_prop(self, pspec: GObject.ParamSpec):
         res, value = self.source.get_child_property(pspec.name)
         assert res, pspec.name
         if pspec.name == "text":
@@ -346,5 +378,3 @@ class TitleProperties(Gtk.Expander, Loggable):
             self.outline_color_button.set_rgba(color)
         elif pspec.name == "draw-shadow":
             self.drop_shadow_checkbox.set_active(value)
-
-        self.app.project_manager.current_project.pipeline.commit_timeline()
diff --git a/pitivi/clipproperties.py b/pitivi/clipproperties.py
index d809fb161..75ad59600 100644
--- a/pitivi/clipproperties.py
+++ b/pitivi/clipproperties.py
@@ -154,15 +154,15 @@ class ClipProperties(Gtk.ScrolledWindow, Loggable):
         label.set_xalign(0)
         box.pack_start(label, False, False, SPACING)
 
-        title_button = Gtk.Button()
-        title_button.set_label(_("Create a title clip"))
-        title_button.connect("clicked", self.create_title_clip_cb)
-        box.pack_start(title_button, False, False, SPACING)
-
-        color_button = Gtk.Button()
-        color_button.set_label(_("Create a color clip"))
-        color_button.connect("clicked", self.create_color_clip_cb)
-        box.pack_start(color_button, False, False, SPACING)
+        self._title_button = Gtk.Button()
+        self._title_button.set_label(_("Create a title clip"))
+        self._title_button.connect("clicked", self.create_title_clip_cb)
+        box.pack_start(self._title_button, False, False, SPACING)
+
+        self._color_button = Gtk.Button()
+        self._color_button.set_label(_("Create a color clip"))
+        self._color_button.connect("clicked", self.create_color_clip_cb)
+        box.pack_start(self._color_button, False, False, SPACING)
 
         box.show_all()
         return box
diff --git a/pitivi/viewer/overlay_stack.py b/pitivi/viewer/overlay_stack.py
index 6c4f2f0dc..9b2f4c695 100644
--- a/pitivi/viewer/overlay_stack.py
+++ b/pitivi/viewer/overlay_stack.py
@@ -77,7 +77,7 @@ class OverlayStack(Gtk.Overlay, Loggable):
         for overlay in self.__overlays.values():
             overlay.update_from_source()
 
-    def __overlay_for_source(self, source):
+    def __overlay_for_source(self, source: GES.Source):
         if source in self.__overlays:
             return self.__overlays[source]
 
diff --git a/pitivi/viewer/title_overlay.py b/pitivi/viewer/title_overlay.py
index 327d5e527..2a785c8cb 100644
--- a/pitivi/viewer/title_overlay.py
+++ b/pitivi/viewer/title_overlay.py
@@ -17,6 +17,7 @@
 import cairo
 import numpy
 
+from pitivi.undo.timeline import CommitTimelineFinalizingAction
 from pitivi.viewer.overlay import Overlay
 
 
@@ -71,7 +72,7 @@ class TitleOverlay(Overlay):
         self.queue_draw()
 
     def on_hover(self, cursor_position):
-        if (self.__position < cursor_position).all() and (cursor_position < self.__position + 
self.__size).all():
+        if (self.__position <= cursor_position).all() and (cursor_position < self.__position + 
self.__size).all():
             if self._is_selected():
                 self.stack.set_cursor("grab")
             self._hover()
@@ -85,11 +86,19 @@ class TitleOverlay(Overlay):
         if self._is_hovered():
             self._select()
             self.stack.set_cursor("grabbing")
-            self.stack.selected_overlay = self
-        elif self._is_selected():
-            self._deselect()
+            self.stack.app.action_log.begin("Title drag",
+                                            finalizing_action=CommitTimelineFinalizingAction(
+                                                self.stack.app.project_manager.current_project.pipeline),
+                                            toplevel=True)
+        else:
+            # We're not hovered anymore, make sure we're not selected.
+            if self._is_selected():
+                self._deselect()
 
     def on_button_release(self, cursor_position):
+        if self._is_selected():
+            self.stack.app.action_log.commit("Title drag")
+
         self.on_hover(cursor_position)
         if self._is_hovered():
             self.stack.set_cursor("grab")
diff --git a/tests/common.py b/tests/common.py
index 3af01ae5e..5b0404e34 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -111,14 +111,16 @@ def create_project():
     return project
 
 
-def create_pitivi(**settings):
+def create_pitivi(**settings) -> Pitivi:
+    """Creates a Pitivi app with the specified settings, ready to be tested."""
     app = Pitivi()
     app._setup()
 
+    app.settings = __create_settings(**settings)
+
+    # Patch a main window object so things depending on it can work properly.
     app.gui = mock.Mock()
     app.gui.editor.viewer.action_group = Gio.SimpleActionGroup()
-
-    app.settings = __create_settings(**settings)
     app.gui.editor.editor_state = EditorState(app.project_manager)
 
     return app
diff --git a/tests/test_utils_misc.py b/tests/test_utils_misc.py
index 99a19a1b6..0d9a184b6 100644
--- a/tests/test_utils_misc.py
+++ b/tests/test_utils_misc.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 # Pitivi video editor
-# Copyright (c) 2020, Alexandru Băluț <alexandru balut gmail com>
+# Copyright (c) 2020, Alex Băluț <alexandru balut gmail com>
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
diff --git a/tests/test_viewer_titleoverlay.py b/tests/test_viewer_titleoverlay.py
new file mode 100644
index 000000000..4a1e3cd89
--- /dev/null
+++ b/tests/test_viewer_titleoverlay.py
@@ -0,0 +1,92 @@
+# -*- coding: utf-8 -*-
+# Pitivi video editor
+# Copyright (c) 2021, Alex Băluț <alexandru balut gmail com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, see <http://www.gnu.org/licenses/>.
+"""Tests for the pitivi.viewer.title_overlay module."""
+# pylint: disable=protected-access,no-self-use,attribute-defined-outside-init
+from unittest import mock
+
+from gi.repository import Gdk
+from gi.repository import GES
+
+from pitivi.viewer.title_overlay import TitleOverlay
+from tests import common
+
+
+class TitleOverlayTest(common.TestCase):
+    """Tests for the TitleOverlay class."""
+
+    def assert_title_source_absolute_position(self, source: GES.TitleSource, expected_x: float, expected_y: 
float):
+        res, val = source.get_child_property("x-absolute")
+        self.assertTrue(res)
+        self.assertAlmostEqual(val, expected_x)
+        res, val = source.get_child_property("y-absolute")
+        self.assertTrue(res)
+        self.assertAlmostEqual(val, expected_y)
+
+    def test_dragging_creates_an_undoable_operation(self):
+        mainloop = common.create_main_loop()
+
+        app = common.create_pitivi()
+        app.gui = None
+        app.create_main_window()
+        app.project_manager.new_blank_project()
+        mainloop.run(until_empty=True)
+        self.assertDictEqual(app.gui.editor.viewer.overlay_stack._OverlayStack__overlays, {})
+
+        app.gui.editor.clipconfig._title_button.clicked()
+        mainloop.run(until_empty=True)
+
+        self.assertEqual(len(app.gui.editor.viewer.overlay_stack._OverlayStack__overlays), 1)
+        source = app.gui.editor.clipconfig.title_expander.source
+        overlay = app.gui.editor.viewer.overlay_stack._OverlayStack__overlays[source]
+        self.assertIsInstance(overlay, TitleOverlay)
+
+        overlay_stack = app.gui.editor.viewer.overlay_stack
+        # The overlay of the selected clip's video source should have been
+        # selected.
+        self.assertIs(overlay_stack.selected_overlay, overlay)
+
+        # Simulate hover so it can be dragged.
+        w, h = overlay_stack.window_size
+        self.assertIs(overlay_stack.hovered_overlay, None)
+        event = mock.Mock(type=Gdk.EventType.MOTION_NOTIFY, x=w / 2, y=h / 2)
+        overlay_stack.do_event(event)
+        self.assertIs(overlay_stack.hovered_overlay, overlay)
+
+        # Simulate drag&drop.
+        event = mock.Mock(type=Gdk.EventType.BUTTON_PRESS, x=w / 2, y=h / 2)
+        overlay_stack.do_event(event)
+        self.assert_title_source_absolute_position(source, 0.5, 0.5)
+
+        event = mock.Mock(type=Gdk.EventType.MOTION_NOTIFY, x=0, y=0)
+        overlay_stack.do_event(event)
+
+        event = mock.Mock(type=Gdk.EventType.BUTTON_RELEASE, x=0, y=0)
+        overlay_stack.do_event(event)
+        mainloop.run(until_empty=True)
+        # These values are weird.
+        self.assert_title_source_absolute_position(source, -0.24980483996877437, -0.08686210640608036)
+
+        # Undo movement.
+        app.action_log.undo()
+        self.assert_title_source_absolute_position(source, 0.5, 0.5)
+        # Undo title creation.
+        app.action_log.undo()
+
+        app.action_log.redo()
+        self.assert_title_source_absolute_position(source, 0.5, 0.5)
+        app.action_log.redo()
+        self.assert_title_source_absolute_position(source, -0.24980483996877437, -0.08686210640608036)
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]