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

[Marius Hanl]

On Sun, 21 Dec 2025 13:15:53 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

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

Should we snap the (here intermediate) value of `getSpacing()` as well? We could do that once before the whole for loop, and use the value. But I'm not sure if that even makes a difference?

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

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