Hi Jim, I didn't notice anything functionally wrong with reviewing the webrev. I'm > going to do some basic testing on it now while you create a JBS bug for it > and submit for a formal review. But I did notice some discrepancies by > diffing the sources against each other which it would be nice to know if > they were oversights or "future work". If there is something you think > should be addressed now, do that in the process of switching to a formal > review (with JBS #) on it... >
I will create the JBS bug asap. > I did a bit of diffing: > > - Marlin2D against MarlinFX > - *NoAA against the AA versions > > and noticed: > Thanks for your carefull analysis, here are my comments below: - RendererNoAA and DRendererNoAA line 38 are missing a "d" suffix. > - RendererNoAA and DRendererNoAA lines 61,90 - missing comment about test > I forgot to synchronize recent changes into RendererNoAA variants: good catch ! > - Helpers and DHelpers - some adjustment of "pts[ off+1 ]" lines that > didn't happen in 2D > I decided to revert those minor syntax changes. > - Configurable constants for Inc/Dec/Quad/Cubic in 2D, but not FX > I hesitated but decided to backport the support for these configurable constants in all (D)Renderer(NoAA) (4 variants) > - DRendererSharedMemory in FX, but not 2D > > - FloatMath.ceil_f deleted from FX, but not 2D (not used in 2D) > - (FloatMath.floor_f is also only in 2D, but it is used in > MarlinRenderingEngine) > I decided to leave those FloatMath differences as Marlin2D (and FloatMath Tests) do use these methods. Finally I propose another MarlinFX webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-075.2/ Changes: - (D)Helpers: revert syntax changed in cubicRootsInAB() + fixed comment in subdivide() - (D)MarlinRenderingEngine: added logs for cubic/quad properties - (D)Renderer and (D)RendererNoAA: use cubic/quad properties like Marlin2D - (D)RendererNoAA: fixed syntax changes mentioned: missing "d" suffix + comment about test - MarlinProperties: added quality settings (cubic/quad) PS: I will send an updated webrev for the Marlin2D patch. Regards, Laurent
