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 <[email protected]> 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 [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/poppler > > > > _______________________________________________ > > poppler mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler -- Fabio _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
