Fix notdef handling when subsetting Type 1 fonts#31928
Open
QuLogic wants to merge 1 commit into
Open
Conversation
There's no guarantee that `.notdef` will appear as character code 0 in
the encoding table [1], so always inserting it there may clobber an
existing glyph. And in fact, with the strange encoding of Computer
Modern, its symbol font _does_ have an entry in 0 for the minus sign.
We only actually need `.notdef` in the `todo` list of charstrings to
simulate, not in the `encoding` dictionary (which is only used again in
`_postscript_encoding`, and that specifically ignores `.notdef`), so
move it there.
Also, fix an inefficiency when simulating charstrings: once we simulate
a glyph, it should be added to the `done` set immediately, otherwise it
remains on the `todo` set, and gets simulated a second time (or possibly
more since the set may `pop` in any order.)
[1] See the Adobe Type 1 Font Format specification in
https://adobe-type-tools.github.io/font-tech-notes/pdfs/T1_SPEC.pdf
which says that "A Type 1 font program must have a `.notdef`
character defined in its CharStrings dictionary, even if it is not
referenced by the encoding vector." but does _not_ say it must be 0.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
PR summary
There's no guarantee that
.notdefwill appear as character code 0 in the encoding table [1], so always inserting it there may clobber an existing glyph. And in fact, with the strange encoding of Computer Modern, its symbol font does have an entry in 0 for the minus sign.We only actually need
.notdefin thetodolist of charstrings to simulate, not in theencodingdictionary (which is only used again in_postscript_encoding, and that specifically ignores.notdef), so move it there.Also, fix an inefficiency when simulating charstrings: once we simulate a glyph, it should be added to the
doneset immediately, otherwise it remains on thetodoset, and gets simulated a second time (or possibly more since the set maypopin any order.)[1] See the Adobe Type 1 Font Format specification in https://adobe-type-tools.github.io/font-tech-notes/pdfs/T1_SPEC.pdf which says that "A Type 1 font program must have a
.notdefcharacter defined in its CharStrings dictionary, even if it is not referenced by the encoding vector." but does not say it must be 0.Fixes #31925, mostly. If you open the result file in a viewer (evince, ghostscript, firefox), then the minus sign is there, but it isn't there in the converted image used for comparisons. I suspect this may have to do with #30068.
AI Disclosure
None
PR checklist