<div dir="ltr"><div><div><div>Dear Jim,<br><br></div>Here is a new webrev:<br><a href="http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.2/">http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.2/</a><br><br></div>Changes:<br></div>- pathToLoop() rewritten to simplify the logic (skip and pathClosed flags removed)<br><div>- added several createStrokedShape() tests in CrashNaNTest class<br><br></div><div>I hope my changes to the filtering algorithm are correct: <br>I advocate not understanding why it was so complicated (3 flags => 1) but maybe I am wrong and I missed several corner cases...<br><br><br></div><div>My comments below:<br></div><div><div><div><div><div><div><div class="gmail_extra"><br><div class="gmail_quote">2016-03-15 0:34 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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Laurent,<br>
<br>
Did you consider adding a test with a completely degenerate path to make sure that we don't have any state errors if all of the segments are eaten by the "finite path" filter?<br></blockquote><div><br></div><div>Fixed and it produces empty paths (only pathDone):<br><br>I wonder if it is time to remove the following lines 317-322 in MarlinRenderingEngine.strokeTo():<br><span style="font-family:monospace,monospace">                // Every path needs an initial moveTo and a pathDone. If these<br>                // are not there this causes a SIGSEGV in libawt.so (at the time<br>                // of writing of this comment (September 16, 2010)). Actually,<br>                // I am not sure if the moveTo is necessary to avoid the SIGSEGV<br>                // but the pathDone is definitely needed.<br>                pc2d.moveTo(0f, 0f);<br></span></div><div><br>For me, it is useless as only pc2d.pathDone() is mandatory.<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">
<br>
I like your approach.  It is worth noting that:<br>
<br>
- non-uniform scales are not common<br>
- your "always overflow-filter path at device resolution" fixes the issues with measuring overflow in user space that I pointed out in my prior email and only does so at a likely small expense in terms of non-uniform scale performance.  Common cases will see no change (other than the fact that you have new code in the path feeder).<br></blockquote><div><br></div><div>Thanks. <br>It has an important consequence for me:<br>it's possible to introduce a new stage in the pipeline (before inverseDeltaTransformConsumer) that implements path clipping in device-space ONLY compatible with the rectangular bounding box. <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">
<br>
With respect to the optimization that I gave a rough outline of.  It is true that:<br>
<br>
- it should help eliminate all of the inverse transforms in the strokeTo code<br>
- the math is incomplete and would need some work<br>
- it only targets the non-uniform case which may not be a high priority<br>
- it would probably get rid of the entire TransformingPathConsumer module, but that module is not complex or trouble-prone and seems to be working fine<span class=""><font color="#888888"><br></font></span></blockquote><div><br></div><div>I totally agree. <br><br>I think improving Marlin's Stroker to better optimize square caps / mitter joins or adding path clipping would provide more benefits as it would minimize the number of point / edges used later by the Renderer stage.<br><br></div><div>Thanks for your comments,<br></div><div>Laurent</div></div></div></div></div></div></div></div></div></div>