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

Tue Sep 14 17:48:37 UTC 2010

Hello Jim.

> - If the first dash starts in the middle of an "on" phase then you 
> shouldn't send it to the output right away.  Remember its segments
> until it turns "off" then save those segments aside.  If the path is
> closed and if you ended in the middle of a dash "on" phase and you 
> have some of those "initial on" segments saved then send those original
> segments from the first "on" dash as an extension of the closing "on"
> segment. If the path doesn't close, or if it closes but isn't on when
> it gets back to the original point, then send those first segments
> starting with a moveto so that they form their own isolated dash.

I see. That makes sense.

> sx1,sy1 have lost their way along the way somewhere.

I think that was my fault:

> I don't think you actually need sx1, sy1 - instead you need to buffer
> and save aside the first set of dashes and I don't see that buffer 
> anywhere...

sx1, sy1 were the buffer themselves, because we used to do flattening
so there were no curves, and no need for an array. I did not see what
they were used for, so I did not implement a buffer to replace them.
I have now. I also fixed a couple of bugs (one related to drawing square
caps and the other related to drawing joins), and I have tried to
fix the whitespace (please tell me if I missed anything).
The link is the same:
http://icedtea.classpath.org/~dlila/webrevs/noflatten/webrev/

(For some reason, 2 unchanged files are appearing in the webrev. Please
ignore this)
There are a few smallish issues I wanted to discuss:
1. To subdivide at arbitrary t, I use equations like (1-t)*p1 + t*p2.
Should I use the more intuitive p1+t*(p2-p1)? It's clearer and should be
faster since it has 1 fewer multiplication.
2. When computing offset curves, I check for degenerate curves (i.e. equal
control and end points). I don't use == for comparisons. I use a function
that returns true if 2 floats are close to each other, for fear of
catastrophic cancellation. Is this really a good idea? If so, how close
should the numbers be before I return true?
3. Should line 862 in Stroker be enabled (I give some reasons for wanting to
in a comment below it).

Thanks,
Denis.

----- "Jim Graham" <james.graham at oracle.com> wrote:

> Hi Denis,
> 

> 

> 
> All of that support code to save aside that first "on" dash seems to 
> have gone missing and so sx1,sy1 don't make sense any more.
> 
> 
> 			...jim
> 
> On 9/13/2010 4:01 PM, Denis Lila wrote:
> > Hello Jim.
> >
> > I think I finally have a version without correctness problems:
> > http://icedtea.classpath.org/~dlila/webrevs/noflatten/webrev/
> >
> > Assuming no bugs, there are still a few minor issues:
> > - whitespace (I'll fix this tomorrow)
> > - comments (also tomorrow)
> > - in dasher, there are variables called sx1, sy1. They seem useless
> > to me. It would help a lot if they are. Could you please look at
> this?
> >
> > If anything at all is confusing in it, please contact me (e-mail or
> irc:
> > I'm on OFTC #openjdk. My nickname is dlila).
> >
> > Thank you,
> > Denis.
> >
> > ----- "Jim Graham"<james.graham at oracle.com>  wrote:
> >
> >> Hi Denis,
> >>
> >> Things got really busy for me over the past week so I wasn't able
> to
> >> keep up with the discussion on this, but I will be looking more at
> it
> >>
> >> next week.  In the meantime it sounds like you are on the right
> track.
> >>
> >> I wish I'd have investigated it to the level you are at so I could
> be
> >> of
> >> more immediate help, but hopefully I'll get there when I review
> your
> >> various changes...
> >>
> >> 			...jim
> >>
> >> On 9/7/2010 2:11 PM, Denis Lila wrote:
> >>>> Hello Jim.
> >>>>
> >>>> So, I finally have a webrev for serious consideration:
> >>>> http://icedtea.classpath.org/~dlila/webrevs/noflatten/webrev/
> >>>> There are still some printing statements I used for debugging,
> and
> >>>> the whitespace is probably pretty bad (tell me if this poses a
> >> problem
> >>>> when reading the code, and I'll clean it up), but I don't want
> to
> >>>> waste time removing that stuff unless necessary, since this is
> >>>> doubtlessly not the last version. I also included a Test.java
> >>>> file that I found useful for testing and debugging. It has a
> main
> >>>> method, and it allows pisces to run as a standalong project in
> >>>> eclipse (as long as you set the JRE to be openjdk7 since it
> needs
> >>>> to know about AATileGenerator and some other non public
> >> interfaces).
> >>>>
> >>>>    From testing it, the only problem I noticed is that it doesn't
> do
> >>>> very well with tight loops. So, a path like
> >>>> p.moveTo(0,0);p.curveTo(1000, 1000, 400, 500, 0, -150);
> >>>> isn't stroked very well when using the rotating algorithm. When
> >> using
> >>>> just the "make monotonic" algorithm it is ok (right now, it is
> set
> >> to
> >>>> use the latter - you can change this by uncommenting
> >> Stroker.java:1011
> >>>> and commenting out Stroker.java:1012). This leads me to believe
> >> that
> >>>> we need to detect and perhaps subdivide at loops in addition to
> >> the
> >>>> current subdivision locations. However, I have not yet looked
> too
> >> deeply
> >>>> into why the problem arises and how to fix it. I welcome
> >> suggestions.
> >>>>
> >>>> Thanks,
> >>>> Denis.
> >>>
> >>> I figured out what the problem is. The problem isn't really tight
> >> loops.
> >>> The problem is cusps in the offset curves. These happen when the
> >> line width
> >>> is equal to the radius of curvature of the curve being processed
> >> (although,
> >>> this may be just a necessary condition and not sufficient, but
> this
> >> doesn't
> >>> matter).
> >>>
> >>> It seems like we have to split at values of t where the above
> >> condition
> >>> holds. However, I can't see a way to do this without resorting to
> >> Newton's method
> >>> for finding the roots of RadiusOfCurvature(t) - lineWidth. It
> would
> >> be
> >>> really easy, however, if we had the arc length parametrization of
> >> the curve
> >>> in question, but this won't necessarily be a polynomial. A good
> way
> >> might be
> >>> to find a polynomial approximation to its inverse (this would
> make
> >> dashing considerably
> >>> easier too).
> >>>
> >>> Regards,
> >>> Denis.
> >>>
> >>> ----- "Denis Lila"<dlila at redhat.com>   wrote:
> >>>
> >>>
> >>>>
> >>>> ----- "Jim Graham"<james.graham at oracle.com>   wrote:
> >>>>
> >>>>> OK, I see.  You were doubting that the "thing that came after
> >>>> Pisces"
> >>>>>
> >>>>> could be that much different considering that Pisces is
> rendering
> >>>> many
> >>>>>
> >>>>> more sub-pixels.
> >>>>>
> >>>>> Actually, embarrassingly I think it can.  It just means the
> >> non-AA
> >>>>> renderer has some performance issues.  One thing I can think of
> >> is
> >>>>> that
> >>>>> the SpanShapeIterator uses a native method call per path
> segment
> >> and
> >>>>> the
> >>>>> cost of the context switches into native and back for each path
> >>>>> segment
> >>>>> dominate the performance of long paths.  It was something I was
> >>>>> meaning
> >>>>> to fix for a long time (when that code was first written native
> >> code
> >>>>> was
> >>>>> so much faster than Java and the native transition was quick -
> >> since
> >>>>> then Hotspot came along, got a lot better, and the native
> >>>> transitions
> >>>>>
> >>>>> got much, much slower).
> >>>>>
> >>>>> So, yes, this isn't out of the question...
> >>>>>
> >>>>> 			...jim
> >>>>>
> >>>>> On 9/2/2010 3:40 PM, Denis Lila wrote:
> >>>>>>> Use which?  The stroking code or the rendering code?
> >>>>>>> I believe that the way I set it up was that Pisces replaced
> >> both
> >>>>> the
> >>>>>>> stroke widening/dashing code and the AA renderer - both were
> >>>> parts
> >>>>> that
> >>>>>>> we relied on Ductus for.  But, the widening code would talk
> to
> >>>> one
> >>>>> of
> >>>>>>> our other existing rasterizers for non-AA.  Look at
> >>>>>>> LoopPipe.draw(sg2d, s).  It (eventually) calls
> >>>>> RenderEngine.strokeTo()
> >>>>>>> directed at a SpanShapeIterator...
> >>>>>>
> >>>>>> I think there's a misunderstanding. All I meant was that, even
> >>>> when
> >>>>> AA is off,
> >>>>>> we do use pisces for widening, but it doesn't do any
> >>>> rasterization.
> >>>>>>
> >>>>>>
> >>>>>> ----- "Jim Graham"<james.graham at oracle.com>    wrote:
> >>>>>>
> >>>>>>> 			...jim
> >>>>>>>
> >>>>>>> On 9/2/2010 3:20 PM, Denis Lila wrote:
> >>>>>>>>> Do we use Pisces for non-AA?  Pisces should clock in slower
> >> for
> >>>>> AA
> >>>>>>> than
> >>>>>>>>> non-AA, but I think we use one of the other pipes (not
> >> Ductus)
> >>>>> for
> >>>>>>>>> non-AA and maybe it just isn't as good as Pisces?
> >>>>>>>>
> >>>>>>>> We definitely use it for non-AA.
> >>>>>>>> I traced it.
> >>>>>>>>
> >>>>>>>> Denis.
> >>>>>>>>
> >>>>>>>> ----- "Jim Graham"<james.graham at oracle.com>     wrote:
> >>>>>>>>
> >>>>>>>>> On 9/2/2010 2:43 PM, Denis Lila wrote:
> >>>>>>>>>> Actually, I had a question about the test I wrote which
> >> takes
> >>>>> 20
> >>>>>>>>> seconds. When
> >>>>>>>>>> I turned antialiasing on, the test dropped from 20 seconds
> >> to
> >>>>>>> 2.5.
> >>>>>>>>> This is very
> >>>>>>>>>> puzzling, since antialiasing is a generalization of
> >>>>>>> non-antialiased
> >>>>>>>>> rendering
> >>>>>>>>>> (a generalization where we pretend there are 64 times more
> >>>>> pixels
> >>>>>>>>> than there
> >>>>>>>>>> actually are). Of course, the paths followed after pisces
> >> for
> >>>>> AA
> >>>>>>> and
> >>>>>>>>> non-AA are
> >>>>>>>>>> completely different, but whatever came after pisces in
> the
> >>>>>>> non-AA
> >>>>>>>>> case would
> >>>>>>>>>> have the same input as Renderer has in the AA case (input
> >>>>> gotten
> >>>>>>>>> from Stroker).
> >>>>>>>>>> Can you take a guess as to what was causing such a large
> >>>>>>>>> difference?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I think Pisces was integrated only as a Ductus replacement
> >>>> which
> >>>>>>> means
> >>>>>>>>>
> >>>>>>>>> it was used only for AA, but check if I'm mistaken...
> >>>>>>>>>
> >>>>>>>>> 			...jim