Fwd: Re: Marlin-Renderer and JavaFX

[Jim Graham]

FX enhancement: https://bugs.openjdk.java.net/browse/JDK-8169270

			...jim

On 11/4/2016 8:21 AM, Kevin Rushforth wrote:
> Hi Jim,
>
> Thanks for the summary. We should proceed as you outlined. Can you file
> a new RFE (Enhancement) to integrate Marlin into JavaFX as an optional
> rasterizer (disabled by default) so we have a clean JBS issue to use for
> the JDK 9 feature extension request, rather than using JDK-8092373? You
> can assign the RFE to Laurent. It can be linked to the Java2D/Marlin JEP
> and to JDK-8092373.
>
> Laurent: Since there are multiple reasons for submitting the Path2D fix
> as a separate bug fix, please file a new bug for this, linking it to the
> equivalent Java2D bug and also to the new RFE that Jim will file.
>
> Thanks.
>
> -- Kevin
>
>
> Jim Graham wrote:
>> There are basically 2 isolated changes to the existing code base and
>> then a set of added source files.
>>
>> The first change is to use Marlin if the appropriate property is
>> specified, and those changes are very localized and easy to verify
>> that they won't hurt anything.
>>
>> The second change is to modify the growth pattern of Path2D.  While
>> these changes are live in AWT already and have already been code
>> reviewed, it would probably be better to submit them as a separate FX
>> fix if they are only performance oriented and not strictly required
>> for Marlin to function.  That way we compartmentalize anything that
>> could possibly result in a regression into a separate bugid so we
>> don't have to pull everything if someone complains that the new growth
>> pattern is having negative consequences for their app.  I doubt that
>> will happen, but it is simple enough to break them into 2 separate
>> fixes so it couldn't hurt to do that.
>>
>> After that, this just boils down to adding a bunch of code that has
>> already been vetted elsewhere and a small and simple change to use it
>> only optionally and conditionally, which is a very low risk fix.
>>
>> That would take this change from "no obvious drawbacks" to "obviously
>> no drawbacks" (or, more precisely, one "obviously no drawbacks" fix
>> and one related "no obvious drawbacks" fix)...
>>
>>             ...jim
>>
>> On 11/2/2016 2:54 PM, Laurent Bourgès wrote:
>>> Jim,
>>>
>>> Here is an updated patch for MarlinFX:
>>> http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-s02-ofx9/
>>>
>>> I made big improvements: MarlinFX is now "feature-complete":
>>> - Added MarlinAlphaConsumer & MarlinRenderer interfaces to define new
>>> methods on AlphaConsumer and common methods between AA & noAA renderers
>>> - Renderer: fixed cubic dec/inc lengths (like openpisces) + use
>>> optimized
>>> copyAlphas from MarlinAlphaConsumer (with block flag optimization
>>> derived
>>> from former MarlinCache)
>>> - RendererNoAA: optimized Renderer for non-antialiasing (tested OK)
>>> - Dasher & Stroker: backported changes from openpisces (small dash
>>> phase &
>>> refined cubic / quad mitters)
>>> - Added MarlinPrismUtils & MarlinRasterizer classes to wrap the Marlin
>>> renderer as a JavaFX ShapeRasterizer and implement the
>>> MarlinAlphaConsumer
>>> efficiently (mimics former MarlinCache ie support the block flag
>>> optimization); MarlinPrismUtils performs properly NaN / Infinity
>>> coordinates filtering and use the same pipeline stages (delta / invDelta
>>> transformers for Dasher/Stroker) like in the MarlinRenderingEngine
>>> - Thread safety: MarlinRasterizer completely thread-safe (for future
>>> multi-threaded rendering) using ReentrantContext...
>>> - Modified (OpenJFX) ShapeUtil to use the MarlinRasterizer instead of
>>> the
>>> OpenPiscesRasterizer class (use -Dsun.javafx.marlin=true to enable
>>> Marlin-FX)
>>> - Fixed Path2D growing algorithm (like java2d)
>>>
>>> So MarlinFX is 13K LOC (few unused classes could be removed soon) and
>>> only
>>> few lines are added in ShapeUtil to switch MarlinFX ON:
>>>
>>>         if (PrismSettings.doNativePisces) {
>>>             shapeRasterizer = new NativePiscesRasterizer();
>>>         } else {
>>>
>>>
>>>
>>>
>>>
>>>
>>> *            // TODO: move that new setting into PrismSettings:
>>> // Enable Marlin-FX by setting -Dsun.javafx.marlin=true            if
>>> (MarlinProperties.isMarlinEnabled()) {
>>> System.out.println("Marlin-FX[" + Version.getVersion() + "]
>>> enabled.");                shapeRasterizer = new
>>> MarlinRasterizer();            } else {*
>>> shapeRasterizer =
>>> new OpenPiscesRasterizer();
>>>
>>> *            }*        }
>>>
>>> So the OpenPisces classes are totally left unchanged and MarlinFX is
>>> just
>>> added as another rasterizer and is enabled with the following settings:
>>> -Dsun.javafx.marlin=true and -Dprism.nativepisces=false
>>>
>>> Of course, we could adapt these few lines to satisfy your requirements
>>> (system properties ...); please tell me what you prefer.
>>>
>>> I tested this new release with DemoFX, Guimark HTML5, Ensemble8 and
>>> everything is working fine.
>>>
>>>
>>> Does it look acceptable as a low risk RFE ?
>>>
>>> Finally what do you prefer for OpenJDK9 integration ?
>>> - as a new javafx rasterizer (like the provided marlinFX patch)
>>> - or as a javafx wrapper using OpenJDK9's marlin renderer (java2d) ?
>>> I wonder if it would be better to create another JavaFX ShapeRasterizer
>>> that wraps OpenJDK9 Marlin (java2d) to minimize the code duplication
>>> but it
>>> will add some complexity in the marlin renderer (itself) to introduce
>>> the
>>> AlphaConsumer interface...
>>>
>>>
>>> I can make separate patches for all these changes concerning jfx
>>> Path2D or
>>> Marlin changes for OpenJDK9 first, then MarlinFX.
>>>
>>> PS: MarlinFX is based on my (internal) Marlin-renderer 0.7.5 (a bit
>>> different than OpenJDK9's Marlin as it is compatible with jdk 1.8) so I
>>> will synchronize both branches to provide soon MarlinFX9 patches
>>> closer to
>>> OpenJDK9's Marlin renderer if we are going to work on its integration.
>>>
>>> PS2: I will also work on another MarlinFX variant not using Unsafe
>>> but only
>>> plain arrays to evaluate the performance loss (small) that could
>>> simplify
>>> the integration with Jigsaw ...
>>>
>>> So I made a big step forward, and I am looking forward your feedback,
>>>
>>> Cheers,
>>> Laurent
>>>