<div dir="ltr"><div>Hi Jim,<br><br></div>Thanks for your comments, it helped refining the webrev.<br><br>Here are my answers:<br><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">2017-08-26 2:22 GMT+02: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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
[D]Dasher.java - why the changes from (firstSegIdx > 0) to (firstSegIdx != 0)?<br></blockquote><div><br></div><div>As firstSegIdx is initialized to 0, I prefer testing (firstSegIdx<span class="gmail-"> !</span>= 0) as it looks more obvious. <br></div><div>For me, (firstSegIdx > 0) indicates that the sign has a particular meaning and that firstSegIdx<span class="gmail-"><span class="gmail-"> </span>may be negative</span>.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
[D]Dasher.java - why is there a new goto_starting() which is only used from one place?  To be able to add final to the parameters?<br></blockquote><div><br></div><div>For inlining purposes, the JITWatch jarscan tool reported that this (hotspot) method is larger than 325 byte codes so I wanted to split it in smaller parts.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
[D]Dasher.java, line 268ish - why move the call to out.moveto() down a line?<br></blockquote><div><br></div><div>To set the needsMoveTo flag just below the condition: if (needsMoveTo) {<span class="gmail-removed"></span> needsMoveTo = false; ...}<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

[D]Stroker.java, line 170ish - you added final to the float params, but not the double<br></blockquote><div><br></div><div>Fixed. I also fixed all (D)PathConsumer2D methods overriden in Dasher, Stroker & Renderer.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
[D]Stroker.java, line 196ish - why are ROUND treated differently.  You have a question on that as well in a comment.<br></blockquote><div><br></div><div>I found the answer: C = 4/3 * (SQRT(2) - 1) is used to compute the control points (cubics) to approximate a circle. I fixed the constant estimation according to the math formula.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
[D]Stroker.java, line 196ish - CAP_SQUARE would require more than lw/2 padding.  I think it needs lw*sqrt(2)/2 padding.<br></blockquote><div><br></div><div>Exactly, good point ! <br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

I would think the padding would be (max of the CAP/JOIN values below):<br>
CAP_BUTT - lw/2<br>
CAP_ROUND - lw/2<br>
CAP_SQUARE - lw/2 * sqrt(2)<br>
JOIN_BEVEL - lw/2<br>
JOIN_ROUND - lw/2<br>
JOIN_MITER - max(lw/2, miter_limit * lw)<br>
<br>
In other words:<br>
- lw/2 by default<br>
- if CAP_SQUARE then multiply that by sqrt(2)<br>
- if JOIN_MITER then max it with limit<br></blockquote><div><br></div><div>I agree your new rules.<br></div><div>I fixed the (D)Stroker init() methods according the latter rules and tested again.</div><div><br></div><div>Probably I should write a new Clip test rendering Z shapes with all  (cap / join) combinations and their bounds aligned to the Stroker's outside clip rules.</div><div><br></div><div>Here is an updated webrev (Marlin2D only):<br></div><div><a href="http://cr.openjdk.java.net/~lbourges/marlin/marlin-080.1/">http://cr.openjdk.java.net/~lbourges/marlin/marlin-080.1/</a></div><div><br></div><div><br></div><div>PS: I can send you an updated MarlinFX patch (when you need it).<br></div><div><br></div><div>Cheers,</div><div>Laurent<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

I'm still looking through the management of the closed path detection coordinates, but I thought I'd get at least these questions out first before the weekend...<br>
<br>
                                ...jim<span class="gmail-"><br>
<br>
On 8/25/17 1:09 PM, Laurent Bourgès wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">
Hi,<br>
<br>
Please review Marlin/FX upgrades that provide an efficient path clipper in<br>
Stroker (float / double variants) fixing the bug JDK-8184429<br></span>
<<a href="https://bugs.openjdk.java.net/browse/JDK-8184429" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net<wbr>/browse/JDK-8184429</a>><div><div class="gmail-h5"><br>
<br>
Marlin2D patch (jdk10):<br>
<a href="http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-080.0/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~lb<wbr>ourges/marlin/marlin-080.0/</a><br>
MarlinFX patch (openjfx10):<br>
<a href="http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlin-080.0/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~lb<wbr>ourges/marlinFX/marlin-080.0/</a><br>
<br>
Path Clipper in (D)Stroker:<br>
- it uses outcode computation (cohen - sutherland) for segment edges (2 for<br>
lines, 3 for quads, 4 for cubics)<br>
- it opens the path when a segment is invisible ie special moveTo() and<br>
ignore invisible joins; it does not compute any intersection (line /<br>
curve), it just skips useless segment for better performance and accuracy<br>
- the ClosedPathDetector is needed to infer if the path is closed or not<br>
(call to closePath) to produce caps when appropriate. It reuses the former<br>
PolyStack (moved into Helper classes) to store the forward segments<br>
- the clip rectangle is adjusted either in Stroker but also in<br>
Transformer2D to take into account the mitter limit, stroker width and also<br>
the Renderer offset<br>
<br>
That's why it can not be applied to closed paths (fill operations) as the<br>
path must remain closed in such case (concave polygon).<br>
This could be implemented later as it needs to insert corner points when<br>
needed to avoid artefacts; so the algorithm seem more complicated to me.<br>
<br>
Marlin2D / FX Patches are slightly different to handle the Renderer offsets.<br>
<br>
I tested these patches against my MapBench test with a small clip and<br>
several affine transforms: it does not produce any artefact (0 pixel<br>
difference between clip=true/false)<br>
<br>
PS: I also improved the accuracy of Renderer's AFD by using the kaham<br>
compensated-sum approach (later patch)<br>
<br>
Cheers,<br>
Laurent Bourgès<br>
<br>
</div></div></blockquote>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">-- <br>Laurent Bourgès</div>
</div></div></div></div></div>