MatthiasValvekens / pyHanko

pyHanko: sign and stamp PDF files
MIT License
483 stars 71 forks source link

Fix signature rotation with rotate_with_page == false #266

Open DominikDostal opened 1 year ago

DominikDostal commented 1 year ago

Description of the changes

Adds a counter rotation to a signature if it is added to a rotated pdf while rotate_with_page is false (See Issue https://github.com/MatthiasValvekens/pyHanko/issues/265)

Caveats

Currently this solution adds the transformation matrix to the signature field first and then copies it to the appearence of the signature. This could be prevented if the already added /N field is reused instead of overwritten. Note: /N or rather /AP was added twice even without my change, once in fields.py at ap_dict[pdf_name('/N')] = pdf_out.add_object(ap_stream) and once in cms_embedder.py where my change adds the /Matrix field

Checklist

Please go over this checklist to increase the chances of your PR being worked on in a timely manner. Deviations are allowed with proper justification (see previous section).

Additional comments

I know that my solution is not perfect, since it creates a transformation matrix field that wont be used by the pdf in any way, but with the bug where 2 appearence objects are created, this was the only solution i could think of without also having to fix that.

I wouldnt mind if you reject this PR because of this, but i hope this can at least help you with fixing the bug.

DominikDostal commented 1 year ago

Hey Matthias,

thanks for you feedback.

As i was trying to implement the suggestions and fix the mistakes i made, i had to admit to myself that i managed to be stupid and not know which parts of all my trial and error were actually needed for the solution.

The solution should have consisted of 3 parts: 1) Add a transformation matrix to the appearence stream to rotate the signature 2) Change the box coordinates to be rotated This was something i had done outside the pyHanko code, therefore it wasnt included in the PR. I converted the box coordinates like this for /Rotate 90:

rect = [generic.FloatObject(rect[0]), generic.FloatObject(rect[1]), generic.FloatObject((rect[3] - rect[1]) * -1 + rect[0]), generic.FloatObject(rect[2] - rect[0] + rect[1])]

3) Swap width and height of stamp This was something that i happened to avoid with StaticStampStyle having the inner_content_scaling=InnerScaling.STRETCH_FILL setting. As an example i printed out the parameter inside the fit method:

boxwidth:  96
boxheight:  532
inner_nat_width:  532
inner_nat_height:  96
x_pos:  0.0
y_pos:  0.0
x_scale:  0.18045112781954886
y_scale:  5.541666666666667

As i have already added 1) to the PR, i will try to implement the other 2 parts too.

2): Thinking back on it, this seems to be a wrong calculation as i am just rotating the box with the upper left corner of the box staying on the same coordinate. I thinkt the best solution would be to not change the coordinates at all. Like if the box starts at 10|10 in portrait orientation, which is in the bottom left corner, then it should also be in the bottom left corner in landscape orientation

3): I actually have no clue how im supposed to do this. Maybe you have an idea where this would fit?

MatthiasValvekens commented 1 year ago

Hi Dominik,

Regarding transforming the box: this is surprisingly subtle. The BBox of a form XObject (in casu: the annotation's appearance stream) is in "form space" (i.e. the XObject's internal coordinate system), whereas the annotation's Rect is always in "default user space" (i.e. the page's coordinate system). The affine transformation matrix transforming from the former to the latter is what you put in the Matrix entry.

On the other hand, the Rotate entry transforms between the page coordinate system and what you see on the screen (let's call that "screen space"). So, if you want to specify your signature box in screen space (which seems to be the end goal here), you need to ensure that you apply the inverse transformation to Rect as well.

... as I mentioned in the issue thread, it's not as straightforward as it first looks ;). Hence my insistence on tests.

Does that answer your questions, or was it rather the maths that you needed some assistance with?