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

[Alexander Scherbatiy]

On 11/21/2013 8:13 PM, Jim Graham wrote:
> Hi Alexander,
>
> I just noticed that the new interface was created in com.sun.  I also 
> note that you discuss a number of issues below that relate to a 
> developer creating one of their own Multi-Resolution images.  We 
> should not be exporting an interface at this point for a developer to 
> do any of that.  Any use of these interfaces beyond our own internal 
> use to support @2x images will be unsupported in JDK8 as we do not 
> have time to properly test and expose an interface with the remaining 
> time in the JDK8 release schedule.  This interface should be moved to 
> the internal sun.* hierarchy until we have time to vet it.

    I see that NSImage on Mac OS X has API to work with representations 
and has methods like 
addRepresentation/removeRepresentation/bestRepresentationForRect.

    Does it mean that it is possible to use not only @2x images in an 
application but create NSImage with different images resolutions as well?
    Should we support the same thing in Java? For example, a user wants 
to provide images with resolutions 0.5, 1, 2, and 3. According to the 
current transform the best resolution variant should be chosen.

   The MultiResolutionImage is in com.sun* package and it still means 
that it can be changed or removed. It can require the CCC request, though.
   Putting it to sun.com prevents using this feature in applets.

   I do not see any compatibility risks with the current fix. If a user 
does not provide @2x images or explicitly overrides the 
MultiResolutionImage nothing should be changed in his code.

   The time testing of this API is the same as testing TollkitImages 
that can hold @2x images.
   We also will have the feedback about these API earlier.  It is better 
than introducing a public API in next release that will be difficult to 
change.

>
> All of the places that call containsResolutionVariant() are pointing 
> out a bug with this implementation.  The resolution variants are 
> internal implementation details and should never be leaked through any 
> current interfaces.  It looks like most of the cases involve 
> imageUpdate() methods that should be receiving a reference to the 
> original image, not the resolution variant. These pieces of could 
> should not be changing and the fact that you had to change them points 
> out that you've created a huge compatibility issue that blocks this 
> solution.

     If someone uses the API with multi-resolution images he needs also 
update his code according to this image usage.
     It should be up to a user to preload all resolutions or only part 
of them using the MediaTracker.

     The only place where an image is replaced to a resolution variant 
and the image observer is invoked is the drawImage(Image,..,ImageObserver).

    There are 3 solutions how it can be handled:
      - Preload an image with all resolution variants. The image 
observer is not invoked in this case.
      - Using the original image in the image observer for the 
resolution variant (x,y,w,h should be rescaled).
      - Using the resolution variant in the observer

    The first one is not good solution for the ToolkitImage because it 
is designed to load asynchronously.
    The second one still  can be surprising for a user because he has 
notification that the original image has been already preloaded. Note 
that a user can do something with this image so his actions will be 
based on the image with another resolution.
    The third one provides the actual information to the user. However, 
it requires that the user update his code in the image observer.  There 
are no compatibility problems. If multi-resolution images are not used 
nothing should be changed. If they used, image observers should be 
updated accordingly.

    Thanks,
    Alexandr.

>
> On 11/21/2013 6:15 AM, Alexander Scherbatiy wrote:
>>>
>>> To check if it is "identity-ish", I'd use:
>>>     ((type & ~(TYPE_TRANSLATION | TYPE_FLIP)) == 0)
>>> To check for scale, use:
>>>     ((type & ~(TYPE_TRANSLATION | TYPE_FLIP | TYPE_SCALE_MASK)) == 0)
>>> and then the distance/hypot equations are in the final else as they 
>>> are now.
>>      Updated.
>
> The test at line 3152 is sub-optimal.  If the transform includes both 
> a SCALE and a TRANSLATE then this optimization will fail to trigger 
> and the code will fall through to the default case (which produces the 
> correct output, but misses the optimization).  Note above that I 
> include the FLIP and TRANSLATE flags in with the SCALE mask to take 
> care of this in my pseudo-code above.
>
>>>>       - Some related to the ToolkitImage staff is moved from 
>>>> SunToolkit to ToolkitImageUtil
>>>
>>> I'm not sure why this was done - it added a bunch of busy-work for 
>>> approving the webrev that I don't have time for. Hopefully someone 
>>> else can verify the cut/paste and refactoring.
>>      I have reverted it back.
>
> Thanks, it is much easier to verify what was modified now.
>
>>> I'm still not sure why it isn't just left as DEFAULT on all 
>>> platforms.  The other platforms don't even create resolution images 
>>> anyway so their default is to just use the existing image which is 
>>> fine.  We should not have platform issues affecting the default 
>>> values for hints.
>>
>>      A user can create custom MultiResolution image.  If our target 
>> is to show the resolution variant according to the current transform 
>> on all platforms then it is ok.
>
> If and when we expose the ability to create these images then the 
> developer would want them to be used on all platforms if they are 
> going to the trouble of manually creating them.  Until then the @2x 
> images are only created on MacOS anyway so the value of the hint is 
> irrelevant on other platforms.  Arguably, we might want to start 
> loading the @2x images on other platforms too, but I think we can go 
> into the 8.0 release with just MacOS support and worry about using it 
> on other platforms in an 8 update release.  In either case, the 
> default hint value should be DEFAULT.
>
>>     However, a user can create it's own MultiResolutionImage 
>> implementation. So I added the static containsResolutionVariant() 
>> method and updated
>>     necessary observers in our code. Usually the code "image!= img" 
>> is changed to "MultiResolutionImage.containsResolutionVariant(image, 
>> img)"
>
> We should not be catering to custom uses at this point until a later 
> release.
>
> If any of this code requires changes to ImageObservers then that is a 
> bug as I mentioned above.
>
>>     and x, y, width, and height values are recalculated.  The same 
>> should be done in a user's code.
>
> We should not require changes to developer's ImageObserver code to 
> support @2x.
>
>>> I was going to ask about that new method.  Why does the developer 
>>> need to manually intercede here?  They should be able to grab an 
>>> image that has an @2x variant, use MediaTracker or some other 
>>> mechanism to make sure it is loaded, then create the cursor and the 
>>> code that creates the cursor should notice the resolution variations 
>>> and deal with it on its own...
>>       Mac OS X gets a necessary image based on the current transform. 
>> A user can create its own MultiResolutionImage that contains images 
>> for the scales, for example, 0.5, 0.7, 1, 3.2.
>>       We need to know a list of all images to create the NSImage.
>
> Any code that triggers an image to load and then looks for that same 
> reference in its imageUpdate() method (which is the case in the 
> MediaTracker code) should continue to work.  That was the point I was 
> trying to make and I've reiterated this above. MediaTracker (which 
> should support @2x images without any code changes) simply makes a 
> good test case that we've handled the compatibility issues here...
>
>             ...jim