RFR: 8287604: Update MarlinFX to 0.9.4.5 [v3]

[Kevin Rushforth]

On Thu, 7 Jul 2022 22:46:53 GMT, Laurent Bourgès <lbourges at openjdk.org> wrote:

>> Changelog for this MarlinFX 0.9.4.5 release:
>> 
>> The Marlin-renderer 0.9.4.5 release provides bug fixes on Marlin's path clipper:
>> - improved Stroker to handle huge coordinates, up to 1E15
>> - improved PathClipFilter (filler) to handle huge coordinates, up to 1E15
>> 
>> 
>> This is the Marlin-renderer 0.9.4.3 release providing few bug / enhancement fixes in the MarlinRenderingEngine:
>> - Update DPQS to latest OpenJDK 14 patch
>> - Improve cubic curve offset computation
>> 
>> 
>> The Marlin-renderer 0.9.4.2 release provides a single long-standing bug fix in the MarlinRenderingEngine: 
>> - JDK-8230728, https://bugs.openjdk.java.net/browse/JDK-8230728.
>> 
>> 
>> Marlin-renderer 0.9.4.1 provides only a single bug fix in the path clipper, reported first against JavaFX 11: 
>> - JDK-8226789, https://bugs.openjdk.java.net/browse/JDK-8226789.
>> 
>> 
>> This is the Marlin-renderer 0.9.4 release providing an updated Dual Pivot Quick Sort (19.05) as its internal sorter faster than the Marlin's optimized MergeSort (x-position + edge indices) for arrays larger than 256.
>> 
>> Special Thanks to Vladimir Yaroslavskiy that provided me up-to-date DPQS 19.05 with many variants, improving almost-sorted datasets. We are collaborating to provide a complete Sort framework (15 algorithms, many various datasets, JMH benchmarks) publicly on github:
>> see https://github.com/bourgesl/nearly-optimal-mergesort-code
>
> Laurent Bourgès has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed pixel color tests on hi-dpi

I've tested this on all three platforms and everything looks good. I can confirm that the newly added test fails without the fix and passes with the fix, as does the test program attached to [JDK-8274066](https://bugs.openjdk.org/browse/JDK-8274066).

I've reviewed most of the code, except for the DPQS sorter itself; I'll do at least a cursory review of that in the next couple of days. I left an inline question as to whether it might make sense to initially leave it disabled by default for this release in order to reduce the risk of regression.

modules/javafx.graphics/src/main/java/com/sun/marlin/MarlinProperties.java line 206:

> 204: 
> 205:     public static boolean isUseDPQS() {
> 206:         return getBoolean("prism.marlin.useDPQS", "true");

In order to reduce risk, it might make sense to disable DPQS by default initially. It would then be available to anyone who wants to try it out with more complex paths, but would be less prone to possible regressions. What do you think?

tests/system/src/test/java/test/com/sun/marlin/HugePolygonClipTest.java line 149:

> 147:             err.initCause(ex);
> 148:             throw err;
> 149:         }

Suggestion: if you add `throws Exception` to the method signature, this try/catch block can be simplified to a single assert statement:


    assertTrue("Timeout waiting for Application to launch",
            launchLatch.await(TIMEOUT, TimeUnit.MILLISECONDS));


Many of our older tests use the patter in this method, but most newer tests use the simplified check.

-------------

PR: https://git.openjdk.org/jfx/pull/674