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.
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.
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
