[poppler] Annot Improving (III)

[Iñigo Martínez]

El sáb, 08-12-2007 a las 10:54 +1100, Brad Hards escribió: 
> On Thursday 06 December 2007 10:45:09 am Iñigo Martínez wrote:
> > I have changed again the last patch with the required changes and synced
> > it with the git head.
> This is good stuff.
> 
> > I have tried it with some annot testing PDF documents I have around and
> > everything looks like is working correctly, but I have some (more)
> > things I have notes about thought.
> >
> > 1. When the dash numbers in the arrays are wrong, poppler old code does
> > give them 1 value, but reading the pdf reference there is this:
> >
> > "Note that no dash phase is specified; the phase is assumed to be 0."
> >
> > So I have assumed that it should have 0, so I have changed it. There are
> > some error handling I don't know have to handle too, so I have added a
> > TODO comment to do it later.
> I'm not sure I fully understand your concern, because I don't know what you 
> mean by "the dash numbers in the arrays are wrong". However my reading of the 
> PDF reference (1.7, Table 8.17) leads me to understand that the lack of a 
> phase is just how PDF does it. Try looking at Table 4.6 in the PDF reference.
> Another way to think about it: imagine you have a dash pattern (D array) of [3 
> 4]:
> ---    ---    ---    --- 
> What the lack of a phase means is that there is no way to do express this:
>   ---    ---    ---    ---
> (note the "offset" at the start of the line)
> 
> I think if there is an error (e.g. D array contains something that isn't a 
> dash pattern), you should assume the default of [3] (i.e. three units on, 
> three units off, repeating).

Yes I was referring to this case, wrong data where it shouldn't be (PDF
reference isn't very clear about this cases and it's where all my doubts
come from as I can ensure anything). I didn't thought about reverting
back to the default case if the data was wrong, but I think it's the
best option (at reference side) too, so I will change it.

> 
> > 2. When handling the colors, poppler old code does default red and green
> > to 0 but blue to 1. There is nothing about that in the reference. My
> > code does default all colors to 0, as it does support more color spaces,
> > maybe I shoudl change it but, where are those 'magic' numbers come
> > from ?
> I'm not sure. I agree this is missing information in the PDF reference.
> 
> > I think I should do some testing with Acrobat Reader to test everything.
> > How do you do those testings ? How do you know what values does use
> > Acrobat once it detects a wrong object/value ?
> You can't. You can only observe a reasonable selection, and wait for the bug 
> reports :-( If you need particular test cases, you may have to hand-edit a 
> PDF file to get the right setup. Also, 
> http://acroeng.adobe.com/Test_Files/comments/ might be useful.
> 
> I thought there was a bug report about annotation colours, but it is only on 
> link colours.
> 
> > As always any comment is welcome.
> I have a couple of extra comments:
> 
> The change to Gfx looks good (much better style), but I'm a bit concerned that 
> the improvements will complicate the next xpdf merge. Albert did the last 
> round of merges - I'd like his feedback on this. Is there likely to be 
> problems with restricting the changes to the Annotation classes (i.e. keep 
> the AnnotBorder, AnnotBorderArray, AnnotBorderBS, AnnotColor classes, but 
> only use them behind the current AnnotBorderStyle interface)?
> 
> I think we should extract the corners (irrespective of what acroread does, per 
> Implementation Note 81). If we don't want to use them for rendering the image 
> backends then we should omit using them there (rather than in the annotation 
> parser).  This shouldn't be too hard - it looks like you have all the code 
> already, just commented out.

I will uncomment it then.

> Brad
> 
> 
-------------- 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/20071208/667f2443/attachment.pgp