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

[Jim Graham]

On 11/7/13 6:18 AM, Sergey Bylokhov wrote:
> On 07.11.2013 11:01, Jim Graham wrote:
>> On 11/6/13 6:15 PM, Sergey Bylokhov wrote:
>>>>> In this example there is a problem. For example we have 2
>>>>> BufferedImages/ToolkiImagest A and B; Both wants be scaled perfectly.
>>>>>   - Image A draws to the image B
>>>>>   - Image B draws to the window.
>>>>>
>>>>> When window is moving from the screen x1 to the screen x2 and back.
>>>>> How
>>>>> to handle this situation?
>>>>> In this case getResolutionVariant() can return
>>>>>
>>>>>
>>>>> Related  discussion:
>>>>> http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005580.html
>>>>>
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005581.html
>>>>>
>>>>>
>>>>
>>>> How does calling getScaledInstance() deal with that?  Note the above
>>>> was simply to demonstrate the visibility of the interface, not to be a
>>>> complete implementation...
>>> For example:
>>>   - getScaledInstance() will be called() when image B will be painted to
>>> the window with scale x2.
>>>   - Inside B.getScaledInstance() the user can create BufferedImage C
>>>   - get graphics from C and set new render hint to IMAGE_SCALING_ON ans
>>> set scalex2.
>>>   - Apply to C absolutely the same rendering  which was used in B.
>>>   - getScaledInstance() will be called for A.
>>
>> In particular, drawImage() has always been a "copy some pixels and
>> return *now*" API.  Even when we had asynchronous loading of images it
>> returned immediately if the image wasn't loaded, it didn't fork off or
>> branch to another rendering process.  I really don't think we should
>> be encouraging creating a new image inside a request to draw an image.
> That's not true we already do some kind of of new image creation inside
> DrawImage.makeBufferedImage() for example;

That is pure pixel copying and uses only internal interfaces to move the 
pixels into a location that we can access from more code modules, we 
don't do callbacks and we don't publish anything there as 
developer-accessible public API.

>> Nothing in those related discussions pointed at this either.  What I
>> see the need for is:
>>
>> - Toolkit images automatically supporting loading an @2x image if one
>> is associated with the image they are requested to load.  I think they
>> already do this and you just need a way for drawImage() to grab that
>> image if necessary.
> I think no. They do not support.

Apologies.  I was confusing code I saw in this current fix with stuff 
that was already integrated.  What I'm getting at is that we already 
have a reasonable fix for that aspect of this list of action items, but 
it is exposed via the getScaledInstance() method here and that is not an 
acceptable way to convey that information to drawImage().

(While I haven't done an in depth code review of the toolkit image part 
of the current fix, the main issue I'm addressing here is just the part 
where those images are obtained by calling getScaledInstance()...)

>> - All images to have an associated resolution or pixel scale.  @2x
>> variants of images, if we ever expose them directly, would claim
>> either a pixel scale of 2 or a DPI of ?144?, i.e. double the implicit
>> dpi of 72 that most images claim or that we've assumed by default.
> That's why all related information: DPI,Scale, etc are not mentioned in
> the fix and api. just width and height are used, which is general and
> simple.

I've already mentioned issues with width and height, but the main point 
here is that they describe that they were intended to be used with a 
different resolution in some manner.  If it's just internal interface 
for now then the exact specification doesn't matter, but if and when we 
find a way to expose that, then we need to talk about what type of 
specifier we want to lock in at that point.

>> - Destination drawables having a resolution or pixel scaling parameter
>> associated with them that lets G2D decide if it is going to use hiDPI
>> media or not.  It would request a HiDPI version of an image if needed,
>> but this should be a simple "choose this set of pixels or that set"
>> API, not an "I'll make you one right now" type of API.
> It is already in the current version of the fix.

I realize that, I'm just outlining the parts of what is needed in the 
short term here.  Also, the API in the current fix that is used to 
request the HiDPI version of an image (getScaledInstance()) is not an 
acceptable implementation of this bullet item.

>> - (not necessarily now, but soon) A way for someone to associate a
>> higher DPI version of an image with an existing image and to query any
>> alternate resolutions that we've already prepared (such as any @2x
>> version that we've already loaded).  This could either be
>> "Image.createMultiResImage(... list of images and resolutions ...)" or
>> perhaps a convention for listing alternate images in the properties.
> It was intended to do not provide any mapping from some scale to the
> image, and leave it up to the user. So loading of the images will be
> lazily. Only if the image really needed it will be created and cached by
> the user.

Have the user's requested this flexibility?  All I've seen is that they 
want to supply retina-resolution images for buttons, which simply 
requires us to recognize and use @2x media files when we find them.

If such a mechanism has any laziness about it, then that laziness has to 
be done asynchronously wrt drawImage().  The drawImage method is 
specified to only use currently available pixels and return immediately 
(which is why it has the Observer parameter).  If a factory interface is 
needed (and currently I have not seen any requests specifically for that 
kind of functionality) then it would have to be something that is 
triggered/notified by the actions drawImage(), but drawImage() will not 
wait for it to complete its work.  Personally, I don't think this is 
really necessary.

>> As far as the "images for buttons" example in the macosx-port thread,
>> if those images are loaded using Toolkit.getImage() and there is an
>> associated @2x image and we load that internally and make it available
>> via some sort of (internal for now) MultiResImage interface like I
>> described, then we've solved that problem for buttons.  It seemed like
>> that was all that they were asking for there and I see no reason to
>> create a "solve every hypothetical resolution issue" API now to meet
>> that need...
> But we do not create any new API except two hints in Image and SG2D.
> Note that if compatibility is a problem from your point of view, we can
> control new hint in SG2D by some desktop property, and disable it by
> default. I think it can be useful to disable all hidpi support in this
> case.

The use of getScaledInstance() in this case far exceeded its original 
goals and created an interaction with those new hints.  Also drawImage 
has never called that method and should never call that method and if it 
is to be used as you describe here then we'd have to document that it 
will call a method that it never should have called in the first place 
and we'd have to document that overriding that method has new 
capabilities.  All of that documentation of new uses and interactions is 
public API.

Also, while the fix being proposed here was simply a way to get the 
internal toolkit images to return their @2x variants, the discussion 
clearly indicated that the getScaledInstance() method was intended to be 
the global solution I was referring to there.  I was saying that API to 
solve the issue of loading from multiple related media of different 
resolutions does not need to be constrained by the requirement that it 
also supports arbitrary creation of synthetic images at a variety of 
resolutions.  The former is needed now and is simple to provide, whereas 
I haven't seen any requests for the latter and it would need a fairly 
serious formal review about how it affects all aspects of our 
architecture...

			...jim