On Sun, 21 Dec 2025 03:36:47 GMT, John Hendrikx <[email protected]> wrote:
>> Fix overflow menu triggering due to floating-point precision error.
>>
>> At 1.25 display scaling on Windows, floating-point comparison errors (e.g.
>> 109.60000000000001 > 109.6) cause the overflow menu to appear when the
>> lefthand value is regarded as larger than the righthand value.
>>
>> These should be treated as equal (and therefore not display the overflow
>> menu).
>>
>> This bug can happen in both horizontal and vertical toolbar orientation.
>>
>> The new tests added fail without this fix, and pass with it. An existing
>> test has been re-factored slightly to allow re-use and more flexibility in
>> specifying the scene's root node.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ToolBarSkin.java
> line 706:
>
>> 704: // Use a small epsilon (1e-9 / 0.000000001) to tolerate
>> floating-point rounding error when comparing
>> 705: // doubles. E.g. 117.60000000000001 should be regarded as
>> equal to 117.6.
>> 706: if (x - length > 1e-9) {
>
> We don't really use tolerances like this in the UI code; instead the problem
> is almost always because some floating point operations were done, but the
> result wasn't (re)snapped.
>
> In the code above I see that `x` is modified without re-snapping. I think a
> `snapPositionX/Y` should be applied on `x`, something like this:
>
> if (node.isManaged()) {
> if (getSkinnable().getOrientation() == Orientation.VERTICAL) {
> x += snapPositionY(snapSizeY(node.prefHeight(-1)) +
> getSpacing());
> } else {
> x += snapPositionX(snapSizeX(node.prefWidth(-1)) +
> getSpacing());
> }
> }
Thanks @hjohn.
The problem here is caused by the repeated operations on x itself (+=), not the
added getSpacing(), so the bug _still_ happens with the above (although, I'm
sure getSpacing() could be a causing factor also if the value was something
particular; in my case I haven't noticed it so so far).
Anyway, this minor change fixes it and I think is in the spirit of your
suggestion:
if (node.isManaged()) {
if (getSkinnable().getOrientation() == Orientation.VERTICAL) {
x = snapPositionY(x + snapSizeY(node.prefHeight(-1)) + getSpacing());
} else {
x = snapPositionX(x + snapSizeX(node.prefWidth(-1)) + getSpacing());
}
}
Pushed now...
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2016#discussion_r2637734595
