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