HiDPI icons for window system icons on Windows and Mac #859
Conversation
This commit introduces vector-drawn Icon implementations for the icons used in the window system's Windows 8 Look-and-Feel. Specifically, this replaces most of the bitmap icons that are used on Windows from the o.n.swing.tabcontrol and openide.awt modules. An abstract class VectorIcon is added in the UI Utilities Module (openide.awt) as a general-purpose starting point for creating new vector icons. It handles and documents a number of tricky adjustments that are needed to draw icons that appear sharp on HiDPI screens. This class can be used to implement HiDPI icons in other LAFs at a later time, e.g. for MacOS retina displays. A small utility, VectorIconTester, was written to preview new icons at multiple resolutions, as well as to compare them with existing bitmap icons. This is less polished code that is nevertheless included here, in tabcontrol's test package. A screenshot of its output, as well as screenshots of the NetBeans IDE before and after the patch on various scaling levels, are attached to the JIRA ticket for NETBEANS-1238.
|
Note that this PR introduces a new public class VectorIcon in the openide.awt module, so it should increment that module's API version before being merged. (I don't think I have permission to add the "API Change" label to the pull request.) |
…code cosmetic/comment adjustments.
This commit vectorizes window system icons for the Aqua (MacOS) LAF, as was done for the Windows LAF in the previous commits. This makes NetBeans look good on MacBook Pro and other Apple monitors that use the 2x "Retina" resolution scaling by default. While the new icons match the old ones exactly in terms of size and alignment, I took the opportunity to make a few modernizing improvements. See the Javadoc for the new class o.n.swing.tabcontrol.plaf.AquaVectorTabControlIcon for a full description. Screenshots will be attached to the JIRA issue and pull request.
eirikbakke
changed the title
|
Don't know enough to comment on the API change, but I have ran this PR locally and verified the Icons show as expected and do look a lot better. Thanks @eirikbakke |
|
Are these icons for specific look and feels only? (Hence why only for Mac and Windows and not Linux) |
|
@darkyellow Yes, these are only the icons used in the window system itself, which are specific to the look-and-feel. Most other icons (in the project pane etc.) are not LAF-specific. |
|
Seems to be agreement here and no objections and these UI changes make NetBeans better, not worse. Merging. |
|
Before merging, let’s discuss the need for an API version change being needed. Has this been done, should something additional be done? |
|
Looking at the various utilities modules, I propose putting the new VectorIcon class in org.openide.util.ui ("Utilities API") rather than in org.openide.awt ("UI Utilities API"), so that it's in the same module as ImageUtilities, in case the latter ever needs to depend on VectorIcon for future HiDPI improvements. Does that make sense? (Attaching a diagram of the existing module dependencies that I just drew for myself.)
I can make another commit that does the appropriate changes to manifest.mf, project.xml, apichanges.xml etc.--it's a good time to learn it. (I'll use https://github.com/apache/incubator-netbeans/pull/848/files as a guide.) |
This will put VectorIcon in the same module as ImageUtilities. ImageUtilities might at some point need to make use of VectorIcon, for other HiDPI-related improvements. Note that openide.awt depends on openide.util.ui (not the other way around).
… VectorIcons class. Added some documentation.
|
I moved VectorIcon into org.openide.util.ui together with ImageUtilities, updated the API versions and added an entry to apichanges.xml. Also tested again on Windows to confirm nothing broke. |
|
This commit is ready to merge if no one objects to the addition of the "VectorIcon" class to the public API of org.openide.awt module. |
|
@emilianbold Would you mind having a quick look to OK the API change here, which is just the addition of a new utility class "VectorIcon"? I looked at the retina-related work you did in the past. This PR should should not interfere with any of that, as this PR only touches the LAF-related icons, and does not change ImageUtilities in any way. It would be great to eventually merge your work as well. (There are basically two possible approaches to doing HiDPI icons: either via MultiResolutionImage, or via vector-painted custom implementations of the Icon class. I think both approaches should be supported--this PR deals with the vector painting approach, while your previous retina work deals with the MultiResolutionImage approach. The MultiResolutionImage approach currently does not work properly on Windows, due to https://bugs.openjdk.java.net/browse/JDK-8212226 , but surely it will work in the long-term.) |
There was a problem hiding this comment.
Oh this is just excellent stuff! And so great to do this at LnF level.
The patch is basically flawless: tight, with comments, tests. What more to ask?
One thing I wonder is if this won't get in the way of some branding attempts? I see
closeTabImage = ImageUtilities.loadImageIcon("org/openide/awt/resources/win8_bigclose_enabled.png", true); // NOI18N
being replaced with Windows8VectorCloseButton.DEFAULT.
I'm also curious how this works in dark mode or with something like Darcula.
Other than these minor concerns a big +1.
|
I just tested with Darcula on both Windows and MacOS. Darcula is unaffected, except for the non-LAF specific icons (arrow, toolbar_arrow_horizontal/vertical), which do get applied to Darcula as well. Those will need some more contrast on dark themes; I can take care of that in a separate commit. Looking at Darcula, its LAF icons seem to be based on the Windows 8 theme. So it should be fairly easy to adapt this work to that plugin. But that would have to be done on the Darcula side. Separate work would also have to be done to vectorize icons on whichever LAF is the default on Linux. Yes, branding attempts that replace the PNGs but not the LAF identifier will be thwarted by this commit. I think that's OK, except for the contrast issue noted above. I'll merge this patch and start creating some issues for future HiDPI work. |
|
Fixed dark theme appearance for the three LAF-independent icons at 8f1f57b . Tested on Darcula on Windows. |
|
Created an issue https://issues.apache.org/jira/browse/NETBEANS-1583 for the future, once we eventually want to update ImageUtilities to work with MultiResolutionImage instances. |
(Update: This PR now includes new window system icons for both Windows and
MacOS. Since this is the same work done twice with small visual variations, I am
combining these in a single PR.)
This pull request introduces vector-drawn Icon implementations for the icons
used in the window system's Windows 8 and Aqua (MacOS) LAFs. Specifically, this
replaces most of the bitmap icons for these two LAFs o.n.swing.tabcontrol and
openide.awt modules.
An abstract class VectorIcon is added in the UI Utilities Module (openide.awt)
as a general-purpose starting point for creating new vector icons. It handles
and documents a number of tricky adjustments that are needed to draw icons that
appear sharp on HiDPI screens. This class can be used to implement HiDPI icons
in other LAFs at a later time, e.g. on Linux.
A small utility, VectorIconTester, was written to preview new icons at multiple
resolutions, as well as to compare them with existing bitmap icons. This is less
polished code that is nevertheless included here, in tabcontrol's test package.
A screenshot of its output, as well as screenshots of the NetBeans IDE before
and after the patch on both Windows and MacOS, are attached to the JIRA
tickets below and attached to this pull request as well.
The relevant JIRA tickets are:
https://issues.apache.org/jira/browse/NETBEANS-1238 (for Windows)
https://issues.apache.org/jira/browse/NETBEANS-1260 (for MacOS)
To test this patch on Windows, use Windows 10 on Java 9 or Java 10 with a HiDPI
screen, and use the workaround in
https://issues.apache.org/jira/browse/NETBEANS-1227 to tell Windows to let
NetBeans manage DPI scaling on its own.
To test this patch on MacOS, use a MacBook Pro.
NetBeans screenshots before and after patch on Windows (be sure to view at 100% resolution):
NetBeans screenshots before and after patch on MacOS (be sure to view at 100% resolution):
Output of the VectorIconTest utility, showing the old icons next to the new scalable ones:
