From 37a01f80a24d80c4ae1ba7038ecd617027c8208f Mon Sep 17 00:00:00 2001 From: Rebecca Breu Date: Sun, 4 Apr 2021 12:15:23 +0200 Subject: [PATCH] Add rubberband selection. There's a slight inaccuracy when getting an already selected item outside of the rubberband: the additional margin of the selection handles needs to be outside of the rubberband too (since it's part of the item's bounding box), making it slightly harder to get an item out of the selection than it should be. Let's see if it's very noticeable/annoying in practice. --- beeref/scene.py | 27 ++++++++-- beeref/selection.py | 79 +++++++++++++++++++++-------- tests/test_scene.py | 107 ++++++++++++++++++++++++++++++++++++++-- tests/test_selection.py | 84 ++++++++++++++++++++++--------- 4 files changed, 244 insertions(+), 53 deletions(-) diff --git a/beeref/scene.py b/beeref/scene.py index 956ebb3..37aa061 100644 --- a/beeref/scene.py +++ b/beeref/scene.py @@ -20,7 +20,7 @@ from PyQt6 import QtCore, QtWidgets from PyQt6.QtCore import Qt from beeref import commands -from beeref.selection import MultiSelectItem +from beeref.selection import MultiSelectItem, RubberbandItem logger = logging.getLogger('BeeRef') @@ -31,9 +31,11 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): def __init__(self, undo_stack): super().__init__() self.move_active = False + self.rubberband_active = False self.undo_stack = undo_stack self.max_z = 0 self.multi_select_item = MultiSelectItem() + self.rubberband_item = RubberbandItem() self.selectionChanged.connect(self.on_selection_change) def normalize_width_or_height(self, mode): @@ -105,15 +107,30 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): return if event.button() == Qt.MouseButtons.LeftButton: - self.move_active = True - self.move_start = event.scenePos() + self.event_start = event.scenePos() + if self.itemAt(event.scenePos(), self.views()[0].transform()): + self.move_active = True + else: + self.rubberband_active = True super().mousePressEvent(event) - def mouseReleaseEvent(self, event): + def mouseMoveEvent(self, event): + if self.rubberband_active: + if not self.rubberband_item.scene(): + logger.debug('Activating rubberband selection') + self.addItem(self.rubberband_item) + self.rubberband_item.bring_to_front() + self.rubberband_item.fit(self.event_start, event.scenePos()) + self.setSelectionArea(self.rubberband_item.shape()) + super().mouseMoveEvent(event) + def mouseReleaseEvent(self, event): + if self.rubberband_active: + self.removeItem(self.rubberband_item) + self.rubberband_active = False if self.move_active and self.has_selection(): - delta = event.scenePos() - self.move_start + delta = event.scenePos() - self.event_start if not delta.isNull(): self.undo_stack.push( commands.MoveItemsBy(self.selectedItems(), diff --git a/beeref/selection.py b/beeref/selection.py index 3851445..eeeb5b0 100644 --- a/beeref/selection.py +++ b/beeref/selection.py @@ -27,12 +27,31 @@ from beeref.config import CommandlineArgs commandline_args = CommandlineArgs() logger = logging.getLogger('BeeRef') +SELECT_COLOR = QtGui.QColor(116, 234, 231, 255) -class SelectableMixin: +class BaseItemMixin: + + def setScale(self, factor): + if factor <= 0: + return + + logger.debug(f'Setting scale for {self} to {factor}') + self.prepareGeometryChange() + super().setScale(factor) + + def setZValue(self, value): + logger.debug(f'Setting z-value for {self} to {value}') + super().setZValue(value) + self.scene().max_z = max(self.scene().max_z, value) + + def bring_to_front(self): + self.setZValue(self.scene().max_z + 0.001) + + +class SelectableMixin(BaseItemMixin): """Common code for selectable items: Selection outline, handles etc.""" - select_color = QtGui.QColor(116, 234, 231, 255) SELECT_LINE_WIDTH = 4 # line width for the selection box SELECT_HANDLE_SIZE = 15 # size of selection handles for scaling SELECT_RESIZE_SIZE = 20 # size of hover area for scaling @@ -48,22 +67,6 @@ class SelectableMixin: self.viewport_scale = 1 self.conf_debug_shapes = commandline_args.draw_debug_shapes - def setScale(self, factor): - if factor <= 0: - return - - logger.debug(f'Setting scale for {self} to {factor}') - self.prepareGeometryChange() - super().setScale(factor) - - def setZValue(self, value): - logger.debug(f'Setting z-value for image {self} to {value}') - super().setZValue(value) - self.scene().max_z = max(self.scene().max_z, value) - - def bring_to_front(self): - self.setZValue(self.scene().max_z + 0.001) - def fixed_length_for_viewport(self, value): """The interactable areas need to stay the same size on the screen so we need to adjust the values according to the scale @@ -101,7 +104,7 @@ class SelectableMixin: if not self.has_selection_outline(): return - pen = QtGui.QPen(self.select_color) + pen = QtGui.QPen(SELECT_COLOR) pen.setWidth(self.SELECT_LINE_WIDTH) pen.setCosmetic(True) painter.setPen(pen) @@ -277,8 +280,9 @@ class SelectableMixin: return super().itemChange(change, value) -class MultiSelectItem(SelectableMixin, QtWidgets.QGraphicsRectItem): - """Class for images added by the user.""" +class MultiSelectItem(SelectableMixin, + QtWidgets.QGraphicsRectItem): + """The multi selection outline around all selected items.""" def __init__(self): super().__init__() @@ -329,3 +333,36 @@ class MultiSelectItem(SelectableMixin, QtWidgets.QGraphicsRectItem): return super().mousePressEvent(event) + + +class RubberbandItem(BaseItemMixin, QtWidgets.QGraphicsRectItem): + """The outline for the rubber band selection.""" + + def __init__(self): + super().__init__() + color = QtGui.QColor(SELECT_COLOR) + color.setAlpha(40) + self.setBrush(QtGui.QBrush(color)) + + def __str__(self): + return (f'RubberbandItem {self.width} x {self.height}') + + @property + def width(self): + return self.rect().width() + + @property + def height(self): + return self.rect().height() + + def fit(self, point1, point2): + """Updates itself to fit the two given points.""" + + topleft = QtCore.QPointF( + min(point1.x(), point2.x()), + min(point1.y(), point2.y())) + bottomright = QtCore.QPointF( + max(point1.x(), point2.x()), + max(point1.y(), point2.y())) + self.setRect(QtCore.QRectF(topleft, bottomright)) + logger.debug(f'Updated rubberband {self}') diff --git a/tests/test_scene.py b/tests/test_scene.py index 9dccf3d..e4f4bc9 100644 --- a/tests/test_scene.py +++ b/tests/test_scene.py @@ -127,16 +127,115 @@ class BeeGraphicsSceneNormalizeTestCase(BeeTestCase): mouse_mock.assert_not_called() @patch('PyQt6.QtWidgets.QGraphicsScene.mousePressEvent') - def test_mouse_press_event_when_left_click(self, mouse_mock): + def test_mouse_press_event_when_left_click_over_item(self, mouse_mock): + self.scene.itemAt = MagicMock( + return_value=BeePixmapItem(QtGui.QImage())) event = MagicMock( button=MagicMock(return_value=Qt.MouseButtons.LeftButton), - scenePos=MagicMock(return_value=QtCore.QPoint(10, 20)), + scenePos=MagicMock(return_value=QtCore.QPointF(10, 20)), ) self.scene.mousePressEvent(event) event.accept.assert_not_called() mouse_mock.assert_called_once_with(event) assert self.scene.move_active is True - assert self.scene.move_start == QtCore.QPoint(10, 20) + assert self.scene.rubberband_active is False + assert self.scene.event_start == QtCore.QPointF(10, 20) + + @patch('PyQt6.QtWidgets.QGraphicsScene.mousePressEvent') + def test_mouse_press_event_when_left_click_not_over_item(self, mouse_mock): + self.scene.itemAt = MagicMock(return_value=None) + event = MagicMock( + button=MagicMock(return_value=Qt.MouseButtons.LeftButton), + scenePos=MagicMock(return_value=QtCore.QPointF(10, 20)), + ) + self.scene.mousePressEvent(event) + event.accept.assert_not_called() + mouse_mock.assert_called_once_with(event) + assert self.scene.move_active is False + assert self.scene.rubberband_active is True + assert self.scene.event_start == QtCore.QPointF(10, 20) + + @patch('PyQt6.QtWidgets.QGraphicsScene.mouseMoveEvent') + def test_mouse_move_event_when_rubberband_new(self, mouse_mock): + item = BeePixmapItem(QtGui.QImage(self.imgfilename3x3)) + self.scene.addItem(item) + self.scene.rubberband_active = True + self.scene.addItem = MagicMock() + self.scene.event_start = QtCore.QPointF(0, 0) + self.scene.rubberband_item.bring_to_front = MagicMock() + self.scene.removeItem(self.scene.rubberband_item) + event = MagicMock( + scenePos=MagicMock(return_value=QtCore.QPointF(10, 20)), + ) + + self.scene.mouseMoveEvent(event) + + self.scene.addItem.assert_called_once_with(self.scene.rubberband_item) + self.scene.rubberband_item.bring_to_front.assert_called_once() + self.scene.rubberband_item.rect().topLeft().x() == 0 + self.scene.rubberband_item.rect().topLeft().y() == 0 + self.scene.rubberband_item.rect().bottomRight().x() == 10 + self.scene.rubberband_item.rect().bottomRight().y() == 20 + assert item.isSelected() is True + assert mouse_mock.called_once_with(event) + + @patch('PyQt6.QtWidgets.QGraphicsScene.mouseMoveEvent') + def test_mouse_move_event_when_rubberband_not_new(self, mouse_mock): + item = BeePixmapItem(QtGui.QImage(self.imgfilename3x3)) + self.scene.addItem(item) + self.scene.rubberband_active = True + self.scene.event_start = QtCore.QPointF(0, 0) + self.scene.rubberband_item.bring_to_front = MagicMock() + self.scene.addItem(self.scene.rubberband_item) + self.scene.addItem = MagicMock() + event = MagicMock( + scenePos=MagicMock(return_value=QtCore.QPointF(10, 20)), + ) + + self.scene.mouseMoveEvent(event) + + self.scene.addItem.assert_not_called() + self.scene.rubberband_item.bring_to_front.assert_not_called() + self.scene.rubberband_item.rect().topLeft().x() == 0 + self.scene.rubberband_item.rect().topLeft().y() == 0 + self.scene.rubberband_item.rect().bottomRight().x() == 10 + self.scene.rubberband_item.rect().bottomRight().y() == 20 + assert item.isSelected() is True + assert mouse_mock.called_once_with(event) + + @patch('PyQt6.QtWidgets.QGraphicsScene.mouseMoveEvent') + def test_mouse_move_event_when_no_rubberband(self, mouse_mock): + item = BeePixmapItem(QtGui.QImage(self.imgfilename3x3)) + self.scene.addItem(item) + self.scene.rubberband_active = False + self.scene.event_start = QtCore.QPointF(0, 0) + self.scene.rubberband_item.bring_to_front = MagicMock() + self.scene.addItem = MagicMock() + event = MagicMock( + scenePos=MagicMock(return_value=QtCore.QPointF(10, 20)), + ) + + self.scene.mouseMoveEvent(event) + + self.scene.addItem.assert_not_called() + self.scene.rubberband_item.bring_to_front.assert_not_called() + self.scene.rubberband_item.rect().topLeft().x() == 0 + self.scene.rubberband_item.rect().topLeft().y() == 0 + self.scene.rubberband_item.rect().bottomRight().x() == 0 + self.scene.rubberband_item.rect().bottomRight().y() == 0 + assert item.isSelected() is False + assert mouse_mock.called_once_with(event) + + @patch('PyQt6.QtWidgets.QGraphicsScene.mouseReleaseEvent') + def test_mouse_release_event_when_rubberband_active(self, mouse_mock): + event = MagicMock() + self.scene.rubberband_active = True + self.scene.removeItem = MagicMock() + + self.scene.mouseReleaseEvent(event) + self.scene.removeItem.assert_called_once_with( + self.scene.rubberband_item) + self.scene.rubberband_active is False @patch('PyQt6.QtWidgets.QGraphicsScene.mouseReleaseEvent') def test_mouse_release_event_when_move_active(self, mouse_mock): @@ -146,7 +245,7 @@ class BeeGraphicsSceneNormalizeTestCase(BeeTestCase): event = MagicMock( scenePos=MagicMock(return_value=QtCore.QPoint(10, 20))) self.scene.move_active = True - self.scene.move_start = QtCore.QPoint(0, 0) + self.scene.event_start = QtCore.QPoint(0, 0) self.scene.undo_stack = MagicMock(push=MagicMock()) self.scene.mouseReleaseEvent(event) diff --git a/tests/test_selection.py b/tests/test_selection.py index 35d8dc3..7218241 100644 --- a/tests/test_selection.py +++ b/tests/test_selection.py @@ -5,34 +5,14 @@ from PyQt6.QtCore import Qt from beeref.items import BeePixmapItem from beeref.scene import BeeGraphicsScene -from beeref.selection import MultiSelectItem +from beeref.selection import MultiSelectItem, RubberbandItem from .base import BeeTestCase -class SelectableMixinBaseTestCase(BeeTestCase): +class BaseItemMixinTestCase(BeeTestCase): def setUp(self): self.scene = BeeGraphicsScene(None) - self.item = BeePixmapItem(QtGui.QImage()) - self.scene.addItem(self.item) - self.view = MagicMock(get_scale=MagicMock(return_value=1)) - views_patcher = patch('beeref.scene.BeeGraphicsScene.views', - return_value=[self.view]) - views_patcher.start() - self.addCleanup(views_patcher.stop) - width_patcher = patch('beeref.items.BeePixmapItem.width', - new_callable=PropertyMock, - return_value=100) - width_patcher.start() - self.addCleanup(width_patcher.stop) - height_patcher = patch('beeref.items.BeePixmapItem.height', - new_callable=PropertyMock, - return_value=80) - height_patcher.start() - self.addCleanup(height_patcher.stop) - - -class SelectableMixinTestCase(SelectableMixinBaseTestCase): def test_set_scale(self): item = BeePixmapItem( @@ -79,6 +59,32 @@ class SelectableMixinTestCase(SelectableMixinBaseTestCase): assert item2.zValue() > item1.zValue() assert item2.zValue() == self.scene.max_z + +class SelectableMixinBaseTestCase(BeeTestCase): + + def setUp(self): + self.scene = BeeGraphicsScene(None) + self.item = BeePixmapItem(QtGui.QImage()) + self.scene.addItem(self.item) + self.view = MagicMock(get_scale=MagicMock(return_value=1)) + views_patcher = patch('beeref.scene.BeeGraphicsScene.views', + return_value=[self.view]) + views_patcher.start() + self.addCleanup(views_patcher.stop) + width_patcher = patch('beeref.items.BeePixmapItem.width', + new_callable=PropertyMock, + return_value=100) + width_patcher.start() + self.addCleanup(width_patcher.stop) + height_patcher = patch('beeref.items.BeePixmapItem.height', + new_callable=PropertyMock, + return_value=80) + height_patcher.start() + self.addCleanup(height_patcher.stop) + + +class SelectableMixinTestCase(SelectableMixinBaseTestCase): + def test_on_view_scale_change(self): item = BeePixmapItem(QtGui.QImage()) with patch('beeref.items.BeePixmapItem.prepareGeometryChange') as m: @@ -466,7 +472,7 @@ class SelectableMixinMouseEventsTestCase(SelectableMixinBaseTestCase): assert self.item.scale_active is False -class MultiSelectItemItemTestCase(BeeTestCase): +class MultiSelectItemTestCase(BeeTestCase): def setUp(self): self.scene = BeeGraphicsScene(None) @@ -555,3 +561,35 @@ class MultiSelectItemItemTestCase(BeeTestCase): item.mousePressEvent(event) event.ignore.assert_not_called() mouse_mock.assert_called_once_with(event) + + +class RubberbandItemTestCase(BeeTestCase): + + def setUp(self): + self.scene = BeeGraphicsScene(None) + + def test_width(self): + item = RubberbandItem() + item.setRect(5, 5, 100, 80) + assert item.width == 100 + + def test_height(self): + item = RubberbandItem() + item.setRect(5, 5, 100, 80) + assert item.height == 80 + + def test_fit_topleft_to_bottomright(self): + item = RubberbandItem() + item.fit(QtCore.QPointF(-10, -20), QtCore.QPointF(30, 40)) + assert item.rect().topLeft().x() == -10 + assert item.rect().topLeft().y() == -20 + assert item.rect().bottomRight().x() == 30 + assert item.rect().bottomRight().y() == 40 + + def test_fit_topright_to_bottomleft(self): + item = RubberbandItem() + item.fit(QtCore.QPointF(50, -20), QtCore.QPointF(-30, 40)) + assert item.rect().topLeft().x() == -30 + assert item.rect().topLeft().y() == -20 + assert item.rect().bottomRight().x() == 50 + assert item.rect().bottomRight().y() == 40