From 5ab305c4cf8ef68f815f39071ec0b030ac66d4af Mon Sep 17 00:00:00 2001
From: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date: Thu, 1 Aug 2024 13:42:33 -0700
Subject: [PATCH] Fix GUI crash on scroll (#1883)

* Only pass wheelEvent to children that can handle it

* Add test for wheelEvent
---
 sleap/gui/widgets/video.py     | 27 ++++++++++-----------
 tests/gui/test_video_player.py | 44 +++++++++++++++++++++++++++++-----
 2 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/sleap/gui/widgets/video.py b/sleap/gui/widgets/video.py
index 745908048..04965bbbb 100644
--- a/sleap/gui/widgets/video.py
+++ b/sleap/gui/widgets/video.py
@@ -1150,8 +1150,13 @@ def mouseDoubleClickEvent(self, event: QMouseEvent):
         QGraphicsView.mouseDoubleClickEvent(self, event)
 
     def wheelEvent(self, event):
-        """Custom event handler. Zoom in/out based on scroll wheel change."""
-        # zoom on wheel when no mouse buttons are pressed
+        """Custom event handler to zoom in/out based on scroll wheel change.
+
+        We cannot use the default QGraphicsView.wheelEvent behavior since that will
+        scroll the view.
+        """
+
+        # Zoom on wheel when no mouse buttons are pressed
         if event.buttons() == Qt.NoButton:
             angle = event.angleDelta().y()
             factor = 1.1 if angle > 0 else 0.9
@@ -1159,20 +1164,10 @@ def wheelEvent(self, event):
             self.zoomFactor = max(factor * self.zoomFactor, 1)
             self.updateViewer()
 
-        # Trigger wheelEvent for all child elements. This is a bit of a hack.
-        # We can't use QGraphicsView.wheelEvent(self, event) since that will scroll
-        # view.
-        # We want to trigger for all children, since wheelEvent should continue rotating
-        # an skeleton even if the skeleton node/node label is no longer under the
-        # cursor.
-        # Note that children expect a QGraphicsSceneWheelEvent event, which is why we're
-        # explicitly ignoring TypeErrors. Everything seems to work fine since we don't
-        # care about the mouse position; if we did, we'd need to map pos to scene.
+        # Trigger only for rotation-relevant children (otherwise GUI crashes)
         for child in self.items():
-            try:
+            if isinstance(child, (QtNode, QtNodeLabel)):
                 child.wheelEvent(event)
-            except TypeError:
-                pass
 
     def keyPressEvent(self, event):
         """Custom event hander, disables default QGraphicsView behavior."""
@@ -1590,7 +1585,9 @@ def mouseReleaseEvent(self, event):
     def wheelEvent(self, event):
         """Custom event handler for mouse scroll wheel."""
         if self.dragParent:
-            angle = event.delta() / 20 + self.parentObject().rotation()
+            angle = (
+                event.angleDelta().x() + event.angleDelta().y()
+            ) / 20 + self.parentObject().rotation()
             self.parentObject().setRotation(angle)
             event.accept()
 
diff --git a/tests/gui/test_video_player.py b/tests/gui/test_video_player.py
index b0661a4e1..c246f0489 100644
--- a/tests/gui/test_video_player.py
+++ b/tests/gui/test_video_player.py
@@ -3,14 +3,13 @@
 from sleap.gui.widgets.video import (
     QtVideoPlayer,
     GraphicsView,
-    QtInstance,
     QtVideoPlayer,
     QtTextWithBackground,
     VisibleBoundingBox,
 )
 
 from qtpy import QtCore, QtWidgets
-from qtpy.QtGui import QColor
+from qtpy.QtGui import QColor, QWheelEvent
 
 
 def test_gui_video(qtbot):
@@ -20,10 +19,6 @@ def test_gui_video(qtbot):
 
     assert vp.close()
 
-    # Click the button 20 times
-    # for i in range(20):
-    #     qtbot.mouseClick(vp.btn, QtCore.Qt.LeftButton)
-
 
 def test_gui_video_instances(qtbot, small_robot_mp4_vid, centered_pair_labels):
     vp = QtVideoPlayer(small_robot_mp4_vid)
@@ -144,3 +139,40 @@ def test_VisibleBoundingBox(qtbot, centered_pair_labels):
     # Check if bounding box scaled appropriately
     assert inst.box.rect().width() - initial_width == 2 * dx
     assert inst.box.rect().height() - initial_height == 2 * dy
+
+
+def test_wheelEvent(qtbot):
+    """Test the wheelEvent method of the GraphicsView class."""
+    graphics_view = GraphicsView()
+
+    # Create a QWheelEvent
+    position = QtCore.QPointF(100, 100)  # The position of the wheel event
+    global_position = QtCore.QPointF(100, 100)  # The global position of the wheel event
+    pixel_delta = QtCore.QPoint(0, 120)  # The distance in pixels the wheel is rotated
+    angle_delta = QtCore.QPoint(0, 120)  # The distance in degrees the wheel is rotated
+    buttons = QtCore.Qt.NoButton  # No mouse button is pressed
+    modifiers = QtCore.Qt.NoModifier  # No keyboard modifier is pressed
+    phase = QtCore.Qt.ScrollUpdate  # The phase of the scroll event
+    inverted = False  # The scroll direction is not inverted
+    source = (
+        QtCore.Qt.MouseEventNotSynthesized
+    )  # The event is not synthesized from a touch or tablet event
+
+    event = QWheelEvent(
+        position,
+        global_position,
+        pixel_delta,
+        angle_delta,
+        buttons,
+        modifiers,
+        phase,
+        inverted,
+        source,
+    )
+
+    # Call the wheelEvent method
+    print(
+        "Testing GraphicsView.wheelEvent which will result in exit code 127 "
+        "originating from a segmentation fault if it fails."
+    )
+    graphics_view.wheelEvent(event)