Hi Alexander,

On 11/22/2013 2:53 AM, Alexander Scherbatiy wrote:
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.

I think we could provide that eventually, but I believe that the requests had 
to do with automatic support for @2x and we should focus on that.

   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.

It doesn't prevent supplying an @2x image for an applet, but it would prevent 
arbitrary resolution variants through custom developer code.  I think that is 
OK for the late time we are at in the 8.0 release cycle.

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.

   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.

If a developer uses a stock Java module in their app and they hand it some 
@2x-enabled images then that code could fail.

If a developer creates an app and then later a graphic artist replaces its 
media with a new set of media that includes @2x images then the app could fail.

If an applet is deployed that uses stock imagery from its own web site and 
someone on the web team for that same company/organization decides to start 
introducing @2x media for a better web experience, then that code could fail.

And, the changes introduced represent not "new functionality" but existing 
behaviors that we've specified that are being violated - and violated for reasons 
external to that code (i.e. whether or not a particular file is found in the file system 
or on a web site).

Also, even if it could be attributed to something that was also fully complicit 
amongst all parties involved, this feature can be deployed without requiring 
code changes and we should do that.  The current issues with the image being 
returned in imageUpdate are avoidable, inconvenient, and in violation of the 
existing drawImage/ImageObserver contract.

   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.

Release cycles have vetting of new public API built in to their scheduling in a 
responsible manner.  An 11th hour sudden introduction of a new API, especially 
one that is necessary for some developers to use a new feature, is a lot more 
irresponsible.

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.

First, their "use" of the new feature is not a code change, it is the 
appearance of a new file in their deployment, or on a web site that they tried to access. 
 There isn't as much explicit intent there as you are making it out to be.

     It should be up to a user to preload all resolutions or only part of them 
using the MediaTracker.

(And, btw, I didn't see MediaTracker on the list of places that were changed to 
be aware of the new images.  Also, the prepareImage/checkImage methods that MT 
uses to trigger loading weren't necessarily up to the task of preloading the 
necessary image variants.)

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

Component.prepareImage() and Component.checkImage() are also intended to 
trigger and track the loading of the required representation of an image for 
the indicated Component.

    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.

Agreed.

    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.

It would be far less surprising than the third option which starts to mention 
an image that he never interacted with.

It would also be similar to the behavior in 1.0 where we could asynchronously 
reload the image in order to perform simple scaling on it.  In other words, 
this may be outdated behavior, but it is not new behavior.

    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.

The fact that it requires the user to update his code to avoid bugs is a non-starter.  It means 
that they have work to do to use @2x images, they can't just install the new files into their 
deployment.  It means that they will experience all sorts of "left hand didn't coordinate with 
right hand" issues in their deployment and testing as often media is managed by a different 
team than the code.  It also means we are exposing an internal implementation detail rather than 
making it "just work".  It also means that we are explicitly violating a long-standing 
API contract in a way that directly impacts them.

The added information is not required for them to use the image and is of 
questionable value given that the interactions they make with the image are 
based on the design space of the unscaled variant - information relative to a 
scaled variant then requires a bit of work for them to use.  We may want to 
provide this information at some point, but for now we should not be 
introducing new behavior to a really old API contract...

                        ...jim

Reply via email to