On Fri, 11 Jul 2025 10:16:28 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> When JRadioButtonMenuItem is called with imageIcon, then only imageIcon is
>> shown without radiobutton in WIndowsLookAndFeel as there was no provision of
>> drawing the radiobutton alongside icon.
>> If icon is not there, the radiobutton is drawn. Added provision of drawing
>> the radiobutton windows Skin even when imageIcon is present.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Adjust offset for varying size imageicon
src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java
line 932:
> 930: skin.paintSkin(g, x + OFFSET, y +
> OFFSET, state);
> 931: } else {
> 932: if (icon.getIconHeight() > 16) {
Why does height > 16 matter ? How does it make icons align ?
Also - if that is satisfactorily answered then wouldn't the conditions here be
more succinctly coded as
if (icon == null || icon.getIconHeight() > 16) {
skin.paintSkin(g, x + OFFSET, y +
icon.getIconHeight() / 2, state);
} else {
skin.paintSkin(g, x + OFFSET, y +
OFFSET, state);
}
src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsIconFactory.java
line 947:
> 945: } else {
> 946: icon.paintIcon(c, g, x + 6*OFFSET,
> 947: y + OFFSET);
Where did the magic multiplier of 6 come from ?
src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsMenuItemUI.java
line 248:
> 246: paintIcon(g, lh, lr, holdc);
> 247:
> 248: boolean isWindows11OrLater =
> Integer.parseInt(System.getProperty("os.name")
Haven't we learned that it 'worked' on Windows 10 in an accidental and not
ideal way?
ie. it didn't really work even there.
So do we even need to bother with this check ? Shouldn't Windows 10 do it the
new way too ?
test/jdk/javax/swing/JMenuItem/TestRadioAndCheckMenuItemWithIcon.java line 31:
> 29: * JRadioButtonMenuItem and JCheckboxMenuItem
> 30: * is rendered with ImageIcon in WindowsLookAndFeel
> 31: * @requires (os.family == "windows")
Can't we run this with all L&Fs on all platforms ?
What about it would be so different in some other case ?
Also that last update for different sized icons isn't being tested - so can we
vary the icon size in this test ?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2201467717
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2201467227
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2201466381
PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2201463390
