[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] <AWT Dev> [8] Review request for 8011059 [macosx] Make JDK demos look perfect o
From: Peter Levart <peter.levart () gmail ! com>
Date: 2013-10-31 20:46:35
Message-ID: 5272C1AB.9030202 () gmail ! com
[Download RAW message or body]
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@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
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 10/29/2013 05:45 PM, Alexander
Scherbatiy wrote:<br>
</div>
<blockquote cite="mid:526FE63C.1020408@oracle.com" type="cite"><!--[if !IE]><DIV \
style="border-left: 2px solid #009900; border-right: 2px solid #009900; padding: 0px \
15px; margin: 2px 0px;"><![endif]--> <blockquote type="cite" style="color: \
#000000;"><!--[if !IE]><DIV style="border-left: 2px solid #009900; border-right: 2px \
solid #009900; padding: 0px 15px; margin: 2px 0px;"><![endif]-->2. I'm not sure \
that the proposed getScaledImageName() implementation in ScalableToolkitImage works \
perfectly for URLs like this:
<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://www.exampmle.com/dir/image">http://www.exampmle.com/dir/image</a>
<br>
<br>
In this case it will try to find 2x image here:
<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://www.example@2x.com/dir/image">http://www.example@2x.com/dir/image</a>
<br>
<br>
which doesn't look correct.
<br>
<!--[if !IE]></DIV><![endif]--></blockquote>
Fixed. Only path part of a URL is \
converted to path2x. <!--[if !IE]></DIV><![endif]--></blockquote>
<br>
Hi Alexander,<br>
<br>
URLs like this:<br>
<br>
<a class="moz-txt-link-freetext" \
href="http://www.example.com/dir.ext/image">http://www.example.com/dir.ext/image</a><br>
<br>
will still be translated to:<br>
<br>
<a class="moz-txt-link-freetext" \
href="http://www.example.com/dir@2x.ext/image">http://www.example.com/dir@2x.ext/image</a><br>
<br>
<br>
I think you need to search for last '.' after the last '/' in the
getScaledImageName();<br>
<br>
<br>
Also the following code has some additional bugs:<br>
<br>
<pre><span class="new"> 853 static Image toScalableImage(Image image, URL \
url) {</span> <span class="new"> 854 </span>
<span class="new"> 855 if (url != null && \
!url.toString().contains("@2x")</span> <span class="new"> 856 \
&& !(image instanceof ScalableToolkitImage)) {</span> <span class="new"> 857 \
String protocol = url.getProtocol();</span> <span class="new"> 858 \
String host = url.getHost();</span> <span class="new"> 859 String \
file = url.getPath();</span> <span class="new"> 860 String file2x = \
<b>host + </b>getScaledImageName(file);</span> <span class="new"> 861 \
try {</span> <span class="new"> 862 URL url2x = new URL(protocol, \
host, file2x);</span> <span class="new"> 863 \
url2x.openStream();</span> <span class="new"> 864 return new \
ScalableToolkitImage(image, getDefaultToolkit().getImage(url2x));</span> <span \
class="new"> 865 } catch (Exception ignore) {</span> <span \
class="new"> 866 }</span> <span class="new"> 867 }</span>
<span class="new"> 868 return image;</span>
<span class="new"> 869 }
</span></pre>
Why are you prepending <b>host</b> to getScaledImageName(file) in
line 860? Let's take the following URL for example:<br>
<br>
<a class="moz-txt-link-freetext" \
href="http://www.example.com/dir/image.jpg">http://www.example.com/dir/image.jpg</a><br>
<br>
protocol = "http"<br>
host = "<a class="moz-txt-link-abbreviated" \
href="http://www.example.com">www.example.com</a>"<br> file = "/dir/image.jpg"<br>
file2x = "<b><a class="moz-txt-link-abbreviated" \
href="http://www.example.com">www.example.com</a></b>/dir/image@2x.jpg"<br> url2x = \
URL("<a class="moz-txt-link-freetext" \
href="http://www.example.com">http://www.example.com</a><b><a \
class="moz-txt-link-abbreviated" \
href="http://www.example.com">www.example.com</a></b>/dir/image@2x.jpg")<br> <br>
<br>
You are missing a part in URL (de)construction - the optional port!
For example in the following URL:<br>
<br>
<a class="moz-txt-link-freetext" \
href="http://www.example.com:8080/dir/image.jpg">http://www.example.com:8080/dir/image.jpg</a><br>
<br>
You should extract the port from original URL and use it in new URL
construction if present (!= -1).<br>
<br>
<br>
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):<br>
<br>
try (InputStream probe = \
url.openStream()) {}<br> <br>
<br>
<br>
Regards, Peter<br>
<br>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic