From ac2054ec3b4aa357fb6350bd525e6f3653c7c8d5 Mon Sep 17 00:00:00 2001 From: Oliver Stevns <1045397+MrStevns@users.noreply.github.com> Date: Sun, 18 Aug 2024 16:41:09 +0200 Subject: [PATCH] Fix fill misbehaving when drawing was partly outside border (#1865) * issue-1864: Fix fill misbehaving when drawing was partly outside border * issue-1864 - Fix fill not accounting for bounds properly * Update tests to account for bounds being expanded by 1 pixel * Remove obsolete fill logic As this is now handled by the fill algorithm, no need for additional checks. * 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. * Adjust tests after modifying where to start fill When the point is outside the fill bounds --- core_lib/src/graphics/bitmap/bitmapbucket.cpp | 10 ++- core_lib/src/graphics/bitmap/bitmapimage.cpp | 64 ++++--------------- core_lib/src/graphics/bitmap/bitmapimage.h | 5 +- tests/src/test_bitmapbucket.cpp | 2 +- 4 files changed, 22 insertions(+), 59 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 e2be3641e..6ec5b019f 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -799,40 +799,18 @@ 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(); - // 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)); QRect newBounds; - bool shouldFillBorder = false; - bool *filledPixels = floodFillPoints(targetImage, fillBounds, 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) { @@ -871,17 +849,13 @@ 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, - bool& fillBorder) + QRect& newBounds) { 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); // Preparations QList queue; // queue all the pixels of the filled area (as they are found) @@ -893,19 +867,10 @@ 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 - bool *filledPixels = new bool[maxBounds.height()*maxBounds.width()]{}; - - // True if the algorithm has attempted to fill a pixel outside the search bounds - bool checkOutside = false; + bool *filledPixels = new bool[searchBounds.height()*searchBounds.width()]{}; BlitRect blitBounds(point); while (!queue.empty()) @@ -917,15 +882,10 @@ 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 (!borderBounds.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--; @@ -941,9 +901,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())) { @@ -965,13 +925,11 @@ 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++; } } - 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 ead411bfb..f1ac37375 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -80,11 +80,10 @@ 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, - 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); diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index cf27e52c1..fccc455d6 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);