[OpenJDK 2D-Dev] RFC: Fix for 7036754

Jim Graham james.graham at oracle.com
Wed Apr 20 00:21:54 UTC 2011


Hi Denis,

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

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

Finally, line 754 - I can't figure out where leftoff[0,1,4,5] were set 
up?  Is that a bug?

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.



More information about the 2d-dev mailing list