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

Denis Lila dlila at redhat.com
Tue Jul 20 00:10:03 UTC 2010


Hello again.

I know I promised this last week, and I'm sorry it's late, but some nasty 
bugs popped up. The webrev is at: 
http://icedtea.classpath.org/~dlila/webrevs/floatingPoint/webrev/

I took this opportunity to make some changes that aren't related to floating
point conversion, since this effort wasn't really directed at solving any one
particular bug, but towards general refactoring and improving of Pisces (although
it does solve a certain bug).
1. 
    I removed some write-only variables in Renderer and Stroker.

2. 
    I removed Dasher and Stroker's ability for the same object to be used with
more than one output, transform, width, etc. Jim Graham said we should consider
removing the no argument constructor for Dasher in a previous e-mail (which
is equivalent to doing what I've done since you can't have this functionality
without a no argument constructor and methods like setParameters and setOutput).
I thought this was a good idea because this functionality was never being used,
it clutters things up (among other things, it necessitates making the clients 
call setOutput and setParameters before the object can be used. This sort of
thing is not a good idea since methods should be as self-contained as possible),
and even if it is used, all it does is save us the creation of some objects. Since
a huge number of these objects would never be needed and since they are not 
very expensive to create (Stroker is the biggest, and it has about 38 members), 
this is a premature optimization.

3.
    (2) allowed me to declare a whole bunch of members final (things like output,
lineWidth, transformation matrix entries and anything that can't change).

4.
    I changed the crossing sorting algorithm in Renderer from bubble sort to 
insertion sort. Not a huge difference, but mine is shorter, it should perform
slightly better, and I think it the algorithm is easier to understand.

5.
    I inserted comments on things which used to confuse me. Please check these -
when reading code, there is nothing worse than a wrong explanation in a comment.

6.
    I removed the if(false &&... block in Renderer. I tried to make it work some
time ago, but it was complicated and more trouble than it was worth - it would
only be used when filling a rectangle, and not even a rectangle that was part of
some path. The rectangle had to be the only thing being rendered. This is fast enough
without being treated as a special case.

I think that's it...
As for testing: I tested this fairly thoroughly. I used dashed strokes, various 
affine transformations, lines longer than 2^16, and complicated GeneralPaths.
Everything looks good.

I also did some performance measurements. Everything is faster than it used to be,
although AA rendering is still much slower than closed source java. One of the
problems here is that when rendering large objects, the strips in Renderer:_endRendering
will be small, and _endRenderer's complexity, not taking into account methods it calls,
is O(numStrips*numEdges), because for every strip it has to go through what's left of the edge 
list. A better way to iterate through edges that are in a strip is needed.

NOTE: I did not make changes (2) and (3) to Renderer.java because I don't have time
today, and I wanted to put out something fairly polished as soon as possible. Also,
I think Renderer needs quite a bit more refactoring than just that. In particular the 
way it stores and iterates through edges, scalines, and crossings needs to be improved.
Right now it is too error-prone, and many variables have a far larger scope than
they should.

I would very much appreciate it if anyone could look over my changes, or comment
on the last two paragraphs above.

Thank you, 
Denis. 

----- "Denis Lila" <dlila at redhat.com> wrote:

> Hello.
> 
> And, I just finished it. At least it compiled successfully. I'm sure
> there
> are a few runtime bugs. I'll try to have a webrev out by today.
> 
> Regards,
> Denis.
> 
> ----- "Jim Graham" <james.graham at oracle.com> wrote:
> 
> > Hi Denis,
> >
> > float operations are pretty fast and they are usually done in a
> > separate
> > part of the processor so the compiler can schedule a lot of
> > bookkeeping
> > instructions to run in parallel with waiting for the results of the
> FP
> >
> > instruction.  In the end, they often end up being "free" if there
> are
> >
> > enough bookkeeping instructions (branches, fetching data, etc.) in
> > amongst the data.
> >
> > Given how many alternate instructions you are pointing out for the
> > fixed
> > point code it would be very likely that this would be a "win".
> >
> > The main reason that Pisces is implemented heavily in fixed point is
> > that it was originally written for the mobile platform where there
> are
> >
> > no FP instructions.  We don't have to worry about that on the
> desktop
> >
> > (any more).
> >
> > I strongly support you converting things to fp if you want to do
> > it...
> >
> > 			...jim
> >
> > On 7/12/2010 8:05 AM, Denis Lila wrote:
> > > Hello.
> > >
> > > Is it ok if I do this? I already started working on it on Friday.
> > > I think I should be done by tomorrow.
> > >
> > > But yes, I agree that we should convert to floating point. As for
> > > performance, there's also the fact that right now we're trading
> > > one double multiplication for 2 casts to long, 1 long
> > multiplication,
> > > 1 bit shift of a long, and 1 cast back to an int. I don't know
> much
> > > about how these are done in hardware, but it doesn't seem like
> > they'd
> > > be faster than the double multiplication.
> > >
> > > As for large coordinates, there's a bug report about it (one not
> > > reported by me :) ) here:
> > https://bugzilla.redhat.com/show_bug.cgi?id=597227
> > > I submitted a matching bug report on bugs.sun.com (ID 6967436),
> but
> > I
> > > can't find it when I search for it.
> > >
> > > Denis.
> > >
> > > ----- "Jim Graham"<james.graham at oracle.com>  wrote:
> > >
> > >> Sigh...
> > >>
> > >> Pisces could really stand to upgrade to floats/doubles
> everywhere,
> > for
> > >>
> > >> several reasons:
> > >>
> > >> - FPU ops are likely as fast if not faster on modern hardware due
> > to
> > >> parallel execution of FP instructions alongside regular
> > instructions.
> > >>
> > >> - Don't have to worry about getting coordinates larger than 32K
> (I
> > >> don't
> > >> think this is well tested, but I imagine that Pisces will not
> deal
> > >> with
> > >> it very gracefully).
> > >>
> > >> - Lots of code is greatly simplified not having to deal with the
> > >> semantics of how to do fixed point multiplies, divides, and
> > >> conversions.
> > >>
> > >> I meant to do this during the original integration, but I wanted
> > to
> > >> get
> > >> the task done as quickly as possible so that we could have an
> open
> > >> source alternative to the closed Ductus code so I left that task
> > for a
> > >>
> > >> later update.  But, now that a lot of work is being done on the
> > code
> > >> to
> > >> fix it up, it might be better to do the float conversion now so
> > that
> > >> the
> > >> maintenance is easier and before we end up creating a lot of new
> > fixed
> > >>
> > >> point code.
> > >>
> > >> My plate is full right now, but hopefully I can interest someone
> > else
> > >> in
> > >> doing a cleanup cycle?  (Donning my Tom Sawyer hat... ;-)
> > >>
> > >> 			...jim



More information about the 2d-dev mailing list