Fix ctkSliderWidget assert when changing decimals with DecimalsByValue

By blocking signals in ctkSliderWidget::changeValue, the change of decimals
(because of ctkDoubleSpinBox::DecimalsByValue) is not catched by
ctkSliderWidget and not propagated to the slider which causes a discrepancy
with the minimum/maximum values between the spinbox and the slider.
The test values come from a real case that was asserting in
ctkSliderWidget.

Libs/Widgets/Testing/Cpp/CMakeLists.txt

@@ -71,6 +71,7 @@ set(TEST_SOURCES
   ctkSettingsTest1.cpp
   ctkSettingsDialogTest1.cpp
   ctkSignalMapperTest1.cpp
+  ctkSliderWidgetTest.cpp
   ctkSliderWidgetTest1.cpp
   ctkSliderWidgetTest2.cpp
   ctkThumbnailListWidgetTest1.cpp
@@ -188,6 +189,7 @@ QT4_GENERATE_MOCS(
   ctkPathListWidgetWithButtonsTest.cpp
   ctkRangeSliderTest.cpp
   ctkSettingsPanelTest.cpp
+  ctkSliderWidgetTest.cpp
   )
 set(Tests_UI_CPP)
 if(TEST_UI_FORMS)
@@ -277,6 +279,7 @@ SIMPLE_TEST( ctkSettingsPanelTest )
 SIMPLE_TEST( ctkSettingsPanelTest1 )
 SIMPLE_TEST( ctkSettingsTest1 )
 SIMPLE_TEST( ctkSignalMapperTest1 )
+SIMPLE_TEST( ctkSliderWidgetTest )
 SIMPLE_TEST( ctkSliderWidgetTest1 )
 SIMPLE_TEST( ctkSliderWidgetTest2 )
 SIMPLE_TEST( ctkThumbnailListWidgetTest1 )

Libs/Widgets/Testing/Cpp/ctkSliderWidgetTest.cpp

@@ -0,0 +1,118 @@
+/*=========================================================================
+
+  Library:   CTK
+
+  Copyright (c) Kitware Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0.txt
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+=========================================================================*/
+
+// Qt includes
+#include <QApplication>
+#include <QDoubleSpinBox>
+#include <QLineEdit>
+#include <QString>
+#include <QStyle>
+#include <QStyleOptionSlider>
+#include <QTimer>
+
+// CTK includes
+#include "ctkDoubleSlider.h"
+#include "ctkDoubleSpinBox.h"
+#include "ctkSliderWidget.h"
+#include "ctkTest.h"
+
+// ----------------------------------------------------------------------------
+class ctkSliderWidgetTester: public QObject
+{
+  Q_OBJECT
+private slots:
+  void testUI();
+
+  void testDecimalsByValue();
+  /// This test makes sure the number of decimals increased with Ctrl+'+' does
+  /// not break the synchronization between the value of the slider and the
+  /// value of the spinbox.
+  void testDecimalsByShortcuts();
+
+};
+
+// ----------------------------------------------------------------------------
+void ctkSliderWidgetTester::testUI()
+{
+  ctkSliderWidget slider;
+  slider.setMinimum(-100.);
+  slider.setMaximum(100.);
+  slider.setValue(26.2110001);
+  slider.setPrefix("A: ");
+  slider.show();
+  QTest::qWaitForWindowShown(&slider);
+  QObject::connect(&slider, SIGNAL(valueChanged(double)),
+                   &slider, SLOT(setValue(double)), Qt::QueuedConnection);
+
+
+  //qApp->exec();
+}
+
+// ----------------------------------------------------------------------------
+void ctkSliderWidgetTester::testDecimalsByValue()
+{
+  ctkSliderWidget slider;
+  slider.spinBox()->setDecimalsOption(
+    ctkDoubleSpinBox::DecimalsByValue | ctkDoubleSpinBox::DecimalsByShortcuts );
+  slider.setValue(-12.4);
+
+  //slider.show();
+  //QTest::qWaitForWindowShown(&slider);
+  //qApp->exec();
+  //QSignalSpy spy(&slider, SIGNAL(decimalsChanged(int)));
+
+  slider.setSingleStep(1.3);
+  slider.setRange(-87.2949, 81.7045);
+  slider.setValue(-15);
+
+  slider.slider()->triggerAction(QAbstractSlider::SliderSingleStepSub);
+  slider.setSingleStep(1.3);
+}
+
+// ----------------------------------------------------------------------------
+void ctkSliderWidgetTester::testDecimalsByShortcuts()
+{
+  ctkSliderWidget slider;
+  slider.spinBox()->setDecimalsOption(ctkDoubleSpinBox::DecimalsByShortcuts);
+  slider.setSingleStep(1.299995422363281);
+  slider.setPageStep(1.299995422363281);
+  slider.setRange(-100., 100.);
+  slider.setValue( -2.145195007324205 );
+
+  slider.show();
+  QTest::qWaitForWindowShown(&slider);
+  //qApp->exec();
+  //QSignalSpy spy(&slider, SIGNAL(decimalsChanged(int)));
+
+  slider.slider()->triggerAction(QAbstractSlider::SliderSingleStepAdd);
+  slider.slider()->triggerAction(QAbstractSlider::SliderSingleStepAdd);
+  slider.slider()->triggerAction(QAbstractSlider::SliderSingleStepAdd);
+
+  QTest::keyClick(slider.spinBox(), Qt::Key_Plus, Qt::ControlModifier);
+  QTest::keyClick(slider.spinBox(), Qt::Key_Plus, Qt::ControlModifier);
+  QTest::keyClick(slider.spinBox(), Qt::Key_Plus, Qt::ControlModifier);
+  QCOMPARE(slider.decimals(), 5);
+}
+
+// ----------------------------------------------------------------------------
+CTK_TEST_MAIN(ctkSliderWidgetTest)
+#include "moc_ctkSliderWidgetTest.cpp"
+
+

Libs/Widgets/ctkSliderWidget.cpp

@@ -56,6 +56,7 @@ public:
   bool   Tracking;
   bool   Changing;
   double ValueBeforeChange;
+  bool   BlockSetSliderValue;
   ctkSliderWidget::SynchronizeSiblings SynchronizeMode;
   ctkPopupWidget* SliderPopup;
 };
@@ -69,6 +70,7 @@ ctkSliderWidgetPrivate::ctkSliderWidgetPrivate(ctkSliderWidget& object)
   this->Tracking = true;
   this->Changing = false;
   this->ValueBeforeChange = 0.;
+  this->BlockSetSliderValue = false;
   this->SynchronizeMode =
     ctkSliderWidget::SynchronizeWidth | ctkSliderWidget::SynchronizeDecimals;
   this->SliderPopup = 0;
@@ -166,13 +168,13 @@ ctkSliderWidget::ctkSliderWidget(QWidget* _parent) : Superclass(_parent)
   d->Slider->setMaximum(d->SpinBox->maximum());
   d->Slider->setMinimum(d->SpinBox->minimum());
 
-  this->connect(d->SpinBox, SIGNAL(valueChanged(double)), d->Slider, SLOT(setValue(double)));
+  this->connect(d->SpinBox, SIGNAL(valueChanged(double)), this, SLOT(setSliderValue(double)));
   this->connect(d->SpinBox, SIGNAL(decimalsChanged(int)), this, SLOT(setDecimals(int)));
 
-  //this->connect(d->Slider, SIGNAL(valueChanged(double)), SIGNAL(valueChanged(double)));
   this->connect(d->Slider, SIGNAL(sliderPressed()), this, SLOT(startChanging()));
   this->connect(d->Slider, SIGNAL(sliderReleased()), this, SLOT(stopChanging()));
-  this->connect(d->Slider, SIGNAL(valueChanged(double)), this, SLOT(changeValue(double)));
+  // setSpinBoxValue will fire the valueChanged signal.
+  this->connect(d->Slider, SIGNAL(valueChanged(double)), this, SLOT(setSpinBoxValue(double)));
   d->SpinBox->installEventFilter(this);
 }
 
@@ -201,9 +203,10 @@ double ctkSliderWidget::maximum()const
 void ctkSliderWidget::setMinimum(double min)
 {
   Q_D(ctkSliderWidget);
-  bool wasBlocked = d->SpinBox->blockSignals(true);
+  bool wasBlockSetSliderValue = d->BlockSetSliderValue;
+  d->BlockSetSliderValue = true;
   d->SpinBox->setMinimum(min);
-  d->SpinBox->blockSignals(wasBlocked);
+  d->BlockSetSliderValue = wasBlockSetSliderValue;
 
   // SpinBox can truncate min (depending on decimals).
   // use Spinbox's min to set Slider's min
@@ -218,9 +221,10 @@ void ctkSliderWidget::setMinimum(double min)
 void ctkSliderWidget::setMaximum(double max)
 {
   Q_D(ctkSliderWidget);
-  bool wasBlocked = d->SpinBox->blockSignals(true);
+  bool wasBlockSetSliderValue = d->BlockSetSliderValue;
+  d->BlockSetSliderValue = true;
   d->SpinBox->setMaximum(max);
-  d->SpinBox->blockSignals(wasBlocked);
+  d->BlockSetSliderValue = wasBlockSetSliderValue;
 
   // SpinBox can truncate max (depending on decimals).
   // use Spinbox's max to set Slider's max
@@ -236,9 +240,10 @@ void ctkSliderWidget::setRange(double min, double max)
 {
   Q_D(ctkSliderWidget);
   
-  bool wasBlocked = d->SpinBox->blockSignals(true);
+  bool wasBlockSetSliderValue = d->BlockSetSliderValue;
+  d->BlockSetSliderValue = true;
   d->SpinBox->setRange(min, max);
-  d->SpinBox->blockSignals(wasBlocked);
+  d->BlockSetSliderValue = wasBlockSetSliderValue;
   
   // SpinBox can truncate the range (depending on decimals).
   // use Spinbox's range to set Slider's range
@@ -282,7 +287,7 @@ void ctkSliderWidget::setValue(double _value)
 {
   Q_D(ctkSliderWidget);
   // disable the tracking temporally to emit the
-  // signal valueChanged if changeValue() is called
+  // signal valueChanged if setSpinBoxValue() is called
   bool isChanging = d->Changing;
   d->Changing = false;
   d->SpinBox->setValue(_value);
@@ -324,22 +329,34 @@ void ctkSliderWidget::stopChanging()
 }
 
 // --------------------------------------------------------------------------
-void ctkSliderWidget::changeValue(double newValue)
+void ctkSliderWidget::setSliderValue(double spinBoxValue)
+{
+  Q_D(ctkSliderWidget);
+  if (d->BlockSetSliderValue)
+    {
+    return;
+    }
+  d->Slider->setValue(spinBoxValue);
+}
+
+// --------------------------------------------------------------------------
+void ctkSliderWidget::setSpinBoxValue(double sliderValue)
 {
   Q_D(ctkSliderWidget);
   
-  bool wasBlocked = d->SpinBox->blockSignals(true);
-  d->SpinBox->setValue(newValue);
-  d->SpinBox->blockSignals(wasBlocked);
+  bool wasBlockSetSliderValue = d->BlockSetSliderValue;
+  d->BlockSetSliderValue = true;
+  d->SpinBox->setValue(sliderValue);
+  d->BlockSetSliderValue = wasBlockSetSliderValue;
   Q_ASSERT(d->equal(d->SpinBox->value(), d->Slider->value()));
   
   if (!d->Tracking)
     {
-    emit this->valueIsChanging(newValue);
+    emit this->valueIsChanging(sliderValue);
     }
   if (!d->Changing)
     {
-    emit this->valueChanged(newValue);
+    emit this->valueChanged(sliderValue);
     }
 }
 

Libs/Widgets/ctkSliderWidget.h

@@ -279,7 +279,8 @@ protected Q_SLOTS:
   
   void startChanging();
   void stopChanging();
-  void changeValue(double value);
+  void setSpinBoxValue(double sliderValue);
+  void setSliderValue(double spinBoxValue);
 
 protected:
   virtual bool eventFilter(QObject *obj, QEvent *event);