Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue-1864: Fix fill misbehaving when drawing was partly outside border #1865

Merged
merged 6 commits into from
Aug 18, 2024
Merged

issue-1864: Fix fill misbehaving when drawing was partly outside border #1865

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4c78d3f
issue-1864: Fix fill misbehaving when drawing was partly outside border
MrStevns Jul 23, 2024
2dc933a
issue-1864 - Fix fill not accounting for bounds properly
MrStevns Jul 24, 2024
e21f997
Update tests to account for bounds being expanded by 1 pixel
MrStevns Jul 24, 2024
63b044f
Remove obsolete fill logic
MrStevns Aug 18, 2024
ba65c8a
Remove contradictory logic
MrStevns Aug 18, 2024
1e8d0ca
Adjust tests after modifying where to start fill
MrStevns Aug 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions core_lib/src/graphics/bitmap/bitmapimage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
J5lx marked this conversation as resolved.
Show resolved Hide resolved

QRect translatedSearchBounds = newBounds.translated(-maxBounds.topLeft());

Expand Down Expand Up @@ -871,17 +871,14 @@ 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)
{
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 @@ -902,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;
Expand All @@ -917,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 (!borderBounds.contains(point)) {
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--;
Expand All @@ -941,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())) {
Expand All @@ -965,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++;
}
}
Expand Down
2 changes: 1 addition & 1 deletion core_lib/src/graphics/bitmap/bitmapimage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions tests/src/test_bitmapbucket.cpp
View file
Open in desktop
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 Expand Up @@ -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<LayerBitmap*>(strokeLayer)->getLastBitmapImageAtFrame(1);

Expand All @@ -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<LayerBitmap*>(strokeLayer)->getLastBitmapImageAtFrame(1);

Expand All @@ -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<LayerBitmap*>(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<LayerBitmap*>(strokeLayer)->getLastBitmapImageAtFrame(1);
image->writeFile(resultsPath + "test2a-second.png");
Expand All @@ -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<LayerBitmap*>(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<LayerBitmap*>(strokeLayer)->getLastBitmapImageAtFrame(1);
image->writeFile(resultsPath + "test3a-second.png");
Expand Down
Loading