From aca966c17a980e2d200e76d7c9088fe1ea952381 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Thu, 30 Sep 2021 07:48:26 +0200 Subject: [PATCH 01/16] Fix a few cases where fill drag behaviour didn't work as expected - Filling on layer below when reference layer was all layers, would not allow stroke filling - Filling with overlay would result in multiple undo operations for one paint operation... - Click fill with tablet would result in two fill events. --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 33 +++++----- core_lib/src/graphics/bitmap/bitmapbucket.h | 4 +- tests/src/test_bitmapbucket.cpp | 64 +++++++++++++++++++ 3 files changed, 83 insertions(+), 18 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 51a2eae75b..8f90248201 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -65,10 +65,10 @@ BitmapBucket::BitmapBucket(Editor* editor, const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); - mReferenceColor = image->constScanLine(point.x(), point.y()); + mFillToImageColor = image->constScanLine(point.x(), point.y()); } -bool BitmapBucket::shouldFill(QPointF checkPoint) const +bool BitmapBucket::allowFill(QPointF checkPoint) const { const QPoint point = QPoint(qFloor(checkPoint.x()), qFloor(checkPoint.y())); @@ -84,12 +84,10 @@ bool BitmapBucket::shouldFill(QPointF checkPoint) const if (!targetImage.isLoaded()) { return false; } - BitmapImage referenceImage = mReferenceImage; - - QRgb pixelColor = referenceImage.constScanLine(point.x(), point.y()); + QRgb colorOfreferenceImage = mReferenceImage.constScanLine(point.x(), point.y()); QRgb targetPixelColor = targetImage.constScanLine(point.x(), point.y()); - if (mProperties.fillMode == 2 && pixelColor != 0) + if (mProperties.fillMode == 2 && colorOfreferenceImage != 0) { // don't try to fill because we won't be able to see it anyway... return false; @@ -101,17 +99,20 @@ bool BitmapBucket::shouldFill(QPointF checkPoint) const return true; } - // Ensure that when dragging that we're only filling on either transparent or same color - if ((mReferenceColor == targetPixelColor && targetPixelColor == pixelColor) || (pixelColor == 0 && targetPixelColor == 0)) - { - return true; + QRgb fillToColor = mFillToImageColor; + + // Using either of these modes applies premultiplied colors, so we have to unpremultiply to compare the colors + if (mProperties.bucketFillReferenceMode == 1) { + colorOfreferenceImage = qUnpremultiply(colorOfreferenceImage); + } + if (mProperties.bucketFillToLayerMode == 1) { + fillToColor = qUnpremultiply(fillToColor); } - // When filling with various blending modes we need to verify that the applied color - // doesn't match the target color, otherwise it will fill the same color for no reason. - // We still expect to only fill on either transparent or same color. - if (mAppliedColor != targetPixelColor && pixelColor == 0) - { + // Ensure that when dragging that we're only filling on either transparent or same color + if (targetPixelColor != mAppliedColor && + targetPixelColor == mFillToImageColor && + (colorOfreferenceImage == fillToColor || colorOfreferenceImage == 0)) { return true; } @@ -130,7 +131,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functioncurrentFrame(); const QRgb origColor = fillColor; - if (!shouldFill(updatedPoint)) { return; } + if (!allowFill(updatedPoint)) { return; } BitmapImage* targetImage = static_cast(targetLayer->getLastKeyFrameAtPosition(currentFrameIndex)); diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index d73b941953..4c151552fb 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -57,7 +57,7 @@ class BitmapBucket * @param checkPoint * @return True if you are allowed to fill, otherwise false */ - bool shouldFill(QPointF checkPoint) const; + bool allowFill(QPointF checkPoint) const; std::pair findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const; BitmapImage flattenBitmapLayersToImage(); @@ -67,7 +67,7 @@ class BitmapBucket BitmapImage mReferenceImage; QRgb mBucketColor = 0; - QRgb mReferenceColor = 0; + QRgb mFillToImageColor = 0; QRgb mAppliedColor = 0; QPointF mBucketStartPoint; diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index d496ce237c..d2bf34aaf5 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -135,4 +135,68 @@ TEST_CASE("BitmapBucket - Fill drag logic") verifyPixels(pressPoint, image, fillColor.rgba()); } + + SECTION("Ensure that we only fill with same color on reference color - " + "layerMode: current layer and reference mode: current") + { + properties.bucketFillToLayerMode = 0; + properties.bucketFillReferenceMode = 0; + properties.fillMode = 0; + + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test5a.png"); + + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + + image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test5b.png"); + + verifyPixels(pressPoint, image, fillColor.rgba()); + } + + SECTION("Ensure that we only fill with same color on reference color - " + "layer mode: all layers and referenceMode: layer below") + { + properties.bucketFillToLayerMode = 1; + properties.bucketFillReferenceMode = 1; + properties.fillMode = 1; + + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + BitmapImage* image = static_cast(editor->layers()->currentLayer(-1))->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test6a.png"); + + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + + image = static_cast(editor->layers()->currentLayer(-1))->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test6b.png"); + + // Because layers reference all layers are used and we're filling to layer below + // the color has to be premultiplied because the reference pixel is premultiplied + verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + } + + SECTION("Ensure that we only fill with same color on reference color - " + "layer mode: current layer and referenceMode: all layers") + { + properties.bucketFillToLayerMode = 0; + properties.bucketFillReferenceMode = 1; + properties.fillMode = 1; + + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test7a.png"); + + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + + image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test7b.png"); + + // Because layers reference all layers are used and we're filling to layer below + // the color has to be premultiplied because the reference pixel is premultiplied + verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + } } From 4ce86c3cdcde1aac6b408df3b5563e70f5f608b6 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Tue, 5 Oct 2021 17:53:49 +0200 Subject: [PATCH 02/16] Fix mouse press event could be fired after a tablet release event --- core_lib/src/interface/scribblearea.cpp | 26 +++++++++++++++++++++++-- core_lib/src/interface/scribblearea.h | 13 +++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/core_lib/src/interface/scribblearea.cpp b/core_lib/src/interface/scribblearea.cpp index 81703b490b..8a21e36d6e 100644 --- a/core_lib/src/interface/scribblearea.cpp +++ b/core_lib/src/interface/scribblearea.cpp @@ -60,15 +60,18 @@ bool ScribbleArea::init() { mPrefs = mEditor->preference(); mDoubleClickTimer = new QTimer(this); + mMouseFilterTimer = new QTimer(this); connect(mPrefs, &PreferenceManager::optionChanged, this, &ScribbleArea::settingUpdated); connect(mDoubleClickTimer, &QTimer::timeout, this, &ScribbleArea::handleDoubleClick); + connect(mMouseFilterTimer, &QTimer::timeout, this, &ScribbleArea::tabletReleaseEventFired); connect(mEditor->select(), &SelectionManager::selectionChanged, this, &ScribbleArea::onSelectionChanged); connect(mEditor->select(), &SelectionManager::needPaintAndApply, this, &ScribbleArea::applySelectionChanges); connect(mEditor->select(), &SelectionManager::needDeleteSelection, this, &ScribbleArea::deleteSelection); mDoubleClickTimer->setInterval(50); + mMouseFilterTimer->setInterval(50); const int curveSmoothingLevel = mPrefs->getInt(SETTING::CURVE_SMOOTHING); mCurveSmoothingLevel = curveSmoothingLevel / 20.0; // default value is 1.0 @@ -86,7 +89,6 @@ bool ScribbleArea::init() updateCanvasCursor(); setMouseTracking(true); // reacts to mouse move events, even if the button is not pressed - #if QT_VERSION >= QT_VERSION_CHECK(5, 9, 0) setTabletTracking(true); // tablet tracking first added in 5.9 #endif @@ -581,6 +583,8 @@ void ScribbleArea::tabletEvent(QTabletEvent *e) } else if (event.eventType() == QTabletEvent::TabletRelease) { + mTabletReleaseMillisAgo = 0; + mMouseFilterTimer->start(); if (mTabletInUse) { mStrokeManager->pointerReleaseEvent(&event); @@ -706,6 +710,22 @@ void ScribbleArea::handleDoubleClick() } } +void ScribbleArea::tabletReleaseEventFired() +{ + // Under certain circumstances a mouse press event will fire after a tablet release event. + // This causes unexpected behaviours for some of the tools, eg. the bucket. + // The problem only seems to occur on windows and only when tapping. + // prior to this fix, the event queue would look like this: + // eg: TabletPress -> TabletRelease -> MousePress + // The following will filter mouse events created after a tablet release event. + mTabletReleaseMillisAgo += 100; + + if (mTabletReleaseMillisAgo >= MOUSE_FILTER_THRESHOLD) { + mTabletReleaseMillisAgo = 0; + mMouseFilterTimer->stop(); + } +} + bool ScribbleArea::isLayerPaintable() const { Layer* layer = mEditor->layers()->currentLayer(); @@ -716,7 +736,7 @@ bool ScribbleArea::isLayerPaintable() const void ScribbleArea::mousePressEvent(QMouseEvent* e) { - if (mTabletInUse) + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; @@ -732,6 +752,7 @@ void ScribbleArea::mousePressEvent(QMouseEvent* e) void ScribbleArea::mouseMoveEvent(QMouseEvent* e) { + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; } PointerEvent event(e); mStrokeManager->pointerMoveEvent(&event); @@ -741,6 +762,7 @@ void ScribbleArea::mouseMoveEvent(QMouseEvent* e) void ScribbleArea::mouseReleaseEvent(QMouseEvent* e) { + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; } PointerEvent event(e); mStrokeManager->pointerReleaseEvent(&event); diff --git a/core_lib/src/interface/scribblearea.h b/core_lib/src/interface/scribblearea.h index 2348609e47..d05bf07f9b 100644 --- a/core_lib/src/interface/scribblearea.h +++ b/core_lib/src/interface/scribblearea.h @@ -267,6 +267,15 @@ public slots: QColor mOnionColor; private: + + /* Under certain circumstances a mouse press event will fire after a tablet release event. + This causes unexpected behaviours for some of the tools, eg. the bucket. + The problem only seems to occur on windows and only when tapping. + prior to this fix the event queue would look like this: + eg: TabletPress -> TabletRelease -> MousePress + The following will filter mouse events created after a tablet release event. + */ + void tabletReleaseEventFired(); bool mKeyboardInUse = false; bool mMouseInUse = false; bool mMouseRightButtonInUse = false; @@ -279,6 +288,10 @@ public slots: // Microsoft suggests that a double click action should be no more than 500 ms const int DOUBLE_CLICK_THRESHOLD = 500; QTimer* mDoubleClickTimer = nullptr; + int mTabletReleaseMillisAgo; + const int MOUSE_FILTER_THRESHOLD = 200; + + QTimer* mMouseFilterTimer = nullptr; QPoint mCursorCenterPos; QPointF mTransformedCursorPos; From 2b08e68d08a8cfb1b6c515196125eb6abc2678bf Mon Sep 17 00:00:00 2001 From: scribblemaniac Date: Fri, 15 Oct 2021 18:31:35 -0600 Subject: [PATCH 03/16] Fix variable case --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 8f90248201..cbb9a79b2e 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -84,10 +84,10 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const if (!targetImage.isLoaded()) { return false; } - QRgb colorOfreferenceImage = mReferenceImage.constScanLine(point.x(), point.y()); + QRgb colorOfReferenceImage = mReferenceImage.constScanLine(point.x(), point.y()); QRgb targetPixelColor = targetImage.constScanLine(point.x(), point.y()); - if (mProperties.fillMode == 2 && colorOfreferenceImage != 0) + if (mProperties.fillMode == 2 && colorOfReferenceImage != 0) { // don't try to fill because we won't be able to see it anyway... return false; @@ -103,7 +103,7 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const // Using either of these modes applies premultiplied colors, so we have to unpremultiply to compare the colors if (mProperties.bucketFillReferenceMode == 1) { - colorOfreferenceImage = qUnpremultiply(colorOfreferenceImage); + colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); } if (mProperties.bucketFillToLayerMode == 1) { fillToColor = qUnpremultiply(fillToColor); @@ -112,7 +112,7 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const // Ensure that when dragging that we're only filling on either transparent or same color if (targetPixelColor != mAppliedColor && targetPixelColor == mFillToImageColor && - (colorOfreferenceImage == fillToColor || colorOfreferenceImage == 0)) { + (colorOfReferenceImage == fillToColor || colorOfReferenceImage == 0)) { return true; } From ddaee959b9ca9c61aee68ec5c1d5886145890da4 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 17 Oct 2021 12:37:51 +0200 Subject: [PATCH 04/16] Use pixelformat to check if color is premultiplied when blending occurs - Use compareColor instead of raw pixel comparison for the ability to fill when the pixel is in average the same. --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 28 +++++++++++-------- core_lib/src/graphics/bitmap/bitmapbucket.h | 7 +++++ tests/src/test_bitmapbucket.cpp | 8 ++---- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index cbb9a79b2e..caeffab8c9 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -48,6 +48,8 @@ BitmapBucket::BitmapBucket(Editor* editor, mTargetFillToLayer = initialLayer; mTargetFillToLayerIndex = initialLayerIndex; + mTolerance = mProperties.toleranceEnabled ? static_cast(mProperties.tolerance) : 0; + if (properties.bucketFillToLayerMode == 1) { auto result = findBitmapLayerBelow(initialLayer, initialLayerIndex); @@ -61,11 +63,15 @@ BitmapBucket::BitmapBucket(Editor* editor, { mReferenceImage = flattenBitmapLayersToImage(); } + mReferenceLayerPixelFormat = mReferenceImage.image()->pixelFormat(); const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); mFillToImageColor = image->constScanLine(point.x(), point.y()); + mFillToLayerPixelFormat = image->image()->pixelFormat(); + + mPixelCache = new QHash(); } bool BitmapBucket::allowFill(QPointF checkPoint) const @@ -101,18 +107,19 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const QRgb fillToColor = mFillToImageColor; - // Using either of these modes applies premultiplied colors, so we have to unpremultiply to compare the colors - if (mProperties.bucketFillReferenceMode == 1) { - colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); - } - if (mProperties.bucketFillToLayerMode == 1) { + // Ensure that when dragging that we're only filling on either transparent or same color + + if (mFillToLayerPixelFormat.premultiplied() == QPixelFormat::Premultiplied) { fillToColor = qUnpremultiply(fillToColor); } - // Ensure that when dragging that we're only filling on either transparent or same color - if (targetPixelColor != mAppliedColor && - targetPixelColor == mFillToImageColor && - (colorOfReferenceImage == fillToColor || colorOfReferenceImage == 0)) { + if (mReferenceLayerPixelFormat.premultiplied() == QPixelFormat::Premultiplied) { + colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); + } + + if (!targetImage.compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && + targetImage.compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && + (mReferenceImage.compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0)) { return true; } @@ -127,7 +134,6 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function(mProperties.tolerance) : 0; const int currentFrameIndex = mEditor->currentFrame(); const QRgb origColor = fillColor; @@ -157,7 +163,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function *mPixelCache; + BitmapImage mReferenceImage; QRgb mBucketColor = 0; QRgb mFillToImageColor = 0; QRgb mAppliedColor = 0; + QPixelFormat mFillToLayerPixelFormat; + QPixelFormat mReferenceLayerPixelFormat; + QPointF mBucketStartPoint; QRectF mMaxFillRegion; + int mTolerance = 0; + int mTargetFillToLayerIndex = -1; Properties mProperties; diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index d2bf34aaf5..5d17bf4aa4 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -173,9 +173,7 @@ TEST_CASE("BitmapBucket - Fill drag logic") image = static_cast(editor->layers()->currentLayer(-1))->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test6b.png"); - // Because layers reference all layers are used and we're filling to layer below - // the color has to be premultiplied because the reference pixel is premultiplied - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyPixels(pressPoint, image, fillColor.rgba()); } SECTION("Ensure that we only fill with same color on reference color - " @@ -195,8 +193,6 @@ TEST_CASE("BitmapBucket - Fill drag logic") image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test7b.png"); - // Because layers reference all layers are used and we're filling to layer below - // the color has to be premultiplied because the reference pixel is premultiplied - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyPixels(pressPoint, image, fillColor.rgba()); } } From a197d52be3a3b5ad00c4e8ec8c7bd7f608a273b4 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 17 Oct 2021 17:31:57 +0200 Subject: [PATCH 05/16] Fix linux compilation error --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 6 +++--- core_lib/src/graphics/bitmap/bitmapimage.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index caeffab8c9..e958ee2bf8 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -117,9 +117,9 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); } - if (!targetImage.compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && - targetImage.compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && - (mReferenceImage.compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0)) { + if (!BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && + BitmapImage::compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && + (BitmapImage::compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0)) { return true; } diff --git a/core_lib/src/graphics/bitmap/bitmapimage.h b/core_lib/src/graphics/bitmap/bitmapimage.h index 94d799f994..8c9c6591f2 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -74,7 +74,7 @@ class BitmapImage : public KeyFrame void clear(QRect rectangle); void clear(QRectF rectangle) { clear(rectangle.toRect()); } - static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache); + static bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache); static bool floodFill(BitmapImage* replaceImage, BitmapImage* targetImage, QRect cameraRect, QPoint point, QRgb fillColor, int tolerance); static void expandFill(BitmapImage* targetImage, QRgb fillColor, int expand); static QVector > manhattanDistance(BitmapImage* image, QRgb& searchColor); From 2b3fefb9e8328413c592f9ac9f8b08714eb428e2 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Fri, 13 May 2022 10:31:37 +0200 Subject: [PATCH 06/16] Fix compiler error caused by inline function definition --- core_lib/src/graphics/bitmap/bitmapimage.cpp | 43 ------------------- core_lib/src/graphics/bitmap/bitmapimage.h | 45 +++++++++++++++++++- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index 9f0448b017..b40e2aa3a9 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -18,7 +18,6 @@ GNU General Public License for more details. #include #include -#include #include #include #include "util.h" @@ -747,48 +746,6 @@ void BitmapImage::clear(QRect rectangle) modification(); } -/** Compare colors for the purposes of flood filling - * - * Calculates the Eulcidian difference of the RGB channels - * of the image and compares it to the tolerance - * - * @param[in] newColor The first color to compare - * @param[in] oldColor The second color to compare - * @param[in] tolerance The threshold limit between a matching and non-matching color - * @param[in,out] cache Contains a mapping of previous results of compareColor with rule that - * cache[someColor] = compareColor(someColor, oldColor, tolerance) - * - * @return Returns true if the colors have a similarity below the tolerance level - * (i.e. if Eulcidian distance squared is <= tolerance) - */ -inline bool BitmapImage::compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache) -{ - // Handle trivial case - if (newColor == oldColor) return true; - - if(cache && cache->contains(newColor)) return cache->value(newColor); - - // Get Eulcidian distance between colors - // Not an accurate representation of human perception, - // but it's the best any image editing program ever does - int diffRed = static_cast(qPow(qRed(oldColor) - qRed(newColor), 2)); - int diffGreen = static_cast(qPow(qGreen(oldColor) - qGreen(newColor), 2)); - int diffBlue = static_cast(qPow(qBlue(oldColor) - qBlue(newColor), 2)); - // This may not be the best way to handle alpha since the other channels become less relevant as - // the alpha is reduces (ex. QColor(0,0,0,0) is the same as QColor(255,255,255,0)) - int diffAlpha = static_cast(qPow(qAlpha(oldColor) - qAlpha(newColor), 2)); - - bool isSimilar = (diffRed + diffGreen + diffBlue + diffAlpha) <= tolerance; - - if(cache) - { - Q_ASSERT(cache->contains(isSimilar) ? isSimilar == (*cache)[newColor] : true); - (*cache)[newColor] = isSimilar; - } - - return isSimilar; -} - bool BitmapImage::floodFill(BitmapImage** replaceImage, const BitmapImage* targetImage, const QRect& cameraRect, diff --git a/core_lib/src/graphics/bitmap/bitmapimage.h b/core_lib/src/graphics/bitmap/bitmapimage.h index 507efbc9c8..6122b92559 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -19,6 +19,8 @@ GNU General Public License for more details. #include #include "keyframe.h" +#include +#include class BitmapImage : public KeyFrame @@ -73,7 +75,6 @@ class BitmapImage : public KeyFrame void clear(QRect rectangle); void clear(QRectF rectangle) { clear(rectangle.toRect()); } - static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache); static bool floodFill(BitmapImage** replaceImage, const BitmapImage* targetImage, const QRect& cameraRect, const QPoint& point, const QRgb& fillColor, int tolerance, const int expandValue); static bool* floodFillPoints(const BitmapImage* targetImage, QRect searchBounds, const QRect& maxBounds, @@ -120,6 +121,48 @@ class BitmapImage : public KeyFrame Status writeFile(const QString& filename); + /** Compare colors for the purposes of flood filling + * + * Calculates the Eulcidian difference of the RGB channels + * of the image and compares it to the tolerance + * + * @param[in] newColor The first color to compare + * @param[in] oldColor The second color to compare + * @param[in] tolerance The threshold limit between a matching and non-matching color + * @param[in,out] cache Contains a mapping of previous results of compareColor with rule that + * cache[someColor] = compareColor(someColor, oldColor, tolerance) + * + * @return Returns true if the colors have a similarity below the tolerance level + * (i.e. if Eulcidian distance squared is <= tolerance) + */ + static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache) + { + // Handle trivial case + if (newColor == oldColor) return true; + + if(cache && cache->contains(newColor)) return cache->value(newColor); + + // Get Eulcidian distance between colors + // Not an accurate representation of human perception, + // but it's the best any image editing program ever does + int diffRed = static_cast(qPow(qRed(oldColor) - qRed(newColor), 2)); + int diffGreen = static_cast(qPow(qGreen(oldColor) - qGreen(newColor), 2)); + int diffBlue = static_cast(qPow(qBlue(oldColor) - qBlue(newColor), 2)); + // This may not be the best way to handle alpha since the other channels become less relevant as + // the alpha is reduces (ex. QColor(0,0,0,0) is the same as QColor(255,255,255,0)) + int diffAlpha = static_cast(qPow(qAlpha(oldColor) - qAlpha(newColor), 2)); + + bool isSimilar = (diffRed + diffGreen + diffBlue + diffAlpha) <= tolerance; + + if(cache) + { + Q_ASSERT(cache->contains(isSimilar) ? isSimilar == (*cache)[newColor] : true); + (*cache)[newColor] = isSimilar; + } + + return isSimilar; + } + protected: void updateBounds(QRect rectangle); void extend(const QPoint& p); From a100078e3d43334f0381aecf26e7e2857402c493 Mon Sep 17 00:00:00 2001 From: Jakob Gahde Date: Sun, 30 Oct 2022 23:39:45 +0100 Subject: [PATCH 07/16] Cleanup --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 48 +++++++------------ core_lib/src/graphics/bitmap/bitmapbucket.h | 2 +- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index df6724f782..2a238a87bb 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -74,10 +74,8 @@ BitmapBucket::BitmapBucket(Editor* editor, mPixelCache = new QHash(); } -bool BitmapBucket::allowFill(QPointF checkPoint) const +bool BitmapBucket::allowFill(const QPoint& checkPoint) const { - const QPoint point = QPoint(qFloor(checkPoint.x()), qFloor(checkPoint.y())); - if (mProperties.fillMode == 0 && qAlpha(mBucketColor) == 0) { // Filling in overlay mode with a fully transparent color has no @@ -90,8 +88,8 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const if (!targetImage.isLoaded()) { return false; } - QRgb colorOfReferenceImage = mReferenceImage.constScanLine(point.x(), point.y()); - QRgb targetPixelColor = targetImage.constScanLine(point.x(), point.y()); + QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y()); + QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y()); if (mProperties.fillMode == 2 && colorOfReferenceImage != 0) { @@ -105,10 +103,10 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const return true; } - QRgb fillToColor = mFillToImageColor; - - // Ensure that when dragging that we're only filling on either transparent or same color + // The remainder applies to drag fill: Ensure that we're only filling on either transparent or same color + // and avoid filling the same area repeatedly + QRgb fillToColor = mFillToImageColor; if (mFillToLayerPixelFormat.premultiplied() == QPixelFormat::Premultiplied) { fillToColor = qUnpremultiply(fillToColor); } @@ -117,35 +115,23 @@ bool BitmapBucket::allowFill(QPointF checkPoint) const colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); } - if (!BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && + return !BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && BitmapImage::compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && - (BitmapImage::compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0)) { - return true; - } - - return false; + (BitmapImage::compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0); } void BitmapBucket::paint(const QPointF updatedPoint, std::function state) { - const Layer* targetLayer = mTargetFillToLayer; - int targetLayerIndex = mTargetFillToLayerIndex; - QRgb fillColor = mBucketColor; - const QPoint point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y())); - const QRect cameraRect = mMaxFillRegion; - const int tolerance = mProperties.toleranceEnabled ? static_cast(mProperties.tolerance) : 0; const int currentFrameIndex = mEditor->currentFrame(); - const QRgb origColor = fillColor; - if (!allowFill(updatedPoint)) { return; } + if (!allowFill(point)) { return; } - BitmapImage* targetImage = static_cast(targetLayer->getLastKeyFrameAtPosition(currentFrameIndex)); + BitmapImage* targetImage = static_cast(mTargetFillToLayer->getLastKeyFrameAtPosition(currentFrameIndex)); if (targetImage == nullptr || !targetImage->isLoaded()) { return; } // Can happen if the first frame is deleted while drawing - BitmapImage referenceImage = mReferenceImage; - + QRgb fillColor = mBucketColor; if (mProperties.fillMode == 1) { // Pass a fully opaque version of the new color to floodFill @@ -161,11 +147,11 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functionpaste(replaceImage, QPainter::CompositionMode_DestinationOut); // Reduce the opacity of the fill to match the new color - BitmapImage properColor(replaceImage->bounds(), QColor::fromRgba(origColor)); + BitmapImage properColor(replaceImage->bounds(), QColor::fromRgba(mBucketColor)); properColor.paste(replaceImage, QPainter::CompositionMode_DestinationIn); // Write reduced-opacity fill image on top of target image targetImage->paste(&properColor); @@ -201,7 +187,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functionmodification(); delete replaceImage; - state(BucketState::DidFillTarget, targetLayerIndex, currentFrameIndex); + state(BucketState::DidFillTarget, mTargetFillToLayerIndex, currentFrameIndex); } BitmapImage BitmapBucket::flattenBitmapLayersToImage() diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index e819d79623..643d3e4d5d 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -57,7 +57,7 @@ class BitmapBucket * @param checkPoint * @return True if you are allowed to fill, otherwise false */ - bool allowFill(QPointF checkPoint) const; + bool allowFill(const QPoint& checkPoint) const; std::pair findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const; BitmapImage flattenBitmapLayersToImage(); From f5b3236a142c387da2e11070defb52f59c2406f9 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Mon, 31 Oct 2022 17:48:50 +0100 Subject: [PATCH 08/16] Fix now unnecessary check for premult The real problem causing this has been fixed via #1729 --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 9 --------- core_lib/src/graphics/bitmap/bitmapbucket.h | 3 --- 2 files changed, 12 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index c6d360877b..e0f2079f1f 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -63,13 +63,11 @@ BitmapBucket::BitmapBucket(Editor* editor, { mReferenceImage = flattenBitmapLayersToImage(); } - mReferenceLayerPixelFormat = mReferenceImage.image()->pixelFormat(); const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); mFillToImageColor = image->constScanLine(point.x(), point.y()); - mFillToLayerPixelFormat = image->image()->pixelFormat(); mPixelCache = new QHash(); } @@ -107,13 +105,6 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const // and avoid filling the same area repeatedly QRgb fillToColor = mFillToImageColor; - if (mFillToLayerPixelFormat.premultiplied() == QPixelFormat::Premultiplied) { - fillToColor = qUnpremultiply(fillToColor); - } - - if (mReferenceLayerPixelFormat.premultiplied() == QPixelFormat::Premultiplied) { - colorOfReferenceImage = qUnpremultiply(colorOfReferenceImage); - } return !BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && BitmapImage::compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index 643d3e4d5d..59d80afcd8 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -72,9 +72,6 @@ class BitmapBucket QRgb mFillToImageColor = 0; QRgb mAppliedColor = 0; - QPixelFormat mFillToLayerPixelFormat; - QPixelFormat mReferenceLayerPixelFormat; - QPointF mBucketStartPoint; QRect mMaxFillRegion; From 1040f9dc2619a7f7510bd783fc72e7a36e2e932b Mon Sep 17 00:00:00 2001 From: MrStevns Date: Mon, 31 Oct 2022 20:03:22 +0100 Subject: [PATCH 09/16] Fix filling was not allowed when fillTo mode was "layer below" Layer setup: Bitmap Layer Stroke Bitmap Layer Fill 1. draw a circle on the stroke layer and slice it up with lines, so you can fill individual parts 2. set fillTo mode to "layer below" and reference to "all layers" 3. fill and drag a color, eg. red.. across the slices 4. Change fillTo mode to "current layer" 5. repeat step 3 with a different color --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 15 +++++---------- core_lib/src/graphics/bitmap/bitmapbucket.h | 2 -- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index e0f2079f1f..97b9a2fcc5 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -95,20 +95,16 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const return false; } - // First paint is allowed with no rules applied - if (mFirstPaint) - { - return true; - } - // The remainder applies to drag fill: Ensure that we're only filling on either transparent or same color // and avoid filling the same area repeatedly - QRgb fillToColor = mFillToImageColor; + if (targetPixelColor == 0) { + // The target pixel color is transparent, so we can fill the pixel + return true; + } return !BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && - BitmapImage::compareColor(targetPixelColor, mFillToImageColor, mTolerance, mPixelCache) && - (BitmapImage::compareColor(colorOfReferenceImage, fillToColor, mTolerance, mPixelCache) || colorOfReferenceImage == 0); + (BitmapImage::compareColor(colorOfReferenceImage, mFillToImageColor, mTolerance, mPixelCache)); } void BitmapBucket::paint(const QPointF updatedPoint, std::function state) @@ -173,7 +169,6 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functionconstScanLine(point.x(), point.y()); - mFirstPaint = false; targetImage->modification(); delete replaceImage; diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index 59d80afcd8..b2102295f9 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -80,8 +80,6 @@ class BitmapBucket int mTargetFillToLayerIndex = -1; Properties mProperties; - - bool mFirstPaint = true; }; #endif // BITMAPBUCKET_H From 2e9141f9b9110d22040b27d1d1a689c4dad11d58 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Tue, 1 Nov 2022 20:00:32 +0100 Subject: [PATCH 10/16] Rewrite test cases --- tests/src/test_bitmapbucket.cpp | 201 +++++++++++++++++++------------- 1 file changed, 122 insertions(+), 79 deletions(-) diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index 9f0b7d674a..56b0c2b20b 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -15,7 +15,7 @@ #include "basetool.h" -void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, Properties properties) { +void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, Properties properties, int fillCountThreshold) { int moveX = 0; BitmapBucket bucket = BitmapBucket(editor, color, bounds, movePoint, properties); @@ -34,12 +34,10 @@ void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, }); } - // Make sure that only 4 fills has been applied - // otherwise we're spilling onto unwanted pixels - REQUIRE(fillCount == 4); + REQUIRE(fillCount == fillCountThreshold); } -void verifyPixels(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) +void verifyOnlyPixelsInsideSegmentsAreFilled(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) { REQUIRE(image->constScanLine(referencePoint.x(), referencePoint.y()) == fillColor); @@ -49,7 +47,27 @@ void verifyPixels(QPoint referencePoint, const BitmapImage* image, QRgb fillColo REQUIRE(image->constScanLine(referencePoint.x()+6, referencePoint.y()) != fillColor); } -TEST_CASE("BitmapBucket - Fill drag logic") +void verifyAllPixelsInsideSegmentsAreFilled(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) +{ + REQUIRE(image->constScanLine(referencePoint.x(), referencePoint.y()) == fillColor); + + REQUIRE(image->constScanLine(referencePoint.x()+4, referencePoint.y()) == fillColor); + REQUIRE(image->constScanLine(referencePoint.x()+5, referencePoint.y()) == fillColor); + REQUIRE(image->constScanLine(referencePoint.x()+6, referencePoint.y()) == fillColor); +} + + +/** + * Ascii representation of test project + * The "*" represent black strokes. + * The space inbetween represents transparency + * *************** + * * * * * * + * *************** + * + * The test cases are based around filling on the initially transparent area and dragging across the four segments. + */ +TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { FileManager fm; Object* obj = fm.load(":/fill-drag-test/fill-drag-test.pcl"); @@ -82,117 +100,142 @@ TEST_CASE("BitmapBucket - Fill drag logic") REQUIRE(beforeFill.constScanLine(pressPoint.x(), pressPoint.y()) == 0); - SECTION("FillTo: CurrentLayer - Reference: Current Layer") - { - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + // The dragging logic is based around that we only fill on either transparent or the same color as the fill color. + SECTION("Filling on current layer - layer is not pre filled") { + properties.bucketFillToLayerMode = 0; + Layer* strokeLayer = editor->layers()->currentLayer(); + SECTION("When reference is current layer, only transparent color is filled") + { + properties.bucketFillReferenceMode = 0; + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test1.png"); + image->writeFile(resultsPath + "test1a.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); - } + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } - SECTION("FillTo: Layer below - reference: current layer") - { - properties.bucketFillToLayerMode = 1; + SECTION("When reference is all layers, only transparent color is filled") + { + properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - // Verify that colors are correct on layer below - BitmapImage* image = static_cast(editor->layers()->getLayer(editor->currentLayerIndex()-1))->getLastBitmapImageAtFrame(1); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test2.png"); + image->writeFile(resultsPath + "test1b.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } } + SECTION("Filling on current layer - layer is pre-filled") { + properties.bucketFillToLayerMode = 0; - SECTION("FillTo: Layer below - reference: all layers") - { - properties.bucketFillToLayerMode = 1; - properties.bucketFillReferenceMode = 1; + // Fill mode is set to `replace` because it makes it easier to compare colors... + properties.fillMode = 1; + Layer* strokeLayer = editor->layers()->currentLayer(); + SECTION("When reference is current layer, only pixels matching the fill color are filled"){ + properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test2a-first.png"); - BitmapImage* image = static_cast(editor->layers()->getLayer(editor->currentLayerIndex()-1))->getLastBitmapImageAtFrame(1); + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - image->writeFile(resultsPath + "test3.png"); + image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test2a-second.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); - } + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, fillColor.rgba()); + } - SECTION("FillTo: Current layer - reference: all layers") - { - properties.bucketFillToLayerMode = 0; - properties.bucketFillReferenceMode = 1; + SECTION("When reference is all layers") + { + properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test3a-first.png"); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - image->writeFile(resultsPath + "test4.png"); + image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test3a-second.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, fillColor.rgba()); + } } - SECTION("Ensure that we only fill with same color on reference color - " - "layerMode: current layer and reference mode: current") - { - properties.bucketFillToLayerMode = 0; - properties.bucketFillReferenceMode = 0; - properties.fillMode = 0; + // The behaviour changes here because we'll be filling on the layer below, but that layer is blank, + // yet the reference mode tells the flood fill algorithm to fill using the pixel data from the the reference layer. + // In this case it means that all pixels will be filled as we drag across the segments. + SECTION("Filling on layer below - layer is not pre-filled") { + properties.bucketFillToLayerMode = 1; + + Layer* fillLayer = editor->layers()->currentLayer(-1); + SECTION("When reference is current layer, then all pixels not matching the fill color are filled once") + { + properties.bucketFillReferenceMode = 0; + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + + // Verify that colors are correct on layer below + BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); + + image->writeFile(resultsPath + "test4a.png"); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test5a.png"); + verifyAllPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } - fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); - image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test5b.png"); + SECTION("When reference is all layers, then all pixels not matching the fill color are filled") + { + properties.bucketFillReferenceMode = 1; - verifyPixels(pressPoint, image, fillColor.rgba()); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + + BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); + + image->writeFile(resultsPath + "test5b.png"); + + verifyAllPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } } - SECTION("Ensure that we only fill with same color on reference color - " - "layer mode: all layers and referenceMode: layer below") - { + SECTION("Filling on layer below - layer is pre-filled") { properties.bucketFillToLayerMode = 1; - properties.bucketFillReferenceMode = 1; properties.fillMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); - BitmapImage* image = static_cast(editor->layers()->currentLayer(-1))->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test6a.png"); + SECTION("when reference is all layers, then only pixels matching the fill color are filled") + { + properties.bucketFillReferenceMode = 1; - fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + Layer* strokeLayer = editor->layers()->currentLayer(); + Layer* fillLayer = editor->layers()->currentLayer(-1); - image = static_cast(editor->layers()->currentLayer(-1))->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test6b.png"); + // Because the layer is blank, all pixels will be filled once + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + BitmapImage* image1 = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image1->writeFile(resultsPath + "test6a-first.png"); - verifyPixels(pressPoint, image, fillColor.rgba()); - } + fillColor = QColor(0,255,0,255); - SECTION("Ensure that we only fill with same color on reference color - " - "layer mode: current layer and referenceMode: all layers") - { - properties.bucketFillToLayerMode = 0; - properties.bucketFillReferenceMode = 1; - properties.fillMode = 1; + // Changes fillTo mode to current layer + properties.bucketFillToLayerMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test7a.png"); + editor->layers()->setCurrentLayer(strokeLayer); - fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + // Now the layer has been filled with pixel data, so we'll only fill when the color matches the fill color + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test7b.png"); + BitmapImage* image2 = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); + image1->writeFile(resultsPath + "test6a-second.png"); + image2->writeFile(resultsPath + "test6a-third.png"); - verifyPixels(pressPoint, image, fillColor.rgba()); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image1, fillColor.rgba()); + } } } From 4c6ab70b598cb5582abd6a535b7d306ff103dfc9 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Tue, 1 Nov 2022 20:05:56 +0100 Subject: [PATCH 11/16] Fix typo --- core_lib/src/interface/scribblearea.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core_lib/src/interface/scribblearea.cpp b/core_lib/src/interface/scribblearea.cpp index 3c878ae376..c96722cc7f 100644 --- a/core_lib/src/interface/scribblearea.cpp +++ b/core_lib/src/interface/scribblearea.cpp @@ -725,7 +725,7 @@ void ScribbleArea::tabletReleaseEventFired() // prior to this fix, the event queue would look like this: // eg: TabletPress -> TabletRelease -> MousePress // The following will filter mouse events created after a tablet release event. - mTabletReleaseMillisAgo += 100; + mTabletReleaseMillisAgo += 50; if (mTabletReleaseMillisAgo >= MOUSE_FILTER_THRESHOLD) { mTabletReleaseMillisAgo = 0; From b17c40fc292129c39be0c1c8d4f0a616e66d433d Mon Sep 17 00:00:00 2001 From: MrStevns Date: Tue, 8 Nov 2022 21:12:43 +0100 Subject: [PATCH 12/16] Simplify allowFill function --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 97b9a2fcc5..5262a35b35 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -89,12 +89,6 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y()); QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y()); - if (mProperties.fillMode == 2 && colorOfReferenceImage != 0) - { - // don't try to fill because we won't be able to see it anyway... - return false; - } - // The remainder applies to drag fill: Ensure that we're only filling on either transparent or same color // and avoid filling the same area repeatedly From 8252390454d46a1987609a8a0614b6802d87e987 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Fri, 11 Nov 2022 18:13:19 +0100 Subject: [PATCH 13/16] Rework drag to fill behavior again --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 18 +++++++++------ core_lib/src/graphics/bitmap/bitmapbucket.h | 2 +- tests/src/test_bitmapbucket.cpp | 22 +++++-------------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 5262a35b35..37ad511865 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -63,8 +63,8 @@ BitmapBucket::BitmapBucket(Editor* editor, { mReferenceImage = flattenBitmapLayersToImage(); } - const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); + mStartReferenceColor = mReferenceImage.constScanLine(point.x(), point.y()); BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); mFillToImageColor = image->constScanLine(point.x(), point.y()); @@ -92,13 +92,19 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const // The remainder applies to drag fill: Ensure that we're only filling on either transparent or same color // and avoid filling the same area repeatedly - if (targetPixelColor == 0) { - // The target pixel color is transparent, so we can fill the pixel + // Rule 1: allow filling if the reference pixel is transparent and the target pixel is transparent + if (mStartReferenceColor == 0 && colorOfReferenceImage == 0 && targetPixelColor == 0) + { + return true; + } + + if ((BitmapImage::compareColor(colorOfReferenceImage, targetPixelColor, mTolerance, mPixelCache) && + BitmapImage::compareColor(mStartReferenceColor, targetPixelColor, mTolerance, mPixelCache)) || + (BitmapImage::compareColor(colorOfReferenceImage, mStartReferenceColor, mTolerance, mPixelCache) && targetPixelColor == 0)) { return true; } - return !BitmapImage::compareColor(targetPixelColor, mAppliedColor, mTolerance, mPixelCache) && - (BitmapImage::compareColor(colorOfReferenceImage, mFillToImageColor, mTolerance, mPixelCache)); + return false; } void BitmapBucket::paint(const QPointF updatedPoint, std::function state) @@ -162,8 +168,6 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functionpaste(&properColor); } - mAppliedColor = targetImage->constScanLine(point.x(), point.y()); - targetImage->modification(); delete replaceImage; diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index b2102295f9..d57e5cdef8 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -70,7 +70,7 @@ class BitmapBucket BitmapImage mReferenceImage; QRgb mBucketColor = 0; QRgb mFillToImageColor = 0; - QRgb mAppliedColor = 0; + QRgb mStartReferenceColor = 0; QPointF mBucketStartPoint; QRect mMaxFillRegion; diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index 56b0c2b20b..3fa0624e30 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -47,16 +47,6 @@ void verifyOnlyPixelsInsideSegmentsAreFilled(QPoint referencePoint, const Bitmap REQUIRE(image->constScanLine(referencePoint.x()+6, referencePoint.y()) != fillColor); } -void verifyAllPixelsInsideSegmentsAreFilled(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) -{ - REQUIRE(image->constScanLine(referencePoint.x(), referencePoint.y()) == fillColor); - - REQUIRE(image->constScanLine(referencePoint.x()+4, referencePoint.y()) == fillColor); - REQUIRE(image->constScanLine(referencePoint.x()+5, referencePoint.y()) == fillColor); - REQUIRE(image->constScanLine(referencePoint.x()+6, referencePoint.y()) == fillColor); -} - - /** * Ascii representation of test project * The "*" represent black strokes. @@ -180,14 +170,14 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") SECTION("When reference is current layer, then all pixels not matching the fill color are filled once") { properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); // Verify that colors are correct on layer below BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test4a.png"); - verifyAllPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); } @@ -195,13 +185,13 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test5b.png"); + image->writeFile(resultsPath + "test5a.png"); - verifyAllPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); } } @@ -217,7 +207,7 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") Layer* fillLayer = editor->layers()->currentLayer(-1); // Because the layer is blank, all pixels will be filled once - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 12); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image1 = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image1->writeFile(resultsPath + "test6a-first.png"); From 7dc8f676582e2a555cc4456729b38de681d6e538 Mon Sep 17 00:00:00 2001 From: Jakob Gahde Date: Wed, 4 Jan 2023 00:09:37 +0100 Subject: [PATCH 14/16] Fix / simplify allowFill() --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 25 ++++++------------- core_lib/src/graphics/bitmap/bitmapbucket.h | 2 -- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 37ad511865..88ba46b223 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -34,7 +34,6 @@ BitmapBucket::BitmapBucket(Editor* editor, QPointF fillPoint, Properties properties): mEditor(editor), - mBucketStartPoint(fillPoint), mMaxFillRegion(maxFillRegion), mProperties(properties) @@ -66,9 +65,6 @@ BitmapBucket::BitmapBucket(Editor* editor, const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); mStartReferenceColor = mReferenceImage.constScanLine(point.x(), point.y()); - BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); - mFillToImageColor = image->constScanLine(point.x(), point.y()); - mPixelCache = new QHash(); } @@ -89,22 +85,17 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y()); QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y()); - // The remainder applies to drag fill: Ensure that we're only filling on either transparent or same color - // and avoid filling the same area repeatedly - - // Rule 1: allow filling if the reference pixel is transparent and the target pixel is transparent - if (mStartReferenceColor == 0 && colorOfReferenceImage == 0 && targetPixelColor == 0) + if (targetPixelColor == mBucketColor &&(mProperties.fillMode == 1 || qAlpha(targetPixelColor) == 255)) { - return true; - } - - if ((BitmapImage::compareColor(colorOfReferenceImage, targetPixelColor, mTolerance, mPixelCache) && - BitmapImage::compareColor(mStartReferenceColor, targetPixelColor, mTolerance, mPixelCache)) || - (BitmapImage::compareColor(colorOfReferenceImage, mStartReferenceColor, mTolerance, mPixelCache) && targetPixelColor == 0)) { - return true; + // Avoid filling if target pixel color matches fill color + // to avoid creating numerous seemingly useless undo operations + return false; } - return false; + // Allow filling if the reference pixel matches the start reference color, and + // the target pixel is either transparent or matches the start reference color + return BitmapImage::compareColor(colorOfReferenceImage, mStartReferenceColor, mTolerance, mPixelCache) && + (targetPixelColor == 0 || BitmapImage::compareColor(targetPixelColor, mStartReferenceColor, mTolerance, mPixelCache)); } void BitmapBucket::paint(const QPointF updatedPoint, std::function state) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index d57e5cdef8..88464994e8 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -69,10 +69,8 @@ class BitmapBucket BitmapImage mReferenceImage; QRgb mBucketColor = 0; - QRgb mFillToImageColor = 0; QRgb mStartReferenceColor = 0; - QPointF mBucketStartPoint; QRect mMaxFillRegion; int mTolerance = 0; From 0489fc4e3e4182676fa5c7916d33d58c954d04d0 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 8 Jan 2023 10:08:23 +0100 Subject: [PATCH 15/16] Reference mode should enforce fillTo mode when currentLayer is set. - Also fixed a few confusing variables --- app/src/bucketoptionswidget.cpp | 15 +++++++++++---- app/src/bucketoptionswidget.h | 1 + core_lib/src/managers/toolmanager.cpp | 16 +++++++++++++--- core_lib/src/managers/toolmanager.h | 8 ++++++-- core_lib/src/tool/basetool.cpp | 2 +- core_lib/src/tool/basetool.h | 2 +- core_lib/src/tool/buckettool.cpp | 4 ++-- core_lib/src/tool/buckettool.h | 2 +- 8 files changed, 36 insertions(+), 14 deletions(-) diff --git a/app/src/bucketoptionswidget.cpp b/app/src/bucketoptionswidget.cpp index 0ec11d2c13..e32cf984f6 100644 --- a/app/src/bucketoptionswidget.cpp +++ b/app/src/bucketoptionswidget.cpp @@ -76,7 +76,7 @@ BucketOptionsWidget::BucketOptionsWidget(Editor* editor, QWidget* parent) : connect(mEditor->tools(), &ToolManager::toolPropertyChanged, this, &BucketOptionsWidget::onPropertyChanged); connect(mEditor->layers(), &LayerManager::currentLayerChanged, this, &BucketOptionsWidget::onLayerChanged); - connect(ui->fillToLayerComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayer); + connect(ui->fillToLayerComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayerMode); connect(ui->referenceLayerComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillReferenceMode); connect(ui->blendModeComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setFillMode); @@ -123,7 +123,7 @@ void BucketOptionsWidget::updatePropertyVisibility() ui->blendModeComboBox->hide(); ui->blendModeLabel->hide(); break; - case Layer::BITMAP: + case Layer::BITMAP: { ui->strokeThicknessSlider->hide(); ui->strokeThicknessSpinBox->hide(); @@ -135,11 +135,11 @@ void BucketOptionsWidget::updatePropertyVisibility() ui->expandCheckbox->show(); ui->expandSlider->show(); ui->expandSpinBox->show(); - ui->referenceLayerComboBox->show(); - ui->referenceLayerDescLabel->show(); + disableFillToLayerComboBox(mEditor->tools()->enforceFillToLayerMode(ui->referenceLayerComboBox->currentIndex())); ui->blendModeComboBox->show(); ui->blendModeLabel->show(); break; + } default: ui->strokeThicknessSlider->hide(); ui->strokeThicknessSpinBox->hide(); @@ -235,6 +235,13 @@ void BucketOptionsWidget::setFillReferenceMode(int referenceMode) { QSignalBlocker b(ui->referenceLayerComboBox); ui->referenceLayerComboBox->setCurrentIndex(referenceMode); + disableFillToLayerComboBox(mEditor->tools()->enforceFillToLayerMode(referenceMode)); +} + +void BucketOptionsWidget::disableFillToLayerComboBox(bool state) +{ + ui->fillToLayerComboBox->setDisabled(state); + ui->fillToDescLabel->setDisabled(state); } void BucketOptionsWidget::setStrokeWidth(qreal value) diff --git a/app/src/bucketoptionswidget.h b/app/src/bucketoptionswidget.h index 34b98c54d3..a53644f33f 100644 --- a/app/src/bucketoptionswidget.h +++ b/app/src/bucketoptionswidget.h @@ -48,6 +48,7 @@ class BucketOptionsWidget : public QWidget void onLayerChanged(int); private: + void disableFillToLayerComboBox(bool state); void updatePropertyVisibility(); Ui::BucketOptionsWidget *ui; diff --git a/core_lib/src/managers/toolmanager.cpp b/core_lib/src/managers/toolmanager.cpp index 026ee73de8..add88969db 100644 --- a/core_lib/src/managers/toolmanager.cpp +++ b/core_lib/src/managers/toolmanager.cpp @@ -72,7 +72,7 @@ Status ToolManager::save(Object*) return Status::OK; } -BaseTool* ToolManager::currentTool() +BaseTool* ToolManager::currentTool() const { if (mTemporaryTool != nullptr) { @@ -251,14 +251,19 @@ void ToolManager::setBucketFillExpand(int expandValue) emit toolPropertyChanged(currentTool()->type(), BUCKETFILLEXPAND); } -void ToolManager::setBucketFillToLayer(int layerIndex) +void ToolManager::setBucketFillToLayerMode(int layerMode) { - currentTool()->setFillToLayer(layerIndex); + currentTool()->setFillToLayerMode(layerMode); emit toolPropertyChanged(currentTool()->type(), BUCKETFILLLAYERMODE); } void ToolManager::setBucketFillReferenceMode(int referenceMode) { + // If the bucket reference mode is current layer, enforce fillTo is also set to current layer + if (bucketReferenceModeIsCurrentLayer(referenceMode)) { + currentTool()->setFillToLayerMode(0); + emit toolPropertyChanged(currentTool()->type(), BUCKETFILLLAYERMODE); + } currentTool()->setFillReferenceMode(referenceMode); emit toolPropertyChanged(currentTool()->type(), BUCKETFILLLAYERREFERENCEMODE); } @@ -301,6 +306,11 @@ void ToolManager::setCameraPathDotColor(int dotColorNum) emit toolPropertyChanged(cameraTool->type(), CAMERAPATH); } +bool ToolManager::bucketReferenceModeIsCurrentLayer(int referenceMode) const +{ + return referenceMode == 0; +} + // Switches on/off two actions // eg. if x = true, then y = false int ToolManager::propertySwitch(bool condition, int tool) diff --git a/core_lib/src/managers/toolmanager.h b/core_lib/src/managers/toolmanager.h index efd3f02242..02373d2d47 100644 --- a/core_lib/src/managers/toolmanager.h +++ b/core_lib/src/managers/toolmanager.h @@ -36,7 +36,7 @@ class ToolManager : public BaseManager Status load(Object*) override; Status save(Object*) override; - BaseTool* currentTool(); + BaseTool* currentTool() const; BaseTool* getTool(ToolType eToolType); void setDefaultTool(); void setCurrentTool(ToolType eToolType); @@ -77,7 +77,7 @@ public slots: void setTolerance(int); void setBucketColorToleranceEnabled(bool enabled); void setBucketFillExpandEnabled(bool enabled); - void setBucketFillToLayer(int layerIndex); + void setBucketFillToLayerMode(int layerMode); void setBucketFillReferenceMode(int referenceMode); void setBucketFillExpand(int); void setUseFillContour(bool); @@ -87,6 +87,10 @@ public slots: void setCameraPathDotColor(int); void resetCameraTransform(CameraFieldOption option); + /// Layer mode will be enforced by the the choice the reference mode selected. + /// @return Returns true if reference mode is ``current layer`, otherwise false. + bool bucketReferenceModeIsCurrentLayer(int referenceMode) const; + private: void setTemporaryTool(ToolType eToolType); diff --git a/core_lib/src/tool/basetool.cpp b/core_lib/src/tool/basetool.cpp index 7671c3469d..56bd359bc7 100644 --- a/core_lib/src/tool/basetool.cpp +++ b/core_lib/src/tool/basetool.cpp @@ -429,7 +429,7 @@ void BaseTool::setFillExpand(const int fillExpandValue) properties.bucketFillExpand = fillExpandValue; } -void BaseTool::setFillToLayer(int layerMode) +void BaseTool::setFillToLayerMode(int layerMode) { properties.bucketFillToLayerMode = layerMode; } diff --git a/core_lib/src/tool/basetool.h b/core_lib/src/tool/basetool.h index bf1405d5ee..9124dcf6d0 100644 --- a/core_lib/src/tool/basetool.h +++ b/core_lib/src/tool/basetool.h @@ -126,7 +126,7 @@ class BaseTool : public QObject virtual void setToleranceEnabled(const bool enabled); virtual void setFillExpand(const int fillExpandValue); virtual void setFillExpandEnabled(const bool enabled); - virtual void setFillToLayer(int layerMode); + virtual void setFillToLayerMode(int layerMode); virtual void setFillReferenceMode(int referenceMode); virtual void setUseFillContour(const bool useFillContour); virtual void setShowSelectionInfo(const bool b); diff --git a/core_lib/src/tool/buckettool.cpp b/core_lib/src/tool/buckettool.cpp index c09902c36c..74d3ed8ea2 100644 --- a/core_lib/src/tool/buckettool.cpp +++ b/core_lib/src/tool/buckettool.cpp @@ -74,7 +74,7 @@ void BucketTool::resetToDefault() setFillMode(0); setFillExpand(2); setFillExpandEnabled(true); - setFillToLayer(0); + setFillToLayerMode(0); setToleranceEnabled(false); setFillReferenceMode(0); } @@ -163,7 +163,7 @@ void BucketTool::setFillExpand(const int fillExpandValue) settings.sync(); } -void BucketTool::setFillToLayer(int layerMode) +void BucketTool::setFillToLayerMode(int layerMode) { properties.bucketFillToLayerMode = layerMode; diff --git a/core_lib/src/tool/buckettool.h b/core_lib/src/tool/buckettool.h index 14ff571b9a..631c19938a 100644 --- a/core_lib/src/tool/buckettool.h +++ b/core_lib/src/tool/buckettool.h @@ -47,7 +47,7 @@ class BucketTool : public StrokeTool void setWidth(const qreal width) override; void setFillExpand(const int fillExpandValue) override; void setFillExpandEnabled(const bool enabled) override; - void setFillToLayer(int layerIndex) override; + void setFillToLayerMode(int layerMode) override; void setFillReferenceMode(int referenceMode) override; void setFillMode(int mode) override; From 33e72b8464dd0aafd8c5605c9e95499e2497d64a Mon Sep 17 00:00:00 2001 From: Jakob Gahde Date: Sun, 8 Jan 2023 23:31:12 +0100 Subject: [PATCH 16/16] Fix build error --- app/src/bucketoptionswidget.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/bucketoptionswidget.cpp b/app/src/bucketoptionswidget.cpp index e32cf984f6..ecbd16729e 100644 --- a/app/src/bucketoptionswidget.cpp +++ b/app/src/bucketoptionswidget.cpp @@ -135,7 +135,7 @@ void BucketOptionsWidget::updatePropertyVisibility() ui->expandCheckbox->show(); ui->expandSlider->show(); ui->expandSpinBox->show(); - disableFillToLayerComboBox(mEditor->tools()->enforceFillToLayerMode(ui->referenceLayerComboBox->currentIndex())); + disableFillToLayerComboBox(mEditor->tools()->bucketReferenceModeIsCurrentLayer(ui->referenceLayerComboBox->currentIndex())); ui->blendModeComboBox->show(); ui->blendModeLabel->show(); break; @@ -235,7 +235,7 @@ void BucketOptionsWidget::setFillReferenceMode(int referenceMode) { QSignalBlocker b(ui->referenceLayerComboBox); ui->referenceLayerComboBox->setCurrentIndex(referenceMode); - disableFillToLayerComboBox(mEditor->tools()->enforceFillToLayerMode(referenceMode)); + disableFillToLayerComboBox(mEditor->tools()->bucketReferenceModeIsCurrentLayer(referenceMode)); } void BucketOptionsWidget::disableFillToLayerComboBox(bool state)