Skip to content

Commit

Permalink
Fix fill misbehaving when drawing was partly outside border (#1865)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@MrStevns
MrStevns authored Aug 18, 2024
1 parent 5d40d27 commit ac2054e
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 59 deletions.
10 changes: 8 additions & 2 deletions core_lib/src/graphics/bitmap/bitmapbucket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,18 @@ bool BitmapBucket::allowContinuousFill(const QPoint& checkPoint, const QRgb& che

void BitmapBucket::paint(const QPointF& updatedPoint, std::function<void(BucketState, int, int)> state)
{
const QPoint& point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y()));
const int currentFrameIndex = mEditor->currentFrame();

BitmapImage* targetImage = static_cast<LayerBitmap*>(mTargetFillToLayer)->getLastBitmapImageAtFrame(mEditor->currentFrame(), 0);
BitmapImage* targetImage = static_cast<LayerBitmap*>(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)) {
Expand Down
64 changes: 11 additions & 53 deletions core_lib/src/graphics/bitmap/bitmapimage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(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) {
Expand Down Expand Up @@ -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<QPoint> queue; // queue all the pixels of the filled area (as they are found)
Expand All @@ -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())
Expand All @@ -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--;
Expand All @@ -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())) {
Expand All @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions core_lib/src/graphics/bitmap/bitmapimage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/src/test_bitmapbucket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit ac2054e

Please sign in to comment.