[10] RFR 8188062: Use Marlin renderer in JavaFX BasicStroke

[Kevin Rushforth]

I did a fairly quick review and ran tests on two platforms. It looks 
good to me. I have some minor formating nits:

PathConsumer2D.java:

1. In the following:

+    public DPathConsumer2D wrapPath2d(Path2D p2d)
+    {


the opening curly brace can go at the end of the method declaration 
(same pattern appears in a few other files)


2. Can you add a blank line before the Path2DWrapper class declaration?

Other than that, as long as you address Phil's concerns it is OK to go in.

Thanks.

-- Kevin


Laurent Bourgès wrote:
> Just a gentle reminder...
> Kevin, could you have a look ?
>
> I have other patches coming soon...
>
> Cheers,
> Laurent
>
> Le 27 oct. 2017 8:02 PM, "Laurent Bourgès" <bourges.laurent at gmail.com 
> <mailto:bourges.laurent at gmail.com>> a écrit :
>
>     Hi Phil,
>     Thanks for your comments.
>
>     Le 27 oct. 2017 18:28, "Phil Race" <philip.race at oracle.com
>     <mailto:philip.race at oracle.com>> a écrit :
>
>           38     private final Path2DWrapper        wp_Path2DWrapper        = new Path2DWrapper();
>
>         Are there tabs  here ? The formatting looks odd.
>
>
>     It is manually aligned with followings lines (only space).
>
>
>           44     public DPathConsumer2D wrapPath2d(Path2D p2d)
>         The method naming pattern I see elsewhere is  "2D" not "2d" .. so can we fix this one ?
>         ie make it wrapPath2D
>         This occurs in at least two different files in this webrev: [D]TransformingPathConsumer2D.java
>               
>
>     I agree and I can fix it here and also in Marlin2D (same code).
>
>         I really don't like the changes in BasicStroke that not use direct literals instead of copying
>         the values from Stroker. It can't possibly help performance and you lose the implied relationship.
>
>     I agree your point of view. However the reference or origin was
>     the public BasicStroke in 2d and Marlin checks that constants
>     match in static initializers (MarlinRenderingEngine). 
>
>     I prefer applying the same pattern in FX so I did that change
>     which removes the reference to OpenPisces.Stroker.
>
>         -    public static final int CAP_BUTT = Stroker.CAP_BUTT;
>         -    public static final int CAP_ROUND = Stroker.CAP_ROUND;
>         -    public static final int CAP_SQUARE = Stroker.CAP_SQUARE;
>         -
>         -    public static final int JOIN_BEVEL = Stroker.JOIN_BEVEL;
>         -    public static final int JOIN_MITER = Stroker.JOIN_MITER;
>         -    public static final int JOIN_ROUND = Stroker.JOIN_ROUND;
>         +
>         +    /** Constant value for end cap style. */
>         +    public static final int CAP_BUTT = 0;
>         +    /** Constant value for end cap style. */
>         +    public static final int CAP_ROUND = 1;
>         +    /** Constant value for end cap style. */
>         +    public static final int CAP_SQUARE = 2;
>         +
>         +    /** Constant value for join style. */
>         +    public static final int JOIN_MITER = 0;
>         +    /** Constant value for join style. */
>         +    public static final int JOIN_ROUND = 1;
>         +    /** Constant value for join style. */
>         +    public static final int JOIN_BEVEL = 2;
>
>          
>         The next one is not something I think must be fixed but an observation that it
>         seems odd to have to decide which Rasterizer to use every time some one calls this
>         API rather than making it a one time initialisation.
>               
>
>     I wondered the same question. As PrismSettings are constants,
>     hotspot will optimize that code as a direct call to the proper
>     method (dead code elimination).
>
>         +    public static Shape createCenteredStrokedShape(Shape s, BasicStroke stroke)
>         +    {
>         +        if (PrismSettings.rasterizerSpec == RasterizerType.DoubleMarlin) {
>         +            return DMarlinRasterizer.createCenteredStrokedShape(s, stroke);
>         +        }
>         +        if (PrismSettings.rasterizerSpec == RasterizerType.FloatMarlin) {
>         +            return MarlinRasterizer.createCenteredStrokedShape(s, stroke);
>         +        }
>         +        // JavaPisces fallback:
>         +        return createCenteredStrokedShapeOpenPisces(s, stroke);
>         +    }
>         +
>
>     Laurent
>
>         -phil.
>
>
>
>         On 10/25/2017 02:11 PM, Laurent Bourgès wrote:
>>         Please review this simple patch that moves the
>>         BasicStroke.createCenteredStrokedShape() implementation into ShapeUtil in
>>         order to use Marlin instead of OpenPisces:
>>
>>         JBS: https://bugs.openjdk.java.net/browse/JDK-8188062 <https://bugs.openjdk.java.net/browse/JDK-8188062>
>>         Webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-8188062.0/ <http://cr.openjdk.java.net/%7Elbourges/marlinFX/marlinFX-8188062.0/>
>>
>>         Changes:
>>         - BasicStroke: fixed definition of CAP / JOIN constants +
>>         createCenteredStrokedShape() calls ShapeUtil createCenteredStrokedShape()
>>         (delegation)
>>         - ShapeUtil.createCenteredStrokedShape() delegates to (D)MarlinRasterizer
>>         (if enabled) or to OpenPisces (fallback)
>>         - (D)MarlinRasterizer: applied the same approach as in Marlin2D
>>         createStrokedShape() except I adopted the lineWidth trick as in OpenPisces
>>         - (D)MarlinPrismUtils: some refactoring to let the new strokeTo() method
>>         reuse the initPipeline() + simplified Path2D special handling
>>         - (D)RendererContext / (D)TransformingPathConsumer2D: added cached Path2D
>>         instance and Path2DWrapper (like in Marlin2D)
>>
>>
>>         PS: Removing OpenPisces in the future will be easier as remaining
>>         references are in ShapeUtil, OpenPiscesPrismUtils and few sw pipeline
>>         classes. Use the following command to get usages:
>>         find . -name "*.java" -exec grep -H "openpisces" {} \;)
>>
>>         Cheers,
>>         Laurent
>>                 
>
>