Re: MarlinFX upgrade 0.7.5

[Jim Graham]

That's fine, I just wanted to make sure the difference was intentional and not 
something that fell through the cracks...

                        ...jim

On 5/16/17 2:17 PM, Laurent Bourgès wrote:

One more comment about '(D)RendererSharedMemory in FX, but not 2D':

This reduces the memory footprint of the RendererContext among Renderer and
RendererNoAA implementations (shared arrays) that can co-exist at runtime
if both AA and NonAA rendering is needed.

As this behavior is unique to JavaFX, I wonder if it is worth to backport
this small difference in Marlin2D where there will be no gain.

Laurent


2017-05-16 23:11 GMT+02:00 Laurent Bourgès <bourges.laur...@gmail.com>:

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