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
