Fix bugs in moving/scaling/flipping items

1. moving/scaling/flipping an item caused a MoveItem action put on the undo stack
2. pressing the mouse button on a flip edge but releasing outside of a flip edge would not clear the flip_active status, leaving the app in a buggy state.
This commit is contained in:
Rebecca Breu 2021-05-16 11:55:05 +02:00
parent 8425f9bbc1
commit 679248380c
4 changed files with 93 additions and 8 deletions

View file

@ -44,6 +44,14 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene):
self.items_to_add = Queue()
self.internal_clipboard = []
def addItem(self, item):
logger.debug(f'Adding item {item}')
super().addItem(item)
def removeItem(self, item):
logger.debug(f'Removing item {item}')
super().removeItem(item)
def copy_selection_to_internal_clipboard(self):
self.internal_clipboard = []
for item in self.selectedItems(user_only=True):
@ -258,7 +266,10 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene):
logger.debug('Ending rubberband selection')
self.removeItem(self.rubberband_item)
self.rubberband_active = False
if self.move_active and self.has_selection():
if (self.move_active
and self.has_selection()
and not self.multi_select_item.is_action_active()
and not self.selectedItems()[0].is_action_active()):
delta = event.scenePos() - self.event_start
if not delta.isNull():
self.undo_stack.push(
@ -339,11 +350,9 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene):
self.multi_select_item.fit_selection_area(
self.itemsBoundingRect(selection_only=True))
if self.has_multi_selection() and not self.multi_select_item.scene():
logger.debug('Adding multi select outline')
self.addItem(self.multi_select_item)
self.multi_select_item.bring_to_front()
if not self.has_multi_selection() and self.multi_select_item.scene():
logger.debug('Removing multi select outline')
self.removeItem(self.multi_select_item)
def on_change(self, region):

View file

@ -120,11 +120,20 @@ class SelectableMixin(BaseItemMixin):
QtWidgets.QGraphicsItem.GraphicsItemFlag.ItemIsMovable
| QtWidgets.QGraphicsItem.GraphicsItemFlag.ItemIsSelectable)
self.viewport_scale = 1
self.reset_actions()
def reset_actions(self):
self.scale_active = False
self.rotate_active = False
self.flip_active = False
self.just_selected = False
self.viewport_scale = 1
def is_action_active(self):
return any((self.scale_active,
self.rotate_active,
self.flip_active,
self.just_selected))
def fixed_length_for_viewport(self, value):
"""The interactable areas need to stay the same size on the
@ -379,6 +388,8 @@ class SelectableMixin(BaseItemMixin):
for edge in self.get_flip_bounds():
if edge['rect'].contains(event.pos()):
self.flip_active = True
event.accept()
return
super().mousePressEvent(event)
@ -470,8 +481,8 @@ class SelectableMixin(BaseItemMixin):
self.get_scale_factor(event),
self.event_anchor,
ignore_first_redo=True))
self.scale_active = False
event.accept()
self.reset_actions()
return
elif self.rotate_active:
self.scene().on_selection_change()
@ -481,10 +492,10 @@ class SelectableMixin(BaseItemMixin):
self.get_rotate_delta(event.scenePos()),
self.event_anchor,
ignore_first_redo=True))
self.rotate_active = False
event.accept()
self.reset_actions()
return
elif not just_selected:
elif self.flip_active and not just_selected:
for edge in self.get_flip_bounds():
if edge['rect'].contains(event.pos()):
self.scene().undo_stack.push(
@ -493,8 +504,9 @@ class SelectableMixin(BaseItemMixin):
self.center_scene_coords,
vertical=self.get_edge_flips_v(edge)))
event.accept()
self.flip_active = False
self.reset_actions()
return
self.reset_actions()
super().mouseReleaseEvent(event)
def on_view_scale_change(self):
@ -514,6 +526,7 @@ class MultiSelectItem(SelectableMixin,
def __init__(self):
super().__init__()
logger.debug(f'Initialized {self}')
self.init_selectable()
def __str__(self):

View file

@ -23,6 +23,13 @@ class BeeGraphicsSceneTestCase(BeeTestCase):
views_patcher.start()
self.addCleanup(views_patcher.stop)
def test_add_remove_item(self):
item = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item)
assert self.scene.items() == [item]
self.scene.removeItem(item)
assert self.scene.items() == []
def test_normalize_height(self):
item1 = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item1)
@ -579,6 +586,42 @@ class BeeGraphicsSceneTestCase(BeeTestCase):
mouse_mock.assert_called_once_with(event)
assert self.scene.move_active is False
@patch('PyQt6.QtWidgets.QGraphicsScene.mouseReleaseEvent')
def test_mouse_release_event_when_item_action_active(self, mouse_mock):
item = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item)
item.setSelected(True)
event = MagicMock(
scenePos=MagicMock(return_value=QtCore.QPoint(10, 20)))
self.scene.move_active = True
item.scale_active = True
self.scene.undo_stack = MagicMock(push=MagicMock())
self.scene.mouseReleaseEvent(event)
self.scene.undo_stack.push.assert_not_called()
mouse_mock.assert_called_once_with(event)
assert self.scene.move_active is False
@patch('PyQt6.QtWidgets.QGraphicsScene.mouseReleaseEvent')
def test_mouse_release_event_when_multiselect_action_active(
self, mouse_mock):
item1 = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item1)
item1.setSelected(True)
item2 = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item2)
item2.setSelected(True)
event = MagicMock(
scenePos=MagicMock(return_value=QtCore.QPoint(10, 20)))
self.scene.move_active = True
self.scene.multi_select_item.scale_active = True
self.scene.undo_stack = MagicMock(push=MagicMock())
self.scene.mouseReleaseEvent(event)
self.scene.undo_stack.push.assert_not_called()
mouse_mock.assert_called_once_with(event)
assert self.scene.move_active is False
def test_selected_items(self):
item1 = BeePixmapItem(QtGui.QImage())
self.scene.addItem(item1)

View file

@ -167,6 +167,23 @@ class SelectableMixinBaseTestCase(BeeTestCase):
class SelectableMixinTestCase(SelectableMixinBaseTestCase):
def test_init_selectable(self):
item = BeePixmapItem(QtGui.QImage())
assert item.viewport_scale == 1
assert item.scale_active is False
assert item.rotate_active is False
assert item.flip_active is False
assert item.just_selected is False
def test_is_action_active_when_no_action(self):
item = BeePixmapItem(QtGui.QImage())
assert item.is_action_active() is False
def test_is_action_active_when_action(self):
item = BeePixmapItem(QtGui.QImage())
item.scale_active = True
assert item.is_action_active() is True
def test_on_view_scale_change(self):
item = BeePixmapItem(QtGui.QImage())
with patch('beeref.items.BeePixmapItem.prepareGeometryChange') as m:
@ -782,11 +799,13 @@ class SelectableMixinMouseEventsTestCase(SelectableMixinBaseTestCase):
m.assert_not_called()
def test_mouse_release_event_when_no_action(self):
self.item.flip_active = True
self.event.pos = MagicMock(return_value=QtCore.QPointF(-100, -100))
with patch('PyQt6.QtWidgets.QGraphicsPixmapItem'
'.mouseReleaseEvent') as m:
self.item.mouseReleaseEvent(self.event)
m.assert_called_once_with(self.event)
self.item.flip_active is False
def test_mouse_release_event_when_scale_action(self):
self.event.scenePos = MagicMock(return_value=QtCore.QPointF(20, 90))
@ -838,6 +857,7 @@ class SelectableMixinMouseEventsTestCase(SelectableMixinBaseTestCase):
assert cmd.items == [self.item]
assert cmd.anchor == QtCore.QPointF(50, 40)
assert cmd.vertical is False
assert self.item.flip_active is False
class MultiSelectItemTestCase(BeeTestCase):