RomanKubiak / ctrlr

Ctrlr
BSD 3-Clause "New" or "Revised" License
497 stars 58 forks source link

uiSlider bug: Maximum value not reached when Minimum value negative #650

Open mtarenskeen opened 3 weeks ago

mtarenskeen commented 3 weeks ago

When I create a uiSlider (rotary, linear bar, inc/dec button) and set Minimum value to a negative value (for example -99) and the maximum value to a positive value (for example 99) the maximum can not the be reached by moving the slider/knob/incdec. It will only go from -99 to +98.

I am running Ctrlr 5.7.1, built myself from GIT sources, on Linux Fedora 40. Also tried pre-built recent Ctrlr versions on Windows, same issue. Older "stable" version 5.3.201 on Windows does not have this issue. (Ctrlr Version 5.3.201 has become unusable on my Linux Fedora (mouseclicks not responding) but that's a totally different issue, not releated to the bug I am reporting here)

There is a workaround for this issue: Setting the Maximum value to 99.0001 or 99.1 solves the problem. The problem also does not arise when Minimum value=0. Only when Minimum value is a negative number.

damiensellier commented 2 weeks ago

I think ctrlr considers value 0 as a positive value, with negative value it maps -99 to -1 for neg and 0 to 98 for positive. setting max value to 100 is a nice workaround :) I'll take a look at the code and see if I can fix that because it's definitely a bug.

EDIT: it's because of the mapping of the different values is not properly rounded.

5.2.198 Capture d’écran, le 2024-06-15 à 22 15 47

5.6.30 Capture d’écran, le 2024-06-15 à 22 14 45

It makes no sens :) I'll take a look at it, it's in my to-do list for CtrlrX

Thanks for reporting

Damien

damiensellier commented 1 week ago

I fixed the problem for the wrong values mapped on the slider :

SEE : https://github.com/damiensellier/CtrlrX/issues/24#issuecomment-2193012443

https://github.com/RomanKubiak/ctrlr/blob/8aa00d82127acda42ad9ac9b7b479461e9436aa4/Source/UIComponents/CtrlrComponents/Sliders/CtrlrSlider.cpp#L175

else if (property == Ids::uiSliderInterval || property == Ids::uiSliderMax || property == Ids::uiSliderMin)
    {
        // This was the script in v5.2.198
        //ctrlrSlider.setRange ( (float) getProperty (Ids::uiSliderMin), (float) getProperty (Ids::uiSliderMax), (float) getProperty (Ids::uiSliderInterval) ); // Added back from v5.2.198 to v5.6.31 with float instead of double. Note: Float (32bit) is less precise than double (64bit).
        //owner.setProperty (Ids::modulatorMax, ctrlrSlider.getMaximum());
        //owner.setProperty (Ids::modulatorMin, ctrlrSlider.getMinimum());

        // The following script is different since v5.6.26+
        float max = getProperty (Ids::uiSliderMax); // v5.6.31 (float) instead of (double). (double) was giving false values when negative
        float min = getProperty (Ids::uiSliderMin); // v5.6.31 (float) instead of (const double). (const double) was giving false values when negative
        float interval = getProperty (Ids::uiSliderInterval); // v5.6.31 (float) instead of (double). (double) was giving false values when negative
        if (interval == 0)
            interval = std::abs(max-min) + 1;
        // For JUCE MAX must be >= min
        if (max <= min) {
            // samething between 0.5 and 1 times the interval
            // to avoid rounding errors
            max = min + interval * 0.66;
        }
        ctrlrSlider.setRange ( min, max, interval );
        owner.setProperty (Ids::modulatorMax, ctrlrSlider.getMaximum());
        owner.setProperty (Ids::modulatorMin, ctrlrSlider.getMinimum());
    }
damiensellier commented 1 week ago

NOTE : Same problem is happening in CtrlrImageSlider.cpp for uiImageSlider :

Source/UIComponents/CtrlrComponents/Sliders/CtrlrImageSlider.cpp#L173

FIX :

else if (property == Ids::uiSliderInterval || property == Ids::uiSliderMax || property == Ids::uiSliderMin)
    {
        ctrlrSlider->setRange ( (float) getProperty (Ids::uiSliderMin), (float) getProperty (Ids::uiSliderMax), (float) getProperty (Ids::uiSliderInterval) );  // v5.6.31 (float) instead of plain values. plain values and (double) were giving false values when negative.
        owner.setProperty (Ids::modulatorMax, ctrlrSlider->getMaximum());
        owner.setProperty (Ids::modulatorMin, ctrlrSlider->getMinimum());
    }
mtarenskeen commented 5 days ago

Thanks for the suggested fixes. Can someone turn these fixes into proper pullrequest(s) so that they can be pushed to the git sources? (I still need to learn how all these Git commands work...)