<AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays

Eirik Bakke ebakke at MIT.EDU
Tue Nov 12 09:57:48 PST 2013


> I have created an issue to track this: JDK-8028212 Custom cursor HiDPI
>support
> https://bugs.openjdk.java.net/browse/JDK-8028212

Perfect--thanks!



On 11/12/13, 10:05 AM, "Alexander Scherbatiy"
<alexandr.scherbatiy at oracle.com> wrote:

>On 11/7/2013 8:23 PM, Eirik Bakke wrote:
>> Just passing by...
>>
>> Will retina-enabled Image objects work with Toolkit.createCustomCursor
>>[1]
>> under this patch?
>
>    This patch allows to create an image with different resolutions that
>can be used for the custom cursor as well.
>    To make it work with custom cursor it needs to properly  pass these
>images to the native system.
>
>    I have created an issue to track this: JDK-8028212 Custom cursor
>HiDPI support
>       https://bugs.openjdk.java.net/browse/JDK-8028212
>
>   Thanks,
>   Alexandr.
>
>>
>> (I have a spreadsheet application that requires an Excel-style "plus"
>> cursor when the user hovers over it, and I'd like to supply a
>> retina-enabled cursor image as well.)
>>
>> -- Eirik
>>
>> [1]
>> 
>>http://docs.oracle.com/javase/6/docs/api/java/awt/Toolkit.html#createCust
>>om
>> Cursor(java.awt.Image, java.awt.Point, java.lang.String)
>>
>> On 11/5/13, 6:16 AM, "Alexander Scherbatiy"
>> <alexandr.scherbatiy at oracle.com> 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
>>>>
>>>>
>



More information about the macosx-port-dev mailing list