[OpenJDK 2D-Dev] RFR 8144938: Handle properly coordinate overflow in Marlin Renderer

[Laurent Bourgès]

Dear Jim,

Here is a new webrev:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.2/

Changes:
- pathToLoop() rewritten to simplify the logic (skip and pathClosed flags
removed)
- added several createStrokedShape() tests in CrashNaNTest class

I hope my changes to the filtering algorithm are correct:
I advocate not understanding why it was so complicated (3 flags => 1) but
maybe I am wrong and I missed several corner cases...


My comments below:

2016-03-15 0:34 GMT+01:00 Jim Graham <james.graham at oracle.com>:

> Hi Laurent,
>
> 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?
>

Fixed and it produces empty paths (only pathDone):

I wonder if it is time to remove the following lines 317-322 in
MarlinRenderingEngine.strokeTo():
                // Every path needs an initial moveTo and a pathDone. If
these
                // are not there this causes a SIGSEGV in libawt.so (at the
time
                // of writing of this comment (September 16, 2010)).
Actually,
                // I am not sure if the moveTo is necessary to avoid the
SIGSEGV
                // but the pathDone is definitely needed.
                pc2d.moveTo(0f, 0f);

For me, it is useless as only pc2d.pathDone() is mandatory.


>
> I like your approach.  It is worth noting that:
>
> - non-uniform scales are not common
> - 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).
>

Thanks.
It has an important consequence for me:
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.


>
> With respect to the optimization that I gave a rough outline of.  It is
> true that:
>
> - it should help eliminate all of the inverse transforms in the strokeTo
> code
> - the math is incomplete and would need some work
> - it only targets the non-uniform case which may not be a high priority
> - 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
>

I totally agree.

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.

Thanks for your comments,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160315/87017d7e/attachment.html>