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

Jim Graham james.graham at oracle.com
Thu Mar 17 00:49:48 UTC 2016


Hi Laurent,

I'm not sure why you added the pathClosed variable back.  Before your 
fix we used to rely on all of the downstream consumers correctly 
implementing the case of CLOSE followed by a non-MOVE command so 
specifically enforcing a MOVE here should be unnecessary and just 
complicates the code...?

			...jim

On 3/16/16 8:16 AM, Laurent Bourgès wrote:
> Jim,
>
> Here is another webrev:
> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.3/
>
> Changes:
> - restored pathClosed flag to fix the 'hour glass' issue below
> - added pathClosedTest in CrashNaNTest
>
> Comments below:
>
> 2016-03-15 23:45 GMT+01:00 Jim Graham <james.graham at oracle.com
> <mailto:james.graham at oracle.com>>:
>
>     The new tests look good.
>
>
> Thanks.
>
>
>     There is one logic error in the new version of the degenerate
>     coordinate filter.
>
>     It is valid to have a PATH_CLOSE immediately followed by a <geom>To
>     call and the subpath will start at the previous close/moveto point.
>     For example:
>
>     moveto 40, 40
>     lineto  0,  0
>     lineto 80,  0
>     closepath
>     lineto 80, 80
>     lineto  0, 80
>     closepath
>
>     should draw an hourglass, but with your fix it will lose the lower
>     half because the lineto after the close will be turned into a moveto
>     (and the second subpath then becomes empty.
>
>
> I tested my previous patch and you were right: it was buggy.
>
>     I think it should be fine just getting rid of the subpath=false in
>     the close case.
>
>
> I prefered reverting to the ductus approach even if your proposal seems
> working.
>
> I agree my patch is a bit tricky but it explicitely emits a moveTo()
> after pathClosed() instead of relying to the path consumer to implement
> the same behaviour (in dasher / stroker)...
>
>     It is arguable, though, if we have closepath followed by a NaN if
>     perhaps that NaN should break the subpath from the previous close,
>     but I don't think that is in keeping with the current philosophy of
>     simply pretending that bad coordinates don't exist.  Essentially,
>     the close of a previously valid path gives us a valid start to the
>     new subpath regardless if it immediately feeds into more bad
>     coordinates - it still started its run on a valid point.  So, I'd
>     argue that simply passing along the prior value of the subpath flag
>     would be appropriate, whether it is true or false (and, obviously,
>     also closing the path in the consumer if it is true)...
>
>
> Please tell me what solution do you prefer.
> I can adopt yours which seems really simple.
>
> Laurent



More information about the 2d-dev mailing list