Re: [OpenJDK 2D-Dev] X11 uniform scaled wide lines and dashed lines; STROKE_CONTROL in Pisces

2010-08-04 Thread Jim Graham

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



Re: [OpenJDK 2D-Dev] X11 uniform scaled wide lines and dashed lines; STROKE_CONTROL in Pisces

2010-08-04 Thread Denis Lila
Hello Jim.

So, I've now implemented both suggestions you had for implementing
stroke control: an intermediate normalizing path iterator, and 
doing flattening in pisces at the lowest level. Respectively,
the webrevs are:
http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
http://icedtea.classpath.org/~dlila/webrevs/fpWithSCandPiscesFlattening/webrev/

Again, these include the floating point conversion changes, but all of the
changes relevant to this e-mail are in PiscesRenderingEngine.java.

As for performance, the version with low level flattening is faster, but this
is only noticeable when pisces is run on it's own i.e. not actually drawing
anything (by the way, I've included a commented out main method in the second
webrev. This allowed me to run pisces on it's own which was useful for debugging
and performance testing). 
However, when drawing stuff, whatever happens after pisces takes up so much
time that it's hard to tell the difference.

Nevertheless, it might be worth to keep the somewhat ugly, low level version.
It might be useful for antialiasing if Stroker moves to outputting curves 
instead
of just lines (because AA still needs to flatten, unless someone comes up with 
an
algorithm to do it without flattening, but I can't think of anything).

I'm sorry to ask you specifically for all these reviews - you've already spent
a lot of time looking at my work, but no one else has replied to any of my 
pisces
inquiries (except Clemens Eisserer, on this issue). Anyway, I would appreciate 
it
if you, or anyone, could take a look at one or both of the above webrevs.

Or maybe we could leave it for when the floating point conversion has been 
pushed.
Then the webrevs would have a lot less clutter.

Thanks,
Denis.

- "Denis Lila"  wrote:

> Hi Jim.
> 
> I implemented STROKE_CONTROL today. I used the intermediate
> NormalizingPathIterator, instead of implementing flattening in
> pisces, because I wanted to get something working asap, and this
> would be the easiest way.
> 
> The webrev is
> http://icedtea.classpath.org/~dlila/webrevs/fpWithStrokeControl/webrev/
> 
> I guess I'm not asking that you take a look at it, because it's
> probably
> not the way we're going to end up doing things, but I wrote it, so
> I'm sending the link just in case anyone wants to see it.
> 
> The webrev is big because it includes the floating point conversion,
> but all the
> STROKE_CONTROL changes are in PiscesRenderingEngine.java.
> 
> Thanks,
> Denis.
> 
> - "Jim Graham"  wrote:
> 
> > Hi Denis,
> >
> > It would be ill-advised to normalize the coordinates after
> flattening.
> >
> > The quality would be really bad.
> >
> > Perhaps this is a good reason to start looking at updating Pisces to
> > take curves and flatten at the lowest level?
> >
> > Or, I suppose we could get a non-flattened iterator from the source
> > path, send it through a "normalizing" filter, and then through a
> > flattening filter (the way many of the existing objects do
> flattening
> > is
> > to just get their regular iterator and run it through an instance of
> > FlatteningPathIterator so we could do this manually with an
> > intervening
> > "NormalizingPathIterator" if normalization is needed)...
> >
> > ...jim
> >
> > Denis Lila wrote:
> > > Hello Jim.
> > >
> > > Thanks for that. I'll get to work on implementing it.
> > >
> > > One thing though, about normalizing the control points of bezier
> > > curves: pisces never gets any bezier curves as input. It only gets
> > > lines that are the product of flattening bezier curves.
> > >
> > > Pisces receives its input from flattening path iterators which get
> > it
> > > from other path iterators. Of course we can't require these to
> send
> > out
> > > normalized points. In order to normalize the control points we
> need
> > to
> > > be able to look at the bezier curves in Pisces, so we can't just
> > take
> > > all the input from the flattener. However, pisces can't handle
> > curves
> > > (yet, hopefully), so after the normalization, they must be
> > flattened, and
> > > this is the problem. I think it's a pretty good idea to do this by
> > > storing the input form the iterator into pisces (after
> > normalization),
> > > creating a nameless path iterator that just iterates through all
> > that,
> > > and using this iterator to create a flattening iterator, which
> then
> > > is used as before.
> > >
> > > Does anyone have any other ideas?
> > >
> > > Thank you,
> > > Denis.
> > >
> > >
> > > - "Jim Graham"  wrote:
> > >
> > >> For AA this is exactly what we do (round to nearest pixel centers
> > for
> > >>
> > >> strokes).  Note that this is done prior to any line widening code
> > is
> > >> executed.
> > >>
> > >> For non-AA we normalize coordinates to, I believe the (0.25,
> 0.25)
> >
> > >> sub-pixel location.  This is so that the transitions between
> > widening
> > >> of
> > >> lines occurs evenly (particularly for horizontal and vertical
> wide

Re: [OpenJDK 2D-Dev] Various fixes to pisces stroke widening code

2010-08-04 Thread Denis Lila
Hello Jim.

> I'd need to see a final webrev and approve it and then anyone with an
> OpenJDK user id can push it.  
The final webrev is 
http://icedtea.classpath.org/~dlila/webrevs/fpBetterAAv2/webrev/
The only things that have changed since the last one you commented on
are the "y == boundsMaxY - 1" check, and a few comments in Renderer.java.

> I'll also have to create a bugid for the change set.
I've already submitted a bug that this patch would fix. The ID is 6967436.

> Are you a registered OpenJDK developer?
Not just yet. I will be soon, I think.

Thanks,
Denis.

- "Jim Graham"  wrote:

> Hi Denis,

> 
> 
>   ...jim
> 
> Denis Lila wrote:
> > Hello Jim.
> > 
> >> I'm guessing the test for "y == boundsMaxY-1" at line 470 could
> >> probably also be deleted now (since it will be handled by the end 
> >> test when y reaches maxY)?
> > 
> > Indeed it can. I've done this, and also updated a few comments that
> > might have been misleading (they were written before certain
> changes
> > were made).
> > 
> >> It looks fine.  Hopefully we can eventually figure out why the
> sorting
> >> on the fly didn't pan out.
> > 
> > Wonderful! So, will it be committed?
> > 
> > Thanks,
> > Denis.
> > 
> > - "Jim Graham"  wrote:
> > 
> >> Hi Denis,
> >>
> > 
> >> Denis Lila wrote:
> >>> Hi Jim. 
> >>>
> >>> Thanks for all your suggestions. I fixed the edge array
> >> indexing
> >>> issue, the moveTo bug (not assigning x0), and the double
> >>> initialization issue. I also improved the emission of the last
> row
> >>> to what you said. The link is the same as the last one I sent.
> > 
> >> But everything looks in order!
> >>
> >>...jim