<AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Nov 13 04:33:35 PST 2013
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.example@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/dir@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/image at 2x.jpg"
>>>> url2x =
>>>> URL("http://www.example.com*www.example.com*/dir/image@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.
More information about the macosx-port-dev
mailing list