Hello, Alexander.
The fix looks good to me too. Thanks!
On 3/14/14 10:38 PM, Petr Pchelko wrote:
Hello, Alexander.
The updated version of the fix looks good to me.
With best regards. Petr.
14 марта 2014 г., в 7:53 после полудня, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> написал(а):
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8035069/webrev.01
On 3/13/2014 12:21 PM, Petr Pchelko wrote:
Hello, Alexander.
1. As Sergey always says, could you please split the long lines.
2. Instead of the MultiResolutionImageMapper you could use a BiFunction<Image,
Integer, Integer>
3. About the ImageCache. As it's uses an AppContext, could you please mention
in the JavaDoc that is must be used from the thread with an AppContext only?
1. 2. and 3 are updated.
4. I don't really like that you are duplicating the RecyclableSingleton class.
May be it's better to make also move it out from com.apple.laf and reuse?
I have added the getSoftReferenceValue(Object key, Supplier<T> supplier)
method to the AppContext class. It should reduce the code duplication.
5. Looks like the old ImageCache contained the following lines:
116 if (state.is(JRSUIConstants.Animating.YES)) {
117 return false;
118 }
I agree that these are probably not needed, but could you please verify that?
Also after these were removed the ImageCache.setImage never returns false, so
it could be made void.
Thank you for pointing it out. This is the necessary check for the animated
images in the Aqua L&F. I just forgot to move it to the AquaPainter.
I have found one more issue that ImageIcon preloads images by calling
image.getProperty("comment", imageObserver) where the imageObserver
is usually null. The MultiResolutionBufferedImage created non-preloaded
resolution variants and they were not shown because JMenuItem as an image
observer
returns false for the image loading. This is described in the issue 8037405
JMenuItem should check L&F icons in the image observer
https://bugs.openjdk.java.net/browse/JDK-8037405
It seems as a common problem so I added resolution variants preloading to
the MultiResolutionBufferedImage.
Thanks,
Alexandr.
With best regards. Petr.
On 12.03.2014, at 19:03, Alexander Scherbatiy <alexandr.scherba...@oracle.com>
wrote:
Hello,
Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8035069
webrev: http://cr.openjdk.java.net/~alexsch/8035069/webrev.00
Image resolution variants are generated by request and cached in the
ImageCache.
- ImageCache is refactored to store different type of images and moved to
sun.awt.image package.
- An object is used as the cache key instead of hash code to prevent
inetsection of hash codes for
different type of images.
- The base image for MultiResolutionBufferedImage is not cached and used for
the hash code calculation
in the getResolutionVariant method.
Thanks,
Alexandr.
--
Best regards, Sergey.
