Hi Jim.
> I noticed that you went through all of the Pisces files and udpated
> their cast spacing. In general, we tend to leave style issues alone
> until we are actually doing something else with the code so I think it
> is a bit overkill. It might be best to just restrict this change to
> Stroker.java for now. (But, in general, I'm all for code cleanup. ;-)
Ok.
> Finally, line 754 - I can't figure out where leftoff[0,1,4,5] were set
> up? Is that a bug?
Oops, yeah, it's a bug. Setting leftoff[0,1,4,5] used to be done before
the if, but then I moved it after for some reason, and it broke it.
To fix it I just moved {right,left}Off[0,1,4,5] before the if. Now the
stuff inside the if parallels the code outside very nicely.
> While you are fixing indents. Stroker.java line 252 got shifted by a
> character from its former alignment. And somehow line 701 got shifted
> by a character, but no change is showing up in the webrevs...?
I think I fixed this too.
FYI: I also inserted this check: && !isNotFinite(lenSq)
in drawMiter to guard against computeIntersection returning computing
NaN values.
Regards,
Denis.
----- Original Message -----
> Hi Denis,
>
>
>
>
> Also, your suggestion below makes sense to me...
>
> ...jim
>
> On 4/19/11 11:11 AM, Denis Lila wrote:
> >
> > Jim, what do you think about completely removing this code:
> >
> > final boolean p1eqp2 = within(x1,y1,x2,y2, 6 * ulp(y2));
> > final boolean p2eqp3 = within(x2,y2,x3,y3, 6 * ulp(y3));
> > if (p1eqp2 || p2eqp3) {
> > getLineOffsets(x1, y1, x3, y3, leftOff, rightOff);
> > return 4;
> > }
> >
> > // if p2-p1 and p3-p2 are parallel, that must mean this
> > curve is a line
> > float dotsq = (dx1 * dx3 + dy1 * dy3);
> > dotsq = dotsq * dotsq;
> > float l1sq = dx1 * dx1 + dy1 * dy1, l3sq = dx3 * dx3 + dy3
> > * dy3;
> > if (Helpers.within(dotsq, l1sq * l3sq, 4 * ulp(dotsq))) {
> > getLineOffsets(x1, y1, x3, y3, leftOff, rightOff);
> > return 4;
> > }
> >
> > This sort of duplicates logic in the caller. I put it in anyway,
> > because I was afraid that the subdivision would introduce degenerate
> > curves, despite the checks in somethingTo passing. However, looking
> > at it again, I don't really think it's necessary. The parallel check
> > in particular was introduced because I was afraid that if a curve's
> > control vectors are close to parallel, computeIntersection would
> > return crazy, inaccurate values. But I tested that and it doesn't
> > seem to happen. The isNotFinite checks should be enough to deal
> > with quads that are actually lines.
> >
> > If we just replaced that whole chunk above with
> > if ((x1 == x2&& y1 == y2) || (x2 == x3&& y2 == y3)) {
> > getLineOffsets(x1, y1, x3, y3, leftOff, rightOff);
> > return 4;
> > }
> >
> > I think we could squeeze a fair bit of performance out of this.
> >
> > I've updated the webrev.
