Fix issues around selecting/deselecting (segfaults and hangs)

beeref/items.py

@@ -89,11 +89,10 @@ class BeePixmapItem(QtWidgets.QGraphicsPixmapItem):
         return self.pixmap().size().height()
 
     def itemChange(self, change, value):
-        if (change == QGraphicsItem.GraphicsItemChange.ItemSelectedChange
-                and value
-                and self.scene()
-                and not self.scene().has_selection()):
-            self.bring_to_front()
+        if change == QGraphicsItem.GraphicsItemChange.ItemSelectedChange:
+            self.prepareGeometryChange()
+            if(value and self.scene() and not self.scene().has_selection()):
+                self.bring_to_front()
         return super().itemChange(change, value)
 
     def pixmap_to_bytes(self):
@@ -118,12 +117,12 @@ class BeePixmapItem(QtWidgets.QGraphicsPixmapItem):
 
         if self.scene():
             scale = self.scene().views()[0].get_scale()
-            return value / scale / self.scale()
-        else:
-            # This can happen when the item is already removed from
-            # the scene but its boundingRect is still needed. Use the
-            # last known scaling factor instead
-            return value * self.viewport_scale
+            self._view_scale = scale
+
+        # It can happen that the item is already removed from
+        # the scene but its boundingRect is still needed. Keep the
+        # last known scaling factor for that case
+        return value / getattr(self, '_view_scale', 1) / self.scale()
 
     @property
     def select_resize_size(self):
@@ -203,15 +202,8 @@ class BeePixmapItem(QtWidgets.QGraphicsPixmapItem):
             shape_ = shape_ + path
         return shape_
 
-    def update_selection(self):
-        new_scale = self.fixed_length_for_viewport(1)
-        if new_scale != self.viewport_scale:
-            logger.debug('Selection geometry changed')
-            self.prepareGeometryChange()
-            self.viewport_scale = new_scale
-
     def hoverMoveEvent(self, event):
-        if not self.isSelected():
+        if not self.isSelected() or not self.scene().has_single_selection():
             return
         if self.bottom_right_scale_bounds.contains(event.pos()):
             self.setCursor(Qt.CursorShape.SizeFDiagCursor)
@@ -221,7 +213,7 @@ class BeePixmapItem(QtWidgets.QGraphicsPixmapItem):
             self.setCursor(Qt.CursorShape.ArrowCursor)
 
     def hoverEnterEvent(self, event):
-        if not self.isSelected():
+        if not self.isSelected() or not self.scene().has_single_selection():
             self.setCursor(Qt.CursorShape.ArrowCursor)
 
     def mousePressEvent(self, event):
@@ -258,3 +250,6 @@ class BeePixmapItem(QtWidgets.QGraphicsPixmapItem):
             event.accept()
         else:
             super().mouseReleaseEvent(event)
+
+    def on_view_scale_change(self):
+        self.prepareGeometryChange()

beeref/scene.py

@@ -30,7 +30,6 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene):
         super().__init__()
         self.move_active = False
         self.undo_stack = undo_stack
-        self.selectionChanged.connect(self.on_selection_changed)
         self.max_z = 0
 
     def normalize_width_or_height(self, mode):
@@ -127,9 +126,6 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene):
         for item in self.items_for_save():
             item.save_id = None
 
-    def on_selection_changed(self):
-        self.update_selection()
-
-    def update_selection(self):
+    def on_view_scale_change(self):
         for item in self.selectedItems():
-            item.update_selection()
+            item.on_view_scale_change()

beeref/view.py

@@ -402,8 +402,8 @@ class BeeGraphicsView(QtWidgets.QGraphicsView):
 
     def scale(self, *args, **kwargs):
         super().scale(*args, **kwargs)
+        self.scene.on_view_scale_change()
         self.recalc_scene_rect()
-        self.scene.update_selection()
 
     def get_scale(self):
         return self.transform().m11()

tests/test_commands.py

@@ -1,4 +1,4 @@
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 from PyQt6 import QtGui
 
@@ -10,8 +10,11 @@ from .base import BeeTestCase
 
 class InsertItemsTestCase(BeeTestCase):
 
-    def test_redo_undo(self):
+    @patch('beeref.scene.BeeGraphicsScene.views')
+    def test_redo_undo(self, views_mock):
         scene = BeeGraphicsScene(None)
+        view = MagicMock(get_scale=MagicMock(return_value=1))
+        views_mock.return_value = [view]
         scene.update_selection = MagicMock()
         scene.max_z = 5
         item1 = BeePixmapItem(QtGui.QImage())

tests/test_items.py

@@ -92,20 +92,23 @@ class BeePixmapItemPaintstuffTestCase(BeeTestCase):
     def test_fixed_length_for_viewport_when_default_scales(self):
         self.view.get_scale = MagicMock(return_value=1)
         assert self.item.fixed_length_for_viewport(100) == 100
+        assert self.item._view_scale == 1
 
     def test_fixed_length_for_viewport_when_viewport_scaled(self):
         self.view.get_scale = MagicMock(return_value=2)
         assert self.item.fixed_length_for_viewport(100) == 50
+        assert self.item._view_scale == 2
 
     def test_fixed_length_for_viewport_when_item_scaled(self):
         self.view.get_scale = MagicMock(return_value=1)
         self.item.setScale(5)
         assert self.item.fixed_length_for_viewport(100) == 20
+        assert self.item._view_scale == 1
 
     def test_fixed_length_for_viewport_when_no_scene(self):
         item = BeePixmapItem(QtGui.QImage())
-        item.viewport_scale = 0.5
-        assert item.fixed_length_for_viewport(100) == 50
+        item._view_scale = 0.5
+        assert item.fixed_length_for_viewport(100) == 200
 
     def test_resize_size_when_scaled(self):
         self.view.get_scale = MagicMock(return_value=2)