From f6b4c9491b8f4e8b868f5f58c9b9a1c6bd721cee Mon Sep 17 00:00:00 2001 From: Rebecca Breu Date: Thu, 28 Dec 2023 13:23:05 +0100 Subject: [PATCH] Improve performance of Select All/Deselect All --- CHANGELOG.rst | 11 +++++++++-- beeref/commands.py | 6 +++--- beeref/fileio/export.py | 2 +- beeref/scene.py | 16 ++++++++++------ beeref/view.py | 9 ++------- tests/conftest.py | 2 +- tests/test_scene.py | 16 ++++++++-------- tests/test_view.py | 6 +++--- 8 files changed, 37 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3075794..cd2d3a5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,8 +10,8 @@ Added * Images can be set to display as grayscale (Images -> Grayscale). -Fixed: ------- +Fixed +----- * Scene Export: Fix output image size and margins when scene had been scaled or moved. @@ -20,6 +20,13 @@ Fixed: of resulting in a confusing error message. +Changed +------- + +* Improved performance of Select All/Deselect All + + + 0.3.1 - 2023-12-10 ================== diff --git a/beeref/commands.py b/beeref/commands.py index 1c9acb7..7c5c94a 100644 --- a/beeref/commands.py +++ b/beeref/commands.py @@ -26,6 +26,7 @@ class InsertItems(QtGui.QUndoCommand): self.ignore_first_redo = ignore_first_redo def redo(self): + self.scene.deselect_all_items() if self.ignore_first_redo: self.ignore_first_redo = False return @@ -35,13 +36,12 @@ class InsertItems(QtGui.QUndoCommand): for item in self.items: self.old_positions.append(item.pos()) item.setPos(item.pos() + self.position - rect.center()) - self.scene.clearSelection() for item in self.items: self.scene.addItem(item) item.setSelected(True) def undo(self): - self.scene.clearSelection() + self.scene.deselect_all_items() for item in self.items: self.scene.removeItem(item) if self.position: @@ -60,7 +60,7 @@ class DeleteItems(QtGui.QUndoCommand): self.scene.removeItem(item) def undo(self): - self.scene.clearSelection() + self.scene.deselect_all_items() for item in self.items: item.setSelected(True) self.scene.addItem(item) diff --git a/beeref/fileio/export.py b/beeref/fileio/export.py index 583690a..223483d 100644 --- a/beeref/fileio/export.py +++ b/beeref/fileio/export.py @@ -32,7 +32,7 @@ class SceneToPixmapExporter: def __init__(self, scene): self.scene = scene self.scene.cancel_crop_mode() - self.scene.set_selected_all_items(False) + self.scene.deselect_all_items() # Selection outlines/handles will be rendered to the exported # image, so deselect first. (Alternatively, pass an attribute # to paint functions to not paint them?) diff --git a/beeref/scene.py b/beeref/scene.py index 538a604..69c428b 100644 --- a/beeref/scene.py +++ b/beeref/scene.py @@ -17,7 +17,7 @@ from queue import Queue import logging import math -from PyQt6 import QtCore, QtWidgets +from PyQt6 import QtCore, QtWidgets, QtGui from PyQt6.QtCore import Qt import rpack @@ -74,7 +74,6 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): self.internal_clipboard.append(item) def paste_from_internal_clipboard(self, position): - self.set_selected_all_items(False) copies = [] for item in self.internal_clipboard: copy = item.create_copy() @@ -257,11 +256,16 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): if item.is_image: item.enter_crop_mode() - def set_selected_all_items(self, value): - """Sets the selection mode of all items to ``value``.""" + def select_all_items(self): self.cancel_crop_mode() - for item in self.items(): - item.setSelected(value) + path = QtGui.QPainterPath() + path.addRect(self.itemsBoundingRect()) + # This is faster than looping through all items and calling setSelected + self.setSelectionArea(path) + + def deselect_all_items(self): + self.cancel_crop_mode() + self.clearSelection() def has_selection(self): """Checks whether there are currently items selected.""" diff --git a/beeref/view.py b/beeref/view.py index adeb975..fb92a0c 100644 --- a/beeref/view.py +++ b/beeref/view.py @@ -238,10 +238,10 @@ class BeeGraphicsView(MainControlsMixin, self.undo_stack.redo() def on_action_select_all(self): - self.scene.set_selected_all_items(True) + self.scene.select_all_items() def on_action_deselect_all(self): - self.scene.set_selected_all_items(False) + self.scene.deselect_all_items() def on_action_delete_items(self): logger.debug('Deleting items...') @@ -425,7 +425,6 @@ class BeeGraphicsView(MainControlsMixin, if not ext: ext = get_file_extension_from_format(formatstr) filename = f'{filename}.{ext}' - print(filename) logger.debug(f'Got export filename {filename}') exporter = SceneToPixmapExporter(self.scene) dialog = widgets.SceneToPixmapExporterDialog( @@ -495,7 +494,6 @@ class BeeGraphicsView(MainControlsMixin, def do_insert_images(self, filenames, pos=None): if not pos: pos = self.get_view_center() - self.scene.clearSelection() self.undo_stack.beginMacro('Insert Images') self.worker = fileio.ThreadedIO( fileio.load_images, @@ -513,7 +511,6 @@ class BeeGraphicsView(MainControlsMixin, self.worker.start() def on_action_insert_images(self): - self.scene.cancel_crop_mode() formats = self.get_supported_image_formats(QtGui.QImageReader) logger.debug(f'Supported image types for reading: {formats}') filenames, f = QtWidgets.QFileDialog.getOpenFileNames( @@ -523,7 +520,6 @@ class BeeGraphicsView(MainControlsMixin, self.do_insert_images(filenames) def on_action_insert_text(self): - self.scene.cancel_crop_mode() item = BeeTextItem() pos = self.mapToScene(self.mapFromGlobal(self.cursor().pos())) item.setScale(1 / self.get_scale()) @@ -549,7 +545,6 @@ class BeeGraphicsView(MainControlsMixin, 'beeref/items', QtCore.QByteArray.number(len(items))) def on_action_paste(self): - self.scene.cancel_crop_mode() logger.debug('Pasting from clipboard...') clipboard = QtWidgets.QApplication.clipboard() pos = self.mapToScene(self.mapFromGlobal(self.cursor().pos())) diff --git a/tests/conftest.py b/tests/conftest.py index 893c13e..cb19bf0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -90,7 +90,7 @@ def tmpfile(tmpdir): @pytest.fixture def item(): from beeref.items import BeePixmapItem - yield BeePixmapItem(QtGui.QImage()) + yield BeePixmapItem(QtGui.QImage(10, 10, QtGui.QImage.Format.Format_RGB32)) @pytest.fixture(scope="session") diff --git a/tests/test_scene.py b/tests/test_scene.py index 3d6e921..fd4e000 100644 --- a/tests/test_scene.py +++ b/tests/test_scene.py @@ -495,31 +495,31 @@ def test_crop_item_when_not_image(view): item.enter_crop_mode.assert_not_called() -def test_set_selected_all_items_when_true(view): - item1 = BeePixmapItem(QtGui.QImage()) +def test_select_all_items_when_true(view): + item1 = BeeTextItem('foo') view.scene.addItem(item1) item1.setSelected(True) - item2 = BeePixmapItem(QtGui.QImage()) + item2 = BeeTextItem('bar') view.scene.addItem(item2) item2.setSelected(True) view.scene.cancel_crop_mode = MagicMock() - view.scene.set_selected_all_items(True) + view.scene.select_all_items() assert item1.isSelected() is True assert item2.isSelected() is True view.scene.cancel_crop_mode.assert_called_once_with() -def test_set_selected_all_items_when_false(view): - item1 = BeePixmapItem(QtGui.QImage()) +def test_deselect_all_items_when_false(view): + item1 = BeeTextItem('foo') view.scene.addItem(item1) item1.setSelected(True) - item2 = BeePixmapItem(QtGui.QImage()) + item2 = BeeTextItem('bar') view.scene.addItem(item2) item2.setSelected(True) view.scene.cancel_crop_mode = MagicMock() - view.scene.set_selected_all_items(False) + view.scene.deselect_all_items() assert item1.isSelected() is False assert item2.isSelected() is False view.scene.cancel_crop_mode.assert_called_once_with() diff --git a/tests/test_view.py b/tests/test_view.py index e13c43b..2f87789 100644 --- a/tests/test_view.py +++ b/tests/test_view.py @@ -548,7 +548,7 @@ def test_on_action_paste_when_empty(img_mock, text_mock, clear_mock, view): view.on_action_paste() assert len(view.scene.items()) == 0 clear_mock.assert_not_called() - view.scene.cancel_crop_mode.assert_called_once_with() + view.scene.cancel_crop_mode.assert_not_called() @patch('beeref.view.BeeGraphicsView.on_action_copy') @@ -591,7 +591,7 @@ def test_on_action_reset_crop(view, item): assert item.crop == QtCore.QRectF(2, 2, 10, 10) item.setSelected(True) view.on_action_reset_crop() - assert item.crop == QtCore.QRectF(0, 0, 0, 0) + assert item.crop == QtCore.QRectF(0, 0, 10, 10) def test_on_action_reset_transforms(view, item): @@ -603,7 +603,7 @@ def test_on_action_reset_transforms(view, item): assert item.crop == QtCore.QRectF(2, 2, 10, 10) item.setSelected(True) view.on_action_reset_transforms() - assert item.crop == QtCore.QRectF(0, 0, 0, 0) + assert item.crop == QtCore.QRectF(0, 0, 10, 10) assert item.flip() == 1 assert item.rotation() == 0 assert item.scale() == 1