-
Notifications
You must be signed in to change notification settings - Fork 917
[NETBEANS-2614,NETBEANS-1586] Improve icon scaling on HiDPI displays, and prepare ImageUtilities for HiDPI icons #1273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The ImageUtilities class now supports HiDPI icons via custom implementations of the Icon interface. For this to work, client code must let ImageUtilities manage the creation of new Icon instances, using "ImageUtilities.image2Icon(image)" instead of "new ImageIcon(image)". This is already common throughout the NetBeans codebase, but there are still quite a few direct uses of ImageIcon. This commit replaces a few of the direct uses of ImageIcon, for most of the icons that are commonly seen during a Java editing session in the IDE.
Most of the icons that are seen in the IDE during a standard Java editing session are already painted via a call to Icon.paintIcon. One exception is the separator icon that is used in the breadcrumb component under the code editor, which is painted via Graphics.drawImage. Migrate this one to Icon.paintIcon to make the icon look better on HiDPI displays (taking advantage of the HiDPI scaling hints that were recently implemented in ImageUtilities). This also serves as an example of how to fix other similar cases in the future.
…I icons Introduced a FilteredIcon class which extends from a new CachedHiDPIIcon class. The CachedHiDPIIcon class will be used again in the future; it was originally designed for an SVG-based Icon implementation, which will be contributed separately in the future. It is generic enough to work with any kind of custom Icon implementation that needs caching. Also make color filtering of icons for dark themes work with HiDPI icons, using the same FilteredIcon class. Also improved a null handling case in ImageUtilities.loadImage. Passing a null resource used to be valid before (yielding a null return value), but caused a NPE on dark LAFs. Since it worked in the common non-dark case, it should arguably work in the dark case as well. This might fix NETBEANS-2401, or it might just lead to an NPE elsewhere in that case.
…olTipImage is being passed around. No public APIs are affected.
This commit prepares ImageUtilities to work with scalable Icon implementations, such as a future SVGIcon class. Conversions such image2icon(icon2Image(icon)) are now lossless with respect to scalable Icon implementations. This allows HiDPI support to be implemented via custom Icon classes while keeping existing Image-based APIs (such as org.openide.nodes.Node.getIcon) unchanged. The internal ImageUtilities.ToolTipImage class, which also implements the Icon interface, is now used in most cases for ImageUtilities methods that return an Image. Modifying ToolTipImage's paint() method to paint an extra debugging stroke yields a quick confirmation that most of the icons visible in the IDE now successfully have their painting delegated through this method. This will come in handy in a subsequent commit, when we try to do some centralized scaling improvements for HiDPI displays. It is no longer safe to assume that ImageUtilities.image2Icon returns an ImageIcon. Grepped the entire codebase for such casts (searched for cast to ImageIcon within 3 lines of 'image2Icon'), and found only one, which was fixed. Manually tested, including for the case where ImageUtilities.getImageIconFilter() returns a non-null filter. Fixed a deadlock bug that was unearthed in the latter case.
Having centralized icon painting into ImageUtilities.ToolTipImage.paintIcon in the previous commits, we can now add a few painting tweaks to improve the appearance of low-resolution bitmap icons that must be scaled up for HiDPI displays. See the before/after screenshots that are attached to JIRA ticket NETBEANS-2614.
The old Image-based implementation of mergeImages remains together with the new Icon-based implementation. This ensures backwards-compatibility when icons are drawn using Graphics.drawImage instead of Icon.paintIcon. (The new implementation was confirmed to be invoked and working by adding some now-removed temporary drawing code.) This completes the changes needed to make all of ImageUtilities' APIs work with scalable Icon implementations. An SVGIcon class will be added in a future commit.
JaroslavTulach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel qualified to review the graphics parts. If the existing tests pass and if some new are written, then OK.
java/jshell.support/src/org/netbeans/modules/jshell/editor/CommandCompletionProvider.java
Show resolved
Hide resolved
…1273. When the aforementioned PR is merged, this branch should be rebased to drop this commit. See apache#1273 .
…1273. When the aforementioned PR is merged, this branch should be rebased to drop this commit. See apache#1273 .
…1273. When the aforementioned PR is merged, this branch should be rebased to drop this commit. See apache#1273 .
This was left over from a previous experiment. Also fix a case where ImageUtilities.icon2Image could return null even when the icon is not null. (Include this fix in this PR rather than in the SVG support PR, to ensure I don't commit bugs if this one is merged before the other.)
|
Fixed a few test failures, and added a bunch more tests. I have had this patch running in my IDE for a week; will soon merge this one (and then continue work on the separate SVG support PR). |
|
Just tested this patch (+the SVG patch) on MacOS as well. Seem not to be causing problems. Will merge this one. |
This PR contains most of the changes that are needed to make ImageUtilities work with HiDPI icons. No publicly visible API changes are needed; the old javax.swing.Icon interface is actually well-suited for providing scalable graphics. The main challenge is to ensure that conversions between javax.swing.Icon and java.awt.Image, which are frequent throughout the NetBeans codebase, always retain the underlying scalable Icon instance.
The main motivation for this PR is to enable SVG icon support, which I will contribute in a separate PR. There's an added bonus, though: With all icon painting now centralized through ImageUtilities.ToolTipImage.paintIcon, we can add some rendering tweaks that make existing low-resolution icons look slightly better on HiDPI displays. See the attached before/after screenshots.
This issue is tracked under https://issues.apache.org/jira/browse/NETBEANS-2614 . SVG icon support is tracked under https://issues.apache.org/jira/browse/NETBEANS-2604 .
Credit: Emilian Bold did a lot of initial work on this before, at emilianbold/nextbeans@0f99dba . The main improvement in this PR is that arbitrary HiDPI scaling factors are supported. This is important on Windows, where non-integral scaling factors such as 150% can occur (MacOS, in contrast, always uses 200% for Retina screens). This PR also avoids the use of MultiResolutionImage, which does not work properly on Windows (see https://bugs.openjdk.java.net/browse/JDK-8212226 ).
To make reviewing easier, I have split up this PR into several commits, which can be reviewed one-at-a-time.
(To compare before/after screenshots, be sure to view the images at 100% scaling.)



