From 31456152d403e57cdd56f6b1dd54d94d930a3029 Mon Sep 17 00:00:00 2001 From: letsfindaway Date: Wed, 5 Jan 2022 11:15:54 +0100 Subject: [PATCH 1/3] refactor: drawBackground mostly in scene - move all drawing of background grid to UBGraphicsScene - only add document border in UBBoardView - avoid some compiler warnings --- src/board/UBBoardView.cpp | 101 ++++----------------------------- src/domain/UBGraphicsScene.cpp | 48 +++++++++++++--- 2 files changed, 51 insertions(+), 98 deletions(-) diff --git a/src/board/UBBoardView.cpp b/src/board/UBBoardView.cpp index 817d8c4e7..c968f17d7 100644 --- a/src/board/UBBoardView.cpp +++ b/src/board/UBBoardView.cpp @@ -29,6 +29,7 @@ #include "UBBoardView.h" +#include #include #include #include @@ -92,11 +93,11 @@ UBBoardView::UBBoardView (UBBoardController* pController, QWidget* pParent, bool , mIsCreatingTextZone (false) , mIsCreatingSceneGrabZone (false) , mOkOnWidget(false) + , _movingItem(nullptr) , suspendedMousePressEvent(NULL) , mLongPressInterval(1000) , mIsDragInProgress(false) , mMultipleSelectionIsEnabled(false) - , _movingItem(nullptr) , bIsControl(isControl) , bIsDesktop(isDesktop) { @@ -117,11 +118,11 @@ UBBoardView::UBBoardView (UBBoardController* pController, QWidget* pParent, bool UBBoardView::UBBoardView (UBBoardController* pController, int pStartLayer, int pEndLayer, QWidget* pParent, bool isControl, bool isDesktop) : QGraphicsView (pParent) , mController (pController) + , _movingItem(nullptr) , suspendedMousePressEvent(NULL) , mLongPressInterval(1000) , mIsDragInProgress(false) , mMultipleSelectionIsEnabled(false) - , _movingItem(nullptr) , bIsControl(isControl) , bIsDesktop(isDesktop) { @@ -685,6 +686,7 @@ bool UBBoardView::itemShouldBeMoved(QGraphicsItem *item) return false; if(currentTool == UBStylusTool::Play) return false; + Q_FALLTHROUGH(); case UBGraphicsSvgItem::Type: case UBGraphicsPixmapItem::Type: @@ -692,10 +694,13 @@ bool UBBoardView::itemShouldBeMoved(QGraphicsItem *item) return true; if (item->isSelected()) return false; + Q_FALLTHROUGH(); + case UBGraphicsMediaItem::Type: case UBGraphicsVideoItem::Type: case UBGraphicsAudioItem::Type: return true; + case UBGraphicsStrokesGroup::Type: case UBGraphicsTextItem::Type: if (currentTool == UBStylusTool::Play) @@ -1714,98 +1719,14 @@ void UBBoardView::paintEvent(QPaintEvent *event) void UBBoardView::drawBackground (QPainter *painter, const QRectF &rect) { + // draw the background of the QGraphicsScene + QGraphicsView::drawBackground(painter, rect); + if (testAttribute (Qt::WA_TranslucentBackground)) { - QGraphicsView::drawBackground (painter, rect); return; } - bool darkBackground = scene () && scene ()->isDarkBackground (); - - if (darkBackground) - { - painter->fillRect (rect, QBrush (QColor (Qt::black))); - } - else - { - painter->fillRect (rect, QBrush (QColor (Qt::white))); - } - - if (transform ().m11 () > 0.5) - { - QColor bgCrossColor; - - if (darkBackground) - bgCrossColor = QColor(UBSettings::settings()->boardCrossColorDarkBackground->get().toString()); - else - bgCrossColor = QColor(UBSettings::settings()->boardCrossColorLightBackground->get().toString()); - - if (transform ().m11 () < 0.7) - { - int alpha = 255 * transform ().m11 () / 2; - bgCrossColor.setAlpha (alpha); // fade the crossing on small zooms - } - - qreal gridSize = scene()->backgroundGridSize(); - bool intermediateLines = scene()->intermediateLines(); - - painter->setPen (bgCrossColor); - - if (scene () && scene ()->pageBackground() == UBPageBackground::crossed) - { - qreal firstY = ((int) (rect.y () / gridSize)) * gridSize; - - for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += gridSize) - { - painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); - } - - qreal firstX = ((int) (rect.x () / gridSize)) * gridSize; - - for (qreal xPos = firstX; xPos < rect.x () + rect.width (); xPos += gridSize) - { - painter->drawLine (xPos, rect.y (), xPos, rect.y () + rect.height ()); - } - - if (intermediateLines) { - QColor intermediateColor = bgCrossColor; - intermediateColor.setAlphaF(0.5 * bgCrossColor.alphaF()); - painter->setPen(intermediateColor); - - for (qreal yPos = firstY - gridSize/2; yPos < rect.y () + rect.height (); yPos += gridSize) - { - painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); - } - - for (qreal xPos = firstX - gridSize/2; xPos < rect.x () + rect.width (); xPos += gridSize) - { - painter->drawLine (xPos, rect.y (), xPos, rect.y () + rect.height ()); - } - } - } - - if (scene() && scene()->pageBackground() == UBPageBackground::ruled) - { - qreal firstY = ((int) (rect.y () / gridSize)) * gridSize; - - for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += gridSize) - { - painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); - } - - if (intermediateLines) { - QColor intermediateColor = bgCrossColor; - intermediateColor.setAlphaF(0.5 * bgCrossColor.alphaF()); - painter->setPen(intermediateColor); - - for (qreal yPos = firstY - gridSize/2; yPos < rect.y () + rect.height (); yPos += gridSize) - { - painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); - } - } - } - } - if (!mFilterZIndex && scene ()) { QSize pageNominalSize = scene ()->nominalSize (); @@ -1821,7 +1742,7 @@ void UBBoardView::drawBackground (QPainter *painter, const QRectF &rect) QColor docSizeColor; - if (darkBackground) + if (scene ()->isDarkBackground ()) docSizeColor = UBSettings::documentSizeMarkColorDarkBackground; else docSizeColor = UBSettings::documentSizeMarkColorLightBackground; diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index aa6734dfe..48850d7e8 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -2690,15 +2690,16 @@ void UBGraphicsScene::drawBackground(QPainter *painter, const QRectF &rect) QGraphicsScene::drawBackground (painter, rect); return; } + bool darkBackground = isDarkBackground (); if (darkBackground) { - painter->fillRect (rect, QBrush (QColor (Qt::black))); + painter->fillRect (rect, QBrush (QColor (Qt::black))); } else { - painter->fillRect (rect, QBrush (QColor (Qt::white))); + painter->fillRect (rect, QBrush (QColor (Qt::white))); } if (mZoomFactor > 0.5) @@ -2709,39 +2710,70 @@ void UBGraphicsScene::drawBackground(QPainter *painter, const QRectF &rect) bgCrossColor = QColor(UBSettings::settings()->boardCrossColorDarkBackground->get().toString()); else bgCrossColor = QColor(UBSettings::settings()->boardCrossColorLightBackground->get().toString()); + if (mZoomFactor < 0.7) { int alpha = 255 * mZoomFactor / 2; bgCrossColor.setAlpha (alpha); // fade the crossing on small zooms } + qreal gridSize = backgroundGridSize(); painter->setPen (bgCrossColor); if (mPageBackground == UBPageBackground::crossed) { - qreal firstY = ((int) (rect.y () / backgroundGridSize())) * backgroundGridSize(); + qreal firstY = ((int) (rect.y () / gridSize)) * gridSize; - for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += backgroundGridSize()) + for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += gridSize) { painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); } - qreal firstX = ((int) (rect.x () / backgroundGridSize())) * backgroundGridSize(); + qreal firstX = ((int) (rect.x () / gridSize)) * gridSize; - for (qreal xPos = firstX; xPos < rect.x () + rect.width (); xPos += backgroundGridSize()) + for (qreal xPos = firstX; xPos < rect.x () + rect.width (); xPos += gridSize) { painter->drawLine (xPos, rect.y (), xPos, rect.y () + rect.height ()); } + + if (mIntermediateLines) + { + QColor intermediateColor = bgCrossColor; + intermediateColor.setAlphaF(0.5 * bgCrossColor.alphaF()); + painter->setPen(intermediateColor); + + for (qreal yPos = firstY - gridSize/2; yPos < rect.y () + rect.height (); yPos += gridSize) + { + painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); + } + + for (qreal xPos = firstX - gridSize/2; xPos < rect.x () + rect.width (); xPos += gridSize) + { + painter->drawLine (xPos, rect.y (), xPos, rect.y () + rect.height ()); + } + } } else if (mPageBackground == UBPageBackground::ruled) { - qreal firstY = ((int) (rect.y () / backgroundGridSize())) * backgroundGridSize(); + qreal firstY = ((int) (rect.y () / gridSize)) * gridSize; - for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += backgroundGridSize()) + for (qreal yPos = firstY; yPos < rect.y () + rect.height (); yPos += gridSize) { painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); } + + if (mIntermediateLines) + { + QColor intermediateColor = bgCrossColor; + intermediateColor.setAlphaF(0.5 * bgCrossColor.alphaF()); + painter->setPen(intermediateColor); + + for (qreal yPos = firstY - gridSize/2; yPos < rect.y () + rect.height (); yPos += gridSize) + { + painter->drawLine (rect.x (), yPos, rect.x () + rect.width (), yPos); + } + } } } } From 95021acc1dbb5ab7c65fb04b7ff543fd064f41a4 Mon Sep 17 00:00:00 2001 From: letsfindaway Date: Tue, 29 Nov 2022 08:49:36 +0100 Subject: [PATCH 2/3] fix: set document modified when changing page size - make sure page size change is persisted - refactor code to refresh background in common function --- src/domain/UBGraphicsScene.cpp | 29 +++++++++++++++-------------- src/domain/UBGraphicsScene.h | 1 + 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index 48850d7e8..25760d0bd 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -1136,22 +1136,17 @@ void UBGraphicsScene::setBackground(bool pIsDark, UBPageBackground pBackground) recolorAllItems(); needRepaint = true; - setModified(true); } if (mPageBackground != pBackground) { mPageBackground = pBackground; needRepaint = true; - setModified(true); } if (needRepaint) { - foreach(QGraphicsView* view, views()) - { - view->resetCachedContent(); - } + updateBackground(); } } @@ -1165,20 +1160,14 @@ void UBGraphicsScene::setBackgroundGridSize(int pSize) { if (pSize > 0) { mBackgroundGridSize = pSize; - setModified(true); - - foreach(QGraphicsView* view, views()) - view->resetCachedContent(); + updateBackground(); } } void UBGraphicsScene::setIntermediateLines(bool checked) { mIntermediateLines = checked; - setModified(true); - - foreach(QGraphicsView* view, views()) - view->resetCachedContent(); + updateBackground(); } void UBGraphicsScene::setDrawingMode(bool bModeDesktop) @@ -2570,6 +2559,7 @@ void UBGraphicsScene::setNominalSize(const QSize& pSize) if (nominalSize() != pSize) { mNominalSize = pSize; + updateBackground(); if(mDocument) mDocument->setDefaultDocumentSize(pSize); @@ -2918,6 +2908,17 @@ void UBGraphicsScene::setDocumentUpdated() } } +void UBGraphicsScene::updateBackground() +{ + setModified(true); + + foreach(QGraphicsView* view, views()) + { + view->resetCachedContent(); + } + +} + void UBGraphicsScene::createEraiser() { if (UBSettings::settings()->showEraserPreviewCircle->get().toBool()) { diff --git a/src/domain/UBGraphicsScene.h b/src/domain/UBGraphicsScene.h index e824e555f..80bad3d89 100644 --- a/src/domain/UBGraphicsScene.h +++ b/src/domain/UBGraphicsScene.h @@ -427,6 +427,7 @@ public slots: private: void setDocumentUpdated(); + void updateBackground(); void createEraiser(); void createPointer(); void createMarkerCircle(); From 1eafb49db2e03b0b878d7d90e621136a6bce1374 Mon Sep 17 00:00:00 2001 From: letsfindaway Date: Tue, 29 Nov 2022 09:12:05 +0100 Subject: [PATCH 3/3] refactor: avoid clang warnings - unneeded temporary container allocations - reference to temporary - possible memory leak - unused variable - use QHash for pointers instead of QMap - possible nullptr reference - not normalized signal/slot signatures - detached container - call to virtual function during construction --- src/domain/UBGraphicsScene.cpp | 65 ++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index 25760d0bd..f3fed4b6f 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -164,7 +164,7 @@ qreal UBZLayerController::changeZLevelTo(QGraphicsItem *item, moveDestination de } //If only one item itself - do nothing, return it's z-value - if (sortedItems.count() == 1 && sortedItems.values().first() == item) { + if (sortedItems.count() == 1 && sortedItems.first() == item) { qDebug() << "only one item exists in layer. Have nothing to change"; return item->data(UBGraphicsItemData::ItemOwnZValue).toReal(); } @@ -385,8 +385,8 @@ UBGraphicsScene::~UBGraphicsScene() void UBGraphicsScene::selectionChangedProcessing() { if (selectedItems().count()){ - UBApplication::showMessage("ZValue is " + QString::number(selectedItems().first()->zValue(), 'f') + "own z value is " - + QString::number(selectedItems().first()->data(UBGraphicsItemData::ItemOwnZValue).toReal(), 'f')); + UBApplication::showMessage("ZValue is " + QString::number(selectedItems().constFirst()->zValue(), 'f') + "own z value is " + + QString::number(selectedItems().constFirst()->data(UBGraphicsItemData::ItemOwnZValue).toReal(), 'f')); } } @@ -749,10 +749,11 @@ bool UBGraphicsScene::inputDeviceRelease(int tool) { if (mUndoRedoStackEnabled) { //should be deleted after scene own undo stack implemented - UBGraphicsItemUndoCommand* udcmd = new UBGraphicsItemUndoCommand(this, mRemovedItems, mAddedItems); //deleted by the undoStack - - if(UBApplication::undoStack) + if (UBApplication::undoStack) + { + UBGraphicsItemUndoCommand* udcmd = new UBGraphicsItemUndoCommand(this, mRemovedItems, mAddedItems); //deleted by the undoStack UBApplication::undoStack->push(udcmd); + } } mRemovedItems.clear(); @@ -1177,7 +1178,7 @@ void UBGraphicsScene::setDrawingMode(bool bModeDesktop) void UBGraphicsScene::recolorAllItems() { - QMap previousUpdateModes; + QHash previousUpdateModes; foreach(QGraphicsView* view, views()) { previousUpdateModes.insert(view, view->viewportUpdateMode()); @@ -1383,7 +1384,7 @@ UBGraphicsScene* UBGraphicsScene::sceneDeepCopy() const copy->setNominalSize(this->mNominalSize); QListIterator itItems(this->mFastAccessItems); - QMap groupClone; + QHash groupClone; while (itItems.hasNext()) { @@ -1507,8 +1508,8 @@ void UBGraphicsScene::clearContent(clearCase pCase) groupsMap.insert(itemGroup, UBGraphicsItem::getOwnUuid(item)); if (itemGroup->childItems().count() == 1) { - groupsMap.insert(itemGroup, UBGraphicsItem::getOwnUuid(itemGroup->childItems().first())); - QGraphicsItem *lastItem = itemGroup->childItems().first(); + groupsMap.insert(itemGroup, UBGraphicsItem::getOwnUuid(itemGroup->childItems().constFirst())); + QGraphicsItem *lastItem = itemGroup->childItems().constFirst(); bool isSelected = itemGroup->isSelected(); itemGroup->destroy(false); lastItem->setSelected(isSelected); @@ -1626,25 +1627,27 @@ UBGraphicsMediaItem* UBGraphicsScene::addMedia(const QUrl& pMediaFileUrl, bool s UBGraphicsMediaItem * mediaItem = UBGraphicsMediaItem::createMediaItem(pMediaFileUrl); - if(mediaItem) + if (mediaItem) + { connect(UBApplication::boardController, SIGNAL(activeSceneChanged()), mediaItem, SLOT(activeSceneChanged())); - mediaItem->setPos(pPos); + mediaItem->setPos(pPos); - mediaItem->setFlag(QGraphicsItem::ItemIsMovable, true); - mediaItem->setFlag(QGraphicsItem::ItemIsSelectable, true); + mediaItem->setFlag(QGraphicsItem::ItemIsMovable, true); + mediaItem->setFlag(QGraphicsItem::ItemIsSelectable, true); - addItem(mediaItem); + addItem(mediaItem); - mediaItem->show(); + mediaItem->show(); - if (mUndoRedoStackEnabled) { //should be deleted after scene own undo stack implemented - UBGraphicsItemUndoCommand* uc = new UBGraphicsItemUndoCommand(this, 0, mediaItem); - UBApplication::undoStack->push(uc); - } + if (mUndoRedoStackEnabled) { //should be deleted after scene own undo stack implemented + UBGraphicsItemUndoCommand* uc = new UBGraphicsItemUndoCommand(this, 0, mediaItem); + UBApplication::undoStack->push(uc); + } - if (shouldPlayAsap) - mediaItem->play(); + if (shouldPlayAsap) + mediaItem->play(); + } setDocumentUpdated(); @@ -1904,7 +1907,7 @@ UBGraphicsTextItem* UBGraphicsScene::addTextWithFont(const QString& pString, con UBApplication::undoStack->push(uc); } - connect(textItem, SIGNAL(textUndoCommandAdded(UBGraphicsTextItem *)), this, SLOT(textUndoCommandAdded(UBGraphicsTextItem *))); + connect(textItem, SIGNAL(textUndoCommandAdded(UBGraphicsTextItem*)), this, SLOT(textUndoCommandAdded(UBGraphicsTextItem*))); textItem->setSelected(true); textItem->setFocus(); @@ -1931,7 +1934,7 @@ UBGraphicsTextItem *UBGraphicsScene::addTextHtml(const QString &pString, const Q UBApplication::undoStack->push(uc); } - connect(textItem, SIGNAL(textUndoCommandAdded(UBGraphicsTextItem *)), this, SLOT(textUndoCommandAdded(UBGraphicsTextItem *))); + connect(textItem, SIGNAL(textUndoCommandAdded(UBGraphicsTextItem*)), this, SLOT(textUndoCommandAdded(UBGraphicsTextItem*))); textItem->setFocus(); @@ -2140,10 +2143,10 @@ QRectF UBGraphicsScene::normalizedSceneRect(qreal ratio) QGraphicsItem *UBGraphicsScene::itemForUuid(QUuid uuid) { QGraphicsItem *result = 0; - QString ui = uuid.toString(); //simple search before implementing container for fast access - foreach (QGraphicsItem *item, items()) { + foreach (QGraphicsItem *item, items()) + { if (UBGraphicsScene::getPersonalUuid(item) == uuid && !uuid.isNull()) { result = item; } @@ -2514,7 +2517,7 @@ QList UBGraphicsScene::relativeDependencies() const UBGraphicsGroupContainerItem* groupItem = dynamic_cast(item); if(groupItem) { - for (auto child : groupItem->childItems()) + foreach (QGraphicsItem* child, groupItem->childItems()) { relativePaths << relativeDependenciesOfItem(child); } @@ -2932,7 +2935,7 @@ void UBGraphicsScene::createEraiser() mEraser->setData(UBGraphicsItemData::itemLayerType, QVariant(itemLayerType::Eraiser)); //Necessary to set if we want z value to be assigned correctly mTools << mEraser; - addItem(mEraser); + UBGraphicsScene::addItem(mEraser); } } @@ -2949,7 +2952,7 @@ void UBGraphicsScene::createPointer() mPointer->setData(UBGraphicsItemData::itemLayerType, QVariant(itemLayerType::Pointer)); //Necessary to set if we want z value to be assigned correctly mTools << mPointer; - addItem(mPointer); + UBGraphicsScene::addItem(mPointer); } void UBGraphicsScene::createMarkerCircle() @@ -2967,7 +2970,7 @@ void UBGraphicsScene::createMarkerCircle() mMarkerCircle->setData(UBGraphicsItemData::itemLayerType, QVariant(itemLayerType::Eraiser)); mTools << mMarkerCircle; - addItem(mMarkerCircle); + UBGraphicsScene::addItem(mMarkerCircle); } } @@ -2986,7 +2989,7 @@ void UBGraphicsScene::createPenCircle() mPenCircle->setData(UBGraphicsItemData::itemLayerType, QVariant(itemLayerType::Eraiser)); mTools << mPenCircle; - addItem(mPenCircle); + UBGraphicsScene::addItem(mPenCircle); } }