Skip to content

Commit ef3d86b

Browse files
lassoanjcfr
lassoan
and
jcfr
committed
BUG: Ensure that the min/max values can be reached in ctkDoubleSlider and ctkDoubleRangeSlider
The minimum and maximum values were not always reachable in ctkDoubleSlider and ctkDoubleRangeSlider widgets when moved the slider to the minimum or maximum position. For example: ``` lo = -1024 hi = 2000 step = (hi - lo) / 1000. thresholdSlider = ctk.ctkRangeWidget() thresholdSlider.spinBoxAlignment = qt.Qt.AlignTop thresholdSlider.setRange(lo, hi) thresholdSlider.singleStep = step thresholdSlider.show() # Impossible to reach 2000.0 slider = ctk.ctkDoubleSlider() slider.minimum = lo slider.maximum = hi slider.singleStep = step slider.show() slider.connect("valueChanged(double)", lambda v: print(v)) # Impossible to reach 2000.0 ``` The problem was that the integer slider range was determined by rounding, which could result in the integer slider range not covering the entire value range. Fixed the problem by extending the integer slider range when it did not fully cover the value range. Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
1 parent 681e208 commit ef3d86b

File tree

5 files changed

+113
-33
lines changed

5 files changed

+113
-33
lines changed

‎Libs/Widgets/Testing/Cpp/ctkDoubleRangeSliderTest1.cpp‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,38 @@ bool checkSlider(const ctkDoubleRangeSlider& slider,
5252
bool checkSlider(const ctkDoubleRangeSlider& slider,
5353
double min, double minVal, double maxVal, double max, double minPos, double maxPos)
5454
{
55-
return qFuzzyCompare(slider.minimum(), min) &&
56-
qFuzzyCompare(slider.minimumValue(), minVal) &&
57-
qFuzzyCompare(slider.maximumValue(), maxVal) &&
58-
qFuzzyCompare(slider.maximum(), max) &&
59-
qFuzzyCompare(slider.minimumPosition(), minPos) &&
60-
qFuzzyCompare(slider.maximumPosition(), maxPos);
55+
bool success = true;
56+
if (!qFuzzyCompare(slider.minimum(), min))
57+
{
58+
std::cerr << "Slider minimum mismatch: expected " << min << " actual " << slider.minimum() << std::endl;
59+
success = false;
60+
}
61+
if (!qFuzzyCompare(slider.minimumValue(), minVal))
62+
{
63+
std::cerr << "Slider minimumValue mismatch: expected " << minVal << " actual " << slider.minimumValue() << std::endl;
64+
success = false;
65+
}
66+
if (!qFuzzyCompare(slider.maximumValue(), maxVal))
67+
{
68+
std::cerr << "Slider maximumValue mismatch: expected " << maxVal << " actual " << slider.maximumValue() << std::endl;
69+
success = false;
70+
}
71+
if (!qFuzzyCompare(slider.maximum(), max))
72+
{
73+
std::cerr << "Slider maximum mismatch: expected " << max << " actual " << slider.maximum() << std::endl;
74+
success = false;
75+
}
76+
if (!qFuzzyCompare(slider.minimumPosition(), minPos))
77+
{
78+
std::cerr << "Slider minimumPosition mismatch: expected " << minPos << " actual " << slider.minimumPosition() << std::endl;
79+
success = false;
80+
}
81+
if (!qFuzzyCompare(slider.maximumPosition(), maxPos))
82+
{
83+
std::cerr << "Slider maximumPosition mismatch: expected " << maxPos << " actual " << slider.maximumPosition() << std::endl;
84+
success = false;
85+
}
86+
return success;
6187
}
6288

6389
//-----------------------------------------------------------------------------

‎Libs/Widgets/Testing/Cpp/ctkRangeWidgetTest.cpp‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,17 +223,30 @@ void ctkRangeWidgetTester::testSetRange()
223223

224224
QFETCH(double, expectedMinimum);
225225
QFETCH(double, expectedMaximum);
226-
ctkTest::COMPARE(rangeWidget.minimum(), expectedMinimum);
227-
ctkTest::COMPARE(rangeWidget.maximum(), expectedMaximum);
228226

227+
// The slider cannot represent infinitely large range, therefore we skip
228+
// testing of range minimum/maximum checks in these cases.
229+
bool testRangeMinMax = (expectedMinimum != std::numeric_limits<double>::min()
230+
&& expectedMinimum != std::numeric_limits<double>::max()
231+
&& expectedMinimum != std::numeric_limits<double>::infinity()
232+
&& expectedMinimum != -std::numeric_limits<double>::infinity());
233+
234+
if (testRangeMinMax)
235+
{
236+
ctkTest::COMPARE(rangeWidget.minimum(), expectedMinimum);
237+
ctkTest::COMPARE(rangeWidget.maximum(), expectedMaximum);
238+
}
229239
const bool minimumChanged = expectedMinimum != 0.;
230240
const bool maximumChanged = expectedMaximum != 99.;
231241
QCOMPARE(rangeChangedSpy.count(),(minimumChanged || maximumChanged) ? 1 : 0);
232242

233243
QFETCH(double, expectedLowerValue);
234244
QFETCH(double, expectedUpperValue);
235-
ctkTest::COMPARE(rangeWidget.minimumValue(), expectedLowerValue);
236-
ctkTest::COMPARE(rangeWidget.maximumValue(), expectedUpperValue);
245+
if (testRangeMinMax)
246+
{
247+
ctkTest::COMPARE(rangeWidget.minimumValue(), expectedLowerValue);
248+
ctkTest::COMPARE(rangeWidget.maximumValue(), expectedUpperValue);
249+
}
237250

238251
const bool lowerValueChanged = expectedLowerValue != 25.;
239252
const bool upperValueChanged = expectedUpperValue != 75.;

‎Libs/Widgets/ctkDoubleRangeSlider.cpp‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class ctkDoubleRangeSliderPrivate
4646
double maxFromInt(int _value)const;
4747
double safeMinFromInt(int _value)const;
4848
double safeMaxFromInt(int _value)const;
49+
void updateIntegerSliderRange();
4950

5051
void init();
5152
void connectSlider();
@@ -157,7 +158,7 @@ int ctkDoubleRangeSliderPrivate::toInt(double doubleValue)const
157158
// --------------------------------------------------------------------------
158159
double ctkDoubleRangeSliderPrivate::minFromInt(int intValue)const
159160
{
160-
double doubleValue = this->SingleStep * (this->MinOffset + intValue) ;
161+
double doubleValue = this->SingleStep * (this->MinOffset + intValue);
161162
return doubleValue;
162163
}
163164

@@ -180,6 +181,23 @@ double ctkDoubleRangeSliderPrivate::safeMaxFromInt(int intValue)const
180181
return qBound(this->Minimum, this->maxFromInt(intValue), this->Maximum);
181182
}
182183

184+
// --------------------------------------------------------------------------
185+
void ctkDoubleRangeSliderPrivate::updateIntegerSliderRange()
186+
{
187+
// Compute the integer slider's range so that the minimum and maximum values can be actually reached
188+
int intMin = this->toInt(this->Minimum);
189+
if (this->minFromInt(intMin) > this->Minimum && intMin > std::numeric_limits<int>::min())
190+
{
191+
intMin -= 1;
192+
}
193+
int intMax = this->toInt(this->Maximum);
194+
if (this->maxFromInt(intMax) < this->Maximum && intMax < std::numeric_limits<int>::max())
195+
{
196+
intMax += 1;
197+
}
198+
this->Slider->setRange(intMin, intMax);
199+
}
200+
183201
// --------------------------------------------------------------------------
184202
void ctkDoubleRangeSliderPrivate::updateMinOffset(double value)
185203
{
@@ -226,16 +244,16 @@ void ctkDoubleRangeSlider::setMinimum(double newMin)
226244
double oldMin = d->Minimum;
227245
d->Minimum = newMin;
228246
if (d->Minimum >= d->MinValue)
229-
{// TBD: use same offset
247+
{
230248
d->updateMinOffset(d->Minimum);
231249
}
232250
if (d->Minimum >= d->MaxValue)
233-
{// TBD: use same offset
251+
{
234252
d->updateMaxOffset(d->Minimum);
235253
}
236254
bool wasSettingRange = d->SettingRange;
237255
d->SettingRange = true;
238-
d->Slider->setMinimum(d->toInt(newMin));
256+
d->updateIntegerSliderRange();
239257
d->SettingRange = wasSettingRange;
240258
if (!wasSettingRange && d->Minimum != oldMin)
241259
{
@@ -268,16 +286,16 @@ void ctkDoubleRangeSlider::setMaximum(double newMax)
268286
double oldMax = d->Maximum;
269287
d->Maximum = newMax;
270288
if (d->Maximum <= d->MinValue)
271-
{// TBD: use same offset
289+
{
272290
d->updateMinOffset(d->Maximum);
273291
}
274292
if (d->Maximum <= d->MaxValue)
275-
{// TBD: use same offset ?
293+
{
276294
d->updateMaxOffset(d->Maximum);
277295
}
278296
bool wasSettingRange = d->SettingRange;
279297
d->SettingRange = true;
280-
d->Slider->setMaximum(d->toInt(newMax));
298+
d->updateIntegerSliderRange();
281299
d->SettingRange = wasSettingRange;
282300
if (!wasSettingRange && d->Maximum != oldMax)
283301
{
@@ -319,24 +337,23 @@ void ctkDoubleRangeSlider::setRange(double newMin, double newMax)
319337
d->Minimum = newMin;
320338
d->Maximum = newMax;
321339
if (d->Minimum >= d->MinValue)
322-
{// TBD: use same offset
340+
{
323341
d->updateMinOffset(d->Minimum);
324342
}
325343
if (d->Minimum >= d->MaxValue)
326-
{// TBD: use same offset
344+
{
327345
d->updateMaxOffset(d->Minimum);
328346
}
329347
if (d->Maximum <= d->MinValue)
330-
{// TBD: use same offset
348+
{
331349
d->updateMinOffset(d->Maximum);
332350
}
333351
if (d->Maximum <= d->MaxValue)
334-
{// TBD: use same offset ?
352+
{
335353
d->updateMaxOffset(d->Maximum);
336354
}
337355
bool wasSettingRange = d->SettingRange;
338-
d->SettingRange = true;
339-
d->Slider->setRange(d->toInt(newMin), d->toInt(newMax));
356+
d->updateIntegerSliderRange();
340357
d->SettingRange = wasSettingRange;
341358
if (!wasSettingRange && (d->Minimum != oldMin || d->Maximum != oldMax))
342359
{
@@ -787,8 +804,8 @@ void ctkDoubleRangeSlider::onRangeChanged(int newIntMin, int newIntMax)
787804
{
788805
return;
789806
}
790-
double newMin = d->minFromInt(newIntMin);
791-
double newMax = d->maxFromInt(newIntMax);
807+
double newMin = d->safeMinFromInt(newIntMin);
808+
double newMax = d->safeMaxFromInt(newIntMax);
792809
if (d->Proxy)
793810
{
794811
newMin = d->Proxy.data()->valueFromProxyValue(newMin);

‎Libs/Widgets/ctkDoubleSlider.cpp‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class ctkDoubleSliderPrivate
6666
double safeFromInt(int value)const;
6767
void init();
6868
void updateOffset(double value);
69+
void updateIntegerSliderRange();
6970

7071
ctkSlider* Slider;
7172
QString HandleToolTip;
@@ -167,6 +168,23 @@ void ctkDoubleSliderPrivate::updateOffset(double value)
167168
this->Offset = (value / this->SingleStep) - this->toInt(value);
168169
}
169170

171+
// --------------------------------------------------------------------------
172+
void ctkDoubleSliderPrivate::updateIntegerSliderRange()
173+
{
174+
// Compute the integer slider's range so that the minimum and maximum values can be actually reached
175+
int intMin = this->toInt(this->Minimum);
176+
if (this->fromInt(intMin) > this->Minimum && intMin > std::numeric_limits<int>::min())
177+
{
178+
intMin -= 1;
179+
}
180+
int intMax = this->toInt(this->Maximum);
181+
if (this->fromInt(intMax) < this->Maximum && intMax < std::numeric_limits<int>::max())
182+
{
183+
intMax += 1;
184+
}
185+
this->Slider->setRange(intMin, intMax);
186+
}
187+
170188
//-----------------------------------------------------------------------------
171189
// ctkDoubleSlider
172190

@@ -236,7 +254,7 @@ void ctkDoubleSlider::setRange(double newMin, double newMax)
236254
}
237255
bool wasSettingRange = d->SettingRange;
238256
d->SettingRange = true;
239-
d->Slider->setRange(d->toInt(newMin), d->toInt(newMax));
257+
d->updateIntegerSliderRange();
240258
d->SettingRange = wasSettingRange;
241259
if (!wasSettingRange && (d->Minimum != oldMin || d->Maximum != oldMax))
242260
{
@@ -365,7 +383,7 @@ void ctkDoubleSlider::setSingleStep(double newStep)
365383
d->updateOffset(d->Value);
366384
// update the new values of the QSlider
367385
bool oldBlockSignals = d->Slider->blockSignals(true);
368-
d->Slider->setRange(d->toInt(d->Minimum), d->toInt(d->Maximum));
386+
d->updateIntegerSliderRange();
369387
d->Slider->setValue(d->toInt(d->Value));
370388
d->Slider->setPageStep(d->toInt(d->PageStep));
371389
d->Slider->blockSignals(oldBlockSignals);
@@ -529,9 +547,9 @@ void ctkDoubleSlider::onValueChanged(int newValue)
529547
Q_D(ctkDoubleSlider);
530548
double doubleNewValue = d->safeFromInt(newValue);
531549
/*
532-
qDebug() << "onValueChanged: " << newValue << "->"<< d->fromInt(newValue+d->Offset)
533-
<< " old: " << d->Value << "->" << d->toInt(d->Value)
534-
<< "offset:" << d->Offset << doubleNewValue;
550+
qDebug() << "onValueChanged: " << newValue << " -> " << d->fromInt(newValue)
551+
<< " (offset: " << d->Offset << ") -> " << doubleNewValue
552+
<< " old: " << d->toInt(d->Value) << " -> " << d->Value;
535553
*/
536554
if (d->Value == doubleNewValue)
537555
{
@@ -556,8 +574,8 @@ void ctkDoubleSlider::onRangeChanged(int newIntMin, int newIntMax)
556574
{
557575
return;
558576
}
559-
double newMin = d->fromInt(newIntMin);
560-
double newMax = d->fromInt(newIntMax);
577+
double newMin = d->safeFromInt(newIntMin);
578+
double newMax = d->safeFromInt(newIntMax);
561579
if (d->Proxy)
562580
{
563581
newMin = d->Proxy.data()->valueFromProxyValue(newMin);

‎Libs/Widgets/ctkRangeWidget.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,16 +372,22 @@ void ctkRangeWidget::setRange(double min, double max)
372372
d->SettingSliderRange = true;
373373
d->Slider->setRange(d->MaximumSpinBox->minimum(), d->MinimumSpinBox->maximum());
374374
d->SettingSliderRange = false;
375+
#ifndef QT_NO_DEBUG
375376
if (!d->useCustomSpinBoxesLimits())
376377
{
377378
if (!(d->equal(d->MinimumSpinBox->minimum(), d->Slider->minimum())) ||
378379
!(d->equal(d->MaximumSpinBox->maximum(), d->Slider->maximum())) ||
379380
!(d->equal(d->Slider->minimumValue(), d->MinimumSpinBox->value())) ||
380381
!(d->equal(d->Slider->maximumValue(), d->MaximumSpinBox->value())))
381382
{
382-
qWarning("ctkRangeWidget::setRange : slider and spinbox are not synchronized");
383+
qWarning() << "ctkRangeWidget::setRange : slider and spinbox are not synchronized "
384+
<< d->MinimumSpinBox->minimum() << d->Slider->minimum()
385+
<< d->MaximumSpinBox->maximum() << d->Slider->maximum()
386+
<< d->Slider->minimumValue() << d->MinimumSpinBox->value()
387+
<< d->Slider->maximumValue() << d->MaximumSpinBox->value();
383388
}
384389
}
390+
#endif
385391
d->updateSpinBoxWidth();
386392
if (!d->useCustomSpinBoxesLimits())
387393
{

0 commit comments

Comments
 (0)