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....@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.rushfo...@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/> >> >> >> >> PS: I am not really satisfied by adding such noise in build.gradle, >> >> but it can be improved later ... >> >> >> >> Laurent >> > >> >>