Buckettool fill bugs

app/src/bucketoptionswidget.cpp

@@ -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"));
@@ -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());
 
@@ -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();
@@ -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();
@@ -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();
@@ -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:
@@ -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)

app/src/bucketoptionswidget.h

@@ -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;

app/src/tooloptionwidget.cpp

@@ -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:

app/ui/bucketoptionswidget.ui

@@ -7,7 +7,7 @@
     <x>0</x>
     <y>0</y>
     <width>400</width>
-    <height>197</height>
+    <height>221</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -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">

core_lib/src/graphics/bitmap/bitmapbucket.cpp

@@ -48,66 +48,82 @@ BitmapBucket::BitmapBucket(Editor* editor,
     mTargetFillToLayerIndex = initialLayerIndex;
 
     mTolerance = mProperties.toleranceEnabled ? static_cast<int>(mProperties.tolerance) : 0;
+    const QPoint& point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y()));
 
-    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));
+    BitmapImage singleLayerImage = *static_cast<BitmapImage*>(initialLayer->getLastKeyFrameAtPosition(frameIndex));
     if (properties.bucketFillReferenceMode == 1) // All layers
     {
         mReferenceImage = flattenBitmapLayersToImage();
+    } else {
+        mReferenceImage = singleLayerImage;
     }
-    const QPoint point = QPoint(qFloor(fillPoint.x()), qFloor(fillPoint.y()));
     mStartReferenceColor = mReferenceImage.constScanLine(point.x(), point.y());
+    mUseDragToFill = canUseDragToFill(point, color, singleLayerImage);
 
     mPixelCache = new QHash<QRgb, bool>();
 }
 
-bool BitmapBucket::allowFill(const QPoint& checkPoint) const
+bool BitmapBucket::canUseDragToFill(const QPoint& fillPoint, const QColor& bucketColor, const BitmapImage& referenceImage)
 {
-    if (mProperties.fillMode == 0 && qAlpha(mBucketColor) == 0)
-    {
-        // Filling in overlay mode with a fully transparent color has no
-        // effect, so we can skip it in this case
+    QRgb pressReferenceColorSingleLayer = referenceImage.constScanLine(fillPoint.x(), fillPoint.y());
+    QRgb startRef = qUnpremultiply(pressReferenceColorSingleLayer);
+
+    if (mProperties.fillMode == 0 && ((QColor(qRed(startRef), qGreen(startRef), qBlue(startRef)) == bucketColor.rgb() && qAlpha(startRef) == 255) || bucketColor.alpha() == 0)) {
+        // In overlay mode: When the reference pixel matches the bucket color and the reference is fully opaque
+        // Otherwise when the bucket alpha is zero.
+        return false;
+    } else if (mProperties.fillMode == 2 && qAlpha(startRef) == 255) {
+        // In behind mode: When the reference pixel is already fully opaque, the output will be invisible.
         return false;
     }
-    Q_ASSERT(mTargetFillToLayer);
 
-    BitmapImage targetImage = *static_cast<LayerBitmap*>(mTargetFillToLayer)->getLastBitmapImageAtFrame(mEditor->currentFrame(), 0);
+    return true;
+}
 
-    if (!targetImage.isLoaded()) { return false; }
+bool BitmapBucket::allowFill(const QPoint& checkPoint, const QRgb& checkColor) const
+{
+    // A normal click to fill should happen unconditionally, because the alternative is utterly confusing.
+    if (!mFilledOnce) {
+        return true;
+    }
+
+    return allowContinuousFill(checkPoint, checkColor);
+}
+
+bool BitmapBucket::allowContinuousFill(const QPoint& checkPoint, const QRgb& checkColor) const
+{
+    if (!mUseDragToFill) {
+        return false;
+    }
 
-    QRgb colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y());
-    QRgb targetPixelColor = targetImage.constScanLine(checkPoint.x(), checkPoint.y());
+    const QRgb& colorOfReferenceImage = mReferenceImage.constScanLine(checkPoint.x(), checkPoint.y());
 
-    if (targetPixelColor == mBucketColor &&(mProperties.fillMode == 1 || qAlpha(targetPixelColor) == 255))
+    if (checkColor == mBucketColor && (mProperties.fillMode == 1 || qAlpha(checkColor) == 255))
     {
         // Avoid filling if target pixel color matches fill color
         // to avoid creating numerous seemingly useless undo operations
         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));
+           (checkColor == 0 || BitmapImage::compareColor(checkColor, mStartReferenceColor, mTolerance, mPixelCache));
 }
 
-void BitmapBucket::paint(const QPointF updatedPoint, std::function<void(BucketState, int, int)> state)
+void BitmapBucket::paint(const QPointF& updatedPoint, std::function<void(BucketState, int, int)> state)
 {
-    const QPoint point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y()));
+    const QPoint& point = QPoint(qFloor(updatedPoint.x()), qFloor(updatedPoint.y()));
     const int currentFrameIndex = mEditor->currentFrame();
 
-    if (!allowFill(point)) { return; }
+    BitmapImage* targetImage = static_cast<LayerBitmap*>(mTargetFillToLayer)->getLastBitmapImageAtFrame(mEditor->currentFrame(), 0);
+    if (targetImage == nullptr || !targetImage->isLoaded()) { return; } // Can happen if the first frame is deleted while drawing
 
-    BitmapImage* targetImage = static_cast<BitmapImage*>(mTargetFillToLayer->getLastKeyFrameAtPosition(currentFrameIndex));
+    const QRgb& targetPixelColor = targetImage->constScanLine(point.x(), point.y());
 
-    if (targetImage == nullptr || !targetImage->isLoaded()) { return; } // Can happen if the first frame is deleted while drawing
+    if (!allowFill(point, targetPixelColor)) {
+        return;
+    }
 
     QRgb fillColor = mBucketColor;
     if (mProperties.fillMode == 1)
@@ -163,6 +179,7 @@ void BitmapBucket::paint(const QPointF updatedPoint, std::function<void(BucketSt
     delete replaceImage;
 
     state(BucketState::DidFillTarget, mTargetFillToLayerIndex, currentFrameIndex);
+    mFilledOnce = true;
 }
 
 BitmapImage BitmapBucket::flattenBitmapLayersToImage()
@@ -184,29 +201,3 @@ BitmapImage BitmapBucket::flattenBitmapLayersToImage()
     }
     return flattenImage;
 }
-
-std::pair<Layer*, int> BitmapBucket::findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const
-{
-    bool foundLayerBelow = false;
-    int layerBelowIndex = layerIndex;
-    for (int i = layerIndex - 1; i >= 0; i--)
-    {
-        Layer* searchlayer = mEditor->layers()->getLayer(i);
-        Q_ASSERT(searchlayer);
-
-        if (searchlayer->type() == Layer::BITMAP && searchlayer->visible())
-        {
-            targetLayer = searchlayer;
-            foundLayerBelow = true;
-            layerBelowIndex = i;
-            break;
-        }
-    }
-
-    if (foundLayerBelow && !targetLayer->keyExists(mEditor->currentFrame()))
-    {
-        targetLayer->addNewKeyFrameAt(mEditor->currentFrame());
-        emit mEditor->updateTimeLine();
-    }
-    return std::make_pair(targetLayer, layerBelowIndex);
-}

core_lib/src/graphics/bitmap/bitmapbucket.h

@@ -43,7 +43,7 @@ class BitmapBucket
      * @param progress - a function that returns the progress of the paint operation,
      * the layer and frame that was affected at the given point.
      */
-    void paint(const QPointF updatedPoint, std::function<void(BucketState, int, int)> progress);
+    void paint(const QPointF& updatedPoint, std::function<void(BucketState, int, int)> progress);
 
 private:
 
@@ -57,9 +57,12 @@ class BitmapBucket
      * @param checkPoint
      * @return True if you are allowed to fill, otherwise false
      */
-    bool allowFill(const QPoint& checkPoint) const;
+    bool allowFill(const QPoint& checkPoint, const QRgb& checkColor) const;
+    bool allowContinuousFill(const QPoint& checkPoint, const QRgb& checkColor) const;
+
+    /** Determines whether fill to drag feature can be used */
+    bool canUseDragToFill(const QPoint& fillPoint, const QColor& bucketColor, const BitmapImage& referenceImage);
 
-    std::pair<Layer*, int> findBitmapLayerBelow(Layer* targetLayer, int layerIndex) const;
     BitmapImage flattenBitmapLayersToImage();
 
     Editor* mEditor = nullptr;
@@ -76,6 +79,8 @@ class BitmapBucket
     int mTolerance = 0;
 
     int mTargetFillToLayerIndex = -1;
+    bool mFilledOnce = false;
+    bool mUseDragToFill = false;
 
     Properties mProperties;
 };

core_lib/src/managers/toolmanager.cpp

@@ -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);
 }

core_lib/src/managers/toolmanager.h

@@ -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);

core_lib/src/tool/basetool.cpp

@@ -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;

core_lib/src/tool/basetool.h

@@ -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;
@@ -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);