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.gra...@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.gra...@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"<dl...@redhat.com> wrote: > >>> > >>> > >>>> > >>>> ----- "Jim Graham"<james.gra...@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.gra...@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.gra...@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
