Fix glyph positioning in some rotation scenarios #403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yob
force-pushed
the
fix-glyph-displacement
branch
from
b6e0259
to
93a8624
Compare
Including some rotated pages, and some rotated text on non-rotated
pages.
When processing glyph displacement after rendering a glyph, the spec is
pretty clear that the calculation should be:
[ 1 0 0 ]
Tm = [ 0 1 0 ] x Tm
[ tx ty 1 ]
However, for years pdf-reader has had it backwards:
[ 1 0 0 ]
Tm = Tm x [ 0 1 0 ]
[ tx ty 1 ]
We'd built up some compensating bugs to cover that for some PDFs too,
like using a calculated font size instead of the raw font size from the
page state. Also a divide by ctm.a that made no sense, and there was
even a comment saying that.
Fixing the order of the matrix multiplication means those compensating
bugs can also go away.
There are some minor changes to the text output of the columns spec,
which I'm willing to wear. Mostly whitespace changes - nothing
significant - so I've updated the spec to match. I suspect these
actually indicate some additional bugs in glyph displacement -
particularly the way we process numeric arguments to the TJ
(show_text_with_positioning) operator. I think this commit is an overall
nett positive as it fixes some significant glyph positioning issues. We
can iterate on the TJ operator handling separately.
Finally, there's a couple of tweaks to the apply_rotation in
PageTextReceiver. This method is still buggy, and I've left a comment
with some details. The current version will shift the characters around
so they're positioned correctly relative to eachother, but the final x
and y values are incorrect relative to the overall page boxes. I'll fix
that up separately.
These changes were driven by a failing spec with a PDF based on the
failure reported at #397. It's a page that's rotated by 270 degrees, and
the rotation is undone in the BT block rather than via the CTM.
yob
force-pushed
the
fix-glyph-displacement
branch
from
93a8624
to
934898f
Compare
This was referenced Dec 19, 2021
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Including some rotated pages, and some rotated text on non-rotated pages.
When processing glyph displacement after rendering a glyph, the spec is pretty clear that the calculation should be:
However, for years pdf-reader has had it backwards:
We'd built up some compensating bugs to cover that for some PDFs too, like using a calculated font size instead of the raw font size from the page state. Also a divide by ctm.a that made no sense, and there was even a comment saying that.
Fixing the order of the matrix multiplication means those compensating bugs can also go away.
There are some minor changes to the text output of the columns spec, which I'm willing to wear. Mostly whitespace changes - nothing significant - so I've updated the spec to match. I suspect these actually indicate some additional bugs in glyph displacement - particularly the way we process numeric arguments to the TJ (show_text_with_positioning) operator. I think this commit is an overall nett positive as it fixes some significant glyph positioning issues. We can iterate on the TJ operator handling separately.
Finally, there's a couple of tweaks to the apply_rotation in PageTextReceiver. This method is still buggy, and I've left a comment with some details. The current version will shift the characters around so they're positioned correctly relative to eachother, but the final x and y values are incorrect relative to the overall page boxes. I'll fix that up separately.
These changes were driven by a failing spec with a PDF based on the failure reported at #397. It's a page that's rotated by 270 degrees, and the rotation is undone in the BT block rather than via the CTM.
Fixes #376
Fixes #316
Fixes #271
Fixes #110