I would say that for doubles, the minimum step size is the one given by ulp = Math.ulp(double). Then the double case should behave like the integer case where the ulp is 1 I think. So, for the angle case of [0, 360), if we are at 360 - ulp, incrementing by N * ulp will get us to (N-1) * ulp.
On Wed, Apr 22, 2020 at 3:41 PM Kevin Rushforth <kevin.rushfo...@oracle.com> wrote: > Hi Ajit, > > Thanks for writing this up. I think we are clear on what should happen > for IntegerSpinnerValueFactory and ListSpinnerValueFactory. To answer > your two questions, I'll take the second one first: > > > 2. Do we really care about negative values in no of steps OR > > amountToStepBy? > > I am asking this as we already have separate increment() and > > decrement() methods. Should we bar such negative values? That’s > > another behavior change though :) > > Once you switch to a proper modulo function, the easy answer is that > negative values will "just work". A negative amountToStepBy might even > make sense in some use cases (think of an increasing debt value), so > there is no reason not to support it. A negative number of steps doesn't > really make sense, since if you want to call increment(-N) you could > equivalently call decrement(N), but again, it will "just work" as > expected once the wrapping function is fixed. If we were designing from > scratch I'd specify that nsteps must be >= 0, but it isn't worth it to > make the change now (which would require a CSR) to prohibit it. > > > 1. Need to decide what would be better way to fix > > DoubleSpinnerValueFactory wrapAround behavior? > > It might be helpful to compare it to the Integer flavor. By definition, > IntegerSpinnerValueFactory represents a set of discrete values: min and > max are two distinct values, and there is a well-defined ordering when > wrapping. If you wrap, it should be a continuous (in both positive and > negative directions) list of values like so: > > min, min+1, min+2, ... max-2, max-1, max, min, min+1, min+2, ... > > The formula is simple. After incrementing or decrementing by N * > amountToStepBy, do the following if wrapAround is true: > > if (val < min || val > max) val = (val - min) % (max - min + 1) + min; > > The above works regardless of step size because integers are naturally > discrete values. It is well-behaved and natural to consider max and min > being 1 apart from each other when wrapping. > > By contrast, it's harder to know what the right answer is for > DoubleSpinnerValueFactory. It's quite possible that in some cases, the > app expects the values of DoubleValue factory to also be discrete when > it comes to wrapping, meaning that min and max are distinct values such > that if your step was 1.0, you might expect to go from max-1.0 to max to > min to min+1.0 (although even that definition has problems for > non-integer values of amountToStepBy, but it's (mostly) solvable). It's > also quite likely for other cases -- especially ones that would lend > themselves to wrapAround mode in graphical apps -- where it isn't a set > of discrete values, but rather is such that min and max represent the > same value (i.e., the same end result). The simplest example is an angle > in degrees where 0.0 and 360.0 mean the same thing when used as a > rotation angle. In this latter use case, stepping from 359.0 to 360.0 to > 0.0 to 1.0 would cause a discontinuous pause in motion if you mapped the > output to a rotation angle. > > I'm not sure what the right answer is, but the latter seems like a valid > use case and I would guess at least as common as a "set of discrete > values" for doubles. Another thing to consider is that even in what I > will call "discrete" mode (no I'm not really proposing that we add a > property to control it), you have to take the step size into account; > otherwise when the step size is < 0.5 you will get stuck at max when > incrementing by 1 amountToStepBy, unless you have special case logic for > that. > > Any thoughts from other developers? > > -- Kevin >
