[poppler] [PATCHES] Support for the NoRotate annotation flag

Tue Apr 2 12:44:28 PDT 2013

On Tuesday, April 02, 2013 08:55:01 PM Adam Reichold wrote:
> Hello Fabio,
> 
> Am 02.04.2013 20:28, schrieb Fabio D'Urso:
> > On Tuesday, April 02, 2013 07:59:57 PM Adam Reichold wrote:
> >> Hello,
> >> 
> >> Am 02.04.2013 19:38, schrieb Albert Astals Cid:
> >>> El Dimarts, 2 d'abril de 2013, a les 18:36:48, Adam Reichold va
> >>> 
> >>> escriure:
> >>>> Hello,
> >>>> [snip]
> >>>> 
> >>>> When implementing this in qpdfview, I spent most of the time
> >>>> figuring out how to correctly transform the annotation boundaries
> >>>> given to me by the qt4 frontend. This is of course not problem in
> >>>> itself, but it aligns with this observation as the applications
> >>>> is bound to use the internal coordinate transformations and needs
> >>>> to correctly recreate their results.
> >>>> 
> >>>> Maybe it would be better to let applications that want to
> >>>> implement this draw those annotations by themselves, hiding
> >>>> Poppler's rendition?
> >>> 
> >>> What's the benefit of everyone doing the same? Wasn't the
> >>> repetition of work why libraries where created?
> >> 
> >> There is some duplication the current setting as well as the
> >> application has to recreate the internal transformations, but of
> >> course this is much less than actually drawing the annotations. I am
> >> just not sure how well the coupling of these interactive elements with
> >> application behavior through an interface designed to render paginated
> >> content will work. (And how prone it will be to breakage by internal
> >> changes.)
> > 
> > I think I see your point, my patches only tell where the annotation is if
> > you render the page with the default rotation.
> > If you ask renderToImage to render the page with a different rotation the
> > position changes, and you want to know this new position, correct?
> 
> Yes, that seems to be one way to ensure that the application uses the
> right coordinates no matter how those flags are implemented.
> 
> > I thought about this when I wrote the patches and came to the conclusion
> > that "by default" we should return coordinates referring to the default
> > rotation, so that we don't break existing applications, at least the
> > "simple" ones (e.g. okular always requests 0-rotated rendering and
> > applies user-requested rotations on his own, and I guess most
> > applications keeping internal caches do the same, but of course I might
> > be wrong :)).
> 
> Actually, I invalidate the cache on rotation change. I was thinking of
> speed and quality improvements if the rotation is done by Splash on the
> vector data instead of using Qt to rotate a QImage. Is doing it like
> this pointless?

I have no idea if it makes any quality difference. I guess that as long as 
pixels are "squares" (i.e. horizontal and vertical DPI are equal) it's the 
same. But I'm just guessing, can't tell for certain.

>From a speed point of view, it really depends on the document. A simple page 
is obviously faster rotated directly in Splash. A page that takes longer to 
render (e.g. a lot of primitives, or images to be upscaled) is better rendered 
once and rotated afterwards each time the user changes rotation.

Still, if I remember correctly, okular too invalidated the cache last time I 
checked. It goes through a full SplashRendering+PostRotation every time the 
user changes rotation. I guess it's like this because not all file-format 
backends can handle rotation internally.

> > For those who need coordinates at different rotations too we have two 
options:
> >  - Let applications do the math themselves. That is, what you have been
> >  doing>  
> >    for qpdfview. I know it's a PITA, I too did that for okular in my first
> >    design.
> 
> I think it is less about doing the math, but we would also enter a
> rather restrictive contract on how not to change the implementation of
> the NoRotate flag. (Is the transformation applied part of the PDF spec?
> Maybe this point is invalid then.)

Behavior for the NoRotate flag is described immediatly after the annotation 
flag list in PDF specs.

By the way, in the "let applications do the math themselves" approach 
applications have to transform not only the Annotation's boundary rectangle, 
but subclass-specific data too (e.g. LineAnnotation's vertices, 
InkAnnotation's paths...)

> >  - Add some hook in poppler qt4 to request coordinates at different
> >  rotations,>  
> >    I can think of two ways to do this:
> > 	a. Add an extra rotation argument to all annotation getters and setters
> > 	b. Add a per-Annotation setting to set the desired coordinate rotation
> > 
> > Both of them would break the ABI (well, #1 can also be implemented by
> > adding a new set of overloaded getters and setters [*]).
> > 
> > #2 would break the API too because this variable would have to be put in
> > the Annotation class, not AnnotationPrivate (AnnotationPrivate is
> > transparently shared across several instances of the same annotation)
> > 
> > I like #1 much more, especially in the light of the thread-safety
> > progresses that have been made, but if we can't break the ABI it adds a
> > lot duplication.
> > 
> > * = still, this breaks souce compatibility if applications use pointers to
> > annotation methods, but I guess this is very unlikely)
> 
> I agree, version a/#1 sounds much nicer.

In the meantime we can also add the scale factor in preparation for the 
NoScale flag, so we don't break the ABI again in the future.

> >>>> (This could also solve the problem of the annotations being
> >>>> rotated out of the page bounding box.)
> >>> 
> >>> I'm sure we can find some other solution for that.
> >> 
> >> Of course there are other solutions, for example adding an annotation
> >> rendering interface to the frontends the results of which the
> >> application can then compose by itself. My point is basically that if
> >> the interactive elements begin to be more than just
> >> some-more-stuff-on-the-page, the API should probably reflect that.
> > 
> > That'd be great! It would also solve the issue of having to redraw the
> > whole page on each single annotation change. I like this idea very very
> > much! But I'm too busy to work on it atm :(
> 
> I would like to try to work something out, but waiting for this would
> probably keep the Okular bug open for rather long time.

Why, can't it be done on top of these patches? Or even current git master, I 
don't think that there are going to be a lot of conflicts.

I would also leave an option to render annotations along with the page like we 
do now (enabled by default), mainly for backward compatibility reasons, but 
also for simpler use cases (e.g. file managers' thumbnailers)

> Best regards, Adam.
> 
> > Fabio
> > 
> >> Best regard, Adam.
> >> 
> >>> Cheers, Albert _______________________________________________
> >>> poppler mailing list poppler at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/poppler
> >> 
> >> _______________________________________________
> >> poppler mailing list
> >> poppler at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/poppler
> > 
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/poppler
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
-- 
Fabio