[OpenJDK 2D-Dev] RFR JDK-8184429: Path clipper added in Marlin2D & MarlinFX 0.8.0

Laurent Bourgès bourges.laurent at gmail.com
Mon Aug 28 21:09:00 UTC 2017

Hi Jim,

Thanks for your comments, it helped refining the webrev.

Here are my answers:

2017-08-26 2:22 GMT+02:00 Jim Graham <james.graham at oracle.com>:

> [D]Dasher.java - why the changes from (firstSegIdx > 0) to (firstSegIdx !=
> 0)?

As firstSegIdx is initialized to 0, I prefer testing (firstSegIdx != 0) as
it looks more obvious.
For me, (firstSegIdx > 0) indicates that the sign has a particular meaning
and that firstSegIdx may be negative.

> [D]Dasher.java - why is there a new goto_starting() which is only used
> from one place?  To be able to add final to the parameters?

For inlining purposes, the JITWatch jarscan tool reported that this
(hotspot) method is larger than 325 byte codes so I wanted to split it in
smaller parts.

> [D]Dasher.java, line 268ish - why move the call to out.moveto() down a
> line?

To set the needsMoveTo flag just below the condition: if (needsMoveTo) {
needsMoveTo = false; ...}

> [D]Stroker.java, line 170ish - you added final to the float params, but
> not the double

Fixed. I also fixed all (D)PathConsumer2D methods overriden in Dasher,
Stroker & Renderer.

> [D]Stroker.java, line 196ish - why are ROUND treated differently.  You
> have a question on that as well in a comment.

I found the answer: C = 4/3 * (SQRT(2) - 1) is used to compute the control
points (cubics) to approximate a circle. I fixed the constant estimation
according to the math formula.

> [D]Stroker.java, line 196ish - CAP_SQUARE would require more than lw/2
> padding.  I think it needs lw*sqrt(2)/2 padding.

Exactly, good point !

I would think the padding would be (max of the CAP/JOIN values below):
> CAP_BUTT - lw/2
> CAP_ROUND - lw/2
> CAP_SQUARE - lw/2 * sqrt(2)
> JOIN_BEVEL - lw/2
> JOIN_ROUND - lw/2
> JOIN_MITER - max(lw/2, miter_limit * lw)
> In other words:
> - lw/2 by default
> - if CAP_SQUARE then multiply that by sqrt(2)
> - if JOIN_MITER then max it with limit

I agree your new rules.
I fixed the (D)Stroker init() methods according the latter rules and tested

Probably I should write a new Clip test rendering Z shapes with all  (cap /
join) combinations and their bounds aligned to the Stroker's outside clip

Here is an updated webrev (Marlin2D only):

PS: I can send you an updated MarlinFX patch (when you need it).


> I'm still looking through the management of the closed path detection
> coordinates, but I thought I'd get at least these questions out first
> before the weekend...
>                                 ...jim
> On 8/25/17 1:09 PM, Laurent Bourgès wrote:
>> Hi,
>> Please review Marlin/FX upgrades that provide an efficient path clipper in
>> Stroker (float / double variants) fixing the bug JDK-8184429
>> <https://bugs.openjdk.java.net/browse/JDK-8184429>
>> Marlin2D patch (jdk10):
>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-080.0/
>> MarlinFX patch (openjfx10):
>> http://cr.openjdk.java.net/~lbourges/marlinFX/marlin-080.0/
>> Path Clipper in (D)Stroker:
>> - it uses outcode computation (cohen - sutherland) for segment edges (2
>> for
>> lines, 3 for quads, 4 for cubics)
>> - it opens the path when a segment is invisible ie special moveTo() and
>> ignore invisible joins; it does not compute any intersection (line /
>> curve), it just skips useless segment for better performance and accuracy
>> - the ClosedPathDetector is needed to infer if the path is closed or not
>> (call to closePath) to produce caps when appropriate. It reuses the former
>> PolyStack (moved into Helper classes) to store the forward segments
>> - the clip rectangle is adjusted either in Stroker but also in
>> Transformer2D to take into account the mitter limit, stroker width and
>> also
>> the Renderer offset
>> That's why it can not be applied to closed paths (fill operations) as the
>> path must remain closed in such case (concave polygon).
>> This could be implemented later as it needs to insert corner points when
>> needed to avoid artefacts; so the algorithm seem more complicated to me.
>> Marlin2D / FX Patches are slightly different to handle the Renderer
>> offsets.
>> I tested these patches against my MapBench test with a small clip and
>> several affine transforms: it does not produce any artefact (0 pixel
>> difference between clip=true/false)
>> PS: I also improved the accuracy of Renderer's AFD by using the kaham
>> compensated-sum approach (later patch)
>> Cheers,
>> Laurent Bourgès

Laurent Bourgès
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170828/020bc6fd/attachment.html>

More information about the 2d-dev mailing list