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

[Jim Graham]

Actually, there is one functional difference between this implementation 
and the Ductus pipeline...

The similar method in the Ductus pipeline is operating entirely on 
device space coordinates after all transformation (which is why it can 
have normalization built-in).

The method you modified is sometimes called on transformed device space 
coordinates, but in the case of a draw operation it can be called on 
user space coordinates if the transform is non-uniform.

Since this change will solve the issue for fills and uniform-scaled 
draws(), it handles the 90% case, but if you use a non-uniform scale of 
more than 2x, then it will still allow overflow in the final rendered 
path if a coordinate is near UPPER_BND...

			...jim

On 3/10/16 4:47 PM, Jim Graham wrote:
> 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