What was the problem - I might have a guess as to the cause if I saw a
picture. You should file a bug against it to make sure it doesn't fall
through the cracks.
But, the webrev you sent looks good to go.
If you want some more optimization comments, I'll always have more (I'm
evil that way)... ;-)
- Testing for y0==y1 in lineTo is redundant. addEdge already ignores
the line with a looser test. It does take more processing to reject the
edge in that case, though, but that test in lineTo is saving less and
less work with every revision.
- pix_sxy0 aren't really needed. close() could simply:
addEdge(x0, y0, sx0, sy0);
this.x0 = sx0;
this.y0 = sy0;
and bypass what little processing is left in lineTo.
- addEdge rejects horizontal edges. It could also reject any of the
following classes of edges:
- completely above clip
- completely below clip
- completely to the right of clip
since those edges will never contribute to the coverage. The algorithm
should skip the above and below edges reasonably quickly, but this would
save storage for them. The edges to the right would have to be updated
every scanline and waste storage, but that isn't a huge hit. This only
helps for corner-case huge paths which aren't common. At least you
aren't iterating over miles and miles of irrelevant geometry which would
be an important performance hit.
But, these 3 are really getting down to the nitty gritty. I'd check it
in before I drive you crazy... ;-)
...jim
Denis Lila wrote:
- Forwarded Message -
From: "Denis Lila"
To: "Jim Graham"
Sent: Monday, August 9, 2010 4:58:10 PM GMT -05:00 US/Canada Eastern
Subject: Re: [OpenJDK 2D-Dev] Various fixes to pisces stroke widening code
Hi Jim.
Good idea. I've implemented it. I also noticed the quicksort
method wasn't very friendly to 0 length arrays. I fixed that.
I ran Java2Demo and a few of my tests, and everything looks good.
http://icedtea.classpath.org/~dlila/webrevs/fpBetterAAv2/webrev/
Everything, except the joins demo, that is. The interior of the
star's right arm (our left) and left leg is not drawn exactly like
in proprietary java or openjdk6. I am not sure if this is a bug
since I don't know exactly how the results of a bs.createStrokedShape
are supposed to look. However, I am almost certain that this
isn't my fault, since I can reproduce the behaviour with an only
slightly changed version of openjdk7 with completely different
changes than what's in this patch. I'll run it next morning with
a fresh build of openjdk7 to be completely sure.
Can I push it?
Thanks,
Denis.
- "Jim Graham" wrote:
To the end of getting rid of "flips" - all they are used for is to
resize the crossings array, right? This is not a huge array that
costs
a lot to resize - why not simply use a default array of, say, 30
elements and then resize it if we ever have more crossings than that?
Only very complicated paths would have more than 30 crossings to
track.
The check for array length is only needed once per scanline since we
know how many active edges are on each scan line (hi-lo) and you can
only have 1 crossing per active edge so with one test per scanline we
can keep the crossings array within range...
...jim
Jim Graham wrote:
Hi Denis,
Well, I guess it's a good thing that Java2Demo had a path like that
in
it - not a very common case, so it's good we found it!
The fix looks fine. It still seems like there is way more logic
there
than is needed - hopefully if we can get rid of flips at some point,
much of it will go away.
Fixes look good to go to me...
...jim
On 8/5/2010 3:58 PM, Denis Lila wrote:
Hi Jim.
I didn't know about Java2Demo. If I did I would have run it
sooner.
But I ran it a few hours ago, and everything looked fine
(surprisingly
high fps too) until I got to the append test.
Apparently I introduced a bug when solving the "2 consecutive
moveTos
bug".
Basically, when there's a close() after a horizontal lineTo(), the
lineTo
in close() won't be executed because it's inside the if
(firstOrientation != 0)
test. So instead of going back to the starting point, close will
stay
where
it is, which will draw a triangle above the rectangle.
I fixed this by introducing a variable that keeps track of the last
method
called (lineTo, moveTo, or close), and instead of checking for
firstOrientation != 0
in close(), I check for (last == LINE_TO).
webrev (hopefully final):
http://icedtea.classpath.org/~dlila/webrevs/fpBetterAAv2/webrev/
I'm sorry about this. I wish I had known about Java2Demo sooner.
Thanks,
Denis.
- "Jim Graham" wrote:
Hi Denis,
That's great! I just did a last minute double-check of your last
(final) webrevs to be sure.
Have you tested Java2Demo with these changes? I'd also run any
regression tests you can find with the changes. If there are no
problems there, then you are good t
