[OpenJDK Rasterizer] Marlin renderer contribution for review

[Laurent Bourgès]

Jim,

> I think it's time to get this in.

Excellent news: marlin is going to be alive in openjdk !

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).

Allright, I will left intact my workdir on cr.ojn.

> 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.

I understand it can be a problem but for me applets are legacy... Anyway
could you give me a code pattern to use... to properly get System
properties in that secure environment.

>
> 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.

Ok I will take care. Could you indicate some of them?

> - 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.

I tried last night to fix the more visible to me!

> 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

I agree some long lines remain but we have very large displays in 2015,
don't you ?

Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20150319/911db26e/attachment.html>