I can confirm that this is the same solution provided for the Ductus
code so it will bring us to par with the former rasterizer.
I would also check what happens when you try to render and
createStrokedPath() for a path that is so full of NaN values that it
never gets any segments out, and/or if it only manages a moveto - just
make sure that we render nothing rather than get an exception because
something in the code isn't set up for that. I'm guessing that this may
already be covered by a test case that ensures that we don't blow up
with g2d.fill/draw(new Path2D.Double()); but it couldn't hurt to add a
quick test.
Finally, I'll mention that I was never very comfortable with the way
that code was written in the Ductus pipeline. It does the trick and I
don't see it passing through any NaN or infinite values, but it just
seems like it might be overly complicated and it's hard to make sure
that it is doing something "sensible" in the face of a partially
constructed secondary sub-path (i.e. pathClosed handling). For one
thing, I'm >80% sure that skip and subpathStarted could be folded into
the same boolean, and/or that our output in the case where
skip/subpathStarted disagree is what we want it to be, but this is all
beyond the scope of this fix.
I'd say add another test for completely bogus path(s) and it's good to
go, but at least keep it in the back of your mind that this code might
not represent the best solution to the problem...
...jim
On 3/10/16 7:09 AM, Laurent Bourgès wrote:
Hi,
Please review this bug fix for the Marlin renderer to properly ignore
point coordinates with NaN/Infinity values:
bug: https://bugs.openjdk.java.net/browse/JDK-8144938
webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.0/
Changes:
- MarlinRenderingEngine.pathToLoop(): added bound checks for point
coordinates by portingDuctusRenderingEngine#feedConsumer()
- CrashNaNTest: check output image to detect incorrect rendering
- minor: unused import in AAShapePipe + Version incremented
Regards,
Laurent
