[OpenJDK 2D-Dev] [10] Marlin2D upgrade 0.7.5

Jim Graham james.graham at oracle.com
Tue Apr 25 23:47:31 UTC 2017


Hi Laurent,

- I found some fp constants with no "d" or "f" modifier.  I sent a grep output in a separate email...

- Why does FloatMath.java copy the constants from sun.misc rather than refer to them?
   (An alternative is a static import or having local copies that copy by
    source-based reference (foo_bar = Foo.bar) rather than copying by
    human-cut-and-paste)

- Should FloatMath be renamed to FPMath or IEEEMath since it now also handles doubles?

- MarlinCache - What was the reason to eliminate the mean crossings calculations from the decision on whether or not to 
use RLE?

- MarlinCache line 548 - why the simpler logic here?

- MarlinProperties, getLog2_XY - 0 to 4 is 1 to 16 subpixel locations in X or Y but you list the subpixel limits as 1 to 
256.  Also, it looks like the real limits are 0 to 8 which really does work out to 1 to 256.  So, I think the "4" in the 
comment is incorrect and should be "8"?

- MarlinProperties - TileSize vs TileWidth.  Is there a reason you haven't created a TileHeight property?  I could see a 
couple of ways of going here:
    - TileSize means height and TileWidth is new which is just odd naming
      In this case, I'd make the default for TileWidth be the value for TileSize
      otherwise setting tilesize used to set both W&H, but now it only sets H?
    - TileSize is legacy and new values are TileWidth and TileHeight
      Both default to TileSize if not specified
In either case, I would think that TileWidth should default to TileSize?

- MarlinProperties - Inc/Dec - constants don't end with d or f.

- MarlinProperties - getFloat() takes doubles for def/min/max?

- MarlinTileGenerator - lines 67,69 - null is the default value for fields

- MarlinTileGenerator,MarlinRenderer - all of the methods called on rdrF and rdrD have the same signature.  Why not have 
MarlinRenderer include those methods and then you just need to store a single MarlinRenderer field and be able to 
manipulate either type...?

- MarlinTileGenerator.TH_AA_ALPHA_00/FF - seem misnamed, really they are low and high thresholds for filling.  Maybe LO 
and HI?

- MarlinTileGenerator, line 416,443,508 - Isn't that "skip" instead of "fill"?

- MarlinTileGenerator, line 420,554,687 - Isn't cx >= aax0?  And isn't x1 >= aax0? ???

- MarlinTileGenerator, line 491 - spacing on byte cast

- MarlinTilegenerator, line 655 - "Clear" or "Fill"?

- Renderer, line 85,114 - maybe add a line saying that is the result of <name> test?  Did we put that test into the repo 
anywhere?

- Renderer, line 1318,1365 - that was fallout from the half-open stuff at 1391, right?

- Stroker, line 112 - "* 8"?  Or perhaps "* 6 + 2" since the endpoints are shared?

- Stroker, line 347,376 - it appears to still use m[off],m[off+1] instead of 0,1...?

- Stroker, line 377 - safecomputeMiter -> safeComputeMiter?  Also, this looks to be some sort of "compute something for 
an offset for quads" method, at least in the way it is used, even if it does look similar to the computeMiter method.

- Stroker, line 841 - "winden" => "widen"

- Stroker, lines 839,847,856,866 - there is no p4

I'm assuming that the D*.java files were all generated from the regular files using a sed script?  I skipped those (for 
now)...

				...jim


On 4/22/17 4:28 AM, Laurent Bourgès wrote:
> Hi,
>
> I am pleased to have successfully upgraded my machine with OpenJDK10 and merged my Marlin 0.7.5 release (dec 2016) with
> its Java2d variant.
>
> Please review this Marlin2D upgrade to Marlin 0.7.5:
>
> JBS: no bug yet for OpenJDK 10
> webrev: http://cr.openjdk.java.net/~lbourges/marlin/Marlin-075.0/
>
> Changes:
> - Improved MarlinTileGenerator to efficiently handle asymetric tiles (W x H) with almost full / empty fills
> - Added Marlin Double-precision pipeline
> - MarlinFX backports (Dasher, Stroker changes from OpenPisces)
> - 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
>
> As this patch is huge, do you want me to provide webrevs against Float vs Double pipelines or against OpenJFX10 ?
>
> PS: I forgot to upgrade the copyright headers to 2017, but will do later
>
> Cheers,
> Laurent



More information about the 2d-dev mailing list