pencil2d · J5lx · Jan 8, 2023 · Sep 30, 2021 · Oct 5, 2021 · Oct 16, 2021
diff --git 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<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged), mEditor->tools(), &ToolManager::setBucketFillToLayer);
+    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);
 
@@ -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
@@ -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
@@ -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<int>(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<LayerBitmap*>(mTargetFillToLayer)->getLastBitmapImageAtFrame(frameIndex, 0);
-    mReferenceColor = image->constScanLine(point.x(), point.y());
+    mPixelCache = new QHash<QRgb, bool>();
 }
 
-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<void(BucketState, int, int)> 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<int>(mProperties.tolerance) : 0;
     const int currentFrameIndex = mEditor->currentFrame();
-    const QRgb origColor = fillColor;
 
-    if (!shouldFill(updatedPoint)) { return; }
+    if (!allowFill(point)) { return; }
 
-    BitmapImage* targetImage = static_cast<BitmapImage*>(targetLayer->getLastKeyFrameAtPosition(currentFrameIndex));
+    BitmapImage* targetImage = static_cast<BitmapImage*>(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::function<void(BucketSt
 
     int expandValue = mProperties.bucketFillExpandEnabled ? mProperties.bucketFillExpand : 0;
     bool didFloodFill = BitmapImage::floodFill(&replaceImage,
-                           &referenceImage,
-                           cameraRect,
+                           &mReferenceImage,
+                           mMaxFillRegion,
                            point,
                            fillColor,
-                           tolerance,
+                           mTolerance,
                            expandValue);
 
     if (!didFloodFill) {
@@ -166,7 +138,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function<void(BucketSt
     }
     Q_ASSERT(replaceImage != nullptr);
 
-    state(BucketState::WillFillTarget, targetLayerIndex, currentFrameIndex);
+    state(BucketState::WillFillTarget, mTargetFillToLayerIndex, currentFrameIndex);
 
     if (mProperties.fillMode == 0)
     {
@@ -181,19 +153,16 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function<void(BucketSt
         // fill mode replace
         targetImage->paste(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
@@ -57,27 +57,27 @@ 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<Layer*, int> findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const;
     BitmapImage flattenBitmapLayersToImage();
 
     Editor* mEditor = nullptr;
     Layer* mTargetFillToLayer = nullptr;
 
+    QHash<QRgb, bool> *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
@@ -18,7 +18,6 @@ GNU General Public License for more details.
 
 #include <cmath>
 #include <QDebug>
-#include <QtMath>
 #include <QFile>
 #include <QFileInfo>
 #include <QPainterPath>
@@ -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<QRgb, bool> *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<int>(qPow(qRed(oldColor) - qRed(newColor), 2));
-    int diffGreen = static_cast<int>(qPow(qGreen(oldColor) - qGreen(newColor), 2));
-    int diffBlue = static_cast<int>(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<int>(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
@@ -19,6 +19,8 @@ GNU General Public License for more details.
 
 #include <QPainter>
 #include "keyframe.h"
+#include <QtMath>
+#include <QHash>
 
 
 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<QRgb, bool> *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<QRgb, bool> *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<int>(qPow(qRed(oldColor) - qRed(newColor), 2));
+        int diffGreen = static_cast<int>(qPow(qGreen(oldColor) - qGreen(newColor), 2));
+        int diffBlue = static_cast<int>(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<int>(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);