On 12.11.2013 23:43, Jim Graham wrote:
Hi Alexander,
Some minor issues with this fix:
- It looks like the transform is used in SG2D to decide if the hiDPI
image should be used. I'm not familiar with the Mac's native use of
@2x, but I thought that they hinged the decision off of the "retina
scale" of the display, not off of the current transform. That way, if
you scale down an icon it doesn't change its look when it reaches .5x
scale (or .75x depending on the cutoff). Personally, I think it is
better to not use the transform to keep animations smooth, but I'm not
sure what Apple did.
- The logic in using the transform is also a bit murky. I think if
you set the scale on a retina display to exactly 1/2 it would use the
HiDPI version even though the scale was 1:1. Since I support not
examining the transform there, I'm going to present this as another
reason why we should just base it on devScale, but the logic could be
fixed if we really plan to use the transform here.
It also allow to the user to use a transform and a hint and force the
images to draw the scaled version even to the BufferedImage fo ex. Which
can be useful in case of sequential drawing of BI-> BI->retina.
- Has any thought been given to my comments about MediaTracker and
image observer states for multi-res images? I don't see any attempt
here to preload the @2x image. The problem would probably only be
seen on multi-headed systems where one display is retina and one is
not - you would find the images suddenly reloading when you move the
window to the other screen and the application might not expect that
to happen. Which image is the Observer registered on? Since the
image is handed to the Observer, will an application be surprised when
their observer gets a handle to an image they've never seen? Is it an
issue if one of the "alternate resolution variant" images leaks into
an application's "hands" via the observer callbacks?
That's could be a problem. Is it possible to wrap imageObserver, which
was passed to the drawImage, and replace one image to another in the
WrapperImageObserver.imageUpdate()?
...jim
On 11/11/13 7:59 AM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.06/
Only internal API is exposed:
- MultiResolutionImage interface with method
"getResolutionVariant(int width, int height)" is added to the
com.sun.awt package
- Hints to switch on/off the resolution variant usage are added to
SunHints class
- Test is updated to use the MultiResolutionImage interface
Thanks,
Alexandr.
On 11/5/2013 3:16 PM, Alexander Scherbatiy wrote:
Thank you for the review.
Could you look at the updated fix:
http://cr.openjdk.java.net/~alexsch/8011059/webrev.05/
- URL is parsed to protocol, host, port, and path parts in the
LWCToolkit class.
I checked that URL(protocol, host, port, file) constructor
correctly handles -1 port value.
- Only last file name after last '/' in the URL path is converted
to @2x name
- Tests that check correct URL and path translation to @2x names are
added to the ScalableImageTest
Thanks,
Alexandr.
On 11/1/2013 12:46 AM, Peter Levart wrote:
On 10/29/2013 05:45 PM, Alexander Scherbatiy wrote:
2. I'm not sure that the proposed getScaledImageName()
implementation in ScalableToolkitImage works perfectly for URLs
like this:
http://www.exampmle.com/dir/image
In this case it will try to find 2x image here:
http://www.exam...@2x.com/dir/image
which doesn't look correct.
Fixed. Only path part of a URL is converted to path2x.
Hi Alexander,
URLs like this:
http://www.example.com/dir.ext/image
will still be translated to:
http://www.example.com/d...@2x.ext/image
I think you need to search for last '.' after the last '/' in the
getScaledImageName();
Also the following code has some additional bugs:
853 static Image toScalableImage(Image image, URL url) {
854
855 if (url != null && !url.toString().contains("@2x")
856 && !(image instanceof
ScalableToolkitImage)) {
857 String protocol = url.getProtocol();
858 String host = url.getHost();
859 String file = url.getPath();
860 String file2x =*host +*getScaledImageName(file);
861 try {
862 URL url2x = new URL(protocol, host, file2x);
863 url2x.openStream();
864 return new ScalableToolkitImage(image,
getDefaultToolkit().getImage(url2x));
865 } catch (Exception ignore) {
866 }
867 }
868 return image;
869 }
Why are you prepending *host* to getScaledImageName(file) in line
860? Let's take the following URL for example:
http://www.example.com/dir/image.jpg
protocol = "http"
host = "www.example.com"
file = "/dir/image.jpg"
file2x = "*www.example.com*/dir/im...@2x.jpg"
url2x =
URL("http://www.example.com*www.example.com*/dir/im...@2x.jpg";)
You are missing a part in URL (de)construction - the optional port!
For example in the following URL:
http://www.example.com:8080/dir/image.jpg
You should extract the port from original URL and use it in new URL
construction if present (!= -1).
I would also close the stream explicitly after probing for existence
of resource rather than delegating to GC which might not be promptly
and can cause resource exhaustion (think of MAX. # of open file
descriptors):
try (InputStream probe = url.openStream()) {}
Regards, Peter
--
Best regards, Sergey.
![http://www.example.com*www.example.com*/dir/im...@2x.jpg"](http://www.example.com*www.example.com*/dir/im...@2x.jpg"){kind=link}
