RFR: 8366739: ToolBar: overflow menu with fractional scale (2)

[Cormac Redmond]

On Sun, 21 Dec 2025 16:10:45 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Ah yes, you are right, I skipped over the `+=` operation; indeed it also needs snapping.  It is simply the reality that even when values are already snapped, adding two snapped values together can still introduce a rounding error that must be resnapped.
>
> Should we snap the (here intermediate) value of `getSpacing()` as well? We could do that once before the whole for loop, and use the value. But I'm not sure if that even makes a difference?

Well, it is already snapped in its own way:


    private final double getSpacing() {
        return spacing == null ? 0.0 : snapSpacing(spacing.get());
    }


And even when it is snapped, once there's potential for decimal component (i.e., introduced by 1.25 scaling) in its value, then there's a risk of these floating point rounding errors when adding it to anything else.

So I don't think it makes any difference to what is done with the spacing value. I think what matters is that the final value of x is rounded off, which is really the point of snapPosition. 

This snapPosition on x call could be done at the end, outside of the for loop either, I imagine, also. As long as a final value looks like 123.62, and not 123.62000000000001.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/2016#discussion_r2637973119