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. 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
0001-Improved-AnnotLink-support.patch
Description: application/mbox
0002-AnnotFreeText-support.patch
Description: application/mbox
0003-Changed-AnnotQuadrilateral-parsing-inside-AnnotLink.patch
Description: application/mbox
signature.asc
Description: Esta parte del mensaje está firmada digitalmente
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
