On Sun, 21 Dec 2025 21:07:30 GMT, Cormac Redmond <[email protected]> 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
