<AWT Dev> [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Jim Graham
james.graham at oracle.com
Wed Nov 27 15:21:24 PST 2013
The things I was talking about in the quoted email were mostly future
directions, but I did point out some issues in another email that should
be addressed before FCS. In particular:
We do need to get the callbacks working whether they draw hi res or not.
I think the current fix delivers no notifications at all because the
observer is not registered anywhere during the course of a drawImage().
That's a big bug. I don't think we need to wrap observers to do that,
there are only a few helper functions that deliver the notifications and
they can be taught how to translate a resolution variant into the base
image easily enough.
You point out that there was no error checking for the hidpi image. I
agree. Also, if the base image errors out, should we draw the @2x
anyway? Or should that base image be required? Minimally I think we
need to not attempt the @2x for FCS if it errored and we can worry about
how to respond to errors in the base image later.
I think that com.sun would require CCC approval for a new interface and
the bar is really high for API in 8.0 right now. Unless a developer
really needs to use it it might be best to move the API to sun.*. I
didn't see any developers clamoring for it in the discussions I read,
but let me know if I missed something. If we get automatic use of @2x
images then I think that satisfies the discussions I saw.
MediaTracker defers to Component.prepareImage() for which variants
should be loaded. I think we need to teach prepareImage() to trigger
the variant appropriate for the screen that the Component is on (perhaps
the default screen if it isn't displayed yet).
Also, there should probably be some MediaTracker, ImageIcon, and Cursor
test cases that show that those mechanisms all work on images that have
@2x variants. Minimally they shouldn't fail entirely, and very
desireably they should use the @2x version when appropriate, but that
could be a follow on bug fix as long as they don't fail for the first fix.
I've included below the quote from my last "review" email about the
specific issues I found. The main "blocking" issue right now is that it
looks to me that no imageUpdate notifications at all happen for a hidpi
image due to the ImageObserver getting dropped on the floor. Can we at
least get imageUpdate() called with the base image, even if the xywh are
wrong before we push the initial fix? I hope my info below helps make
that an easy task.
> 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.)
...jim
On 11/27/13 6:28 AM, Sergey Bylokhov wrote:
> On 27.11.2013 15:12, Sergey Bylokhov wrote:
>> Hello.
>> Probably we are in a point when it is necessary to stop and move out
>> all extensions to the separate bugs.
>> We should give a time to our sqe to test these changes.
>> Actually current version looks good to me.
> Small additional notes, It would be good:
> - to wrap initial observer in the drawImage and replace img/x,y,....
> in the observer.
> - don't draw hidpi image, if it was loaded with errors.
> - sun.com package should be used?
> - If MediaTracker was requested to load MultiResolutionImage,it should
> load all versions?
>>
>> On 26.11.2013 2:31, Jim Graham wrote:
>>>
>>>
>>> On 11/25/13 5:51 AM, Alexander Scherbatiy wrote:
>>>> On 11/23/2013 5:11 AM, Jim Graham wrote:
>>>>> Hi Alexander,
>>>>>
>>>>> If we are going to be advertising it as something that developers use
>>>>> in production code then we would need to file this via CCC. With the
>>>>> current implementation, any user that has their own ImageObservers
>>>>> must use this API and so it becomes not only public, at a time when
>>>>> there is a lockdown on new public API in 8.0, but it also means that
>>>>> we are tied to its implementation and we can't rely on "it was
>>>>> experimental and so we can change it at any point" - you can't say
>>>>> that about an API that is necessary to correctly use a touted feature.
>>>>
>>>> This issue has its own history. It has been already fixed for the
>>>> JDK 7u for the Java2D demo. It was decided to not bring new API for the
>>>> HiDPI support in the JDK 7.
>>>> It was suggested to using code like below to create a
>>>> ScalableImage.
>>>> ------------------------------
>>>> /**
>>>> * Class that loads and returns images according to
>>>> * scale factor of graphic device
>>>> *
>>>> * <pre> {@code
>>>> * Image image = new ScalableImage() {
>>>> *
>>>> * public Image createScaledImage(float scaleFactor) {
>>>> * return (scaleFactor <= 1) ? loadImage("image.png")
>>>> * : loadImage("image at 2x.png");
>>>> * }
>>>> *
>>>> * Image loadImage(String name){
>>>> * // load image
>>>> * }
>>>> * };}</pre>
>>>> */
>>>> ------------------------------
>>>>
>>>> It was based on the display scale factor. The scale factor should be
>>>> manually retrieved from the Graphics2D and was not a part of the public
>>>> API:
>>>> g2d.drawImage(scalbleImage.getScaledImage(scaleFactor),..).
>>>>
>>>> From that time it was clear that there should be 2 basic operations
>>>> for the HiDPI images support:
>>>> - retrieve an image with necessary resolution
>>>> - retrieve images with all resolutions
>>>>
>>>> The second one is useful when it is necessary to create one set of
>>>> images using the given one (for example, to draw a shape on all
>>>> images).
>>>
>>> For now, these are just ToolkitImages and you can't draw on them, so
>>> this is moot for the case of @2x support in getImage().
>>>
>>>> The JDK 7 solution has been revisited for the JDK 8 and the
>>>> neccesary
>>>> CCC request 8011059 has been created and approved.
>>>>
>>>> Both the JDK-8011059 issue description and the approved CCC request
>>>> says:
>>>> -----------------------
>>>> A user should have a unified way to provide high resolution images
>>>> that can be drawn on HIDPI displays according to the user provided
>>>> algorithm.
>>>> -----------------------
>>>
>>> We've implemented the Hint part of that solution, but the mechanism
>>> for creating custom multi-res images was not workable. I no longer
>>> sit as the client rep on the CCC or I would have pointed that out, my
>>> apologies.
>>>
>>>> The 8011059 fix consists of two parts:
>>>> - Provide a way for a custom image creation that holds images with
>>>> different resolutions
>>>> - Load @2x images on Mac OS X
>>>>
>>>> The first one of the fix can be used without the second. it is not
>>>> difficult to crate just a class which loads @2x images using the given
>>>> file path/url.
>>>
>>> If we support @2x transparently behind the scenes as we are doing
>>> now, who is the requester for the way to create a custom set of
>>> resolution variants? I think the most important customer-driven
>>> request was to support @2x for the Mac developers, wasn't it?
>>>
>>>> Now it is suggested to implement the given MultiResolutionImage
>>>> interface and override two methods:
>>>> - Image getResolutionVariant(int width, int height)
>>>> - List<Image> getResolutionVariants()
>>>>
>>>> An image with necessary resolution should be drawn automatically in
>>>> SunGraphics2D.
>>>>
>>>> What we need is to focus on the first part of the fix.
>>>> From this point of view it up to the user' code to load all or some
>>>> resolution variants by MediaTracker and using
>>>> Component.prepareImage/checkImage methods.
>>>
>>> This is all an interesting goal, but I'm not sure it is as high a
>>> priority if we support a convention (based on platform conventions)
>>> for high resolution variants behind the scenes. I do agree that we
>>> need something like this before too long, but I don't think it was
>>> workable to target the getScaledInstance() method as the mechanism to
>>> implement it. That was the only convention that was approved in that
>>> CCC request, so a more complete API based on another mechanism is not
>>> covered there.
>>>
>>> Also, Win8 has its own conventions as well which come into play on
>>> the new high resolution Surface tablets and we should consider those
>>> to guide the API we develop.
>>>
>>> All, in all, though, I think if we get @2x support in with no code
>>> changes needed for apps then we have taken care of the primary use
>>> case. We can probably even handle the Win8 conventions behind the
>>> scenes in an 8u release...
>>>
>>> ...jim
>>
>>
>
>
More information about the awt-dev
mailing list