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

Tue Apr 2 11:55:01 PDT 2013

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,
>>>>
>>>> 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?

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?

> 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.)

>  - 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.

>>>> (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.

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
> 