From 4c78d3f4725cb37f99e5cac690b79cd543175355 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Tue, 23 Jul 2024 15:20:27 +0200 Subject: [PATCH 1/6] issue-1864: Fix fill misbehaving when drawing was partly outside border --- core_lib/src/graphics/bitmap/bitmapimage.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index e2be3641e..23a688387 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -880,8 +880,7 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, { QRgb oldColor = targetImage->constScanLine(point.x(), point.y()); oldColor = qRgba(qRed(oldColor), qGreen(oldColor), qBlue(oldColor), qAlpha(oldColor)); - QRect borderBounds = searchBounds.intersected(maxBounds); - searchBounds = searchBounds.adjusted(-1, -1, 1, 1).intersected(maxBounds); + searchBounds = searchBounds.adjusted(-1, -1, 1, 1).united(maxBounds); // Preparations QList queue; // queue all the pixels of the filled area (as they are found) @@ -921,7 +920,7 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, int yCoord = point.y() - maxBounds.top(); // In case we fill outside the searchBounds, expand the search area to the max. - if (!borderBounds.contains(point)) { + if (!searchBounds.contains(point)) { checkOutside = true; } From 2dc933a077087dcf573929c6f5549fef56e4403e Mon Sep 17 00:00:00 2001 From: MrStevns Date: Wed, 24 Jul 2024 10:43:44 +0200 Subject: [PATCH 2/6] issue-1864 - Fix fill not accounting for bounds properly --- core_lib/src/graphics/bitmap/bitmapimage.cpp | 22 +++++++++----------- core_lib/src/graphics/bitmap/bitmapimage.h | 2 +- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index 23a688387..8ed72bae8 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -799,7 +799,7 @@ bool BitmapImage::floodFill(BitmapImage** replaceImage, const int expandValue) { // Fill region must be 1 pixel larger than the target image to fill regions on the edge connected only by transparent pixels - const QRect& fillBounds = targetImage->mBounds; + const QRect& fillBounds = targetImage->mBounds.adjusted(-1, -1, 1, 1); QRect maxBounds = cameraRect.united(fillBounds).adjusted(-expandValue, -expandValue, expandValue, expandValue); const int maxWidth = maxBounds.width(), left = maxBounds.left(), top = maxBounds.top(); @@ -814,7 +814,7 @@ bool BitmapImage::floodFill(BitmapImage** replaceImage, QRect newBounds; bool shouldFillBorder = false; - bool *filledPixels = floodFillPoints(targetImage, fillBounds, maxBounds, point, tolerance, newBounds, shouldFillBorder); + bool *filledPixels = floodFillPoints(targetImage, maxBounds, point, tolerance, newBounds, shouldFillBorder); QRect translatedSearchBounds = newBounds.translated(-maxBounds.topLeft()); @@ -871,8 +871,7 @@ bool BitmapImage::floodFill(BitmapImage** replaceImage, // Flood filling based on this scanline algorithm // ----- http://lodev.org/cgtutor/floodfill.html bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, - QRect searchBounds, - const QRect& maxBounds, + const QRect& searchBounds, QPoint point, const int tolerance, QRect& newBounds, @@ -880,7 +879,6 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, { QRgb oldColor = targetImage->constScanLine(point.x(), point.y()); oldColor = qRgba(qRed(oldColor), qGreen(oldColor), qBlue(oldColor), qAlpha(oldColor)); - searchBounds = searchBounds.adjusted(-1, -1, 1, 1).united(maxBounds); // Preparations QList queue; // queue all the pixels of the filled area (as they are found) @@ -901,7 +899,7 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, queue.append(point); // Preparations END - bool *filledPixels = new bool[maxBounds.height()*maxBounds.width()]{}; + bool *filledPixels = new bool[searchBounds.height()*searchBounds.width()]{}; // True if the algorithm has attempted to fill a pixel outside the search bounds bool checkOutside = false; @@ -916,15 +914,15 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, xTemp = point.x(); - int xCoord = xTemp - maxBounds.left(); - int yCoord = point.y() - maxBounds.top(); + int xCoord = xTemp - searchBounds.left(); + int yCoord = point.y() - searchBounds.top(); // In case we fill outside the searchBounds, expand the search area to the max. if (!searchBounds.contains(point)) { checkOutside = true; } - if (filledPixels[yCoord*maxBounds.width()+xCoord]) continue; + if (filledPixels[yCoord*searchBounds.width()+xCoord]) continue; while (xTemp >= searchBounds.left() && compareColor(targetImage->constScanLine(xTemp, point.y()), oldColor, tolerance, cache.data())) xTemp--; @@ -940,9 +938,9 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, blitBounds.extend(floodPoint); } - xCoord = xTemp - maxBounds.left(); + xCoord = xTemp - searchBounds.left(); // This pixel is what we're going to fill later - filledPixels[yCoord*maxBounds.width()+xCoord] = true; + filledPixels[yCoord*searchBounds.width()+xCoord] = true; if (!spanLeft && (point.y() > searchBounds.top()) && compareColor(targetImage->constScanLine(xTemp, point.y() - 1), oldColor, tolerance, cache.data())) { @@ -964,7 +962,7 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, spanRight = false; } - Q_ASSERT(queue.count() < (maxBounds.width() * maxBounds.height())); + Q_ASSERT(queue.count() < (searchBounds.width() * searchBounds.height())); xTemp++; } } diff --git a/core_lib/src/graphics/bitmap/bitmapimage.h b/core_lib/src/graphics/bitmap/bitmapimage.h index ead411bfb..db79297d4 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -80,7 +80,7 @@ class BitmapImage : public KeyFrame 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, + const QRect& searchBounds, QPoint point, const int tolerance, QRect& newBounds, From e21f997a4a8cd2f2ca9b966e45aefc43c0418cd9 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Wed, 24 Jul 2024 11:08:24 +0200 Subject: [PATCH 3/6] Update tests to account for bounds being expanded by 1 pixel --- tests/src/test_bitmapbucket.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index cf27e52c1..46401c42c 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -22,7 +22,7 @@ void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, QPointF movingPoint = movePoint; int fillCount = 0; - while (moveX < 40) { + while (moveX < bounds.width()) { moveX++; movingPoint.setX(movingPoint.x()+1); @@ -95,7 +95,7 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") SECTION("When reference is current layer, only transparent color is filled") { properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); @@ -108,7 +108,7 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); @@ -126,12 +126,12 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") SECTION("When reference is current layer, only pixels matching the fill color are filled"){ properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test2a-first.png"); fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test2a-second.png"); @@ -143,12 +143,12 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test3a-first.png"); fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test3a-second.png"); From 63b044f0c649e32da3f41baafb595c53f7d92cc5 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 18 Aug 2024 09:58:39 +0200 Subject: [PATCH 4/6] Remove obsolete fill logic As this is now handled by the fill algorithm, no need for additional checks. --- core_lib/src/graphics/bitmap/bitmapimage.cpp | 31 ++------------------ core_lib/src/graphics/bitmap/bitmapimage.h | 3 +- 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index 8ed72bae8..2215ec1f7 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -813,26 +813,10 @@ bool BitmapImage::floodFill(BitmapImage** replaceImage, tolerance = static_cast(qPow(tolerance, 2)); QRect newBounds; - bool shouldFillBorder = false; - bool *filledPixels = floodFillPoints(targetImage, maxBounds, point, tolerance, newBounds, shouldFillBorder); + bool *filledPixels = floodFillPoints(targetImage, maxBounds, point, tolerance, newBounds); QRect translatedSearchBounds = newBounds.translated(-maxBounds.topLeft()); - if (shouldFillBorder) - { - for (int y = 0; y < maxBounds.height(); y++) - { - for (int x = 0; x < maxBounds.width(); x++) - { - if(!translatedSearchBounds.contains(x, y)) - { - filledPixels[y*maxWidth+x] = true; - } - } - } - newBounds = maxBounds; - } - // The scanned bounds should take the expansion into account const QRect& expandRect = newBounds.adjusted(-expandValue, -expandValue, expandValue, expandValue); if (expandValue > 0) { @@ -874,8 +858,7 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, const QRect& searchBounds, QPoint point, const int tolerance, - QRect& newBounds, - bool& fillBorder) + QRect& newBounds) { QRgb oldColor = targetImage->constScanLine(point.x(), point.y()); oldColor = qRgba(qRed(oldColor), qGreen(oldColor), qBlue(oldColor), qAlpha(oldColor)); @@ -901,9 +884,6 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, bool *filledPixels = new bool[searchBounds.height()*searchBounds.width()]{}; - // True if the algorithm has attempted to fill a pixel outside the search bounds - bool checkOutside = false; - BlitRect blitBounds(point); while (!queue.empty()) { @@ -917,11 +897,6 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, int xCoord = xTemp - searchBounds.left(); int yCoord = point.y() - searchBounds.top(); - // In case we fill outside the searchBounds, expand the search area to the max. - if (!searchBounds.contains(point)) { - checkOutside = true; - } - if (filledPixels[yCoord*searchBounds.width()+xCoord]) continue; while (xTemp >= searchBounds.left() && @@ -967,8 +942,6 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, } } - fillBorder = checkOutside && compareColor(qRgba(0,0,0,0), oldColor, tolerance, cache.data()); - newBounds = blitBounds; return filledPixels; diff --git a/core_lib/src/graphics/bitmap/bitmapimage.h b/core_lib/src/graphics/bitmap/bitmapimage.h index db79297d4..f1ac37375 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -83,8 +83,7 @@ class BitmapImage : public KeyFrame const QRect& searchBounds, QPoint point, const int tolerance, - QRect& newBounds, - bool &fillBorder); + QRect& newBounds); static void expandFill(bool* fillPixels, const QRect& searchBounds, const QRect& maxBounds, int expand); void drawLine(QPointF P1, QPointF P2, QPen pen, QPainter::CompositionMode cm, bool antialiasing); From ba65c8abf1d5b96e6bcaa411789abb32f8cddd6c Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 18 Aug 2024 10:00:51 +0200 Subject: [PATCH 5/6] Remove contradictory logic We can't both check whether we're outside and then inside :3 Assuming we should make it seem like the canvas is infinite, it's more appropriate to move the fill point inside the bounds, when we're filling outside the bounds. --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 10 ++++++++-- core_lib/src/graphics/bitmap/bitmapimage.cpp | 12 ------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index 60e506dda..3e062020b 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -113,12 +113,18 @@ bool BitmapBucket::allowContinuousFill(const QPoint& checkPoint, const QRgb& che void BitmapBucket::paint(const QPointF& updatedPoint, std::function state) { - const QPoint& point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y())); const int currentFrameIndex = mEditor->currentFrame(); - BitmapImage* targetImage = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(mEditor->currentFrame(), 0); + BitmapImage* targetImage = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(currentFrameIndex, 0); if (targetImage == nullptr || !targetImage->isLoaded()) { return; } // Can happen if the first frame is deleted while drawing + QPoint point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y())); + if (!mReferenceImage.contains(point)) + { + // If point is outside the our max known fill area, move the fill point anywhere within the bounds + point = mReferenceImage.topLeft(); + } + const QRgb& targetPixelColor = targetImage->constScanLine(point.x(), point.y()); if (!allowFill(point, targetPixelColor)) { diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index 2215ec1f7..6ec5b019f 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -803,12 +803,6 @@ bool BitmapImage::floodFill(BitmapImage** replaceImage, QRect maxBounds = cameraRect.united(fillBounds).adjusted(-expandValue, -expandValue, expandValue, expandValue); const int maxWidth = maxBounds.width(), left = maxBounds.left(), top = maxBounds.top(); - // If the point we are supposed to fill is outside the max bounds, do nothing - if(!maxBounds.contains(point)) - { - return false; - } - // Square tolerance for use with compareColor tolerance = static_cast(qPow(tolerance, 2)); @@ -873,12 +867,6 @@ bool* BitmapImage::floodFillPoints(const BitmapImage* targetImage, bool spanLeft = false; bool spanRight = false; - if (!searchBounds.contains(point)) - { - // If point is outside the search area, move it anywhere in the 1px transparent border - point = searchBounds.topLeft(); - } - queue.append(point); // Preparations END From 1e8d0caa78255acf89afb906e8aff36a01e36d18 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 18 Aug 2024 10:10:36 +0200 Subject: [PATCH 6/6] Adjust tests after modifying where to start fill When the point is outside the fill bounds --- tests/src/test_bitmapbucket.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index 46401c42c..fccc455d6 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -95,7 +95,7 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") SECTION("When reference is current layer, only transparent color is filled") { properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); @@ -108,7 +108,7 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); @@ -126,12 +126,12 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") SECTION("When reference is current layer, only pixels matching the fill color are filled"){ properties.bucketFillReferenceMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test2a-first.png"); fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test2a-second.png"); @@ -143,12 +143,12 @@ TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test3a-first.png"); fillColor = QColor(0,255,0,255); - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 5); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); image->writeFile(resultsPath + "test3a-second.png");