<AWT Dev> [9] Review request for 8035069 [macosx] Loading resolution variants by demand

[Alexander Scherbatiy]

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.scherbatiy at 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.
>>