MarlinFX upgrade 0.7.5
james.graham at oracle.com
Mon May 15 21:34:14 UTC 2017
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 did a bit of diffing:
- Marlin2D against MarlinFX
- *NoAA against the AA versions
- RendererNoAA and DRendererNoAA line 38 are missing a "d" suffix.
- RendererNoAA and DRendererNoAA lines 61,90 - missing comment about test
- Helpers and DHelpers - some adjustment of "pts[ off+1 ]" lines that didn't happen in 2D
- Configurable constants for Inc/Dec/Quad/Cubic in 2D, but not FX
- 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)
On 5/11/17 2:14 PM, Laurent Bourgès wrote:
> Hi Jim,
> Please review this updated webrev:
> I mainly synchronized again all my Marlin repositories and this provides an
> up-to-date patch in synch with Marlin2D.
> - Renderer (4 variants): use shared memory among AA and NonAA renderer
> - Fixed comments in Helpers, XXXArrayCache
> - Marlin2D backports in ArrayCacheConst, Renderer ...
> - SWUtils: remove import + JavaShapeRenderer made static
> - fixed copyright year to 2017
> You can see the incremental changes by comparing the openjfx10.patch:
> diff marlinFX-075.0/openjfx10.patch marlinFX-075.1/openjfx10.patch
> 2017-04-26 7:06 GMT+02:00 Jim Graham <james.graham at oracle.com>:
>> I've reviewed the code and run a number of tests. Things look fine.
>> I spotted at least one thing that I brought up in the 2D Marlin review,
>> but since the 2 source bases are moving towards synchronizing with each
>> other I didn't look too closely since many of the changes in the 2D Marlin
>> update are things that are already "fixed" in this FX Marlin code, so I
>> thought I would focus my scrutiny more on the 2D review instead. Would this
>> code base be affected by the review comments I made there? Did you want to
>> hold both until they both are ready to go in and then push them at the same
>> time (to keep them in sync)?
>> Minimally, it is time to file a bug against FX for this...
>> On 4/19/17 11:35 PM, Laurent Bourgès wrote:
>>> Please review this MarlinFX upgrade to Marlin 0.7.5:
>>> JBS: no bug yet for OpenJFX 10
>>> webrev: http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-075.0/
>>> - Renderers: fixed block processing
>>> - dead code & few comment removals in Strokers
>>> - fixed all floating-point number literals to be x.0f or x.0d to simplify
>>> the conversion between float & double variants
>>> PS: I plan to run later FindBugs, Netbeans & IntelliJ code analysis tools
>>> to fix any warning
More information about the openjfx-dev