[11] JDK-8204621: Upgrade MarlinFX to 0.9.2

[Kevin Rushforth]

I tested this on all three platforms and the updated rasterizer looks good.

I spot checked the code changes, but didn't get time to do a complete 
review yet. I was mostly looking for diffs between the Java2D version 
which was already reviewed, and this one.

I do have a couple comments on the new ClipShapeTest (which looks like a 
nice accuracy test, btw).

1. The test runs for way too long (about 20x too long) to include in our 
normal test runs. By default the entire class file (all three tests) 
needs to take < 5 minutes and 2 minutes would be better. I measured the 
time on 4 machines that I have and found that if you cut the number of 
iterations down from 5000 to 250 it will be just about the right run 
time. Then you can set the timeout to 120 seconds (the slowest test on 
the slowest of my machines took about 48 seconds, so a 2 minute timeout 
should be plenty).


2. Can you explain the reason for setting the following?

  206         // disable static clipping setting:
  207         System.setProperty("prism.marlin.clip", "false");
  208         System.setProperty("prism.marlin.clip.runtime.enable", 
"true");
  209
  210         // enable subdivider:
  211         System.setProperty("prism.marlin.clip.subdivider", "true");
  212
  213         // disable min length check: always subdivide curves at 
clip edges
  214 System.setProperty("prism.marlin.clip.subdivider.minLength", "-1");
  215
  216         // If any curve, increase curve accuracy:
  217         // curve length max error:
  218         System.setProperty("prism.marlin.curve_len_err", "1e-4");
  219
  220         // cubic min/max error:
  221         System.setProperty("prism.marlin.cubic_dec_d2", "1e-3");
  222         System.setProperty("prism.marlin.cubic_inc_d1", "1e-4"); 
// or disabled ~ 1e-6
  223
  224         // quad max error:
  225         System.setProperty("prism.marlin.quad_dec_d2", "5e-4");

It seems better to test with the default parameters (i.e., it makes a 
better regression test that way).


3. Related to that, I think you should eliminate the following (I don't 
recommend running functional tests with this set), although since you 
don't do any animation, it probably doesn't matter.

  227         System.setProperty("javafx.animation.fullspeed", "true"); 
// full speed


-- Kevin


On 6/8/2018 7:28 AM, Laurent Bourgès wrote:
> Hi,
>
> Please review this large patch to upgrade MarlinFX to 0.9.2 in OpenJFX11:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8198885
> webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-092.0/ 
> <http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlinFX-092.0/>
> PR: https://github.com/javafxports/openjdk-jfx/pull/96 (CI OK)
>
> This patch is almost identical to Marlin(2D) patch, see:
> https://bugs.openjdk.java.net/browse/JDK-8198885
>
> I added the ClipShapeTest (ported to jfx) that compares shape clipping 
> (within threshold) and it works (within large timeouts):
> gradle -PFULL_TEST=true :system:test --tests 
> test.com.sun.marlin.ClipShapeTest
>
> Regards,
> Laurent