Skip to content

Commit

Permalink
BUG: Ensure that the min/max values can be reached in ctkDoubleSlider…
Browse files Browse the repository at this point in the history 
… 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 <[email protected]>
  • Loading branch information
@lassoan @jcfr
lassoan and jcfr committed Mar 11, 2023
1 parent 681e208 commit ef3d86b
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 33 deletions.
38 changes: 32 additions & 6 deletions Libs/Widgets/Testing/Cpp/ctkDoubleRangeSliderTest1.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,38 @@ bool checkSlider(const ctkDoubleRangeSlider& slider,
bool checkSlider(const ctkDoubleRangeSlider& slider,
double min, double minVal, double maxVal, double max, double minPos, double maxPos)
{
return qFuzzyCompare(slider.minimum(), min) &&
qFuzzyCompare(slider.minimumValue(), minVal) &&
qFuzzyCompare(slider.maximumValue(), maxVal) &&
qFuzzyCompare(slider.maximum(), max) &&
qFuzzyCompare(slider.minimumPosition(), minPos) &&
qFuzzyCompare(slider.maximumPosition(), maxPos);
bool success = true;
if (!qFuzzyCompare(slider.minimum(), min))
{
std::cerr << "Slider minimum mismatch: expected " << min << " actual " << slider.minimum() << std::endl;
success = false;
}
if (!qFuzzyCompare(slider.minimumValue(), minVal))
{
std::cerr << "Slider minimumValue mismatch: expected " << minVal << " actual " << slider.minimumValue() << std::endl;
success = false;
}
if (!qFuzzyCompare(slider.maximumValue(), maxVal))
{
std::cerr << "Slider maximumValue mismatch: expected " << maxVal << " actual " << slider.maximumValue() << std::endl;
success = false;
}
if (!qFuzzyCompare(slider.maximum(), max))
{
std::cerr << "Slider maximum mismatch: expected " << max << " actual " << slider.maximum() << std::endl;
success = false;
}
if (!qFuzzyCompare(slider.minimumPosition(), minPos))
{
std::cerr << "Slider minimumPosition mismatch: expected " << minPos << " actual " << slider.minimumPosition() << std::endl;
success = false;
}
if (!qFuzzyCompare(slider.maximumPosition(), maxPos))
{
std::cerr << "Slider maximumPosition mismatch: expected " << maxPos << " actual " << slider.maximumPosition() << std::endl;
success = false;
}
return success;
}

//-----------------------------------------------------------------------------
Expand Down
21 changes: 17 additions & 4 deletions Libs/Widgets/Testing/Cpp/ctkRangeWidgetTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,30 @@ void ctkRangeWidgetTester::testSetRange()

QFETCH(double, expectedMinimum);
QFETCH(double, expectedMaximum);
ctkTest::COMPARE(rangeWidget.minimum(), expectedMinimum);
ctkTest::COMPARE(rangeWidget.maximum(), expectedMaximum);

// The slider cannot represent infinitely large range, therefore we skip
// testing of range minimum/maximum checks in these cases.
bool testRangeMinMax = (expectedMinimum != std::numeric_limits<double>::min()
&& expectedMinimum != std::numeric_limits<double>::max()
&& expectedMinimum != std::numeric_limits<double>::infinity()
&& expectedMinimum != -std::numeric_limits<double>::infinity());

if (testRangeMinMax)
{
ctkTest::COMPARE(rangeWidget.minimum(), expectedMinimum);
ctkTest::COMPARE(rangeWidget.maximum(), expectedMaximum);
}
const bool minimumChanged = expectedMinimum != 0.;
const bool maximumChanged = expectedMaximum != 99.;
QCOMPARE(rangeChangedSpy.count(),(minimumChanged || maximumChanged) ? 1 : 0);

QFETCH(double, expectedLowerValue);
QFETCH(double, expectedUpperValue);
ctkTest::COMPARE(rangeWidget.minimumValue(), expectedLowerValue);
ctkTest::COMPARE(rangeWidget.maximumValue(), expectedUpperValue);
if (testRangeMinMax)
{
ctkTest::COMPARE(rangeWidget.minimumValue(), expectedLowerValue);
ctkTest::COMPARE(rangeWidget.maximumValue(), expectedUpperValue);
}

const bool lowerValueChanged = expectedLowerValue != 25.;
const bool upperValueChanged = expectedUpperValue != 75.;
Expand Down
47 changes: 32 additions & 15 deletions Libs/Widgets/ctkDoubleRangeSlider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class ctkDoubleRangeSliderPrivate
double maxFromInt(int _value)const;
double safeMinFromInt(int _value)const;
double safeMaxFromInt(int _value)const;
void updateIntegerSliderRange();

void init();
void connectSlider();
Expand Down Expand Up @@ -157,7 +158,7 @@ int ctkDoubleRangeSliderPrivate::toInt(double doubleValue)const
// --------------------------------------------------------------------------
double ctkDoubleRangeSliderPrivate::minFromInt(int intValue)const
{
double doubleValue = this->SingleStep * (this->MinOffset + intValue) ;
double doubleValue = this->SingleStep * (this->MinOffset + intValue);
return doubleValue;
}

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

// --------------------------------------------------------------------------
void ctkDoubleRangeSliderPrivate::updateIntegerSliderRange()
{
// Compute the integer slider's range so that the minimum and maximum values can be actually reached
int intMin = this->toInt(this->Minimum);
if (this->minFromInt(intMin) > this->Minimum && intMin > std::numeric_limits<int>::min())
{
intMin -= 1;
}
int intMax = this->toInt(this->Maximum);
if (this->maxFromInt(intMax) < this->Maximum && intMax < std::numeric_limits<int>::max())
{
intMax += 1;
}
this->Slider->setRange(intMin, intMax);
}

// --------------------------------------------------------------------------
void ctkDoubleRangeSliderPrivate::updateMinOffset(double value)
{
Expand Down Expand Up @@ -226,16 +244,16 @@ void ctkDoubleRangeSlider::setMinimum(double newMin)
double oldMin = d->Minimum;
d->Minimum = newMin;
if (d->Minimum >= d->MinValue)
{// TBD: use same offset
{
d->updateMinOffset(d->Minimum);
}
if (d->Minimum >= d->MaxValue)
{// TBD: use same offset
{
d->updateMaxOffset(d->Minimum);
}
bool wasSettingRange = d->SettingRange;
d->SettingRange = true;
d->Slider->setMinimum(d->toInt(newMin));
d->updateIntegerSliderRange();
d->SettingRange = wasSettingRange;
if (!wasSettingRange && d->Minimum != oldMin)
{
Expand Down Expand Up @@ -268,16 +286,16 @@ void ctkDoubleRangeSlider::setMaximum(double newMax)
double oldMax = d->Maximum;
d->Maximum = newMax;
if (d->Maximum <= d->MinValue)
{// TBD: use same offset
{
d->updateMinOffset(d->Maximum);
}
if (d->Maximum <= d->MaxValue)
{// TBD: use same offset ?
{
d->updateMaxOffset(d->Maximum);
}
bool wasSettingRange = d->SettingRange;
d->SettingRange = true;
d->Slider->setMaximum(d->toInt(newMax));
d->updateIntegerSliderRange();
d->SettingRange = wasSettingRange;
if (!wasSettingRange && d->Maximum != oldMax)
{
Expand Down Expand Up @@ -319,24 +337,23 @@ void ctkDoubleRangeSlider::setRange(double newMin, double newMax)
d->Minimum = newMin;
d->Maximum = newMax;
if (d->Minimum >= d->MinValue)
{// TBD: use same offset
{
d->updateMinOffset(d->Minimum);
}
if (d->Minimum >= d->MaxValue)
{// TBD: use same offset
{
d->updateMaxOffset(d->Minimum);
}
if (d->Maximum <= d->MinValue)
{// TBD: use same offset
{
d->updateMinOffset(d->Maximum);
}
if (d->Maximum <= d->MaxValue)
{// TBD: use same offset ?
{
d->updateMaxOffset(d->Maximum);
}
bool wasSettingRange = d->SettingRange;
d->SettingRange = true;
d->Slider->setRange(d->toInt(newMin), d->toInt(newMax));
d->updateIntegerSliderRange();
d->SettingRange = wasSettingRange;
if (!wasSettingRange && (d->Minimum != oldMin || d->Maximum != oldMax))
{
Expand Down Expand Up @@ -787,8 +804,8 @@ void ctkDoubleRangeSlider::onRangeChanged(int newIntMin, int newIntMax)
{
return;
}
double newMin = d->minFromInt(newIntMin);
double newMax = d->maxFromInt(newIntMax);
double newMin = d->safeMinFromInt(newIntMin);
double newMax = d->safeMaxFromInt(newIntMax);
if (d->Proxy)
{
newMin = d->Proxy.data()->valueFromProxyValue(newMin);
Expand Down
32 changes: 25 additions & 7 deletions Libs/Widgets/ctkDoubleSlider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class ctkDoubleSliderPrivate
double safeFromInt(int value)const;
void init();
void updateOffset(double value);
void updateIntegerSliderRange();

ctkSlider* Slider;
QString HandleToolTip;
Expand Down Expand Up @@ -167,6 +168,23 @@ void ctkDoubleSliderPrivate::updateOffset(double value)
this->Offset = (value / this->SingleStep) - this->toInt(value);
}

// --------------------------------------------------------------------------
void ctkDoubleSliderPrivate::updateIntegerSliderRange()
{
// Compute the integer slider's range so that the minimum and maximum values can be actually reached
int intMin = this->toInt(this->Minimum);
if (this->fromInt(intMin) > this->Minimum && intMin > std::numeric_limits<int>::min())
{
intMin -= 1;
}
int intMax = this->toInt(this->Maximum);
if (this->fromInt(intMax) < this->Maximum && intMax < std::numeric_limits<int>::max())
{
intMax += 1;
}
this->Slider->setRange(intMin, intMax);
}

//-----------------------------------------------------------------------------
// ctkDoubleSlider

Expand Down Expand Up @@ -236,7 +254,7 @@ void ctkDoubleSlider::setRange(double newMin, double newMax)
}
bool wasSettingRange = d->SettingRange;
d->SettingRange = true;
d->Slider->setRange(d->toInt(newMin), d->toInt(newMax));
d->updateIntegerSliderRange();
d->SettingRange = wasSettingRange;
if (!wasSettingRange && (d->Minimum != oldMin || d->Maximum != oldMax))
{
Expand Down Expand Up @@ -365,7 +383,7 @@ void ctkDoubleSlider::setSingleStep(double newStep)
d->updateOffset(d->Value);
// update the new values of the QSlider
bool oldBlockSignals = d->Slider->blockSignals(true);
d->Slider->setRange(d->toInt(d->Minimum), d->toInt(d->Maximum));
d->updateIntegerSliderRange();
d->Slider->setValue(d->toInt(d->Value));
d->Slider->setPageStep(d->toInt(d->PageStep));
d->Slider->blockSignals(oldBlockSignals);
Expand Down Expand Up @@ -529,9 +547,9 @@ void ctkDoubleSlider::onValueChanged(int newValue)
Q_D(ctkDoubleSlider);
double doubleNewValue = d->safeFromInt(newValue);
/*
qDebug() << "onValueChanged: " << newValue << "->"<< d->fromInt(newValue+d->Offset)
<< " old: " << d->Value << "->" << d->toInt(d->Value)
<< "offset:" << d->Offset << doubleNewValue;
qDebug() << "onValueChanged: " << newValue << " -> " << d->fromInt(newValue)
<< " (offset: " << d->Offset << ") -> " << doubleNewValue
<< " old: " << d->toInt(d->Value) << " -> " << d->Value;
*/
if (d->Value == doubleNewValue)
{
Expand All @@ -556,8 +574,8 @@ void ctkDoubleSlider::onRangeChanged(int newIntMin, int newIntMax)
{
return;
}
double newMin = d->fromInt(newIntMin);
double newMax = d->fromInt(newIntMax);
double newMin = d->safeFromInt(newIntMin);
double newMax = d->safeFromInt(newIntMax);
if (d->Proxy)
{
newMin = d->Proxy.data()->valueFromProxyValue(newMin);
Expand Down
8 changes: 7 additions & 1 deletion Libs/Widgets/ctkRangeWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,16 +372,22 @@ void ctkRangeWidget::setRange(double min, double max)
d->SettingSliderRange = true;
d->Slider->setRange(d->MaximumSpinBox->minimum(), d->MinimumSpinBox->maximum());
d->SettingSliderRange = false;
#ifndef QT_NO_DEBUG
if (!d->useCustomSpinBoxesLimits())
{
if (!(d->equal(d->MinimumSpinBox->minimum(), d->Slider->minimum())) ||
!(d->equal(d->MaximumSpinBox->maximum(), d->Slider->maximum())) ||
!(d->equal(d->Slider->minimumValue(), d->MinimumSpinBox->value())) ||
!(d->equal(d->Slider->maximumValue(), d->MaximumSpinBox->value())))
{
qWarning("ctkRangeWidget::setRange : slider and spinbox are not synchronized");
qWarning() << "ctkRangeWidget::setRange : slider and spinbox are not synchronized "
<< d->MinimumSpinBox->minimum() << d->Slider->minimum()
<< d->MaximumSpinBox->maximum() << d->Slider->maximum()
<< d->Slider->minimumValue() << d->MinimumSpinBox->value()
<< d->Slider->maximumValue() << d->MaximumSpinBox->value();
}
}
#endif
d->updateSpinBoxWidth();
if (!d->useCustomSpinBoxesLimits())
{
Expand Down

0 comments on commit ef3d86b

Please sign in to comment.