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

John Hendrikx jhendrikx at openjdk.org
Sun Dec 21 22:11:05 UTC 2025


On Sun, 21 Dec 2025 21:42:51 GMT, Cormac Redmond <duke at openjdk.org> wrote:

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

Yeah, that's going to cause problems at some point.  It must be resnapped when doing comparisons with it.

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

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


More information about the openjfx-dev mailing list