Add ImageUtilities methods to help migrating away from "new ImageIcon" (SVG icon related) by eirikbakke · Pull Request #8114 · apache/netbeans

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke · 2025-01-05T23:44:15Z

Background: To ensure that SVG icons are loaded and drawn at full resolution, direct use of ImageIcon constructors should be avoided in the NetBeans codebase. The ImageIcon instances returned from methods in ImageUtilities, by contrast, are instances of a special subclass of ImageIcon that support vector graphics painting.

During work to remove uses of "new ImageIcon" constructors in the codebase (including #8109), I see that a few new utility methods would be useful in ImageUtilities. This PR proposes adding the following methods to ImageUtilities:

loadIcon(String,boolean) works like loadIconImage(String,boolean) but returns only a plain Icon (not IconImage). This would help discourage use of IconImage in the future.
loadIcon(String) is equivalent to loadIcon(String,false)
toImageIcon(Icon) helps cases where an existing API requires an ImageIcon to be returned, but where we only have an Icon.
loadImage(URL) helps migrate away from uses of Toolkit.getDefaultToolkit().createImage(), which has similar problems as "new ImageIcon".

This PR also contains, in a separate commit, migration away from the Toolkit.getDefaultToolkit().createImage() method, using the new loadImage(URL) method. This makes, for instance, the "lightbulb" icons in the editor gutter show up properly with their new SVG icons:

eirikbakke · 2025-01-06T07:28:34Z

There is one more method I could add which might be useful, which is a mergeIcons(Icon,Icon,int,int) which works like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image. It could simplify some of the cases in #8109.

eirikbakke · 2025-01-07T14:08:16Z

I pushed a revision to the two commits in this PR. Changes since your previous review:

Change the new loadImage(URL) to loadImage(URI). Update call sites accordingly in the second commit.
Clean up the Javadoc in ImageUtilities to avoid repeating the explanation of resource path loading semantics (substitution of dark mode suffixes and SVG images) for every similar method. Some other Javadoc edits for grammar, consistency etc.
Add the ImageUtilities.mergeIcons(Icon,Icon,int,int) method (like mergeImages(Image,Image,int,int) but takes and returns Icon instances instead of Image) as well, for future use.

eirikbakke · 2025-01-07T15:29:05Z

(Latest push was just a rebase on master.)

mbien

looks good to me. Please check the comment I added.

mbien · 2025-01-23T23:39:42Z

don't forget to squash, looks like this could be 1-2 commits.

…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke · 2025-01-24T00:10:38Z

Squashed so that two commits remain: The API change in isolation, and the call sites that make use of it.

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

mbien · 2025-01-25T21:14:42Z

i give it a 75% chance that someone will complain that the light bulbs are too large - but I think i like them after running RC1 for ~2 days :)

eirikbakke · 2025-01-25T21:36:10Z

@mbien Yeah the old ones were scaled down a bit relative to the lightbulb-only icon. I might adjust it the next time I do a round of editing on these.

mbien · 2025-01-25T21:41:35Z

its fine - they are now using LEDs and were no longer available in the same size

This is a follow-up on apache#8114 and apache#8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

This is a follow-up on #8114 and #8109 . To render at full HiDPI resolution, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon. This PR, combined with the previously mentioned PRs, handles most of the remaining cases. Specifically: * Search for 'new ImageIcon(' and rewrite each case to use ImageUtilities to load icons instead. * Search for 'instanceof ImageIcon' and generalize to 'instanceof Icon' when appropriate. * Search for 'getLookAndFeel*getDisabledIcon' and switch to ImageUtilities.createDisabledIcon.

eirikbakke added API Change [ci] enable extra API related tests Platform [ci] enable platform tests (platform/*) UI User Interface labels Jan 5, 2025

eirikbakke requested review from matthiasblaesing and mbien January 5, 2025 23:46

eirikbakke mentioned this pull request Jan 5, 2025
Avoid remaining trivial uses of new ImageIcon(Image), for HiDPI icons #8109
Merged

mbien reviewed Jan 6, 2025
View reviewed changes

Comment threadplatform/openide.util.ui/src/org/openide/util/ImageUtilities.java Outdated
Comment threadplatform/openide.util.ui/src/org/openide/util/ImageUtilities.java Outdated

mbien added the Code cleanup Label for cleanup done on the Netbeans IDE label Jan 7, 2025

eirikbakke force-pushed the pr-moreimageutils branch 3 times, most recently from 4ffa602 to 04ca39cCompare January 7, 2025 14:00

eirikbakke force-pushed the pr-moreimageutils branch from 04ca39c to ce87d72Compare January 7, 2025 15:28

eirikbakke mentioned this pull request Jan 14, 2025
The icon for running a test method is not displayed correctly in the dark editor theme #8151
Closed

mbien linked an issue Jan 14, 2025 that may be closed by this pull request
The icon for running a test method is not displayed correctly in the dark editor theme #8151
Closed

mbien approved these changes Jan 15, 2025
View reviewed changes

Comment threadplatform/openide.util.ui/src/org/openide/util/ImageUtilities.java Outdated

mbien reviewed Jan 15, 2025
View reviewed changes

Comment threadplatform/openide.util.ui/src/org/openide/util/ImageUtilities.java

mbien added this to the NB25 milestone Jan 15, 2025

mbien approved these changes Jan 23, 2025
View reviewed changes

Add methods to ImageUtilities, to make migration from other image loa…
05f28f6
…ding methods used throughout the codebase easier. (Additive API change.) Edit Javadoc to avoid repeating semantics that are common to many methods. Some other Javadoc cleanup.

eirikbakke force-pushed the pr-moreimageutils branch from e58bba7 to 46e8409Compare January 24, 2025 00:09

Replace most uses of Toolkit.getDefaultToolkit().getImage(URL) with I…
46e8409
…mageUtilities.loadImage(URI), to make SVG icons work in these cases.

eirikbakke merged commit 3fa8212 into apache:masterJan 24, 2025

eirikbakke mentioned this pull request Jan 25, 2025
More work on centralizing icon loading to ImageUtilities #8194
Merged

eirikbakke mentioned this pull request Feb 12, 2025
Handle both nbres and nbresloc protocols in ImageUtilities.loadImage(URI) #8239
Merged

eirikbakke mentioned this pull request Oct 15, 2025
Suppress 'loadImage called with unusual URI: ergoloc:/...' log message #8917
Merged