From c5e23fdf4fd6ed74b7f5881216cf71ca4ffff403 Mon Sep 17 00:00:00 2001 From: Rebecca Breu Date: Thu, 9 May 2024 15:41:47 +0200 Subject: [PATCH] Display error messages when images can't be loaded from bee file --- CHANGELOG.rst | 1 + beeref/config/settings.py | 2 +- beeref/fileio/sql.py | 15 +++-- beeref/items.py | 79 ++++++++++++++++++++++- beeref/scene.py | 10 ++- beeref/selection.py | 1 + beeref/widgets/settings.py | 2 +- tests/fileio/test_sql.py | 66 ++++++++++++++++++-- tests/items/test_erroritem.py | 114 ++++++++++++++++++++++++++++++++++ tests/items/test_textitem.py | 6 +- tests/test_scene.py | 8 +++ 11 files changed, 286 insertions(+), 18 deletions(-) create mode 100644 tests/items/test_erroritem.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2758c4e..e06b6a2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,7 @@ Added images. If a big image won't load, increase this limit. This setting can be overridden by Qt's default environment variable QT_IMAGEIO_MAXALLOC +* Display error messages when images can't be loaded from bee files 0.3.3 - 2024-05-05 diff --git a/beeref/config/settings.py b/beeref/config/settings.py index 1d767c5..88fe3a4 100644 --- a/beeref/config/settings.py +++ b/beeref/config/settings.py @@ -119,7 +119,7 @@ class BeeSettings(QtCore.QSettings): 'validate': lambda x: 0 <= x <= 200, }, 'Items/image_allocation_limit': { - 'default': QtGui.QImageReader.allocationLimit(), + 'default': 256, 'cast': int, 'validate': lambda x: x >= 0, 'post_save_callback': QtGui.QImageReader.setAllocationLimit, diff --git a/beeref/fileio/sql.py b/beeref/fileio/sql.py index b0d6a83..d211d3b 100644 --- a/beeref/fileio/sql.py +++ b/beeref/fileio/sql.py @@ -34,7 +34,7 @@ import tempfile from PyQt6 import QtGui from beeref import constants -from beeref.items import BeePixmapItem +from beeref.items import BeePixmapItem, BeeErrorItem from .errors import BeeFileIOError, IMG_LOADING_ERROR_MSG from .schema import SCHEMA, USER_VERSION, MIGRATIONS, APPLICATION_ID @@ -216,9 +216,8 @@ class SQLiteIO: item = data['data']['text'] = ( f'Image could not be loaded: {item.filename}\n' + IMG_LOADING_ERROR_MSG) - data['type'] = 'text' + data['type'] = BeeErrorItem.TYPE data['item'] = item - data['data']['is_editable'] = False self.scene.add_item_later(data) @@ -252,7 +251,13 @@ class SQLiteIO: self.write() def write_data(self): - to_delete = self.fetchall('SELECT id from ITEMS') + to_delete = {row[0] for row in self.fetchall('SELECT id from ITEMS')} + # We don't want to touch existing items that are displayed as errors: + keep = {item.original_save_id + for item in self.scene.items_by_type(BeeErrorItem.TYPE)} + logger.debug(f'Not saving error items: {keep}') + to_delete = to_delete - keep + to_save = list(self.scene.items_for_save()) if self.worker: self.worker.begin_processing.emit(len(to_save)) @@ -260,7 +265,7 @@ class SQLiteIO: logger.debug(f'Saving {item} with id {item.save_id}') if item.save_id: self.update_item(item) - to_delete.remove((item.save_id,)) + to_delete.remove(item.save_id) else: self.insert_item(item) if self.worker: diff --git a/beeref/items.py b/beeref/items.py index cac6740..995aeab 100644 --- a/beeref/items.py +++ b/beeref/items.py @@ -602,13 +602,13 @@ class BeeTextItem(BeeItemMixin, QtWidgets.QGraphicsTextItem): TYPE = 'text' - def __init__(self, text=None, is_editable=True, **kwargs): + def __init__(self, text=None, **kwargs): super().__init__(text or "Text") self.save_id = None logger.debug(f'Initialized {self}') self.is_image = False self.init_selectable() - self.is_editable = is_editable + self.is_editable = True self.edit_mode = False self.setDefaultTextColor(QtGui.QColor(*COLORS['Scene:Text'])) @@ -692,3 +692,78 @@ class BeeTextItem(BeeItemMixin, QtWidgets.QGraphicsTextItem): def copy_to_clipboard(self, clipboard): clipboard.setText(self.toPlainText()) + + +@register_item +class BeeErrorItem(BeeItemMixin, QtWidgets.QGraphicsTextItem): + """Class for displaying error messages when an item can't be loaded + from a bee file. + + This item will be displayed instead of the original item. It won't + save to bee files. The original item will be preserved in the bee + file, unless this item gets deleted by the user, or a new bee file + is saved. + """ + + TYPE = 'error' + + def __init__(self, text=None, **kwargs): + super().__init__(text or "Text") + self.original_save_id = None + logger.debug(f'Initialized {self}') + self.is_image = False + self.init_selectable() + self.is_editable = False + self.setDefaultTextColor(QtGui.QColor(*COLORS['Scene:Text'])) + + @classmethod + def create_from_data(cls, **kwargs): + data = kwargs.get('data', {}) + item = cls(**data) + return item + + def __str__(self): + txt = self.toPlainText()[:40] + return (f'Error "{txt}"') + + def contains(self, point): + return self.boundingRect().contains(point) + + def paint(self, painter, option, widget): + painter.setPen(Qt.PenStyle.NoPen) + color = QtGui.QColor(200, 0, 0) + brush = QtGui.QBrush(color) + painter.setBrush(brush) + painter.drawRect(QtWidgets.QGraphicsTextItem.boundingRect(self)) + option.state = QtWidgets.QStyle.StateFlag.State_Enabled + super().paint(painter, option, widget) + self.paint_selectable(painter, option, widget) + + def update_from_data(self, **kwargs): + self.original_save_id = kwargs.get('save_id', self.original_save_id) + self.setPos(kwargs.get('x', self.pos().x()), + kwargs.get('y', self.pos().y())) + self.setZValue(kwargs.get('z', self.zValue())) + self.setScale(kwargs.get('scale', self.scale())) + self.setRotation(kwargs.get('rotation', self.rotation())) + + def create_copy(self): + item = BeeErrorItem(self.toPlainText()) + item.setPos(self.pos()) + item.setZValue(self.zValue()) + item.setScale(self.scale()) + item.setRotation(self.rotation()) + return item + + def flip(self, *args, **kwargs): + """Returns the flip value (1 or -1)""" + # Never display error messages flipped + return 1 + + def do_flip(self, *args, **kwargs): + """Flips the item.""" + # Never flip error messages + pass + + def copy_to_clipboard(self, clipboard): + clipboard.setText(self.toPlainText()) diff --git a/beeref/scene.py b/beeref/scene.py index 91f2f28..0299a85 100644 --- a/beeref/scene.py +++ b/beeref/scene.py @@ -24,7 +24,7 @@ import rpack from beeref import commands from beeref.config import BeeSettings -from beeref.items import item_registry +from beeref.items import item_registry, BeeErrorItem from beeref.selection import MultiSelectItem, RubberbandItem @@ -399,6 +399,12 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): return list(filter(lambda i: hasattr(i, 'save_id'), items)) return items + def items_by_type(self, itype): + """Returns all items of the given type.""" + + return filter(lambda i: getattr(i, 'TYPE', None) == itype, + self.items()) + def items_for_save(self): """Returns the items that are to be saved. @@ -496,7 +502,7 @@ class BeeGraphicsScene(QtWidgets.QGraphicsScene): if not cls: # Just in case we add new item types in future versions logger.warning(f'Encountered item of unknown type: {typ}') - cls = item_registry.get('text') + cls = BeeErrorItem data['data'] = {'text': f'Item of unknown type: {typ}'} item = cls.create_from_data(**data) # Set the values common to all item types: diff --git a/beeref/selection.py b/beeref/selection.py index ebd1180..b9df21e 100644 --- a/beeref/selection.py +++ b/beeref/selection.py @@ -224,6 +224,7 @@ class SelectableMixin(BaseItemMixin): pen.setWidth(self.SELECT_LINE_WIDTH) pen.setCosmetic(True) painter.setPen(pen) + painter.setBrush(QtGui.QBrush()) # Draw the main selection rectangle painter.drawRect(self.bounding_rect_unselected()) diff --git a/beeref/widgets/settings.py b/beeref/widgets/settings.py index c6bdf40..449c639 100644 --- a/beeref/widgets/settings.py +++ b/beeref/widgets/settings.py @@ -98,8 +98,8 @@ class IntegerGroup(GroupBase): def __init__(self): super().__init__() self.input = QtWidgets.QSpinBox() - self.input.setValue(self.settings.valueOrDefault(self.KEY)) self.input.setRange(self.MIN, self.MAX) + self.input.setValue(self.settings.valueOrDefault(self.KEY)) self.input.valueChanged.connect(self.on_value_changed) self.layout.addWidget(self.input) self.layout.addStretch(100) diff --git a/tests/fileio/test_sql.py b/tests/fileio/test_sql.py index 6203fee..5f640ae 100644 --- a/tests/fileio/test_sql.py +++ b/tests/fileio/test_sql.py @@ -10,7 +10,7 @@ import pytest from beeref.fileio import schema, is_bee_file from beeref.fileio.errors import BeeFileIOError from beeref.fileio.sql import SQLiteIO -from beeref.items import BeePixmapItem, BeeTextItem +from beeref.items import BeePixmapItem, BeeTextItem, BeeErrorItem @pytest.mark.parametrize('filename,expected', @@ -304,6 +304,8 @@ def test_sqliteio_write_updates_existing_text_item(tmpfile, view): item.save_id = 1 io = SQLiteIO(tmpfile, view.scene, create_new=True) io.write() + assert io.fetchone('SELECT COUNT(*) from items') == (1,) + item.setScale(0.7) item.setPos(20, 30) item.setZValue(0.33) @@ -341,6 +343,8 @@ def test_sqliteio_write_updates_existing_pixmap_item(tmpfile, view): item.pixmap_to_bytes = MagicMock(return_value=(b'abc', 'png')) io = SQLiteIO(tmpfile, view.scene, create_new=True) io.write() + assert io.fetchone('SELECT COUNT(*) from items') == (1,) + item.setScale(0.7) item.setPos(20, 30) item.setZValue(0.33) @@ -374,6 +378,62 @@ def test_sqliteio_write_updates_existing_pixmap_item(tmpfile, view): assert result[7] == b'abc' +def test_sqliteio_write_keeps_pixmap_item_of_error_item(tmpfile, view): + item = BeePixmapItem(QtGui.QImage(), filename='bee.png') + view.scene.addItem(item) + item.setScale(1.3) + item.setPos(44, 55) + item.setZValue(0.22) + item.setRotation(33) + item.setOpacity(0.2) + item.save_id = 1 + item.crop = QtCore.QRectF(5, 5, 80, 100) + item.pixmap_to_bytes = MagicMock(return_value=(b'abc', 'png')) + io = SQLiteIO(tmpfile, view.scene, create_new=True) + io.write() + view.scene.removeItem(item) + assert io.fetchone('SELECT COUNT(*) from items') == (1,) + + err_item = BeeErrorItem('errormsg') + err_item.original_save_id = 1 + err_item.setScale(0.7) + err_item.setPos(20, 30) + err_item.setZValue(0.33) + err_item.setRotation(100) + view.scene.addItem(err_item) + io.create_new = False + io.write() + + assert io.fetchone('SELECT COUNT(*) from items') == (1,) + result = io.fetchone( + 'SELECT x, y, z, scale, rotation, flip, items.data, sqlar.data ' + 'FROM items ' + 'INNER JOIN sqlar on sqlar.item_id = items.id') + assert result[0] == 44 + assert result[1] == 55 + assert result[2] == 0.22 + assert result[3] == 1.3 + assert result[4] == 33 + assert result[5] == 1 + assert json.loads(result[6]) == { + 'filename': 'bee.png', + 'crop': [5, 5, 80, 100], + 'opacity': 0.2, + 'grayscale': False, + } + assert result[7] == b'abc' + + +def test_sqliteio_doesnt_write_error_item_to_new_file(tmpfile, view): + err_item = BeeErrorItem('errormsg') + err_item.original_save_id = 1 + view.scene.addItem(err_item) + io = SQLiteIO(tmpfile, view.scene, create_new=True) + io.create_new = True + io.write() + assert io.fetchone('SELECT COUNT(*) from items') == (0,) + + def test_sqliteio_write_removes_nonexisting_text_item(tmpfile, view): item = BeeTextItem('foo bar') item.setScale(1.3) @@ -464,7 +524,6 @@ def test_sqliteio_read_reads_readonly_text_item(tmpfile, view): assert len(view.scene.items()) == 1 item = view.scene.items()[0] assert isinstance(item, BeeTextItem) - assert item.is_editable is True assert item.isSelected() is False assert item.save_id == 1 assert item.pos().x() == 22.2 @@ -531,8 +590,7 @@ def test_sqliteio_read_reads_readonly_pixmap_item_error(tmpfile, view): view.scene.add_queued_items() assert len(view.scene.items()) == 1 item = view.scene.items()[0] - assert isinstance(item, BeeTextItem) - assert item.is_editable is False + assert isinstance(item, BeeErrorItem) item.toPlainText().startswith('Unknown') assert view.scene.items_to_add.empty() is True diff --git a/tests/items/test_erroritem.py b/tests/items/test_erroritem.py new file mode 100644 index 0000000..11b4910 --- /dev/null +++ b/tests/items/test_erroritem.py @@ -0,0 +1,114 @@ +from unittest.mock import patch, MagicMock + +from PyQt6 import QtCore, QtWidgets + +from beeref.items import BeeErrorItem, item_registry + + +def test_in_items_registry(): + assert item_registry['error'] == BeeErrorItem + + +@patch('beeref.selection.SelectableMixin.init_selectable') +def test_init(selectable_mock, qapp): + item = BeeErrorItem('foo bar') + assert hasattr(item, 'save_id') is False + assert item.original_save_id is None + assert item.width + assert item.height + assert item.scale() == 1 + assert item.toPlainText() == 'foo bar' + assert item.is_editable is False + assert item.is_image is False + selectable_mock.assert_called_once() + + +@patch('PyQt6.QtWidgets.QGraphicsTextItem.paint') +def test_paint(paint_mock, qapp): + item = BeeErrorItem('foo bar') + item.paint_selectable = MagicMock() + painter = MagicMock() + option = MagicMock() + item.paint(painter, option, 'widget') + item.paint_selectable.assert_called_once() + painter.drawRect.assert_called_once() + assert option.state == QtWidgets.QStyle.StateFlag.State_Enabled + paint_mock.assert_called_once_with(painter, option, 'widget') + + +def test_update_from_data(qapp): + item = BeeErrorItem('foo bar') + item.update_from_data( + save_id=3, + x=11, + y=22, + z=1.2, + scale=2.5, + rotation=45, + flip=-1, + data={'opactiy': 0.5}) + assert item.original_save_id == 3 + assert item.pos() == QtCore.QPointF(11, 22) + assert item.zValue() == 1.2 + assert item.rotation() == 45 + assert item.flip() == 1 + assert hasattr(item, 'save_id') is False + + +def test_update_from_data_keeps_unset_values(qapp): + item = BeeErrorItem('foo bar') + item.setScale(3) + item.update_from_data(rotation=45) + assert item.scale() == 3 + assert item.flip() == 1 + + +def test_create_from_data(qapp): + item = BeeErrorItem.create_from_data(data={'text': 'hello world'}) + item.toPlainText() == 'hello world' + assert hasattr(item, 'save_id') is False + + +def test_create_copy(qapp): + item = BeeErrorItem('foo bar') + item.setPos(20, 30) + item.setRotation(33) + item.setZValue(0.5) + item.setScale(2.2) + + copy = item.create_copy() + assert copy.toPlainText() == 'foo bar' + assert copy.pos() == QtCore.QPointF(20, 30) + assert copy.rotation() == 33 + assert copy.zValue() == 0.5 + assert copy.scale() == 2.2 + assert copy.flip() == 1 + + +def test_item_to_clipboard(qapp): + clipboard = QtWidgets.QApplication.clipboard() + item = BeeErrorItem('foo bar') + item.copy_to_clipboard(clipboard) + assert clipboard.text() == 'foo bar' + + +def test_flip(qapp): + item = BeeErrorItem('foo bar') + item.do_flip() + assert item.flip() == 1 + + +@patch('beeref.items.BeeErrorItem.boundingRect') +def test_contains_when_inside_bounds(brect_mock, qapp): + brect_mock.return_value = QtCore.QRectF(20, 30, 50, 50) + item = BeeErrorItem('foo bar') + item.contains(QtCore.QPointF(33, 45)) is True + brect_mock.assert_called_once_with() + + +@patch('beeref.items.BeeErrorItem.boundingRect') +def test_contains_when_outside_bounds(brect_mock, qapp): + brect_mock.return_value = QtCore.QRectF(20, 30, 50, 50) + item = BeeErrorItem('foo bar') + item.contains(QtCore.QPointF(19, 29)) is False + brect_mock.assert_called_once_with() diff --git a/tests/items/test_textitem.py b/tests/items/test_textitem.py index 740ae7a..09c467e 100644 --- a/tests/items/test_textitem.py +++ b/tests/items/test_textitem.py @@ -202,9 +202,9 @@ def test_create_copy(qapp): assert copy.toPlainText() == 'foo bar' assert copy.pos() == QtCore.QPointF(20, 30) assert copy.rotation() == 33 - assert item.flip() == -1 - assert item.zValue() == 0.5 - assert item.scale() == 2.2 + assert copy.flip() == -1 + assert copy.zValue() == 0.5 + assert copy.scale() == 2.2 def test_enter_edit_mode(view): diff --git a/tests/test_scene.py b/tests/test_scene.py index 7df249b..56a42b2 100644 --- a/tests/test_scene.py +++ b/tests/test_scene.py @@ -1058,6 +1058,14 @@ def test_selected_items_user_only(view): assert item2 in selected +def test_items_by_tpe(view): + item1 = BeePixmapItem(QtGui.QImage()) + view.scene.addItem(item1) + item2 = BeeTextItem('foo') + view.scene.addItem(item2) + assert list(view.scene.items_by_type('text')) == [item2] + + def test_items_for_save(view): item1 = BeePixmapItem(QtGui.QImage()) view.scene.addItem(item1)