[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR 8149338: JVM Crash caused by Marlin renderer not handling NaN coordinates
From: Laurent_Bourgès <bourges.laurent () gmail ! com>
Date: 2016-02-11 8:12:25
Message-ID: CAKjRUT7b78EWP8xWWezMYYRZdgbu0QH=dpDjdg4Knb0ie07Z7Q () mail ! gmail ! com
[Download RAW message or body]
Pushed: http://hg.openjdk.java.net/jdk9/client/jdk/rev/3eda6cd3f504
Thanks for your reviews !
Laurent
2016-02-10 22:27 GMT+01:00 Jim Graham <james.graham@oracle.com>:
> Looks good...
>
> ...jim
>
> On 2/10/16 12:52 PM, Laurent Bourgès wrote:
>
>> Jim & Phil,
>>
>> Here is the updated webrev:
>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8149338.1/
>>
>> I just simplified the exception handling code in both TextClipErrorTest
>> and CrashNaNTest
>>
>> Cheers,
>> Laurent
>>
>> 2016-02-10 0:46 GMT+01:00 Jim Graham <james.graham@oracle.com
>> <mailto:james.graham@oracle.com>>:
>>
>> Let me know if/when you have an updated webrev. It should be good
>> to go, but it couldn't hurt to publish one for the archives...
>>
>> ...jim
>>
>> On 2/9/16 12:51 AM, Laurent Bourgès wrote:
>>
>> Jim,
>>
>> Here are my answers to your 2 questions:
>>
>> 2016-02-09 0:14 GMT+01:00 Jim Graham <james.graham@oracle.com
>> <mailto:james.graham@oracle.com>
>> <mailto:james.graham@oracle.com <mailto:james.graham@oracle.com
>> >>>:
>>
>>
>> In the test case, why are you using a log handler to check
>> for a
>> particular exception? Shouldn't any exception logged be
>> cause for a
>> test failure?
>>
>>
>> I already used that code in 1 other test: TextClipErrorTest. I
>> agree it
>> can be simpler to report any exception like:
>> public void publish(LogRecord record) {
>> Throwable th = record.getThrown();
>> // detect any Throwable:
>> if (th != null) {
>> System.out.println("Test failed:\n" +
>> record.getMessage());
>> th.printStackTrace(System.out);
>>
>> throw new RuntimeException("Test failed:
>> ", th);
>> }
>> }
>>
>> Is there a reason why you reversed the calculations for the
>> slope at
>> line 374?
>>
>> + final double slope = (x1d - x2) / (y1d - y2);
>>
>> I prefer this syntax as it is more explicit that (x1d - x2) and
>> (y1d -
>> y2) are double values (not implicit promotion).
>>
>> Cheers,
>> Laurent
>>
>>
>>
--
--
Laurent Bourgès
[Attachment #3 (text/html)]
<div dir="ltr">Pushed: <a \
href="http://hg.openjdk.java.net/jdk9/client/jdk/rev/3eda6cd3f504">http://hg.openjdk.java.net/jdk9/client/jdk/rev/3eda6cd3f504</a><br><div><br>Thanks \
for your reviews !<br><br></div><div>Laurent<br><br></div></div><div \
class="gmail_extra"><br><div class="gmail_quote">2016-02-10 22:27 GMT+01:00 Jim \
Graham <span dir="ltr"><<a href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a>></span>:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Looks good...<br> <br>
...jim<span class=""><br>
<br>
On 2/10/16 12:52 PM, Laurent Bourgès wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><span class=""> Jim & Phil,<br>
<br>
Here is the updated webrev:<br>
<a href="http://cr.openjdk.java.net/~lbourges/marlin/marlin-8149338.1/" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~lbourges/marlin/marlin-8149338.1/</a><br>
<br>
I just simplified the exception handling code in both TextClipErrorTest<br>
and CrashNaNTest<br>
<br>
Cheers,<br>
Laurent<br>
<br>
2016-02-10 0:46 GMT+01:00 Jim Graham <<a href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a><br></span> <mailto:<a \
href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a>>>:<span class=""><br> <br>
Let me know if/when you have an updated webrev. It should be good<br>
to go, but it couldn't hurt to publish one for the archives...<br>
<br>
...jim<br>
<br>
On 2/9/16 12:51 AM, Laurent Bourgès wrote:<br>
<br>
Jim,<br>
<br>
Here are my answers to your 2 questions:<br>
<br>
2016-02-09 0:14 GMT+01:00 Jim Graham <<a \
href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a><br>
<mailto:<a href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a>><br></span> <mailto:<a \
href="mailto:james.graham@oracle.com" target="_blank">james.graham@oracle.com</a> \
<mailto:<a href="mailto:james.graham@oracle.com" \
target="_blank">james.graham@oracle.com</a>>>>:<div><div class="h5"><br> \
<br>
In the test case, why are you using a log handler to check<br>
for a<br>
particular exception? Shouldn't any exception logged be<br>
cause for a<br>
test failure?<br>
<br>
<br>
I already used that code in 1 other test: TextClipErrorTest. I<br>
agree it<br>
can be simpler to report any exception like:<br>
public void publish(LogRecord record) {<br>
Throwable th = record.getThrown();<br>
// detect any Throwable:<br>
if (th != null) {<br>
System.out.println("Test \
failed:\n" +<br> record.getMessage());<br>
th.printStackTrace(System.out);<br>
<br>
throw new RuntimeException("Test \
failed:<br> ", th);<br>
}<br>
}<br>
<br>
Is there a reason why you reversed the calculations for the<br>
slope at<br>
line 374?<br>
<br>
+ final double slope = (x1d - x2) / (y1d - y2);<br>
<br>
I prefer this syntax as it is more explicit that (x1d - x2) and<br>
(y1d -<br>
y2) are double values (not implicit promotion).<br>
<br>
Cheers,<br>
Laurent<br>
<br>
<br>
</div></div></blockquote>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">-- \
<br>Laurent Bourgès</div> </div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic