[OpenJDK 2D-Dev] <AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Nov 22 02:53:54 PST 2013
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
More information about the macosx-port-dev
mailing list