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.laur...@gmail.com> a
écrit :
> Hi Phil,
> Thanks for your comments.
>
> Le 27 oct. 2017 18:28, "Phil Race" <philip.r...@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
> Webrev: http://cr.openjdk.java.net/~lbourges/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
>
>
>
>
