<AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Tue Nov 5 03:16:21 PST 2013
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