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