Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Annotation] Don't rotate the popups (bug 1819047) #16109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calixteman
Copy link
Contributor

No description provided.

@Snuffleupagus
Copy link
Collaborator

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Mar 4, 2023

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/e3b5085c9fae6c8/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 4, 2023

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/e3b5085c9fae6c8/output.txt

Total script time: 1.27 mins

Published

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Mar 4, 2023

Sorry, but I'm really not convinced that this is correct unfortunately.

If a PDF document wants to actually prevent an Annotation from rotating there's a /NoRotate flag specifically for that purpose, however I don't think that effectively disabling general Popup-rotation is what we want.
It really seems to me that the actual problem in this case is that we don't correctly determine the total rotation for Popups in the case where the /Page and the /Annot has different /Rotate-entries.

@calixteman
Copy link
Contributor Author

I opened the tested pdf in Acrobat and the popup isn't rotated at all and I missed that the NoRotate flag for the popup is set.
So we must make a difference between the popup annotation and the popup we add with a FreeText for example.
The /NoRotate case is a bit specific (I think I've a patch somewhere for it).
What do you think we should do for the FreeText case ? Should we just apply to the popup the rotation of the FreeText or keep the popup unrotated in order to make it readable ?

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Mar 4, 2023

What do you think we should do for the FreeText case ? Should we just apply to the popup the rotation of the FreeText or keep the popup unrotated in order to make it readable ?

I don't really see a problem with having the Popup simply follow the rotation of its underlying Annotation in this case, and would prefer that, given that everything else in the document is being rotated (when rotating the pages in the viewer).
(Unless the /NoRotate flag is set, which I don't think applies to the PDF document in that bug; anyway we already have issue #7631 for the /NoRotate flag case.)

@calixteman
Copy link
Contributor Author

calixteman commented Mar 4, 2023

Afaik, the popup we add for FreeText annotations is not specified in the pdf specifications.
In Foxit reader, a popup is added too and it isn't rotated with the page:
image

I don't really see how it's useful for the user to have to break is neck in order to read the popup contents.

@Snuffleupagus
Copy link
Collaborator

Afaik, the popup we add for FreeText annotations is not specified in the pdf specifications.
In Foxit reader, a popup is added too and it isn't rotated with the page:

Are you suggesting that different Popups should behave differently, depending on their base Annotation, since that feels inconsistent?

To me, it seems like there's basically two separate issues at play here:

  • The fact that we currently don't compute the total initial rotation correctly for any Popup, which is something that can also be easily seen in the files in issue 7631. For bug 1819047 the /Page rotation=180 and the /Annot rotation=180, hence the total initial rotation ought to be 0 degrees for the Popup in that case,
  • The general question if we should ever rotate the Popup itself, since as you mentioned that's not necessarily easy to read.

I do really think that we should fix the first point here, and have a more general UX discussion about the second one separately.
Otherwise we'd just wallpaper over an actual bug, w.r.t. computing the correct initial rotation, as far as I'm concerned.

@calixteman
Copy link
Contributor Author

The problem with the pdf in bug 1819047 is that the FreeText annotation has a /Rotate entry but this one isn't in the pdf specs.
There is no MK entry which could specify a rotation and the Matrix from AP is [-1, 0, 0, -1, ...] which indicates that a the appearance is rotated by 180deg.
In the specs, it's mentioned that Text annotations are always NoRotate (I played with the F value to see if it has an effect for Text annotations and it doesn't).
So I wonder if we should just do the same for the popup associated with a FreeText when it hasn't a popup entry in its dictionary.

@Snuffleupagus
Copy link
Collaborator

So I wonder if we should just do the same for the popup associated with a FreeText when it hasn't a popup entry in its dictionary.

Yes, I suppose that it could work to special-case that particular situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants