diff --git a/app/src/bucketoptionswidget.cpp b/app/src/bucketoptionswidget.cpp index 0ec11d2c13..ecbd16729e 100644 --- a/app/src/bucketoptionswidget.cpp +++ b/app/src/bucketoptionswidget.cpp @@ -76,7 +76,7 @@ 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(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayer); + connect(ui->fillToLayerComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayerMode); connect(ui->referenceLayerComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillReferenceMode); connect(ui->blendModeComboBox, static_cast(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setFillMode); @@ -123,7 +123,7 @@ void BucketOptionsWidget::updatePropertyVisibility() ui->blendModeComboBox->hide(); ui->blendModeLabel->hide(); break; - case Layer::BITMAP: + case Layer::BITMAP: { ui->strokeThicknessSlider->hide(); ui->strokeThicknessSpinBox->hide(); @@ -135,11 +135,11 @@ void BucketOptionsWidget::updatePropertyVisibility() ui->expandCheckbox->show(); ui->expandSlider->show(); ui->expandSpinBox->show(); - ui->referenceLayerComboBox->show(); - ui->referenceLayerDescLabel->show(); + disableFillToLayerComboBox(mEditor->tools()->bucketReferenceModeIsCurrentLayer(ui->referenceLayerComboBox->currentIndex())); ui->blendModeComboBox->show(); ui->blendModeLabel->show(); break; + } default: ui->strokeThicknessSlider->hide(); ui->strokeThicknessSpinBox->hide(); @@ -235,6 +235,13 @@ 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) diff --git a/app/src/bucketoptionswidget.h b/app/src/bucketoptionswidget.h index 34b98c54d3..a53644f33f 100644 --- a/app/src/bucketoptionswidget.h +++ b/app/src/bucketoptionswidget.h @@ -48,6 +48,7 @@ class BucketOptionsWidget : public QWidget void onLayerChanged(int); private: + void disableFillToLayerComboBox(bool state); void updatePropertyVisibility(); Ui::BucketOptionsWidget *ui; diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.cpp b/core_lib/src/graphics/bitmap/bitmapbucket.cpp index dcb16c7f07..88ba46b223 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.cpp +++ b/core_lib/src/graphics/bitmap/bitmapbucket.cpp @@ -34,7 +34,6 @@ BitmapBucket::BitmapBucket(Editor* editor, QPointF fillPoint, Properties properties): mEditor(editor), - mBucketStartPoint(fillPoint), mMaxFillRegion(maxFillRegion), mProperties(properties) @@ -48,6 +47,8 @@ BitmapBucket::BitmapBucket(Editor* editor, mTargetFillToLayer = initialLayer; mTargetFillToLayerIndex = initialLayerIndex; + mTolerance = mProperties.toleranceEnabled ? static_cast(mProperties.tolerance) : 0; + if (properties.bucketFillToLayerMode == 1) { auto result = findBitmapLayerBelow(initialLayer, initialLayerIndex); @@ -61,17 +62,14 @@ BitmapBucket::BitmapBucket(Editor* editor, { mReferenceImage = flattenBitmapLayersToImage(); } - const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y())); + mStartReferenceColor = mReferenceImage.constScanLine(point.x(), point.y()); - BitmapImage* image = static_cast(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0); - mReferenceColor = image->constScanLine(point.x(), point.y()); + mPixelCache = new QHash(); } -bool BitmapBucket::shouldFill(QPointF checkPoint) const +bool BitmapBucket::allowFill(const QPoint& checkPoint) const { - const QPoint point = QPoint(qFloor(checkPoint.x()), qFloor(checkPoint.y())); - if (mProperties.fillMode == 0 && qAlpha(mBucketColor) == 0) { // Filling in overlay mode with a fully transparent color has no @@ -84,60 +82,34 @@ bool BitmapBucket::shouldFill(QPointF checkPoint) const if (!targetImage.isLoaded()) { return false; } - BitmapImage referenceImage = mReferenceImage; + QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y()); + QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y()); - QRgb pixelColor = referenceImage.constScanLine(point.x(), point.y()); - QRgb targetPixelColor = targetImage.constScanLine(point.x(), point.y()); - - if (mProperties.fillMode == 2 && pixelColor != 0) + if (targetPixelColor == mBucketColor &&(mProperties.fillMode == 1 || qAlpha(targetPixelColor) == 255)) { - // don't try to fill because we won't be able to see it anyway... + // Avoid filling if target pixel color matches fill color + // to avoid creating numerous seemingly useless undo operations return false; } - // First paint is allowed with no rules applied - if (mFirstPaint) - { - return true; - } - - // Ensure that when dragging that we're only filling on either transparent or same color - if ((mReferenceColor == targetPixelColor && targetPixelColor == pixelColor) || (pixelColor == 0 && targetPixelColor == 0)) - { - return true; - } - - // When filling with various blending modes we need to verify that the applied color - // doesn't match the target color, otherwise it will fill the same color for no reason. - // We still expect to only fill on either transparent or same color. - if (mAppliedColor != targetPixelColor && pixelColor == 0) - { - return true; - } - - 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)); } void BitmapBucket::paint(const QPointF updatedPoint, std::function state) { - const Layer* targetLayer = mTargetFillToLayer; - int targetLayerIndex = mTargetFillToLayerIndex; - QRgb fillColor = mBucketColor; - const QPoint point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y())); - const QRect cameraRect = mMaxFillRegion; - const int tolerance = mProperties.toleranceEnabled ? static_cast(mProperties.tolerance) : 0; const int currentFrameIndex = mEditor->currentFrame(); - const QRgb origColor = fillColor; - if (!shouldFill(updatedPoint)) { return; } + if (!allowFill(point)) { return; } - BitmapImage* targetImage = static_cast(targetLayer->getLastKeyFrameAtPosition(currentFrameIndex)); + BitmapImage* targetImage = static_cast(mTargetFillToLayer->getLastKeyFrameAtPosition(currentFrameIndex)); if (targetImage == nullptr || !targetImage->isLoaded()) { return; } // Can happen if the first frame is deleted while drawing - BitmapImage referenceImage = mReferenceImage; - + QRgb fillColor = mBucketColor; if (mProperties.fillMode == 1) { // Pass a fully opaque version of the new color to floodFill @@ -153,11 +125,11 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::functionpaste(replaceImage, QPainter::CompositionMode_DestinationOut); // Reduce the opacity of the fill to match the new color - BitmapImage properColor(replaceImage->bounds(), QColor::fromRgba(origColor)); + BitmapImage properColor(replaceImage->bounds(), QColor::fromRgba(mBucketColor)); properColor.paste(replaceImage, QPainter::CompositionMode_DestinationIn); // Write reduced-opacity fill image on top of target image targetImage->paste(&properColor); } - mAppliedColor = targetImage->constScanLine(point.x(), point.y()); - mFirstPaint = false; - targetImage->modification(); delete replaceImage; - state(BucketState::DidFillTarget, targetLayerIndex, currentFrameIndex); + state(BucketState::DidFillTarget, mTargetFillToLayerIndex, currentFrameIndex); } BitmapImage BitmapBucket::flattenBitmapLayersToImage() diff --git a/core_lib/src/graphics/bitmap/bitmapbucket.h b/core_lib/src/graphics/bitmap/bitmapbucket.h index 1f75d698c1..88464994e8 100644 --- a/core_lib/src/graphics/bitmap/bitmapbucket.h +++ b/core_lib/src/graphics/bitmap/bitmapbucket.h @@ -57,7 +57,7 @@ class BitmapBucket * @param checkPoint * @return True if you are allowed to fill, otherwise false */ - bool shouldFill(QPointF checkPoint) const; + bool allowFill(const QPoint& checkPoint) const; std::pair findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const; BitmapImage flattenBitmapLayersToImage(); @@ -65,19 +65,19 @@ class BitmapBucket Editor* mEditor = nullptr; Layer* mTargetFillToLayer = nullptr; + QHash *mPixelCache; + BitmapImage mReferenceImage; QRgb mBucketColor = 0; - QRgb mReferenceColor = 0; - QRgb mAppliedColor = 0; + QRgb mStartReferenceColor = 0; - QPointF mBucketStartPoint; QRect mMaxFillRegion; + int mTolerance = 0; + int mTargetFillToLayerIndex = -1; Properties mProperties; - - bool mFirstPaint = true; }; #endif // BITMAPBUCKET_H diff --git a/core_lib/src/graphics/bitmap/bitmapimage.cpp b/core_lib/src/graphics/bitmap/bitmapimage.cpp index e334e84f92..c4be6a77c2 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.cpp +++ b/core_lib/src/graphics/bitmap/bitmapimage.cpp @@ -18,7 +18,6 @@ GNU General Public License for more details. #include #include -#include #include #include #include @@ -764,48 +763,6 @@ void BitmapImage::clear(QRect rectangle) modification(); } -/** Compare colors for the purposes of flood filling - * - * Calculates the Eulcidian difference of the RGB channels - * of the image and compares it to the tolerance - * - * @param[in] newColor The first color to compare - * @param[in] oldColor The second color to compare - * @param[in] tolerance The threshold limit between a matching and non-matching color - * @param[in,out] cache Contains a mapping of previous results of compareColor with rule that - * cache[someColor] = compareColor(someColor, oldColor, tolerance) - * - * @return Returns true if the colors have a similarity below the tolerance level - * (i.e. if Eulcidian distance squared is <= tolerance) - */ -bool BitmapImage::compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache) -{ - // Handle trivial case - if (newColor == oldColor) return true; - - if(cache && cache->contains(newColor)) return cache->value(newColor); - - // Get Eulcidian distance between colors - // Not an accurate representation of human perception, - // but it's the best any image editing program ever does - int diffRed = static_cast(qPow(qRed(oldColor) - qRed(newColor), 2)); - int diffGreen = static_cast(qPow(qGreen(oldColor) - qGreen(newColor), 2)); - int diffBlue = static_cast(qPow(qBlue(oldColor) - qBlue(newColor), 2)); - // This may not be the best way to handle alpha since the other channels become less relevant as - // the alpha is reduces (ex. QColor(0,0,0,0) is the same as QColor(255,255,255,0)) - int diffAlpha = static_cast(qPow(qAlpha(oldColor) - qAlpha(newColor), 2)); - - bool isSimilar = (diffRed + diffGreen + diffBlue + diffAlpha) <= tolerance; - - if(cache) - { - Q_ASSERT(cache->contains(isSimilar) ? isSimilar == (*cache)[newColor] : true); - (*cache)[newColor] = isSimilar; - } - - return isSimilar; -} - bool BitmapImage::floodFill(BitmapImage** replaceImage, const BitmapImage* targetImage, const QRect& cameraRect, diff --git a/core_lib/src/graphics/bitmap/bitmapimage.h b/core_lib/src/graphics/bitmap/bitmapimage.h index 507efbc9c8..6122b92559 100644 --- a/core_lib/src/graphics/bitmap/bitmapimage.h +++ b/core_lib/src/graphics/bitmap/bitmapimage.h @@ -19,6 +19,8 @@ GNU General Public License for more details. #include #include "keyframe.h" +#include +#include class BitmapImage : public KeyFrame @@ -73,7 +75,6 @@ class BitmapImage : public KeyFrame void clear(QRect rectangle); void clear(QRectF rectangle) { clear(rectangle.toRect()); } - static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache); 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, @@ -120,6 +121,48 @@ class BitmapImage : public KeyFrame Status writeFile(const QString& filename); + /** Compare colors for the purposes of flood filling + * + * Calculates the Eulcidian difference of the RGB channels + * of the image and compares it to the tolerance + * + * @param[in] newColor The first color to compare + * @param[in] oldColor The second color to compare + * @param[in] tolerance The threshold limit between a matching and non-matching color + * @param[in,out] cache Contains a mapping of previous results of compareColor with rule that + * cache[someColor] = compareColor(someColor, oldColor, tolerance) + * + * @return Returns true if the colors have a similarity below the tolerance level + * (i.e. if Eulcidian distance squared is <= tolerance) + */ + static inline bool compareColor(QRgb newColor, QRgb oldColor, int tolerance, QHash *cache) + { + // Handle trivial case + if (newColor == oldColor) return true; + + if(cache && cache->contains(newColor)) return cache->value(newColor); + + // Get Eulcidian distance between colors + // Not an accurate representation of human perception, + // but it's the best any image editing program ever does + int diffRed = static_cast(qPow(qRed(oldColor) - qRed(newColor), 2)); + int diffGreen = static_cast(qPow(qGreen(oldColor) - qGreen(newColor), 2)); + int diffBlue = static_cast(qPow(qBlue(oldColor) - qBlue(newColor), 2)); + // This may not be the best way to handle alpha since the other channels become less relevant as + // the alpha is reduces (ex. QColor(0,0,0,0) is the same as QColor(255,255,255,0)) + int diffAlpha = static_cast(qPow(qAlpha(oldColor) - qAlpha(newColor), 2)); + + bool isSimilar = (diffRed + diffGreen + diffBlue + diffAlpha) <= tolerance; + + if(cache) + { + Q_ASSERT(cache->contains(isSimilar) ? isSimilar == (*cache)[newColor] : true); + (*cache)[newColor] = isSimilar; + } + + return isSimilar; + } + protected: void updateBounds(QRect rectangle); void extend(const QPoint& p); diff --git a/core_lib/src/interface/scribblearea.cpp b/core_lib/src/interface/scribblearea.cpp index 649428c592..c96722cc7f 100644 --- a/core_lib/src/interface/scribblearea.cpp +++ b/core_lib/src/interface/scribblearea.cpp @@ -63,17 +63,20 @@ bool ScribbleArea::init() { mPrefs = mEditor->preference(); mDoubleClickTimer = new QTimer(this); + mMouseFilterTimer = new QTimer(this); connect(mPrefs, &PreferenceManager::optionChanged, this, &ScribbleArea::settingUpdated); connect(mEditor->tools(), &ToolManager::toolPropertyChanged, this, &ScribbleArea::onToolPropertyUpdated); connect(mEditor->tools(), &ToolManager::toolChanged, this, &ScribbleArea::onToolChanged); connect(mDoubleClickTimer, &QTimer::timeout, this, &ScribbleArea::handleDoubleClick); + connect(mMouseFilterTimer, &QTimer::timeout, this, &ScribbleArea::tabletReleaseEventFired); connect(mEditor->select(), &SelectionManager::selectionChanged, this, &ScribbleArea::onSelectionChanged); connect(mEditor->select(), &SelectionManager::needDeleteSelection, this, &ScribbleArea::deleteSelection); mDoubleClickTimer->setInterval(50); + mMouseFilterTimer->setInterval(50); const int curveSmoothingLevel = mPrefs->getInt(SETTING::CURVE_SMOOTHING); mCurveSmoothingLevel = curveSmoothingLevel / 20.0; // default value is 1.0 @@ -91,7 +94,6 @@ bool ScribbleArea::init() updateCanvasCursor(); setMouseTracking(true); // reacts to mouse move events, even if the button is not pressed - #if QT_VERSION >= QT_VERSION_CHECK(5, 9, 0) setTabletTracking(true); // tablet tracking first added in 5.9 #endif @@ -599,6 +601,8 @@ void ScribbleArea::tabletEvent(QTabletEvent *e) } else if (event.eventType() == QTabletEvent::TabletRelease) { + mTabletReleaseMillisAgo = 0; + mMouseFilterTimer->start(); if (mTabletInUse) { mStrokeManager->pointerReleaseEvent(&event); @@ -713,6 +717,22 @@ void ScribbleArea::handleDoubleClick() } } +void ScribbleArea::tabletReleaseEventFired() +{ + // Under certain circumstances a mouse press event will fire after a tablet release event. + // This causes unexpected behaviours for some of the tools, eg. the bucket. + // The problem only seems to occur on windows and only when tapping. + // prior to this fix, the event queue would look like this: + // eg: TabletPress -> TabletRelease -> MousePress + // The following will filter mouse events created after a tablet release event. + mTabletReleaseMillisAgo += 50; + + if (mTabletReleaseMillisAgo >= MOUSE_FILTER_THRESHOLD) { + mTabletReleaseMillisAgo = 0; + mMouseFilterTimer->stop(); + } +} + bool ScribbleArea::isLayerPaintable() const { Layer* layer = mEditor->layers()->currentLayer(); @@ -723,7 +743,7 @@ bool ScribbleArea::isLayerPaintable() const void ScribbleArea::mousePressEvent(QMouseEvent* e) { - if (mTabletInUse) + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; @@ -739,6 +759,7 @@ void ScribbleArea::mousePressEvent(QMouseEvent* e) void ScribbleArea::mouseMoveEvent(QMouseEvent* e) { + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; } PointerEvent event(e); mStrokeManager->pointerMoveEvent(&event); @@ -748,6 +769,7 @@ void ScribbleArea::mouseMoveEvent(QMouseEvent* e) void ScribbleArea::mouseReleaseEvent(QMouseEvent* e) { + if (mTabletInUse || (mMouseFilterTimer->isActive() && mTabletReleaseMillisAgo < MOUSE_FILTER_THRESHOLD)) { e->ignore(); return; } PointerEvent event(e); mStrokeManager->pointerReleaseEvent(&event); diff --git a/core_lib/src/interface/scribblearea.h b/core_lib/src/interface/scribblearea.h index 0110adc7c6..afa3e4e070 100644 --- a/core_lib/src/interface/scribblearea.h +++ b/core_lib/src/interface/scribblearea.h @@ -263,6 +263,15 @@ public slots: QColor mOnionColor; private: + + /* Under certain circumstances a mouse press event will fire after a tablet release event. + This causes unexpected behaviours for some of the tools, eg. the bucket. + The problem only seems to occur on windows and only when tapping. + prior to this fix the event queue would look like this: + eg: TabletPress -> TabletRelease -> MousePress + The following will filter mouse events created after a tablet release event. + */ + void tabletReleaseEventFired(); bool mKeyboardInUse = false; bool mMouseInUse = false; bool mMouseRightButtonInUse = false; @@ -276,6 +285,10 @@ public slots: // Microsoft suggests that a double click action should be no more than 500 ms const int DOUBLE_CLICK_THRESHOLD = 500; QTimer* mDoubleClickTimer = nullptr; + int mTabletReleaseMillisAgo; + const int MOUSE_FILTER_THRESHOLD = 200; + + QTimer* mMouseFilterTimer = nullptr; QPoint mCursorCenterPos; QPointF mTransformedCursorPos; diff --git a/core_lib/src/managers/toolmanager.cpp b/core_lib/src/managers/toolmanager.cpp index 026ee73de8..add88969db 100644 --- a/core_lib/src/managers/toolmanager.cpp +++ b/core_lib/src/managers/toolmanager.cpp @@ -72,7 +72,7 @@ Status ToolManager::save(Object*) return Status::OK; } -BaseTool* ToolManager::currentTool() +BaseTool* ToolManager::currentTool() const { if (mTemporaryTool != nullptr) { @@ -251,14 +251,19 @@ void ToolManager::setBucketFillExpand(int expandValue) emit toolPropertyChanged(currentTool()->type(), BUCKETFILLEXPAND); } -void ToolManager::setBucketFillToLayer(int layerIndex) +void ToolManager::setBucketFillToLayerMode(int layerMode) { - currentTool()->setFillToLayer(layerIndex); + 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); } @@ -301,6 +306,11 @@ void ToolManager::setCameraPathDotColor(int dotColorNum) emit toolPropertyChanged(cameraTool->type(), CAMERAPATH); } +bool ToolManager::bucketReferenceModeIsCurrentLayer(int referenceMode) const +{ + return referenceMode == 0; +} + // Switches on/off two actions // eg. if x = true, then y = false int ToolManager::propertySwitch(bool condition, int tool) diff --git a/core_lib/src/managers/toolmanager.h b/core_lib/src/managers/toolmanager.h index efd3f02242..02373d2d47 100644 --- a/core_lib/src/managers/toolmanager.h +++ b/core_lib/src/managers/toolmanager.h @@ -36,7 +36,7 @@ class ToolManager : public BaseManager Status load(Object*) override; Status save(Object*) override; - BaseTool* currentTool(); + BaseTool* currentTool() const; BaseTool* getTool(ToolType eToolType); void setDefaultTool(); void setCurrentTool(ToolType eToolType); @@ -77,7 +77,7 @@ public slots: void setTolerance(int); void setBucketColorToleranceEnabled(bool enabled); void setBucketFillExpandEnabled(bool enabled); - void setBucketFillToLayer(int layerIndex); + void setBucketFillToLayerMode(int layerMode); void setBucketFillReferenceMode(int referenceMode); void setBucketFillExpand(int); void setUseFillContour(bool); @@ -87,6 +87,10 @@ public slots: void setCameraPathDotColor(int); void resetCameraTransform(CameraFieldOption option); + /// Layer mode will be enforced by the the choice the reference mode selected. + /// @return Returns true if reference mode is ``current layer`, otherwise false. + bool bucketReferenceModeIsCurrentLayer(int referenceMode) const; + private: void setTemporaryTool(ToolType eToolType); diff --git a/core_lib/src/tool/basetool.cpp b/core_lib/src/tool/basetool.cpp index 7671c3469d..56bd359bc7 100644 --- a/core_lib/src/tool/basetool.cpp +++ b/core_lib/src/tool/basetool.cpp @@ -429,7 +429,7 @@ void BaseTool::setFillExpand(const int fillExpandValue) properties.bucketFillExpand = fillExpandValue; } -void BaseTool::setFillToLayer(int layerMode) +void BaseTool::setFillToLayerMode(int layerMode) { properties.bucketFillToLayerMode = layerMode; } diff --git a/core_lib/src/tool/basetool.h b/core_lib/src/tool/basetool.h index bf1405d5ee..9124dcf6d0 100644 --- a/core_lib/src/tool/basetool.h +++ b/core_lib/src/tool/basetool.h @@ -126,7 +126,7 @@ class BaseTool : public QObject virtual void setToleranceEnabled(const bool enabled); virtual void setFillExpand(const int fillExpandValue); virtual void setFillExpandEnabled(const bool enabled); - virtual void setFillToLayer(int layerMode); + virtual void setFillToLayerMode(int layerMode); virtual void setFillReferenceMode(int referenceMode); virtual void setUseFillContour(const bool useFillContour); virtual void setShowSelectionInfo(const bool b); diff --git a/core_lib/src/tool/buckettool.cpp b/core_lib/src/tool/buckettool.cpp index c09902c36c..74d3ed8ea2 100644 --- a/core_lib/src/tool/buckettool.cpp +++ b/core_lib/src/tool/buckettool.cpp @@ -74,7 +74,7 @@ void BucketTool::resetToDefault() setFillMode(0); setFillExpand(2); setFillExpandEnabled(true); - setFillToLayer(0); + setFillToLayerMode(0); setToleranceEnabled(false); setFillReferenceMode(0); } @@ -163,7 +163,7 @@ void BucketTool::setFillExpand(const int fillExpandValue) settings.sync(); } -void BucketTool::setFillToLayer(int layerMode) +void BucketTool::setFillToLayerMode(int layerMode) { properties.bucketFillToLayerMode = layerMode; diff --git a/core_lib/src/tool/buckettool.h b/core_lib/src/tool/buckettool.h index 14ff571b9a..631c19938a 100644 --- a/core_lib/src/tool/buckettool.h +++ b/core_lib/src/tool/buckettool.h @@ -47,7 +47,7 @@ class BucketTool : public StrokeTool void setWidth(const qreal width) override; void setFillExpand(const int fillExpandValue) override; void setFillExpandEnabled(const bool enabled) override; - void setFillToLayer(int layerIndex) override; + void setFillToLayerMode(int layerMode) override; void setFillReferenceMode(int referenceMode) override; void setFillMode(int mode) override; diff --git a/tests/src/test_bitmapbucket.cpp b/tests/src/test_bitmapbucket.cpp index 4b7db42665..3fa0624e30 100644 --- a/tests/src/test_bitmapbucket.cpp +++ b/tests/src/test_bitmapbucket.cpp @@ -15,7 +15,7 @@ #include "basetool.h" -void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, Properties properties) { +void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, Properties properties, int fillCountThreshold) { int moveX = 0; BitmapBucket bucket = BitmapBucket(editor, color, bounds, movePoint, properties); @@ -34,12 +34,10 @@ void dragAndFill(QPointF movePoint, Editor* editor, QColor color, QRect bounds, }); } - // Make sure that only 4 fills has been applied - // otherwise we're spilling onto unwanted pixels - REQUIRE(fillCount == 4); + REQUIRE(fillCount == fillCountThreshold); } -void verifyPixels(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) +void verifyOnlyPixelsInsideSegmentsAreFilled(QPoint referencePoint, const BitmapImage* image, QRgb fillColor) { REQUIRE(image->constScanLine(referencePoint.x(), referencePoint.y()) == fillColor); @@ -49,7 +47,17 @@ void verifyPixels(QPoint referencePoint, const BitmapImage* image, QRgb fillColo REQUIRE(image->constScanLine(referencePoint.x()+6, referencePoint.y()) != fillColor); } -TEST_CASE("BitmapBucket - Fill drag logic") +/** + * Ascii representation of test project + * The "*" represent black strokes. + * The space inbetween represents transparency + * *************** + * * * * * * + * *************** + * + * The test cases are based around filling on the initially transparent area and dragging across the four segments. + */ +TEST_CASE("BitmapBucket - Fill drag behaviour across four segments") { FileManager fm; Object* obj = fm.load(":/fill-drag-test/fill-drag-test.pcl"); @@ -82,57 +90,142 @@ TEST_CASE("BitmapBucket - Fill drag logic") REQUIRE(beforeFill.constScanLine(pressPoint.x(), pressPoint.y()) == 0); - SECTION("FillTo: CurrentLayer - Reference: Current Layer") - { - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + // The dragging logic is based around that we only fill on either transparent or the same color as the fill color. + SECTION("Filling on current layer - layer is not pre filled") { + properties.bucketFillToLayerMode = 0; + Layer* strokeLayer = editor->layers()->currentLayer(); + SECTION("When reference is current layer, only transparent color is filled") + { + properties.bucketFillReferenceMode = 0; + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test1.png"); + image->writeFile(resultsPath + "test1a.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); - } + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } - SECTION("FillTo: Layer below - reference: current layer") - { - properties.bucketFillToLayerMode = 1; + SECTION("When reference is all layers, only transparent color is filled") + { + properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - // Verify that colors are correct on layer below - BitmapImage* image = static_cast(editor->layers()->getLayer(editor->currentLayerIndex()-1))->getLastBitmapImageAtFrame(1); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test2.png"); + image->writeFile(resultsPath + "test1b.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } } + SECTION("Filling on current layer - layer is pre-filled") { + properties.bucketFillToLayerMode = 0; + + // Fill mode is set to `replace` because it makes it easier to compare colors... + properties.fillMode = 1; + Layer* strokeLayer = editor->layers()->currentLayer(); + 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); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test2a-first.png"); + + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + + image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test2a-second.png"); + + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, fillColor.rgba()); + } + + SECTION("When reference is all layers") + { + properties.bucketFillReferenceMode = 1; + + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + BitmapImage* image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test3a-first.png"); + + fillColor = QColor(0,255,0,255); + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + + image = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image->writeFile(resultsPath + "test3a-second.png"); + + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, fillColor.rgba()); + } + } - SECTION("FillTo: Layer below - reference: all layers") - { + // The behaviour changes here because we'll be filling on the layer below, but that layer is blank, + // yet the reference mode tells the flood fill algorithm to fill using the pixel data from the the reference layer. + // In this case it means that all pixels will be filled as we drag across the segments. + SECTION("Filling on layer below - layer is not pre-filled") { properties.bucketFillToLayerMode = 1; - properties.bucketFillReferenceMode = 1; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + Layer* fillLayer = editor->layers()->currentLayer(-1); + SECTION("When reference is current layer, then all pixels not matching the fill color are filled once") + { + properties.bucketFillReferenceMode = 0; + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + + // Verify that colors are correct on layer below + BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); + + image->writeFile(resultsPath + "test4a.png"); + + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } + + + SECTION("When reference is all layers, then all pixels not matching the fill color are filled") + { + properties.bucketFillReferenceMode = 1; + + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - BitmapImage* image = static_cast(editor->layers()->getLayer(editor->currentLayerIndex()-1))->getLastBitmapImageAtFrame(1); + BitmapImage* image = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); - image->writeFile(resultsPath + "test3.png"); + image->writeFile(resultsPath + "test5a.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image, qPremultiply(fillColor.rgba())); + } } - SECTION("FillTo: Current layer - reference: all layers") - { - properties.bucketFillToLayerMode = 0; - properties.bucketFillReferenceMode = 1; + SECTION("Filling on layer below - layer is pre-filled") { + properties.bucketFillToLayerMode = 1; + properties.fillMode = 1; + + SECTION("when reference is all layers, then only pixels matching the fill color are filled") + { + properties.bucketFillReferenceMode = 1; + + Layer* strokeLayer = editor->layers()->currentLayer(); + Layer* fillLayer = editor->layers()->currentLayer(-1); + + // Because the layer is blank, all pixels will be filled once + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); + BitmapImage* image1 = static_cast(strokeLayer)->getLastBitmapImageAtFrame(1); + image1->writeFile(resultsPath + "test6a-first.png"); + + fillColor = QColor(0,255,0,255); + + // Changes fillTo mode to current layer + properties.bucketFillToLayerMode = 0; - dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties); + editor->layers()->setCurrentLayer(strokeLayer); - BitmapImage* image = static_cast(editor->layers()->currentLayer())->getLastBitmapImageAtFrame(1); + // Now the layer has been filled with pixel data, so we'll only fill when the color matches the fill color + dragAndFill(pressPoint, editor, fillColor, beforeFill.bounds(), properties, 4); - image->writeFile(resultsPath + "test4.png"); + BitmapImage* image2 = static_cast(fillLayer)->getLastBitmapImageAtFrame(1); + image1->writeFile(resultsPath + "test6a-second.png"); + image2->writeFile(resultsPath + "test6a-third.png"); - verifyPixels(pressPoint, image, qPremultiply(fillColor.rgba())); + verifyOnlyPixelsInsideSegmentsAreFilled(pressPoint, image1, fillColor.rgba()); + } } }