[OpenJDK Rasterizer] Marlin renderer contribution for review

[Jim Graham]

Hi Laurent,

I think it's time to get this in.  But, please keep these "Marlin vs 
Pisces" diffs around for future reference (typically I never delete any 
webrevs once I publish them to cr.openjdk anyway, but these initial 
diffs in particular are going to be important as I start looking at the 
specific changes in more depth).

One important functional issue that I think we should address early on 
is that there are a number of property references in 
MarlinRenderingEngine and MarlinConst that I don't think are protected 
by AccessController doPrivileged blocks.  These pieces of code cannot be 
run inside an applet or any context managed by a SecurityManager as 
access to arbitrary system properties is one of the first things to be 
disabled by security managers.  I'd recommend addressing that early on - 
your call as to whether you want to address it before the first push.

Here are some additional minor style things I noticed which I'll add 
here just to get them listed:

- There are a number of single-line "/** short comment */" style 
comments sprinkled around.  They aren't really doc comments since they 
don't contain all of the parts that one would expect in a doc comment. 
They should probably just be shortened to a regular comment (/* ... */), 
and any that are embedded in a class talking about a field could just as 
easily be a single-line comment format (// ...), but having them pretend 
to be doc comments (with the "/**") seems to be overkill.

- Also a few places where I would move an open brace starting a method 
after a continued argument list to a new line as per Java2D coding 
style, but none of them were jarring enough that my eye had trouble 
distinguishing method body from argument list so I'll just note it as a 
general comment to keep in mind.

MarlinTileGenerator, line 119 (only noticed because it was changed) - 
when I have multi-line continuations for ?: operations I usually line 
them up as:

      (condition
       ? true-clause
       : false-clause)

for readability.

MarlineTileGenerator, line 192 (not sure why this caught my eye in 
skimming) - 2 open square brackets looks odd, and a semicolon?  Not sure 
what the notation is supposed to mean there.

Renderer.java - still some fairly long lines in there.  (I don't think 
our jcheck will reject those, but checking with Phil just in case)

RendererStats.java - also some pretty long lines

			...jim

On 3/18/15 4:25 PM, Laurent Bourgès wrote:
> Jim,
>
> Here is the new webrev:
> http://cr.openjdk.java.net/~lbourges/marlin/marlin.3/
>
> Changes:
> - discussed changes in Dasher, Stroker, TransformingPathConsumer2D
> - indentation, line breaks (continuation lines)
> - revert many float suffix in math operations (useless ? does the
> compiler make conversions ?)
>
> Hope it looks better ...
>
> Good night,
> Laurent