Re: Fwd: Re: Marlin-Renderer and JavaFX

[Kevin Rushforth]

Hi Laurent,

Great progress.
I agree with Sergey about avoiding unconditional logging, and about 
wanting a different name for the property (work with Jim on the property 
name).

To answer a couple other questions:

Does it look acceptable as a low risk RFE ?

I would need to see the final diff against FX 9-dev, but if the only 
change to the current control flow in the existing logic is the 
additional "if" test for the existence of the new property, and if the 
Marlin code base itself is basically a copy of what is in Java2D, then I 
think it is low-risk enough to propose it. I can't guarantee that it 
will be accepted.

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

The former. It will add risk and complexity to wrap the version of 
Marlin in Java2D for JDK 9.

I can make separate patches for all these changes concerning jfx 
Path2D or
Marlin changes for OpenJDK9 first, then MarlinFX.

Please do file a new JBS bug for Path2D and let's fix that separately. 
As a bug fix it will not need to go through the feature request 
extension process.

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.

We will need a patch based on FX 9 before we can propose this for 9, so 
this sounds like a good plan.

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

I recommend whatever is easiest to get to a patch that will apply to FX 
9-dev and build using JDK 9.

-- Kevin




Sergey Bylokhov wrote:
Probably the logging should be enabled only if we pass the upper case 
"True" to "-Dsun.javafx.marlin=True". I do not know is it correct to 
write to the system output unconditionally, can this affect 
application? And instead of "sun." can we use something different like 
"jdk." I guess the same question can be applied to jdk?

On 03.11.16 0:54, 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