RFR: 8373650: Test javax/swing/JMenuItem/6458123/ManualBug6458123.java fails because the check icons are not aligned properly as expected [v2]

[Alexey Ivanov]

On Mon, 5 Jan 2026 07:01:34 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 922:
>> 
>>> 920:                                                 x + OFFSET,
>>> 921:                                                 (icon.getIconHeight() <= 16) ? y + OFFSET : (y + icon.getIconHeight() / 2), state);
>>> 922:                                     } else if (icon.getIconWidth() <= 16) {
>> 
>> I don't understand why painting an icon and check marks / radio bullets in RTL case depends on the width of the icon but there's no such dependency in LTR case.
>> 
>> Shouldn't the performed menu layout handle these cases?
>
> As I see, MenuItemLayout handler's x position calculation for RTL depends on icon rect width
> https://github.com/openjdk/jdk/blob/6eaabed55ca4670d8c317f0a4323ccea4dd0b9ca/src/java.desktop/share/classes/sun/swing/MenuItemLayoutHelper.java#L737-L741
> but LTR doesn't depend on rect width
> https://github.com/openjdk/jdk/blob/6eaabed55ca4670d8c317f0a4323ccea4dd0b9ca/src/java.desktop/share/classes/sun/swing/MenuItemLayoutHelper.java#L726-L730

Okay, but why do you have tweak the layout produced by `MenuItemLayoutHelper`?

Do Metal and Nimbus have to tweak the layout too?

Is it because Windows icons used to be special and combined the icon and check mark / bullet mark, or rather replaced the check mark / bullet mark with the icon? Should we get rid of that special treatment?

I'd like to have answers to these questions.

To be clear, I'm not opposing this approach, it seems to work, yet it seems superfluous, the code kind of repeats calculations. Both Metal and Nimbus L&F have similar column layouts where a separate column is allocated for marks and icons, Windows L&F should re-use that code.

>> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java line 936:
>> 
>>> 934:                                             skin.paintSkin(g,
>>> 935:                                                 (type == JRadioButtonMenuItem.class) ? (x + 3 * OFFSET) : (x + 4 * OFFSET),
>>> 936:                                                 (icon.getIconHeight() <= 16) ? y + OFFSET : (y + icon.getIconHeight() / 2), state);
>> 
>> I wonder why painting the skin for radio button menu requires such a fix up whereas it's not required for `JCheckBoxMenuItem`.
>
> it is taken care via ternary operator, width/spread of check mark is usually more than radio bullet so starting point of checkmark is considered a little ahead compared to radio bullet

I see that it's taken care by the ternary operator. Why is it needed, though?

The skin for both check mark and radio bullet have the same size, and the mark is positioned accordingly inside the rectangle. This works well for the LTR layout, both marks are rendered at the same location and appear aligned in the end. Why does RTL layout treat these marks differently?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28889#discussion_r2694197701
PR Review Comment: https://git.openjdk.org/jdk/pull/28889#discussion_r2694204711