[NETBEANS-1586] Make ImageUtilities.createDisabledIcon work with HiDPI icons

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .

[eirikbakke]

After the introduction of the org.openide.util.VectorIcon class in #859 , ImageUtilities.createDisabledIcon should be updated to work with such resolution-independent icons, as well as ImageIcon instances that delegate to a MultiResolutionImage. This patch achieves that, without making assumptions about the specific Icon implementation. Tracked as https://issues.apache.org/jira/browse/NETBEANS-1586 .

The new ImageUtilities.createDisabledIcon implementation was tested with both a VectorIcon and an ImageIcon, including with the latter delegating to a MultiResolutionImage, on OpenJDK 10.0.2 on Windows 10 with a HiDPI/non-HiDPI dual monitor setup. Also tested the VectorIcon case on MacOS with Retina and non-Retina configurations.

(This is the only change to ImageUtilities I'm doing for now. The other methods in ImageUtilities will eventually need to be updated to support HiDPI icons as well. That's left for future work, tracked in https://issues.apache.org/jira/browse/NETBEANS-1583 )

[eirikbakke]

The diff comes out confusing; for reviewing purposes, I recommend looking at the raw Java file instead. The private static LazyDisabledIcon class is replaced by the private static DisabledIcon class, and LazyDisabledIcon.createDisabledImage is moved to the private static DisabledButtonFilter class.

[emilianbold]

I doubt icons will be moved around different screens so much. Thus, wouldn't some cache still make sense? Worse case scenario, if the cached isn't a good fit, we create another image.

There's also some leftover debug lines.

[eirikbakke]

Strictly speaking, it's not possible to cache the image here, because we don't know what part of the "Component c" parameter's state the delegate's paintImage method might use. If said state changes, we'd have to invalidate the cache. Though the previous code didn't seem to care about this. There's also a fair bit of extra logic that would have to be added to check if the cache needs to be updated (comparing the FontRenderContext, ensuring thread-safety etc.). I figured the added complexity was not worth it, given that icon images (even HiDPI ones) are typically tiny. If you disagree, and think it's preferable to ignore the "Component c" state problem, I'm happy to add caching logic.

I can remove the deactivated debugging lines.

[emilianbold]

You probably know more about this area than me. There are a few aspects in my mind here:

• generally speaking icons won't move around displays with different scalings. So, it seems better to always cache and risk a bad icon when move than to never cache and pay some computational cost for every repaint. Not caching is correct, but is it really worth the price? (Then again, we have no measurement of the actual cost).

• the old (existing?) NetBeans code cared a lot about image caching, so I assume they had some metrics for this fear.

• there is something odd about the design too. So DisabledIcon wraps an Icon. But then during paint time: it creates a temporary image, paints the icon there, creates a disables image then paints that. Considering icons are the closest we have to vector graphics (ie. non-raster) in AWT/Swing, this roundtrip is annoying to me. I guess I'm surprised we can't do RGBImageFilter at Graphics2D level, like we can do a setTransform.

[eirikbakke]

I added another commit that implements caching. That should at least preserve the previous performance-related behavior.

As far as I know there is no way around the need for a BufferedImage here.

[eirikbakke]

Any objections to this latest version? I've been running my IDE with this patch since November without issue.

[eirikbakke]

I'll be adding a PR that supersedes this one in a while, so you can hold off looking at this one.

[eirikbakke]

There's a new version of this code at ba22cd4 now. I separated the caching logic into an abstract superclass which will also be used for a future SVGIcon class.

Closing this PR since it's now superseded by #1273 .