Fix curve changing with sensors w/ Use Same Curve

Before this commit, curve would change semi-randomly in some cases
if you change from some specific sensors to some other specific sensors
(having a complex curve and clicking randomly should show a bug though)
when "Share curve across all settings" is selected.
This commit fixes that behaviour.

BUG:383909
CCBUG:410158

libs/global/kis_signal_compressor.h

@@ -31,14 +31,14 @@ class QTimer;
  * where this is used is to limit the amount of expensive redraw activity on the
  * canvas.
  *
- * There are three behaviors to choose from.
+ * There are four behaviors to choose from.
  *
  * POSTPONE resets the timer after each call. Therefore if the calls are made
  * quickly enough, the timer will never be activated.
  *
  * FIRST_ACTIVE_POSTPONE_NEXT emits the first signal and postpones all
- * the other actions the other action like in POSTPONE. This mode is
- * used e.g.  in move/remove layer functionality. If you remove a
+ * the other actions like in POSTPONE. This mode is
+ * used e.g. in move/remove layer functionality. If you remove a
  * single layer, you'll see the result immediately. But if you want to
  * remove multiple layers, you should wait until all the actions are
  * finished.

libs/ui/widgets/kis_curve_widget.cpp

@@ -47,6 +47,10 @@
 #include <kis_config.h>
 #include <klocalizedstring.h>
 
+#include <kis_signal_compressor.h>
+#include <kis_thread_safe_signal_compressor.h>
+
+
 // Local includes.
 
 #include "widgets/kis_curve_widget.h"
@@ -73,13 +77,15 @@ KisCurveWidget::KisCurveWidget(QWidget *parent, Qt::WindowFlags f)
     d->m_intIn = 0;
     d->m_intOut = 0;
 
+    connect(&d->m_modifiedSignalsCompressor, SIGNAL(timeout()), SLOT(notifyModified()));
+    connect(this, SIGNAL(compressorShouldEmitModified()), SLOT(slotCompressorShouldEmitModified()));
+
     setMouseTracking(true);
     setAutoFillBackground(false);
     setAttribute(Qt::WA_OpaquePaintEvent);
     setMinimumSize(150, 50);
     setMaximumSize(250, 250);
 
-    d->setCurveModified();
 
     setFocusPolicy(Qt::StrongFocus);
 }
@@ -92,6 +98,8 @@ KisCurveWidget::~KisCurveWidget()
 
 void KisCurveWidget::setupInOutControls(QSpinBox *in, QSpinBox *out, int inMin, int inMax, int outMin, int outMax)
 {
+    dropInOutControls();
+
     d->m_intIn = in;
     d->m_intOut = out;
 
@@ -533,3 +541,12 @@ void KisCurveWidget::leaveEvent(QEvent *)
 {
 }
 
+void KisCurveWidget::notifyModified()
+{
+    emit modified();
+}
+
+void KisCurveWidget::slotCompressorShouldEmitModified()
+{
+    d->m_modifiedSignalsCompressor.start();
+}

libs/ui/widgets/kis_curve_widget.h

@@ -105,9 +105,26 @@ Q_SIGNALS:
      * Emitted whenever the status of whether a control point is selected or not changes
      */
     void pointSelectedChanged();
+    /**
+     * Emitted to notify that the start() function in compressor can be activated.
+     * Thanks to that, blocking signals in curve widget blocks "sending signals"
+     * (calling start() function) *to* the signal compressor.
+     * It effectively makes signals work nearly the same way they worked before
+     * adding the signal compressor in between.
+     */
+    void compressorShouldEmitModified();
+
 
 protected Q_SLOTS:
     void inOutChanged(int);
+    void notifyModified();
+
+    /**
+     * This function is called when compressorShouldEmitModified() is emitted.
+     * For why it's needed, \see compressorShouldEmitModified()
+     */
+    void slotCompressorShouldEmitModified();
+
 
 
 protected:

libs/ui/widgets/kis_curve_widget_p.h

@@ -83,6 +83,10 @@ public:
     inline void setState(enumState st);
     inline enumState state() const;
 
+    /**
+     * Compresses the modified() signals
+     */
+    KisThreadSafeSignalCompressor m_modifiedSignalsCompressor;
 
 
     /*** Internal routines ***/
@@ -136,6 +140,7 @@ public:
 };
 
 KisCurveWidget::Private::Private(KisCurveWidget *parent)
+    : m_modifiedSignalsCompressor(100, KisSignalCompressor::Mode::FIRST_INACTIVE)
 {
     m_curveWidget = parent;
 }
@@ -247,13 +252,12 @@ void KisCurveWidget::Private::syncIOControls()
 
 void KisCurveWidget::Private::setCurveModified(bool rewriteSpinBoxesValues = true)
 {
-
     if (rewriteSpinBoxesValues) {
         syncIOControls();
     }
     m_splineDirty = true;
     m_curveWidget->update();
-    m_curveWidget->emit modified();
+    m_curveWidget->emit compressorShouldEmitModified();
 }
 
 void KisCurveWidget::Private::setCurveRepaint()

plugins/paintops/libpaintop/kis_auto_brush_widget.cpp

@@ -113,7 +113,11 @@ KisAutoBrushWidget::KisAutoBrushWidget(QWidget *parent, const char* name)
     KisCubicCurve topLeftBottomRightLinearCurve;
     topLeftBottomRightLinearCurve.setPoint(0, QPointF(0.0, 1.0));
     topLeftBottomRightLinearCurve.setPoint(1, QPointF(1.0, 0.0));
+
+    bool blockedBefore = softnessCurve->blockSignals(true);
     softnessCurve->setCurve(topLeftBottomRightLinearCurve);
+    softnessCurve->blockSignals(blockedBefore);
+
     connect(softnessCurve, SIGNAL(modified()), m_updateCompressor.data(), SLOT(start()));
 
     m_brush = QImage(1, 1, QImage::Format_RGB32);

plugins/paintops/libpaintop/kis_curve_option.cpp

@@ -43,9 +43,12 @@ KisCurveOption::KisCurveOption(const QString& name, KisPaintOpOption::PaintopCat
     setValue(value);
 
     QList<QPointF> points;
-    points.push_back(QPointF(0,1));
+    // needs to be set to something, weird curve is better for debugging
+    // it will be reset to the curve from the preset anyway though
+    points.push_back(QPointF(0,0));
     points.push_back(QPointF(0.25,0.9));
-    points.push_back(QPointF(0.75,0.1));
+    points.push_back(QPointF(0.5,0));
+    points.push_back(QPointF(0.75,0.6));
     points.push_back(QPointF(1,0));
     m_commonCurve = KisCubicCurve(points);
 }
@@ -114,12 +117,12 @@ void KisCurveOption::writeOptionSetting(KisPropertiesConfigurationSP setting) co
     setting->setProperty(m_name + "UseSameCurve", m_useSameCurve);
     setting->setProperty(m_name + "Value", m_value);
     setting->setProperty(m_name + "curveMode", m_curveMode);
+    setting->setProperty(m_name + "commonCurve", qVariantFromValue(m_commonCurve));
 
 }
 
 void KisCurveOption::readOptionSetting(KisPropertiesConfigurationSP setting)
 {
-    m_curveCache.clear();
     readNamedOptionSetting(m_name, setting);
 }
 
@@ -146,6 +149,9 @@ QString KisCurveOption::valueSuffix() const
 void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisPropertiesConfigurationSP setting)
 {
     if (!setting) return;
+
+    KisCubicCurve commonCurve = m_commonCurve;
+
     //dbgKrita << "readNamedOptionSetting" << prefix;
     // setting->dump();
 
@@ -167,6 +173,7 @@ void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisProp
         if (s) {
             replaceSensor(s);
             s->setActive(true);
+            commonCurve = s->curve();
             //dbgKrita << "\tsingle sensor" << s::id(s->sensorType()) << s->isActive() << "added";
         }
     }
@@ -183,6 +190,7 @@ void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisProp
                     if (s) {
                         replaceSensor(s);
                         s->setActive(true);
+                        commonCurve = s->curve();
                         //dbgKrita << "\tchild sensor" << s::id(s->sensorType()) << s->isActive() << "added";
                     }
                 }
@@ -191,6 +199,9 @@ void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisProp
         }
     }
 
+
+    m_useSameCurve = setting->getBool(m_name + "UseSameCurve", true);
+
     // Only load the old curve format if the curve wasn't saved by the sensor
     // This will give every sensor the same curve.
     //dbgKrita << ">>>>>>>>>>>" << prefix + "Sensor" << setting->getString(prefix + "Sensor");
@@ -199,10 +210,15 @@ void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisProp
         if (setting->getBool("Custom" + prefix, false)) {
             Q_FOREACH (KisDynamicSensorSP s, m_sensorMap.values()) {
                 s->setCurve(setting->getCubicCurve("Curve" + prefix));
+                commonCurve = s->curve();
             }
         }
     }
 
+    if (m_useSameCurve) {
+        m_commonCurve = setting->getCubicCurve(prefix + "commonCurve", commonCurve);
+    }
+
     // At least one sensor needs to be active
     if (activeSensors().size() == 0) {
         m_sensorMap[PRESSURE]->setActive(true);
@@ -214,7 +230,7 @@ void KisCurveOption::readNamedOptionSetting(const QString& prefix, const KisProp
     m_useCurve = setting->getBool(m_name + "UseCurve", true);
     //dbgKrita << "\t" + m_name + "UseCurve" << m_useSameCurve;
 
-    m_useSameCurve = setting->getBool(m_name + "UseSameCurve", true);
+
     //dbgKrita << "\t" + m_name + "UseSameCurve" << m_useSameCurve;
 
     m_curveMode = setting->getInt(m_name + "curveMode");
@@ -299,6 +315,11 @@ void KisCurveOption::setCurveMode(int mode)
     m_curveMode = mode;
 }
 
+void KisCurveOption::setUseSameCurve(bool useSameCurve)
+{
+    m_useSameCurve = useSameCurve;
+}
+
 void KisCurveOption::setCommonCurve(KisCubicCurve curve)
 {
     m_commonCurve = curve;
@@ -306,7 +327,6 @@ void KisCurveOption::setCommonCurve(KisCubicCurve curve)
 
 void KisCurveOption::setCurve(DynamicSensorType sensorType, bool useSameCurve, const KisCubicCurve &curve)
 {
-    // No switch in state, don't mess with the cache
     if (useSameCurve == m_useSameCurve) {
         if (useSameCurve) {
             m_commonCurve = curve;
@@ -320,31 +340,16 @@ void KisCurveOption::setCurve(DynamicSensorType sensorType, bool useSameCurve, c
         }
     }
     else {
-        // moving from not use same curve to use same curve: backup the custom curves
         if (!m_useSameCurve && useSameCurve) {
-            // Copy the custom curves to the cache and set the new curve on all sensors, active or not
-            m_curveCache.clear();
-            Q_FOREACH (KisDynamicSensorSP s, m_sensorMap.values()) {
-                m_curveCache[s->sensorType()] = s->curve();
-            }
             m_commonCurve = curve;
         }
         else { //if (m_useSameCurve && !useSameCurve)
-            // Restore the cached curves
             KisDynamicSensorSP s = 0;
-            Q_FOREACH (DynamicSensorType sensorType, m_curveCache.keys()) {
-                if (m_sensorMap.contains(sensorType)) {
-                    s = m_sensorMap[sensorType];
-                }
-                else {
-                    s = KisDynamicSensor::type2Sensor(sensorType, m_name);
-                }
-                m_sensorMap[sensorType] = s;
-            }
-            s = 0;
             // And set the current sensor to the current curve
             if (!m_sensorMap.contains(sensorType)) {
                 s = KisDynamicSensor::type2Sensor(sensorType, m_name);
+            } else {
+                KisDynamicSensorSP s = sensor(sensorType, false);
             }
             if (s) {
                 s->setCurve(curve);

plugins/paintops/libpaintop/kis_curve_option.h

@@ -99,9 +99,14 @@ public:
     void setValue(qreal value);
     void setCurveMode(int mode);
 
+    /**
+     * Sets the bool indicating whether "Share curve across all settings" is checked.
+     */
+    void setUseSameCurve(bool useSameCurve);
+
     /**
      * Sets the curve that is being used instead of sensor ones
-     * in case "Use the same curve" is checked.
+     * in case "Share curve across all settings" is checked.
      */
     void setCommonCurve(KisCubicCurve curve);
 
@@ -219,7 +224,6 @@ protected:
     int m_curveMode;
 
     QMap<DynamicSensorType, KisDynamicSensorSP> m_sensorMap;
-    QMap<DynamicSensorType, KisCubicCurve> m_curveCache;
 
 private:
 

plugins/paintops/libpaintop/kis_curve_option_widget.cpp

@@ -60,7 +60,7 @@ KisCurveOptionWidget::KisCurveOptionWidget(KisCurveOption* curveOption, const QS
     connect(m_curveOptionWidget->sensorSelector, SIGNAL(parametersChanged()), SLOT(updateLabelsOfCurrentSensor()));
     connect(m_curveOptionWidget->sensorSelector, SIGNAL(highlightedSensorChanged(KisDynamicSensorSP)), SLOT(updateSensorCurveLabels(KisDynamicSensorSP)));
     connect(m_curveOptionWidget->sensorSelector, SIGNAL(highlightedSensorChanged(KisDynamicSensorSP)), SLOT(updateCurve(KisDynamicSensorSP)));
-    connect(m_curveOptionWidget->checkBoxUseSameCurve, SIGNAL(stateChanged(int)), SLOT(slotStateChanged()));
+    connect(m_curveOptionWidget->checkBoxUseSameCurve, SIGNAL(stateChanged(int)), SLOT(slotUseSameCurveChanged()));
 
 
     // set all the icons for the curve preset shapes
@@ -113,9 +113,14 @@ void KisCurveOptionWidget::readOptionSetting(const KisPropertiesConfigurationSP
 
     m_curveOption->readOptionSetting(setting);
 
+    // Signals needs to be blocked, otherwise checking the checkbox will trigger
+    //   setting the common curve to the widget curve, which is incorrect in this case.
+    bool blockedBefore = m_curveOptionWidget->checkBoxUseSameCurve->blockSignals(true);
+    m_curveOptionWidget->checkBoxUseSameCurve->setChecked(m_curveOption->isSameCurveUsed());
+    m_curveOptionWidget->checkBoxUseSameCurve->blockSignals(blockedBefore);
+
     m_curveOptionWidget->checkBoxUseCurve->setChecked(m_curveOption->isCurveUsed());
     m_curveOptionWidget->strengthSlider->setValue(m_curveOption->value()*100);
-    m_curveOptionWidget->checkBoxUseSameCurve->setChecked(m_curveOption->isSameCurveUsed());
     m_curveOptionWidget->curveMode->setCurrentIndex(m_curveOption->getCurveMode());
 
     disableWidgets(!m_curveOption->isCurveUsed());
@@ -123,10 +128,6 @@ void KisCurveOptionWidget::readOptionSetting(const KisPropertiesConfigurationSP
     m_curveOptionWidget->sensorSelector->reload();
     m_curveOptionWidget->sensorSelector->setCurrent(m_curveOption->activeSensors().first());
     updateSensorCurveLabels(m_curveOptionWidget->sensorSelector->currentHighlighted());
-
-    if (m_curveOption->isSameCurveUsed()) {
-        m_curveOption->setCommonCurve(m_curveOptionWidget->sensorSelector->currentHighlighted()->curve());
-    }
     updateCurve(m_curveOptionWidget->sensorSelector->currentHighlighted());
 }
 
@@ -162,18 +163,25 @@ QWidget* KisCurveOptionWidget::curveWidget()
 
 void KisCurveOptionWidget::slotModified()
 {
-    transferCurve();
-}
-
-void KisCurveOptionWidget::slotStateChanged()
-{
-    transferCurve();
+    if (!m_curveOption->isSameCurveUsed()) {
+        m_curveOptionWidget->sensorSelector->currentHighlighted()->setCurve(getWidgetCurve());
+    } else {
+        m_curveOption->setCommonCurve(getWidgetCurve());
+    }
+    emitSettingChanged();
 }
 
-
-void KisCurveOptionWidget::transferCurve()
+void KisCurveOptionWidget::slotUseSameCurveChanged()
 {
-    m_curveOptionWidget->sensorSelector->setCurrentCurve(m_curveOptionWidget->curveWidget->curve(), m_curveOptionWidget->checkBoxUseSameCurve->isChecked());
+    // this is a slot that answers on "Share Curve across all settings" checkbox
+    m_curveOption->setUseSameCurve(m_curveOptionWidget->checkBoxUseSameCurve->isChecked());
+    if (m_curveOption->isSameCurveUsed()) {
+        // !(UseSameCurve) => UseSameCurve
+        // set the current curve to the common curve
+        m_curveOption->setCommonCurve(getWidgetCurve());
+    } else {
+        updateCurve(m_curveOptionWidget->sensorSelector->currentHighlighted());
+    }
     emitSettingChanged();
 }
 
@@ -338,3 +346,15 @@ void KisCurveOptionWidget::updateThemedIcons()
 
 }
 
+
+
+
+KisCubicCurve KisCurveOptionWidget::getWidgetCurve()
+{
+    return m_curveOptionWidget->curveWidget->curve();
+}
+
+KisCubicCurve KisCurveOptionWidget::getHighlightedSensorCurve()
+{
+    return m_curveOptionWidget->sensorSelector->currentHighlighted()->curve();
+}

plugins/paintops/libpaintop/kis_curve_option_widget.h

@@ -54,9 +54,8 @@ protected:
 private Q_SLOTS:
 
     void slotModified();
-    void slotStateChanged();
+    void slotUseSameCurveChanged();
 
-    void transferCurve();
     void updateSensorCurveLabels(KisDynamicSensorSP sensor);
     void updateCurve(KisDynamicSensorSP sensor);
     void updateValues();
@@ -82,6 +81,10 @@ private:
     Ui_WdgCurveOption* m_curveOptionWidget;
     QComboBox* m_curveMode;
     KisCurveOption* m_curveOption;
+
+    KisCubicCurve getWidgetCurve();
+    KisCubicCurve getHighlightedSensorCurve();
+
 };
 
 #endif // KIS_CURVE_OPTION_WIDGET_H