On Sun, 21 Dec 2025 20:37:25 GMT, John Hendrikx <[email protected]> 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
