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
