[OpenJDK 2D-Dev] <AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays

[Jim Graham]

Hi Alexander,

On 11/25/13 5:51 AM, Alexander Scherbatiy wrote:
>    Could you review the updated fix:
>      http://cr.openjdk.java.net/~alexsch/8011059/webrev.10/
>
>   - FLIP and TRANSLATE bit masks are also included for the SCALE
> transform checking

Looks good.

>   - LWCToolkit checks an image in cache before requesting
> multi-resolution image creation to prevent excessive string manipulation.
>     It needs to make imgCache accessible from the LWCToolkit or move
> image2x staff calculation to the SunToolkit to use the right
> synchronization.

Eventually we will probably want to make @2x supported on all platforms 
for cross-platform compatibility (not so much the behavior on retina 
displays that only exist on Mac for now, but since it is invoked by 
scaled rendering, then other platforms also run into the conditions for 
these images).

I think it's OK to release this as Mac-only for now, but I think we 
should eventually strive for a more unified HiDPI support on all platforms.

>     imgCache is not created per Application context so there can be
> issues to make it protected. The image2x staff does not relate to
> SunToolkit.
>     That is why all staff related to ToolkitImage creation used to move
> into ToolkitImageUtil in some previous fix. I just skipped the
> synchronization for this particular case.

We can probably take care of that later when we unify the HiDPI media 
handling?

>   - The containsResolutionVariant method is removed from the
> MultiResolutionImage

OK.  The main thing was that we not require its use for @2x to "just work".

>   - The image observer is nullified in drawImage(Image,..ImageObserver)
> method for the resolution variant.
>     The original image is always loaded because it needs to know the
> image width and height for the resolution variant size calculation.
>      Only the original image sends notifications that an image is
> loaded. The resolution variant is drawn silently so it does not break
> existed image observer implementation.

If they only ever use the image on a retina display (or some scaled 
context) then I don't think we ever send any notifications the way the 
current fix is written.  Also, if you don't send notifications for the 
scaled variant, but load the original and expect its notifications to 
suffice, then we have a race condition - if the original variant is 
fully constructed before the scaled version is done then the final "OK 
to draw everything" notice would not happen and they'd be left with a 
partial image.

I think it should be easy to pass along the observer and simply have 
ImageRep translate the image variant into the base image in its newInfo 
method (and there are a couple of other locations that imageUpdate is 
called that could do the same).  All it takes is adding 
"ToolkitImage.set/getBaseImage()" to ToolkitImage, defaulting to "this", 
but the Multi-Res image will set the base image on both of its variants 
to the Multi-Res instance.

Then we get full notices of all resolution variants.

It would be best to scale the xywh supplied there as well.  That should 
be fairly easy, but if it is a big problem then that is lower priority 
than getting the "ALLBITS" notification right, as that is the main 
trigger for so many uses.

(In the future we can add ImageObserver2 (or MultiResObserver?) which 
has support for receiving notifications of variants without translation.)

I'll need some time to read the other information about what was done in 
7 and respond in a separate email.  And, again, my apologies for not 
having been in the loop for that part of the process...

			...jim