[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR JDK-8191814: Marlin rasterizer spends time computing geometry for stro
From: Laurent_Bourgès <bourges.laurent () gmail ! com>
Date: 2017-12-11 20:32:19
Message-ID: CAKjRUT6iEdLmj4yJ8yg6xf2SQxHbwcdUGmAPzTbu7n80X=ZBkg () mail ! gmail ! com
[Download RAW message or body]
Hi,
I pushed in jdk forrest:
http://hg.openjdk.java.net/jdk/client/rev/fd7fbc929001
Thanks for your reviews, Phil & Sergey !
Laurent
2017-12-07 20:43 GMT+01:00 Sergey Bylokhov <Sergey.Bylokhov@oracle.com>:
> I am not an expert here, I just look to the changes and run the closed
> tests for jdk_desktop group. No new issues were found, so it looks fine to
> me.
>
> On 05/12/2017 06:30, Laurent Bourgès wrote:
>
>> Hi again,
>>
>> Here is a new webrev fixing the ClipShapeTest:
>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-082-8191814.1/
>>
>> Changes:
>> - @run twice to test both Marlin variants (float / double)
>> - use Logger to get & check marlin system properties
>> ("sun.java2d.renderer.clip.runtime.enable") to ensure the test is run
>> against a Marlin renderer having the clipping features
>>
>> I tested the fixed test on JDK9 and it fails:
>> runner finished test: sun/java2d/marlin/ClipShapeTest.java
>> Failed. Execution failed: `main' threw exception:
>> java.lang.RuntimeException: Marlin clipping not enabled at runtime !
>>
>> On JDK10 + patch, it passes:
>> sun/java2d/marlin/ClipShapeTest.java
>> Passed. Execution successful
>>
>>
>> PS: Could anyone review the patch ? before RDP1 deadline ?
>>
>> Regards,
>> Laurent
>>
>>
>> 2017-12-04 23:35 GMT+01:00 Laurent Bourgès <bourges.laurent@gmail.com
>> <mailto:bourges.laurent@gmail.com>>:
>>
>> Hi Sergey,
>>
>> Thanks for your comment.
>>
>> This new test only validates the new clipping algorithms ie it
>> compares the rendering outputs with / without clipping enabled.
>>
>> As such algorithms are only available in Marlin 0.8.2 and the test
>> uses new system properties to enable/disable clipping, I confirm it
>> passes before (jdk9 or jdk10 before patch).
>>
>> To ensure it detects any regression, I manually inserted some bugs
>> in the clipping code, and the test failed.
>>
>> Note: I should add another test @run to check the float variant too
>> (and not only the double variant, the default in jdk10).
>>
>> Finally I could write a new performance test that would prove
>> clipping is more efficient than before.
>> Such test would fail before patch (timeout ?), but it is difficult
>> to make it robust as it depends on the hw.
>> Jim wrote a basic test in the jfx bug showing 300ms without but 2ms
>> now => gain is high.
>> A possible success condition would be: clipping gain > 10 or 50.
>>
>> Regards,
>> Laurent
>>
>>
>> Le 4 déc. 2017 11:11 PM, "Sergey Bylokhov"
>> <Sergey.Bylokhov@oracle.com <mailto:Sergey.Bylokhov@oracle.com>> a
>> écrit :
>>
>> Hi, Laurent.
>>
>> On 29/11/2017 14:30, Laurent Bourgès wrote:
>>
>> - added new ClipShapeTest (jtreg) that checks all possible
>> combinations of (cap / join) for random polyline (Stroker)
>> and polygons (Filler) comparing image outputs rendered with
>> clipping enabled vs disabled
>>
>>
>> I have only one note that the test is passed before the fix, so
>> if we will regress at some point later we will not catch this.
>
>
[Attachment #3 (text/html)]
<div dir="ltr"><div><div>Hi,<br><br>I pushed in jdk forrest:<br><a \
href="http://hg.openjdk.java.net/jdk/client/rev/fd7fbc929001">http://hg.openjdk.java.net/jdk/client/rev/fd7fbc929001</a><br><br></div>Thanks \
for your reviews, Phil & Sergey \
!</div><div><br></div>Laurent<br><div><div><br></div></div><div \
class="gmail_extra"><br><div class="gmail_quote">2017-12-07 20:43 GMT+01:00 Sergey \
Bylokhov <span dir="ltr"><<a href="mailto:Sergey.Bylokhov@oracle.com" \
target="_blank">Sergey.Bylokhov@oracle.com</a>></span>:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">I am not an expert here, I just look to the changes and run \
the closed tests for jdk_desktop group. No new issues were found, so it looks fine to \
me.<span class=""><br> <br>
On 05/12/2017 06:30, 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=""> Hi again,<br>
<br>
Here is a new webrev fixing the ClipShapeTest:<br>
<a href="http://cr.openjdk.java.net/~lbourges/marlin/marlin-082-8191814.1/" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~lb<wbr>ourges/marlin/marlin-082-81918<wbr>14.1/</a><br>
<br>
Changes:<br>
- @run twice to test both Marlin variants (float / double)<br>
- use Logger to get & check marlin system properties \
("sun.java2d.renderer.clip.run<wbr>time.enable") to ensure the test is run \
against a Marlin renderer having the clipping features<br> <br>
I tested the fixed test on JDK9 and it fails:<br>
runner finished test: sun/java2d/marlin/ClipShapeTes<wbr>t.java<br>
Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: \
Marlin clipping not enabled at runtime !<br> <br>
On JDK10 + patch, it passes:<br>
sun/java2d/marlin/ClipShapeTes<wbr>t.java \
Passed. Execution successful<br> <br>
<br>
PS: Could anyone review the patch ? before RDP1 deadline ?<br>
<br>
Regards,<br>
Laurent<br>
<br>
<br></span>
2017-12-04 23:35 GMT+01:00 Laurent Bourgès <<a \
href="mailto:bourges.laurent@gmail.com" target="_blank">bourges.laurent@gmail.com</a> \
<mailto:<a href="mailto:bourges.laurent@gmail.com" \
target="_blank">bourges.laurent@gmail.<wbr>com</a>>>:<span class=""><br> <br>
Hi Sergey,<br>
<br>
Thanks for your comment.<br>
<br>
This new test only validates the new clipping algorithms ie it<br>
compares the rendering outputs with / without clipping enabled.<br>
<br>
As such algorithms are only available in Marlin 0.8.2 and the test<br>
uses new system properties to enable/disable clipping, I confirm it<br>
passes before (jdk9 or jdk10 before patch).<br>
<br>
To ensure it detects any regression, I manually inserted some bugs<br>
in the clipping code, and the test failed.<br>
<br>
Note: I should add another test @run to check the float variant too<br>
(and not only the double variant, the default in jdk10).<br>
<br>
Finally I could write a new performance test that would prove<br>
clipping is more efficient than before.<br>
Such test would fail before patch (timeout ?), but it is difficult<br>
to make it robust as it depends on the hw.<br>
Jim wrote a basic test in the jfx bug showing 300ms without but 2ms<br>
now => gain is high.<br>
A possible success condition would be: clipping gain > 10 or 50.<br>
<br>
Regards,<br>
Laurent<br>
<br>
<br>
Le 4 déc. 2017 11:11 PM, "Sergey Bylokhov"<br></span>
<<a href="mailto:Sergey.Bylokhov@oracle.com" \
target="_blank">Sergey.Bylokhov@oracle.com</a> <mailto:<a \
href="mailto:Sergey.Bylokhov@oracle.com" \
target="_blank">Sergey.Bylokhov@oracle<wbr>.com</a>>> a<span class=""><br> \
écrit :<br> <br>
Hi, Laurent.<br>
<br>
On 29/11/2017 14:30, Laurent Bourgès wrote:<br>
<br>
- added new ClipShapeTest (jtreg) that checks all possible<br>
combinations of (cap / join) for random polyline (Stroker)<br>
and polygons (Filler) comparing image outputs rendered with<br>
clipping enabled vs disabled<br>
<br>
<br>
I have only one note that the test is passed before the fix, so<br>
if we will regress at some point later we will not catch \
this.</span></blockquote></blockquote></div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic