Skip to content

Buckettool fill bugs #1772

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

Merged
MrStevns merged 8 commits into from
Oct 1, 2023
Merged

Buckettool fill bugs #1772

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c80357a
Remove Fill to layer feature
MrStevns Jul 9, 2023
66574a0
Fix floodfill didn't take camera transform into account #1735
MrStevns Jul 9, 2023
64b8c90
UX: No matter what, allow a fill the very first time
MrStevns Jul 10, 2023
7114dfa
Fix case where undo commands would fill up with no visible changes
MrStevns Jul 10, 2023
4078cec
Add explanation
MrStevns Jul 10, 2023
b4f5de3
Remove dead code
J5lx Aug 8, 2023
cc359c1
Fix more cases where continuous fill would flood the undo stack
MrStevns Aug 28, 2023
dc638ed
Fix typos and such
J5lx Aug 30, 2023
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
28 changes: 0 additions & 28 deletions app/src/bucketoptionswidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ BucketOptionsWidget::BucketOptionsWidget(Editor* editor, QWidget* parent) :
ui->colorToleranceSpinbox->setMaximum(MAX_COLOR_TOLERANCE);
ui->strokeThicknessSpinBox->setMinimum(1);

ui->fillToLayerComboBox->addItem(tr("Current layer"), 0);
ui->fillToLayerComboBox->addItem(tr("Layer below"), 1);
ui->fillToLayerComboBox->setToolTip(tr("Fill to the current layer or the layer below"));

ui->referenceLayerComboBox->addItem(tr("Current layer", "Reference Layer Options"), 0);
ui->referenceLayerComboBox->addItem(tr("All layers", "Reference Layer Options"), 1);
ui->referenceLayerComboBox->setToolTip(tr("Refers to the layer that used to flood fill from"));
Expand All @@ -76,15 +72,13 @@ 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<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayerMode);
connect(ui->referenceLayerComboBox, static_cast<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillReferenceMode);
connect(ui->blendModeComboBox, static_cast<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setFillMode);

ui->expandSlider->setValue(settings.value(SETTING_BUCKET_FILL_EXPAND, 2).toInt());
ui->expandSpinBox->setValue(settings.value(SETTING_BUCKET_FILL_EXPAND, 2).toInt());
ui->colorToleranceSlider->setValue(settings.value(SETTING_BUCKET_TOLERANCE, 50).toInt());
ui->colorToleranceSpinbox->setValue(settings.value(SETTING_BUCKET_TOLERANCE, 50).toInt());
ui->fillToLayerComboBox->setCurrentIndex(settings.value(SETTING_BUCKET_FILL_TO_LAYER_MODE, 0).toInt());
ui->referenceLayerComboBox->setCurrentIndex(settings.value(SETTING_BUCKET_FILL_REFERENCE_MODE, 0).toInt());
ui->blendModeComboBox->setCurrentIndex(settings.value(SETTING_FILL_MODE, 0).toInt());

Expand All @@ -110,8 +104,6 @@ void BucketOptionsWidget::updatePropertyVisibility()
ui->strokeThicknessSlider->show();
ui->strokeThicknessSpinBox->show();

ui->fillToLayerComboBox->hide();
ui->fillToDescLabel->hide();
ui->colorToleranceCheckbox->hide();
ui->colorToleranceSlider->hide();
ui->colorToleranceSpinbox->hide();
Expand All @@ -127,8 +119,6 @@ void BucketOptionsWidget::updatePropertyVisibility()
ui->strokeThicknessSlider->hide();
ui->strokeThicknessSpinBox->hide();

ui->fillToLayerComboBox->show();
ui->fillToDescLabel->show();
ui->referenceLayerComboBox->show();
ui->referenceLayerDescLabel->show();
ui->colorToleranceCheckbox->show();
Expand All @@ -137,16 +127,13 @@ void BucketOptionsWidget::updatePropertyVisibility()
ui->expandCheckbox->show();
ui->expandSlider->show();
ui->expandSpinBox->show();
disableFillToLayerComboBox(mEditor->tools()->bucketReferenceModeIsCurrentLayer(ui->referenceLayerComboBox->currentIndex()));
ui->blendModeComboBox->show();
ui->blendModeLabel->show();
break;
}
default:
ui->strokeThicknessSlider->hide();
ui->strokeThicknessSpinBox->hide();
ui->fillToLayerComboBox->hide();
ui->fillToDescLabel->hide();
ui->colorToleranceCheckbox->hide();
ui->colorToleranceSlider->hide();
ui->colorToleranceSpinbox->hide();
Expand Down Expand Up @@ -175,8 +162,6 @@ void BucketOptionsWidget::onPropertyChanged(ToolType, ToolPropertyType propertyT
setFillExpand(static_cast<int>(p.bucketFillExpand)); break;
case ToolPropertyType::USEBUCKETFILLEXPAND:
setFillExpandEnabled(p.bucketFillExpandEnabled); break;
case ToolPropertyType::BUCKETFILLLAYERMODE:
setFillToLayerMode(p.bucketFillToLayerMode); break;
case ToolPropertyType::BUCKETFILLLAYERREFERENCEMODE:
setFillReferenceMode(p.bucketFillReferenceMode); break;
case ToolPropertyType::FILL_MODE:
Expand Down Expand Up @@ -227,23 +212,10 @@ void BucketOptionsWidget::setFillExpand(int value)
ui->expandSpinBox->setValue(value);
}

void BucketOptionsWidget::setFillToLayerMode(int layerMode)
{
QSignalBlocker b(ui->fillToLayerComboBox);
ui->fillToLayerComboBox->setCurrentIndex(layerMode);
}

void BucketOptionsWidget::setFillReferenceMode(int referenceMode)
{
QSignalBlocker b(ui->referenceLayerComboBox);
ui->referenceLayerComboBox->setCurrentIndex(referenceMode);
disableFillToLayerComboBox(mEditor->tools()->bucketReferenceModeIsCurrentLayer(referenceMode));
}

void BucketOptionsWidget::disableFillToLayerComboBox(bool state)
{
ui->fillToLayerComboBox->setDisabled(state);
ui->fillToDescLabel->setDisabled(state);
}

void BucketOptionsWidget::setStrokeWidth(qreal value)
Expand Down
2 changes: 0 additions & 2 deletions app/src/bucketoptionswidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ class BucketOptionsWidget : public QWidget
void setFillExpand(int value);
void setColorTolerance(int tolerance);
void setFillReferenceMode(int referenceMode);
void setFillToLayerMode(int layerMode);
void setFillMode(int mode);

void onPropertyChanged(ToolType, const ToolPropertyType propertyType);
void onLayerChanged(int);

private:
void disableFillToLayerComboBox(bool state);
void updatePropertyVisibility();

Ui::BucketOptionsWidget *ui;
Expand Down
1 change: 0 additions & 1 deletion app/src/tooloptionwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ void ToolOptionWidget::onToolPropertyChanged(ToolType, ToolPropertyType ePropert
case USETOLERANCE: break;
case BUCKETFILLEXPAND: break;
case USEBUCKETFILLEXPAND: break;
case BUCKETFILLLAYERMODE: break;
case BUCKETFILLLAYERREFERENCEMODE: break;
case FILL_MODE: break;
default:
Expand Down
22 changes: 2 additions & 20 deletions app/ui/bucketoptionswidget.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<x>0</x>
<y>0</y>
<width>400</width>
<height>197</height>
<height>221</height>
</rect>
</property>
<property name="windowTitle">
Expand Down Expand Up @@ -35,25 +35,7 @@
<number>6</number>
</property>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_3">
<item>
<widget class="QLabel" name="fillToDescLabel">
<property name="text">
<string>Fill to</string>
</property>
</widget>
</item>
<item>
<widget class="QComboBox" name="fillToLayerComboBox">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
</widget>
</item>
</layout>
<layout class="QHBoxLayout" name="horizontalLayout_3"/>
</item>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_2">
Expand Down
21 changes: 12 additions & 9 deletions core_lib/src/graphics/bitmap/bitmapbucket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ BitmapBucket::BitmapBucket(Editor* editor,

mTolerance = mProperties.toleranceEnabled ? static_cast<int>(mProperties.tolerance) : 0;

if (properties.bucketFillToLayerMode == 1)
{
auto result = findBitmapLayerBelow(initialLayer, initialLayerIndex);
mTargetFillToLayer = result.first;
mTargetFillToLayerIndex = result.second;
}
Q_ASSERT(mTargetFillToLayer);

mReferenceImage = *static_cast<BitmapImage*>(initialLayer->getLastKeyFrameAtPosition(frameIndex));
Expand Down Expand Up @@ -85,15 +79,23 @@ bool BitmapBucket::allowFill(const QPoint& checkPoint) const
QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y());
QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y());

if (targetPixelColor == mBucketColor &&(mProperties.fillMode == 1 || qAlpha(targetPixelColor) == 255))
// A normal click to fill should happen unconditionally, because the alternative is utterly confusing.
if (!mFilledOnce) {
return true;
}

if (targetPixelColor == mBucketColor && (mProperties.fillMode == 1 || qAlpha(targetPixelColor) == 255))
{
// Avoid filling if target pixel color matches fill color
// to avoid creating numerous seemingly useless undo operations
return false;
} else if (QColor::fromRgb(targetPixelColor) == QColor::fromRgb(mBucketColor) && mProperties.fillMode == 0) {
Comment thread
J5lx marked this conversation as resolved.
Outdated
// Avoid filling if the target pixel color matches without the alpha component
// this fixes a case where filling a fully opaque pixel with a less opaque version of itself would cause undo
// fills to happen
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));
}
Expand Down Expand Up @@ -163,6 +165,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function<void(BucketSt
delete replaceImage;

state(BucketState::DidFillTarget, mTargetFillToLayerIndex, currentFrameIndex);
mFilledOnce = true;
}

BitmapImage BitmapBucket::flattenBitmapLayersToImage()
Expand Down
1 change: 1 addition & 0 deletions core_lib/src/graphics/bitmap/bitmapbucket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class BitmapBucket
int mTolerance = 0;

int mTargetFillToLayerIndex = -1;
int mFilledOnce = false;

Properties mProperties;
};
Expand Down
11 changes: 0 additions & 11 deletions core_lib/src/managers/toolmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,19 +251,8 @@ void ToolManager::setBucketFillExpand(int expandValue)
emit toolPropertyChanged(currentTool()->type(), BUCKETFILLEXPAND);
}

void ToolManager::setBucketFillToLayerMode(int layerMode)
{
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);
}
Expand Down
1 change: 0 additions & 1 deletion core_lib/src/managers/toolmanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ public slots:
void setTolerance(int);
void setBucketColorToleranceEnabled(bool enabled);
void setBucketFillExpandEnabled(bool enabled);
void setBucketFillToLayerMode(int layerMode);
void setBucketFillReferenceMode(int referenceMode);
void setBucketFillExpand(int);
void setUseFillContour(bool);
Expand Down
5 changes: 0 additions & 5 deletions core_lib/src/tool/basetool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,6 @@ void BaseTool::setFillExpand(const int fillExpandValue)
properties.bucketFillExpand = fillExpandValue;
}

void BaseTool::setFillToLayerMode(int layerMode)
{
properties.bucketFillToLayerMode = layerMode;
}

void BaseTool::setFillReferenceMode(int referenceMode)
{
properties.bucketFillReferenceMode = referenceMode;
Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/tool/basetool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ class Properties
bool toleranceEnabled = false;
int bucketFillExpand = 0;
bool bucketFillExpandEnabled = 0;
int bucketFillToLayerMode = 0;
int bucketFillReferenceMode = 0;
bool useFillContour = false;
bool showSelectionInfo = true;
Expand Down Expand Up @@ -127,7 +126,6 @@ class BaseTool : public QObject
virtual void setToleranceEnabled(const bool enabled);
virtual void setFillExpand(const int fillExpandValue);
virtual void setFillExpandEnabled(const bool enabled);
virtual void setFillToLayerMode(int layerMode);
virtual void setFillReferenceMode(int referenceMode);
virtual void setUseFillContour(const bool useFillContour);
virtual void setShowSelectionInfo(const bool b);
Expand Down
21 changes: 1 addition & 20 deletions core_lib/src/tool/buckettool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ void BucketTool::loadSettings()

properties.bucketFillExpand = settings.value(SETTING_BUCKET_FILL_EXPAND, 2.0).toInt();
properties.bucketFillExpandEnabled = settings.value(SETTING_BUCKET_FILL_EXPAND_ON, true).toBool();
properties.bucketFillToLayerMode = settings.value(SETTING_BUCKET_FILL_TO_LAYER_MODE, 0).toInt();
properties.bucketFillReferenceMode = settings.value(SETTING_BUCKET_FILL_REFERENCE_MODE, 0).toInt();
properties.fillMode = settings.value(SETTING_FILL_MODE, 0).toInt();
}
Expand All @@ -74,7 +73,6 @@ void BucketTool::resetToDefault()
setFillMode(0);
setFillExpand(2);
setFillExpandEnabled(true);
setFillToLayerMode(0);
setToleranceEnabled(false);
setFillReferenceMode(0);
}
Expand Down Expand Up @@ -163,16 +161,6 @@ void BucketTool::setFillExpand(const int fillExpandValue)
settings.sync();
}

void BucketTool::setFillToLayerMode(int layerMode)
{
properties.bucketFillToLayerMode = layerMode;

// Update settings
QSettings settings(PENCIL2D, PENCIL2D);
settings.setValue(SETTING_BUCKET_FILL_TO_LAYER_MODE, layerMode);
settings.sync();
}

void BucketTool::setFillReferenceMode(int referenceMode)
{
properties.bucketFillReferenceMode = referenceMode;
Expand All @@ -195,7 +183,7 @@ void BucketTool::pointerPressEvent(PointerEvent* event)

mBitmapBucket = BitmapBucket(mEditor,
mEditor->color()->frontColor(),
layerCam ? layerCam->getViewRect() : QRect(),
layerCam ? layerCam->getViewAtFrame(mEditor->currentFrame()).inverted().mapRect(layerCam->getViewRect()) : QRect(),
getCurrentPoint(),
properties);

Expand Down Expand Up @@ -271,13 +259,6 @@ void BucketTool::paintBitmap()
else if (progress == BucketState::DidFillTarget)
{
mEditor->setModified(layerIndex, frameIndex);

// Need to invalidate layer pixmap cache when filling anything else but current layer
// otherwise dragging won't show until release event
if (properties.bucketFillToLayerMode == 1)
{
mScribbleArea->invalidatePainterCaches();
}
}
});
}
Expand Down
1 change: 0 additions & 1 deletion core_lib/src/tool/buckettool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class BucketTool : public StrokeTool
void setWidth(const qreal width) override;
void setFillExpand(const int fillExpandValue) override;
void setFillExpandEnabled(const bool enabled) override;
void setFillToLayerMode(int layerMode) override;
void setFillReferenceMode(int referenceMode) override;
void setFillMode(int mode) override;

Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/util/pencildef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ enum ToolPropertyType
USETOLERANCE,
BUCKETFILLEXPAND,
USEBUCKETFILLEXPAND,
BUCKETFILLLAYERMODE,
BUCKETFILLLAYERREFERENCEMODE,
CAMERAPATH,
};
Expand Down Expand Up @@ -303,7 +302,6 @@ const static float RotationHandleOffset = 50;
#define SETTING_BUCKET_TOLERANCE_ON "BucketToleranceEnabled"
#define SETTING_BUCKET_FILL_EXPAND "BucketFillExpand"
#define SETTING_BUCKET_FILL_EXPAND_ON "BucketFillExpandEnabled"
#define SETTING_BUCKET_FILL_TO_LAYER_MODE "BucketFillToLayerMode"
#define SETTING_BUCKET_FILL_REFERENCE_MODE "BucketFillReferenceMode"

#define SETTING_FILL_MODE "FillMode"
Expand Down
Loading