[11] JDK-8204621: Upgrade MarlinFX to 0.9.2
Kevin Rushforth
kevin.rushforth at oracle.com
Thu Jul 5 13:24:17 UTC 2018
> 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
> >
>
More information about the openjfx-dev
mailing list