这是indexloc提供的服务，不要输入任何密码
Skip to content

Add fill tool transparency mode switch #1590

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
merged 4 commits into from
Apr 22, 2021
Merged

Add fill tool transparency mode switch #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9f32a2
Add fill tool transparency mode switch
scribblemaniac Mar 28, 2021
eadd8ae
Move some of flood fill algorithm to bucket tool
scribblemaniac Apr 22, 2021
77e1daf
Skip overlay filling when fully transparent
scribblemaniac Apr 22, 2021
6d25959
Document fill mode code better
scribblemaniac Apr 22, 2021
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
12 changes: 12 additions & 0 deletions app/src/tooloptionwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ void ToolOptionWidget::updateUI()
setPreserveAlpha(p.preserveAlpha);
setVectorMergeEnabled(p.vectorMergeEnabled);
setAA(p.useAA);
setFillMode(p.fillMode);
setStabilizerLevel(p.stabilizerLevel);
setTolerance(static_cast<int>(p.tolerance));
setFillContour(p.useFillContour);
Expand Down Expand Up @@ -109,6 +110,8 @@ void ToolOptionWidget::makeConnectionToEditor(Editor* editor)
connect(ui->vectorMergeBox, &QCheckBox::clicked, toolManager, &ToolManager::setVectorMergeEnabled);
connect(ui->useAABox, &QCheckBox::clicked, toolManager, &ToolManager::setAA);

connect(ui->fillMode, static_cast<void (QComboBox::*)(int)>(&QComboBox::activated), toolManager, &ToolManager::setFillMode);

connect(ui->inpolLevelsCombo, static_cast<void (QComboBox::*)(int)>(&QComboBox::activated), toolManager, &ToolManager::setStabilizerLevel);

connect(ui->toleranceSlider, &SpinSlider::valueChanged, toolManager, &ToolManager::setTolerance);
Expand All @@ -135,6 +138,7 @@ void ToolOptionWidget::onToolPropertyChanged(ToolType, ToolPropertyType ePropert
case PRESERVEALPHA: setPreserveAlpha(p.preserveAlpha); break;
case VECTORMERGE: setVectorMergeEnabled(p.vectorMergeEnabled); break;
case ANTI_ALIASING: setAA(p.useAA); break;
case FILL_MODE: setFillMode(p.fillMode); break;
case STABILIZATION: setStabilizerLevel(p.stabilizerLevel); break;
case TOLERANCE: setTolerance(static_cast<int>(p.tolerance)); break;
case FILLCONTOUR: setFillContour(p.useFillContour); break;
Expand All @@ -157,6 +161,7 @@ void ToolOptionWidget::setVisibility(BaseTool* tool)
ui->makeInvisibleBox->setVisible(tool->isPropertyEnabled(INVISIBILITY));
ui->preserveAlphaBox->setVisible(tool->isPropertyEnabled(PRESERVEALPHA));
ui->useAABox->setVisible(tool->isPropertyEnabled(ANTI_ALIASING));
ui->fillModeGroup->setVisible(tool->isPropertyEnabled(FILL_MODE));
ui->stabilizerLabel->setVisible(tool->isPropertyEnabled(STABILIZATION));
ui->inpolLevelsCombo->setVisible(tool->isPropertyEnabled(STABILIZATION));
ui->toleranceSlider->setVisible(tool->isPropertyEnabled(TOLERANCE));
Expand Down Expand Up @@ -187,6 +192,7 @@ void ToolOptionWidget::setVisibility(BaseTool* tool)
ui->sizeSlider->setLabel(tr("Stroke Thickness"));
ui->toleranceSlider->setVisible(false);
ui->toleranceSpinBox->setVisible(false);
ui->fillModeGroup->setVisible(false);
break;
default:
ui->sizeSlider->setLabel(tr("Width"));
Expand Down Expand Up @@ -299,6 +305,11 @@ void ToolOptionWidget::setAA(int x)
}
}

void ToolOptionWidget::setFillMode(int x)
{
ui->fillMode->setCurrentIndex(qBound(0, x, ui->fillMode->count() - 1));
}

void ToolOptionWidget::setStabilizerLevel(int x)
{
ui->inpolLevelsCombo->setCurrentIndex(qBound(0, x, ui->inpolLevelsCombo->count() - 1));
Expand Down Expand Up @@ -341,6 +352,7 @@ void ToolOptionWidget::disableAllOptions()
ui->preserveAlphaBox->hide();
ui->vectorMergeBox->hide();
ui->useAABox->hide();
ui->fillModeGroup->hide();
ui->inpolLevelsCombo->hide();
ui->toleranceSlider->hide();
ui->toleranceSpinBox->hide();
Expand Down
1 change: 1 addition & 0 deletions app/src/tooloptionwidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public slots:
void setPreserveAlpha(int);
void setVectorMergeEnabled(int);
void setAA(int);
void setFillMode(int);
void setStabilizerLevel(int);
void setTolerance(int);
void setFillContour(int);
Expand Down
63 changes: 48 additions & 15 deletions app/ui/tooloptions.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,6 @@
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout">
<property name="spacing">
<number>6</number>
</property>
<property name="leftMargin">
<number>0</number>
</property>
<property name="topMargin">
<number>0</number>
</property>
<property name="rightMargin">
<number>0</number>
</property>
<property name="bottomMargin">
<number>2</number>
</property>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_2">
<item>
Expand Down Expand Up @@ -241,6 +226,54 @@
</property>
</widget>
</item>
<item>
<widget class="QWidget" name="fillModeGroup" native="true">
<layout class="QHBoxLayout" name="horizontalLayout_5">
<property name="leftMargin">
<number>0</number>
</property>
<property name="topMargin">
<number>0</number>
</property>
<property name="rightMargin">
<number>0</number>
</property>
<property name="bottomMargin">
<number>0</number>
</property>
<item>
<widget class="QLabel" name="fillModeLabel">
<property name="text">
<string>Transparency</string>
</property>
</widget>
</item>
<item>
<widget class="QComboBox" name="fillMode">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="toolTip">
<string>Defines how the fill will behave when the new color is not opaque</string>
</property>
<item>
<property name="text">
<string>Overlay</string>
</property>
</item>
<item>
<property name="text">
<string>Replace</string>
</property>
</item>
</widget>
</item>
</layout>
</widget>
</item>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_4" stretch="0,1">
<item>
Expand Down
14 changes: 6 additions & 8 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 @@ -790,16 +790,16 @@ bool BitmapImage::compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHas

// Flood fill
// ----- http://lodev.org/cgtutor/floodfill.html
void BitmapImage::floodFill(BitmapImage* targetImage,
BitmapImage* BitmapImage::floodFill(BitmapImage* targetImage,
QRect cameraRect,
QPoint point,
QRgb newColor,
QRgb fillColor,
int tolerance)
{
// If the point we are supposed to fill is outside the image and camera bounds, do nothing
if(!cameraRect.united(targetImage->bounds()).contains(point))
{
return;
return nullptr;
}

// Square tolerance for use with compareColor
Expand Down Expand Up @@ -844,11 +844,11 @@ void BitmapImage::floodFill(BitmapImage* targetImage,
spanLeft = spanRight = false;
while (xTemp <= targetImage->mBounds.right() &&
compareColor(targetImage->constScanLine(xTemp, point.y()), oldColor, tolerance, cache.data()) &&
newPlacedColor != newColor)
newPlacedColor != fillColor)
{

// Set pixel color
replaceImage->scanLine(xTemp, point.y(), newColor);
replaceImage->scanLine(xTemp, point.y(), fillColor);

if (!spanLeft && (point.y() > targetImage->mBounds.top()) &&
compareColor(targetImage->constScanLine(xTemp, point.y() - 1), oldColor, tolerance, cache.data())) {
Expand Down Expand Up @@ -876,7 +876,5 @@ void BitmapImage::floodFill(BitmapImage* targetImage,
}
}

targetImage->paste(replaceImage);
targetImage->modification();
delete replaceImage;
return replaceImage;
}
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 @@ -75,7 +75,7 @@ class BitmapImage : public KeyFrame
void clear(QRectF rectangle) { clear(rectangle.toRect()); }

static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash<QRgb, bool> *cache);
static void floodFill(BitmapImage* targetImage, QRect cameraRect, QPoint point, QRgb newColor, int tolerance);
static BitmapImage* floodFill(BitmapImage* targetImage, QRect cameraRect, QPoint point, QRgb newColor, int tolerance);

void drawLine(QPointF P1, QPointF P2, QPen pen, QPainter::CompositionMode cm, bool antialiasing);
void drawRect(QRectF rectangle, QPen pen, QBrush brush, QPainter::CompositionMode cm, bool antialiasing);
Expand Down
6 changes: 6 additions & 0 deletions core_lib/src/managers/toolmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ void ToolManager::setAA(int usingAA)
emit toolPropertyChanged(currentTool()->type(), ANTI_ALIASING);
}

void ToolManager::setFillMode(int mode)
{
currentTool()->setFillMode(mode);
emit toolPropertyChanged(currentTool()->type(), FILL_MODE);
}

void ToolManager::setStabilizerLevel(int level)
{
currentTool()->setStabilizerLevel(level);
Expand Down
1 change: 1 addition & 0 deletions core_lib/src/managers/toolmanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public slots:
void setBezier(bool);
void setPressure(bool);
void setAA(int);
void setFillMode(int);
void setStabilizerLevel(int);
void setTolerance(int);
void setUseFillContour(bool);
Expand Down
6 changes: 6 additions & 0 deletions core_lib/src/tool/basetool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ BaseTool::BaseTool(QObject* parent) : QObject(parent)
mPropertyEnabled.insert(PRESERVEALPHA, false);
mPropertyEnabled.insert(BEZIER, false);
mPropertyEnabled.insert(ANTI_ALIASING, false);
mPropertyEnabled.insert(FILL_MODE, false);
mPropertyEnabled.insert(STABILIZATION, false);
}

Expand Down Expand Up @@ -443,6 +444,11 @@ void BaseTool::setAA(const int useAA)
properties.useAA = useAA;
}

void BaseTool::setFillMode(const int mode)
{
properties.fillMode = mode;
}

void BaseTool::setStabilizerLevel(const int level)
{
properties.stabilizerLevel = level;
Expand Down
2 changes: 2 additions & 0 deletions core_lib/src/tool/basetool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Properties
bool bezier_state = false;
bool useFeather = true;
int useAA = 0;
int fillMode = 0;
int stabilizerLevel = 0;
qreal tolerance = 0;
bool useFillContour = false;
Expand Down Expand Up @@ -113,6 +114,7 @@ class BaseTool : public QObject
virtual void setPreserveAlpha(const bool preserveAlpha);
virtual void setVectorMergeEnabled(const bool vectorMergeEnabled);
virtual void setAA(const int useAA);
virtual void setFillMode(const int mode);
virtual void setStabilizerLevel(const int level);
virtual void setTolerance(const int tolerance);
virtual void setUseFillContour(const bool useFillContour);
Expand Down
88 changes: 80 additions & 8 deletions core_lib/src/tool/buckettool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ GNU General Public License for more details.

#include <QPixmap>
#include <QPainter>
#include <QPointer>
#include <QtMath>
#include <QSettings>
#include "pointerevent.h"
Expand Down Expand Up @@ -47,20 +48,23 @@ void BucketTool::loadSettings()
{
mPropertyEnabled[TOLERANCE] = true;
mPropertyEnabled[WIDTH] = true;
mPropertyEnabled[FILL_MODE] = true;

QSettings settings(PENCIL2D, PENCIL2D);

properties.width = settings.value("fillThickness", 4.0).toDouble();
properties.feather = 10;
properties.stabilizerLevel = StabilizationLevel::NONE;
properties.useAA = DISABLED;
properties.fillMode = settings.value("fillMode", 0).toInt();
properties.tolerance = settings.value("tolerance", 32.0).toDouble();
}

void BucketTool::resetToDefault()
{
setWidth(4.0);
setTolerance(32.0);
setFillMode(0);
}

QCursor BucketTool::cursor()
Expand All @@ -79,6 +83,17 @@ QCursor BucketTool::cursor()
}
}

void BucketTool::setTolerance(const int tolerance)
{
// Set current property
properties.tolerance = tolerance;

// Update settings
QSettings settings(PENCIL2D, PENCIL2D);
settings.setValue("tolerance", tolerance);
settings.sync();
}

/**
* @brief BrushTool::setWidth
* @param width
Expand All @@ -95,14 +110,14 @@ void BucketTool::setWidth(const qreal width)
settings.sync();
}

void BucketTool::setTolerance(const int tolerance)
void BucketTool::setFillMode(int mode)
{
// Set current property
properties.tolerance = tolerance;
properties.fillMode = mode;

// Update settings
QSettings settings(PENCIL2D, PENCIL2D);
settings.setValue("tolerance", tolerance);
settings.setValue("fillMode", mode);
settings.sync();
}

Expand Down Expand Up @@ -169,11 +184,68 @@ void BucketTool::paintBitmap(Layer* layer)

QPoint point = QPoint(qFloor(getLastPoint().x()), qFloor(getLastPoint().y()));
QRect cameraRect = mScribbleArea->getCameraRect().toRect();
BitmapImage::floodFill(targetImage,
cameraRect,
point,
qPremultiply(mEditor->color()->frontColor().rgba()),
properties.tolerance);

QRgb fillColor = qPremultiply(mEditor->color()->frontColor().rgba());
QRgb origColor = fillColor;
if (properties.fillMode == 0)
{
if (qAlpha(fillColor) == 0)
{
// Filling in overlay mode with a fully transparent color has no
// effect, so we can skip it in this case
return;
}
}
Comment on lines +190 to +198
Copy link
Member

@MrStevns MrStevns Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor thing but this could be simplified to:

Suggested change
if (properties.fillMode == 0)
{
if (qAlpha(fillColor) == 0)
{
// Filling in overlay mode with a fully transparent color has no
// effect, so we can skip it in this case
return;
}
}
if (properties.fillMode == 0 && qAlpha(fillColor) == 0)
{
// Filling in overlay mode with a fully transparent color has no
// effect, so we can skip it in this case
return;
}

It is however not a must have change though, I will merge the PR.

Copy link
Member Author

@scribblemaniac scribblemaniac Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentional so the else if check won't be run if fillMode == 0 and qAlpha(fillColor) != 0. A premature and useless optimization? Yes absolutely, but it's not really any worse readability-wise so 🤷

Copy link
Member

@MrStevns MrStevns Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that nested branches for the sake of premature optimization is unnecessary complexity and does decrease readability, eg. I find it easier to read one branch with logic operators (to a certain extend of course) than to consider a nested branch logic but I guess that's highly subjective. It only goes through the branch once, there is no useful optimization to win here, since you're simply checking if the color is alpha based, which is extremely fast.

You're right with only one branch it's not an issue but it is the beginning of unnecessary complexity by branching.

Copy link
Member Author

@scribblemaniac scribblemaniac Apr 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also feel like this might be changed to a switch statement at a later time if we get more modes (such as that erase mode), in which case a nested if statement would make it a bit more natural to notice and make that change. I'm not really defending this choice, it's neither here nor there for me. If you want to change it go ahead.

else if (properties.fillMode == 1)
{
// Pass a fully opaque version of the new color to floodFill
// This is required so we can fully mask out the existing data before
// writing the new color.
QColor tempColor;
tempColor.setRgba(fillColor);
tempColor.setAlphaF(1);
fillColor = tempColor.rgba();
}

std::unique_ptr<BitmapImage> fillImage(
BitmapImage::floodFill(targetImage,
cameraRect,
point,
fillColor,
properties.tolerance));

if (fillImage == nullptr)
{
// Nothing was filled for whatever reason
return;
}

switch(properties.fillMode)
{
default:
case 0: // Overlay mode
// Write fill image on top of target image
targetImage->paste(fillImage.get());
break;
case 1: // Replace mode
if (qAlpha(origColor) == 0xFF)
{
// When the new color is fully opaque, replace mode
// behaves exactly like overlay mode, and origColor == fillColor
targetImage->paste(fillImage.get());
}
else
{
// Clearly all pixels in the to-be-filled region from the target image
targetImage->paste(fillImage.get(), QPainter::CompositionMode_DestinationOut);
// Reduce the opacity of the fill to match the new color
BitmapImage properColor(targetImage->bounds(), QColor::fromRgba(origColor));
properColor.paste(fillImage.get(), QPainter::CompositionMode_DestinationIn);
// Write reduced-opacity fill image on top of target image
targetImage->paste(&properColor);
}
break;
}

mScribbleArea->setModified(layerNumber, mEditor->currentFrame());
}
Expand Down
Loading