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
>> >
>>
>>
