On Sun, 21 Dec 2025 10:30:50 GMT, Cormac Redmond <[email protected]> wrote:

>> 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...

Ah yes, you are right, I skipped over the `+=` operation; indeed it also needs 
snapping.  It is simply the reality that even when values are already snapped, 
adding two snapped values together can still introduce a rounding error that 
must be resnapped.

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

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

Reply via email to