Browse Source

Fix ctkDoubleSpinBox::DecimalsByValue signals

Extra signals where fired by setValueAlways->setValue->setDecimals.
This is an issue when the intermediate signals are generating errors
(e.g. 0.1 -> 1.0 was generatign a valueChanged(0.) signal).
Julien Finet 11 years ago
parent
commit
f21e12bcc8

+ 36 - 0
Libs/Widgets/Testing/Cpp/ctkDoubleSpinBoxTest.cpp

@@ -66,6 +66,9 @@ private slots:
   void testDecimalsByValue();
   void testDecimalsByValue_data();
 
+  void testDecimalsByValueSignals();
+  void testDecimalsByValueSignals_data();
+
   void testDecimalPointAlwaysVisible();
   void testDecimalPointAlwaysVisible_data();
 };
@@ -577,6 +580,39 @@ void ctkDoubleSpinBoxTester::testDecimalsByValue_data()
 }
 
 // ----------------------------------------------------------------------------
+void ctkDoubleSpinBoxTester::testDecimalsByValueSignals()
+{
+  ctkDoubleSpinBox spinBox;
+  spinBox.setDecimalsOption( ctkDoubleSpinBox::DecimalsByValue );
+  // 0 -> 0.1 with 1 decimal
+  spinBox.setValue(0.1);
+
+  QSignalSpy decimalsChangedSpy(&spinBox, SIGNAL(decimalsChanged(int)));
+  QSignalSpy valueChangedSpy(&spinBox, SIGNAL(valueChanged(double)));
+
+  // 0.1 -> 1. with 0 decimal
+  QFETCH(double, newValue);
+  spinBox.setValue(newValue);
+
+  QCOMPARE(spinBox.value(), newValue);
+  QCOMPARE(decimalsChangedSpy.count(), spinBox.decimals() != 1 ? 1 : 0);
+  QCOMPARE(valueChangedSpy.count(), newValue != 0.1 ? 1 : 0);
+}
+
+// ----------------------------------------------------------------------------
+void ctkDoubleSpinBoxTester::testDecimalsByValueSignals_data()
+{
+  QTest::addColumn<double>("newValue");
+  QTest::newRow("0: change value, remove decimal") << 0.;
+  QTest::newRow("0.01: change value, add decimal") << 0.01;
+  QTest::newRow("0.1: same value, same decimal") << 0.1;
+  QTest::newRow("0.11: change value, add decimal") << 0.11;
+  QTest::newRow("0.2: change value, same decimal") << 0.2;
+  QTest::newRow("1: change value, remove decimal") << 1.;
+  QTest::newRow("1.1: change value, same decimal") << 1.1;
+}
+
+// ----------------------------------------------------------------------------
 void ctkDoubleSpinBoxTester::testDecimalPointAlwaysVisible()
 {
   ctkDoubleSpinBox spinBox;

+ 22 - 1
Libs/Widgets/ctkDoubleSpinBox.cpp

@@ -286,12 +286,24 @@ void ctkDoubleSpinBoxPrivate::setValue(double value, int dec)
 {
   Q_Q(ctkDoubleSpinBox);
   dec = this->boundDecimals(dec);
-  bool changeDecimals = dec != q->decimals();
+  const bool changeDecimals = dec != q->decimals();
   if (changeDecimals)
     {
+    // don't fire valueChanged signal because we will change the value
+    // right after anyway.
+    const bool blockValueChangedSignal = (this->round(this->SpinBox->value(), dec) != value);
+    bool wasBlocked = false;
+    if (blockValueChangedSignal)
+      {
+      wasBlocked = this->SpinBox->blockSignals(true);
+      }
     // don't fire decimalsChanged signal yet, wait for the value to be
     // up-to-date.
     this->SpinBox->setDecimals(dec);
+    if (blockValueChangedSignal)
+      {
+      this->SpinBox->blockSignals(wasBlocked);
+      }
     }
   this->SpinBox->setValue(value); // re-do the text (calls textFromValue())
   if (changeDecimals)
@@ -876,8 +888,17 @@ void ctkDoubleSpinBox::setValueAlways(double newValue)
     newValueToDisplay = d->Proxy.data()->proxyValueFromValue(newValueToDisplay);
     }
   const int decimals = d->decimalsForValue(newValueToDisplay);
+  // setValueAlways already fires the valueChanged() signal if needed, same
+  // thing for d->setValue() with decimalsChanged(). There is no need to
+  // propagate the valueChanged/decimalsChanged signals from the spinbox.
+  // Alternatively we could also have set a flag that prevents onValueChanged()
+  // to trigger the valueChanged() signal.
+  //bool wasBlocking = d->SpinBox->blockSignals(true);
   d->setValue(newValueToDisplay, decimals);
+  //d->SpinBox->blockSignals(wasBlocking);
   const bool signalsEmitted = (newValue != d->InputValue);
+  // Fire the valueChanged signal only if d->setValue() did not fire it
+  // already..
   if (valueModified && !signalsEmitted)
     {
     emit valueChanged(d->InputValue);