Fwd: Re: Marlin-Renderer and JavaFX

[Kevin Rushforth]

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