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

[Cormac Redmond]

On Sun, 21 Dec 2025 20:37:25 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> @hjohn: I'm a bit concerned that given snapPositionX/Y is basically just rounding (up or down), that x could still be wrong when compared to "length" in some scenarios (which I cannot produce)...any high-level thoughts on that?
>> 
>> I'm not up-to-speed on the various operations in ScaledMath and what assumptions I can make as true/false...
>
> `length` is also supposed to be rounded with a similar function; if that's the case, then this should always work correctly.  Floating point operations may introduce slight errors, but if you round them to whole pixels (whether those pixels are of size 1, 0.75, 0.66666 or 0.5) the resulting value should always be the same for the same pixel position/size.
> 
> When I look at the values used in such calculations, and I see something weird like 0.6666664 or 1.000001, then I know it is not properly rounded; so if `length` looks good then it should work correctly.  Alternatively, you can try and see where `length` comes from and if it was the result of some snapping function.

Sorry, I should have mentioned before.

`getOverflowNodeIndex(double length),` the method I've changed, is called in two places from the same method:  `organizeOverflow(double length)`.

The `length` value given into _that_ method, does come from a `snapSizeX/Y` on the computed length, _originally_, (which is a ceil); so that's fine. But there are other concerning operations in `organizeOverflow(double length)` thereafter:


  // and the overflow index recalculated.
        if (newOverflowNodeIndex < getSkinnable().getItems().size()) {
            if (getSkinnable().getOrientation() == Orientation.VERTICAL) {
                length -= snapSizeY(overflowMenu.prefHeight(-1));
            } else {
                length -= snapSizeX(overflowMenu.prefWidth(-1));
            }
            length -= getSpacing();
            newOverflowNodeIndex = getOverflowNodeIndex(length);
        }


E.g., we're doing more floating point operations there, and not rounding or snapping thereafter before calling `getOverflowNodeIndex(double length)`, which also does nothing (to `length`).

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

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