Round 4...

On 10/6/2010 1:36 PM, Denis Lila wrote:

webrev:
http://icedtea.classpath.org/~dlila/webrevs/noflatten/webrev/

BezCurve.java:

I'd add some "set()" methods to BezCurve/Curve and then use a scratch instance in Renderer (and other places?) to reuse those calculations, such as:

Curve constructors (obviously)
Renderer.curveOrQuadTo()
Renderer.initQuad()
Renderer.initCurve()
Stroker.findSubdivPoints()

lines 179,182 - using your d* variables, wouldn't these be:
   a = 2*(dax*dax + day*day)
   b = 3*(dax*dbx + day*dby)
   c = 2*(dax*cx + day*cy) + dbx*dbx + dby*dby
   d = dbx*cx + dby*cy
It has fewer multiplies and more of the multipliers are powers of 2 which are faster than non-power-of-2 multiplies.

line 206,210 - a nit - it didn't really confuse me, but halfway through reading this it occurs to me that these are really t0 and t1, right?

line 212 - if x0 (t0?) is 0 then you don't need to include it in the roots, no?

line 249,257 - these corrections are exponential compared to the sample code on the wikipedia page - was that the "slight modification" that you mentioned in the comments?

line 303 - isn't it enough to just look at the previous T value (or keep a running "prevt" variable) rather than update every T value in the array? Isn't this enough?
    int prevt = 0; /* field in Iterator */
    next() {
        curt = Ts[next];
        split = (curt - prevt) / (1 - prevt);
        prevt = curt;
    }

ROCsq - I looked at the wikipedia article and it wasn't clear how it directly related to your function since the article is dealing with the curvature of a function graphed against its own t, but you are dealing with 2 parametric equations combined and graphed against each other. I think I'm going to have to just trust you on this one for now. :-(

Done with BezCurve.java...

Stroker.java:

lines 558 (et al) - create a helper function for all of these (degenerates to a line) cases?

lines 621-626 and 643-646 - I'm not sure what the derivation of these lines are. I tried to do my own equations, but I seem to be heading in a different direction than you used and it didn't seem like I was going to converge. What equations did you set up to solve to get these calculations? From what I can see we have:
  - new p1,p4 are calculated
  - new p(0.5) is calculated
  - the corresponding dx,dy at t=0,0.5,1 (gives slopes)
  - slopes at t=0, 0.5 and 1 should be the same for all curves
and what you are trying to compute are two scaling constants c1 and c2 that represent how much to scale the dx1,dy1 and dx4,dy4 values to get a curve that interpolates both position and slope at t=0.5. A comment might help here... :-(

line 687 - dup?

line 856 - use a scratch Curve instance and set methods to reduce GC?

line 857 - this is true if the first vector is parallel to either axis, but the comment after it says only "parallel to the x axis" - is that a problem? Also, if both are 0 then no parallel constraint is applied even if it does start out parallel. I imagine that this is all OK because the "both 0" case should be rare/non-existant and the y-axis case will also benefit from the same optimization...?

lines 861-863: sin/cos and atan2 cancel each other out. It is faster to compute the hypotenuse and then divide the x and y by the hypotenuse to get the cos and sin. (cos = x/hypot; sin = y/hypot;)

Cache and TileGenerator look ok...

I think I'm done at this point except for not understanding the "parallel cubic" equations I mentioned above and the ROCsq method...

                        ...jim

Reply via email to