[OpenJDK Rasterizer] Marlin renderer contribution for review

[Laurent Bourgès]

Jim,

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

Good idea: will do it but will try keeping MarlinConst an interface and of
course ensure constants are still final and static to let the compiler do
its magic = constant inlining...

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

Thanks, I will check using that regexp.

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

As I said previously, I shortened lines except in the Renderer class !
It is the most critical code with so many loops and conditional blocks...

I will that effort also to shorten lines while keeping readability but it
will take me some time and require running tests and benchmarks again.

> (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!)

Thanks too !

I need rest: busy week + marlin work. I hope to release the new webrev
early next week.

Good week end,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20150320/a25e7f9c/attachment-0001.html>