Hi Denis,
I think the first version is a better choice for now since you said that
the performance difference isn't noticeable. I think the lower level
flattening might look a little different if we ever decide to upgrade
the pipeline to deal with curves. In particular, you are still
flattening above the dashing/stroking code and I think the flattening
should be done below that code (i.e. in Renderer).
So, I'd go with the first one with the following comments:
- You indent by 8 spaces in a few places. Is that a tabs vs. spaces
issue? We try to stick to 4 space indentations with no tabs for
consistentcy.
- I'd make the internal error message a little less personal. ;-)
"normalization not needed in OFF mode" or something.
- lines 362,363 - you don't need to set cur_adjust variables here, they
are already being set below.
Other than that, it looks good to go...
...jim
Denis Lila wrote:
Hi Jim.
So, I have the nicer webrevs.
FlatteningIterator version:
http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
Pisces flattening version:
http://icedtea.classpath.org/~dlila/webrevs/fpWithSCandPiscesFlattening/webrev/
I dealt with the issue of handling OFF by just not accepting it as an input.
After all, a normalizing iterator only needs to be created, and is only created
if the normalization mode is not OFF.
Thanks,
Denis.
----- "Jim Graham" <james.gra...@oracle.com> wrote:
Hi Denis,
I'll wait for some clean webrevs once you get the float stuff in for a
final review. I did take a really quick look and thought that a
better
way to handle "OFF" would be to set rval to -1 and then check "rval <
0"
as the (quicker) test for OFF in the currentSegment() method. Does
that
make sense?
In any case, let's wait for cleaner webrevs to go further on this
(hopefully in a day or so?)...
...jim
On 8/5/2010 8:06 AM, Denis Lila wrote:
Hi Jim.
I made all the suggested changes.
Links:
http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
http://icedtea.classpath.org/~dlila/webrevs/fpWithSCandPiscesFlattening/webrev/
Thanks,
Denis.
----- "Jim Graham"<james.gra...@oracle.com> wrote:
Hi Denis,
First, comments on the high level normalizer (Normalizing
iterator):
- If there is no normalization going on, I would use the Shape's
own
flattening (i.e. getPathIterator(at, flat)). The reason being
that
some
shapes may know how to flatten themselves better, or faster, than
a
Flattening Iterator. In particular, rectangles and polygons would
simply ignore the argument and save themselves the cost of
wrapping
with
an extra iterator. This would probably only be a big issue for
very
long Polygons.
- Line 331 - the initializations to NaN aren't necessary as far as
I
can
tell...?
- Rather than saving "mode" in the normalizing iterator, how about
saving 2 constants: (0.0, 0.5) for AA and (0.25, 0.25) for non-AA
and
then simply add those constants in rather than having to have the
conditional with the 2 different equations?
...jim
