[poppler] Annot Improving (VIII)

[Iñigo Martínez]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Improved-AnnotLink-support.patch
Type: application/mbox
Size: 6650 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080111/0f685d84/attachment-0003.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-AnnotFreeText-support.patch
Type: application/mbox
Size: 13324 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080111/0f685d84/attachment-0004.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Changed-AnnotQuadrilateral-parsing-inside-AnnotLink.patch
Type: application/mbox
Size: 19739 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080111/0f685d84/attachment-0005.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Esta parte del mensaje =?ISO-8859-1?Q?est=E1?= firmada
	digitalmente
Url : http://lists.freedesktop.org/archives/poppler/attachments/20080111/0f685d84/attachment-0001.pgp