search

heronarts / LX

Core library for 3D LED lighting engines
https://chromatik.co/
Other
41 stars 25 forks source link

Fix bug: MFT normalized value not reaching 1 for DiscreteParameter #82

Closed jkbelcher closed 1 year ago

jkbelcher commented 1 year ago

Need another set of eyes on this. The problem shows up with a high resolution DiscreteParameter, say range 0-255, IncrementMode=NORMALIZED. Easier to see if not wrappable.

In onKnobIncrement(..), value was being constrained to (0,127) prior to calling knob.setNormalized(value/128.). So the value of 1 was never passed to setNormalized() and the max value of the parameter could not be reached by turning the MFT knob.

127 is a max value imposed by the definition of midi CC (0-127). I think this means the number of increments on a midi knob between min and max, inclusive, is 127. It's only 128 including the full wrap. I changed the 128s to 127s in the code, which seems right at the moment but needs double checking.

mcslee commented 1 year ago

Yeah I think this is fine, the only case I'm worried about is a knob that's set to wrap-around. Does this make it impossible to ever hit the maximum value (e.g. normalized value of 1) - does it end up always wrapping around and over-shooting the final value?

I think it's okay but don't have a setup in front of me to confirm. Can you just check that case? If all good, I'm happy to merge this.

jkbelcher commented 1 year ago

Looks correct, tested ranges of 2, 4, 100, and 200 on all combinations of wrappable and IncrementMode. All of them land on min&max and feel like the correct distance when wrapping. Tested alternating with mouse UI moves too.

mcslee commented 1 year ago

Fantastic, thanks as always dude!