[OpenJDK 2D-Dev] Various fixes to pisces stroke widening code

Jim Graham james.graham at oracle.com
Fri Jul 30 00:27:17 UTC 2010


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.



More information about the 2d-dev mailing list