[poppler] [PATCHES] Support for the NoRotate annotation flag
Fabio D'Urso
fabiodurso at hotmail.it
Tue Apr 2 11:32:20 PDT 2013
On Tuesday, April 02, 2013 08:28:12 PM Fabio D'Urso wrote:
> 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,
> > >>
> > >> Am 31.03.2013 15:16, schrieb Carlos Garcia Campos:
> > >>> Fabio D'Urso <fabiodurso at hotmail.it> writes:
> > >>>> Hi,
> > >>>>
> > >>>> the attached patches implement support for the NoRotate
> > >>>> annotation flag, which tells that the annotation must *not*
> > >>>> be rotated according to the page orientation or requested
> > >>>> rendering rotation (more on this later).
> > >>>>
> > >>>> The main reason why I have implemented these patches is
> > >>>> https://bugs.kde.org/show_bug.cgi?id=313177, which consists
> > >>>> in free-text annotations' text being wrongly oriented when
> > >>>> the page has a /Rotate attribute whose value is different
> > >>>> than 0. With the NoRotate flag, it will be possible to create
> > >>>> annotations that always face the viewer, independently from
> > >>>> the page's /Rotate attribute.
> > >>>>
> > >>>> NoRotate annotations are somewhat tricky, because they are
> > >>>> never rotated, neither according to the orientation of page,
> > >>>> nor according to the requested *rendering* rotation. I'm
> > >>>> attaching a "orientation-examples.pdf" that shows this very
> > >>>> important aspect. If you open it in acroread and rotate the
> > >>>> pages, you'll see that NoRotate annotations pivot on their
> > >>>> top-left corner and always face the viewer. I want to stress
> > >>>> on this aspect: what you get if you render a page containing
> > >>>> a NoRotate annotation and then apply a rotation *IS NOT* the
> > >>>> same result that you would obtain by directly rendering the
> > >>>> page at that rotation (see attached image for an example).
> > >>>> This is the reason why I had to write patch 0004.
> > >>>>
> > >>>> ~ PATCHES 0001 and 0002
> > >>>>
> > >>>> Patch 0001 changes the Gfx::drawAnnot method so that it
> > >>>> "unrotates" the coordinate system before drawing NoRotate
> > >>>> annotations. This is the patch that actually implements the
> > >>>> NoRotate flag, and it works in all cases but rasterized
> > >>>> postscript output, which is addressed in patch 0004.
> > >>>>
> > >>>> Patch 0002 adds rotation control in poppler-qt4's demo
> > >>>> application. I've used this patch to quickly test how
> > >>>> NoRotate annotations behave with different rendering
> > >>>> rotations.
> > >>>>
> > >>>> @@ -5144,6 +5157,28 @@ void Gfx::drawAnnot(Object *str,
> > >>>> AnnotBorder *border, AnnotColor *aColor,>> return;
> > >>>>
> > >>>> }
> > >>>>
> > >>>> + // saves gfx state and automatically restores it on
> > >>>> return + GfxStackStateSaver stackStateSaver(this); + + /*
> > >>>> If flag NoRotate is set then we need to "unrotate" the
> > >>>> coordinate system */ + if ((flags & Annot::flagNoRotate) &&
> > >>>> state->getRotate() != 0) { + const double angle_rad =
> > >>>> state->getRotate() * M_PI / 180; + const double c =
> > >>>> cos(angle_rad); + const double s = sin(angle_rad); + +
> > >>>> // Rotation around topleft corner (xMin, yMax) + const
> > >>>> double unrotateMTX[6] = { + +c, +s, + -s, +c, +
> > >>>> -c*xMin + s*yMax + xMin, -c*yMax - s*xMin + yMax + }; + +
> > >>>> state->concatCTM(unrotateMTX[0], unrotateMTX[1],
> > >>>> unrotateMTX[2], + unrotateMTX[3],
> > >>>> unrotateMTX[4], unrotateMTX[5]); + out->updateCTM(state,
> > >>>> unrotateMTX[0], unrotateMTX[1], unrotateMTX[2], +
> > >>>> unrotateMTX[3], unrotateMTX[4], unrotateMTX[5]); + } +
> > >>>>
> > >>>> // draw the appearance stream (if there is one) if
> > >>>> (str->isStream()) {
> > >>>
> > >>> Unfortunately this doesn't work for the cairo backend either,
> > >>> since state->getRotate() will be 0. In cairo the user typically
> > >>> does something like:
> > >>>
> > >>> cairo_create() cairo_translate() cairo_scale() cairo_rotate()
> > >>> poppler_page_render()
> > >>>
> > >>> And we always pass 0 to displaySlice() since the cairo context
> > >>> is already transformed.
> > >>
> > >> 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?
>
> 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 :)).
>
> 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.
>
> - 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 [*]).
Sorry I pressed Send too early :)
I meant that both a and b break the ABI. The "let applications do the math
themselves" approach of course doesn't.
Also, s/#1/option a/
>
> #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.
Again, s/#1/option a/ and s/#2/option b/
> * = still, this breaks souce compatibility if applications use pointers to
> annotation methods, but I guess this is very unlikely)
>
> > >> (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 :(
>
> 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
--
Fabio
More information about the poppler
mailing list