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

Cormac Redmond duke at openjdk.org
Sun Dec 21 21:48:52 UTC 2025


On Sun, 21 Dec 2025 21:07:30 GMT, Cormac Redmond <duke at openjdk.org> wrote:

>> `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 (in the case where the overflow is displayed):
> 
> 
>         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`).

An example of length rounding due to "-=" operations (again, at 1.25 scale), when overflow should show:

<img width="855" height="247" alt="image" src="https://github.com/user-attachments/assets/aca455ab-0d32-4e46-9868-78d786cd4a7b" />

I can't (easily) get it to create any actually problems for me, but the question remains, should be re-snap it also?

If it's not (demonstrably) broken, don't fix it, maybe.

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

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


More information about the openjfx-dev mailing list