[OpenJDK Rasterizer] Marlin renderer contribution for review

[Jim Graham]

On 3/19/2015 3:53 PM, Laurent Bourgès wrote:
>  > 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.

Look at the source for 
java.awt.GraphicsEnvironment.getHeadlessProperty().  It's much more 
complicated than your needs, but it shows the way to wrap some code in a 
doPrivileged block.

I'd recommend moving all of the getProperty() calls into a single static 
initialization block (in the Const class for instance), wrapped in a 
single doPrivileged() block which then sets static booleans to the 
indicated values so that you don't have to fire up a doPrivileged() 
block every time anyone calls any of those methods.  The methods in the 
Const class then just become getters for the saved booleans.

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

I pulled up the patch file (from the top of the webrev index page) and 
just used a regex search for "[/][*][*].*[*][/]".  Lots of lines, but 
they appear to be grouped in just a few new blocks of new code.

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

Yes, but they aren't all large enough to hold 2 pages of text side by 
side if the lines get too long.  Pull up the sdiffs of the webrev and 
notice that you have to scroll side to side to review the changes on 
some of your files (unless you have a really wide screen, but many of us 
work on laptops so 2x80 characters is comfortable and as you get much 
wider then you need to start scrolling side to side - 2x100 is iffy). 
In particular, the sdiff of Renderer.java in your last patch overflows 
the width of my retina MBP screen by about 25-30% mostly because of long 
lines in the "new" version.

(I'll note that with your changes, Dasher.java was actually improved - 
it just barely doesn't overflow that same screen because you shortened 
some of the old lines that were in there from previous fixes - thanks!)

...jim