A Divendres 11 Gener 2008, Iñigo Martínez va escriure: > Here goes an update to the last patch fixing the comments. Anyway, I'm > not very proud of the new code. I don't like the code that parses the > quadrilateral points, it should be generic but as it depends on > annotation fields I have implemented it in AnnotLink. Everything isn't > bad thought, this way it checks for errors in the quadrilaterals points > and it uses the annotation rect instead the quadrilaterals when they are > wrong (as specified in the reference). > > Any comment is welcome.
0003-Changed-AnnotQuadrilateral-parsing-inside-AnnotLink.patch has lots of spacing changes, are they really needed? Besides that if noone objects i think we can commit them if you want. Albert > > PD: I have been some days thinking on the better solution (for that > little piece of code yes!), any idea on a better implementation for > quadrilaterals parsing is welcome too. > > El dom, 30-12-2007 a las 23:54 +0100, Iñigo Martínez escribió: > > El dom, 30-12-2007 a las 17:43 -0500, Jeff Muizelaar escribió: > > > On Sun, Dec 30, 2007 at 11:19:22PM +0100, Iñigo Martínez wrote: > > > > El dom, 30-12-2007 a las 16:56 -0500, Jeff Muizelaar escribió: > > > > > On Sun, Dec 30, 2007 at 09:32:05PM +0100, Iñigo Martínez wrote: > > > > > > Hi: > > > > > > > > > > > > Here goes two new patches. The first one improves AnnotLink > > > > > > support, anyway it lacks a lot of features. The best option > > > > > > should be to merge the actual Link implementation with the > > > > > > AnnotLink one. > > > > > > > > > > > > The second patch adds support for AnnotFreeText. > > > > > > > > > > > > I have tested both (I have figured out the old problems, and now > > > > > > I can test them correctly, anyway I have a few glib test on the > > > > > > way) but I would prefer a review. > > > > > > > > > > I had a quick look and things looked pretty good. My comments > > > > > follow below. > > > > > > > > > > -Jeff > > > > > > > > Thank you very much Jeff. I have done the changes to every if wrong > > > > typed. I wrote them incorrectly in the last patches too. > > > > > > > > I have another question. I think I should move the AnnotQuadPoint > > > > parsing from AnnotFreeTex code to the class implementation itself. > > > > Something like (and yes, I will change quadrilateralsLength, I didn't > > > > liked it neither when I writed it): > > > > > > Yeah, this change looks like a good idea. > > > > > > > I have some doubts with AnnotCalloutLine and AnnotCalloutMultiline. > > > > Think that they use inheritance so they could need dynamic cast at > > > > some point in the future. > > > > > > Can you explain this issue in more detail? > > > > It was recommended not to use dynamic cast as it wasn't used in any > > other place in the poppler code. This case uses inheritance and usually > > to cast between base and child classes a dynamic cast is needed. I'm > > thinking that I should avoid it. Maybe using independent classes instead > > of using inheritance ? > > > > > -Jeff _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
