<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