Just to clarify. My second message that I just sent mostly contained
some additional optimizations to consider for now or later, but this
message below contained at least one (maybe 2) thing(s) that looked like
a bug and a few maintenance issues that I think should be done before
finalizing this set of changes...
...jim
On 7/29/2010 5:27 PM, Jim Graham wrote:
Hi Denis,
The changes look fine and I'm moving on to Renderer.java...
I'd make as many methods private as you can. It looks like all your new
methods are private, but some old methods are still public even though I
don't think they are accessed elsewhere (like addEdge()?). I think
"private" is enough for Hotspot to inline so that should help
performance a bit. I usually use "final", but I think "private" does the
same thing.
You should create constants for the indices into the struct to make the
code more readable (and to simplify updating the EDGE layout):
X0_OFF = 0
Y0_OFF = 1
// etc.
I don't think you touch X0 and Y0 after initializing them and then using
them for the first setCurY so you could just use them as the "curX" and
"curY" and save 2 slots in the table.
You initialize edges twice - once when it is declared, and once in the
constructor. Note that the initialization where it is declared uses
INIT_SIZE which is not a multiple of EDGE size anyway.
It looks like there is a missing statement in moveTo to initialize
this.x0...?
I need some more time on it, but I thought I would send along these
comments in the meantime...
...jim
Denis Lila wrote:
Hello Jim.
I made the changes you point out, except for your second point
(I don't have time to think about it right now).
I stopped precomputing m00_2_m01_2 because it was only being
used in one place. But I guess it worth it to precompute it
if it's going to be used often (and computeOffset is called
for every line).
The new webrev is at
http://icedtea.classpath.org/~dlila/webrevs/fpBetterAAv2/webrev/
Thanks,
Denis.
