[11] JDK-8204621: Upgrade MarlinFX to 0.9.2

[Kevin Rushforth]

 > Kevin & Johan, do you consider the last webrev is approved by 2 
reviewers ?

I took Johan's comment to be a "+1" from him. He can correct us if that 
wasn't his intent.

-- Kevin


On 7/5/2018 6:07 AM, Laurent Bourgès wrote:
> Johan,
> I agree Marlin 0.9.2 provides new clipping algorithms (curve 
> subdivision & clipping in the dasher stage) so it involves lots of 
> maths and tricks ...
>
> I implemented the automated ClipShapeTest that compares rendering with 
> clipping enabled vs disabled for all possible combinations:
> - stroked & dashed paths with all sort of joins / caps
> - filled paths with all filling rules (NZ /EO)
> with
>  lines/quads/curves ...
>
> This intensive test proves new algorithms do not introduce visual 
> regressions except that clipped stroked curves are slightly different 
> due to the curve offsetting algorithm. Such problem has no exact 
> solution and the approximated offsetted curve causes the minor visual 
> differences along the stroked curve.
>
> Finally MarlinFX 0.9.2 is giving same clipping accuracy than Marlin 
> 0.9.1 integrated in jdk11.
> Performance is mainly improved for huge dashed paths.
>
> Kevin & Johan, do you consider the last webrev is approved by 2 
> reviewers ?
>
> Laurent
>
> Le jeu. 5 juil. 2018 à 11:46, Johan Vos <johan.vos at gluonhq.com 
> <mailto:johan.vos at gluonhq.com>> a écrit :
>
>     I had a slightly deeper look and had a few comments, but it's hard
>     to check the math behind it and evaluating performance without
>     doing real tests.
>     However, I feel confident we can merge it. That will also allow
>     eager developers to use it and provide feedback.
>
>     - Johan
>
>     On Wed, Jul 4, 2018 at 9:55 AM Johan Vos <johan.vos at gluonhq.com
>     <mailto:johan.vos at gluonhq.com>> wrote:
>
>         I looked at it, mainly at the differences between the Java2D
>         patch and the JavaFX patch, and it looks ok to me.
>         I'll try to test it on linux and/or mac later today and have a
>         deeper look.
>
>         - Johan
>
>
>         On Tue, Jul 3, 2018 at 6:14 PM Kevin Rushforth
>         <kevin.rushforth at oracle.com
>         <mailto:kevin.rushforth at oracle.com>> wrote:
>
>             Looks good.
>
>             +1 -- note that needs a second reviewer (doesn't need to
>             be a capital-R
>             Reviewer).
>
>             -- Kevin
>
>
>             On 7/3/2018 8:56 AM, Kevin Rushforth wrote:
>             >> PS: I am not really satisfied by adding such noise in
>             build.gradle,
>             >> but it can be improved later ...
>             >
>             > Agreed. This can be a follow-on issue. I'll finish my
>             review shortly.
>             >
>             > -- Kevin
>             >
>             >
>             > On 7/3/2018 8:45 AM, Laurent Bourgès wrote:
>             >> Kevin,
>             >>
>             >>     > I added the system property
>             "ClipShapeTest.numTests" but it
>             >> requires a
>             >>     build.gradle change to pass the parameter:
>             >>
>             >>     Yes, something like this is what I had in mind. As
>             long as we
>             >>     don't add too many of these, it is OK with me. Note
>             that as coded,
>             >>     the build will fail if you don't define
>             ClipShapeTest.numTests, so
>             >>     you will need to check for that. I note also that
>             you used tabs in
>             >>     build.gradle (so please change them to spaces). I
>             recommend the
>             >>     following logic:
>             >>
>             >>             if
>             (rootProject.hasProperty("ClipShapeTest.numTests")) {
>             >>                 systemProperty "ClipShapeTest.numTests",
>             >> rootProject.getProperty("ClipShapeTest.numTests")
>             >>             }
>             >>
>             >>
>             >> I adopted your proposal and updated the webrev:
>             >>
>             http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.2/
>             <http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlinFX-092.2/>
>
>             >>
>             <http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlinFX-092.2/>
>             >>
>             >> PS: I am not really satisfied by adding such noise in
>             build.gradle,
>             >> but it can be improved later ...
>             >>
>             >> Laurent
>             >
>