Fixes issue #688: [tx] subroutinizing with -no_futile may make font larger than without #704
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
1. treat CFF2 subroutines the same as CFF1 tail subroutines, because CFF2 has no return 2. modified buildCallList to list up subroutine candidates by savings, not by mere lengths. there may be slight differences between the two (when hinted, subr number size differ) 3. qsort comp function cmpSubrLengths was not sorting subrs by smallest ofset first as the second key despite the comment. fixed it, and renamed comp function to cmpSubrSavings
fixed sorting by offset in cmpSubrLengths
cjchapman
approved these changes
Dec 20, 2018
There was a problem hiding this comment.
This looks OK to me, but given the extent of the changes, I'd definitely like @readroberts to review as well before we merge.
readroberts
approved these changes
Jan 14, 2019
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.
The original futile subroutine removal option was implemented as a multi-phase subroutine assignment process, where futile (zero/single-use) subroutines are detected & removed after each assignment, then reiterated until there are no more futile subroutines. While this implementation generally makes the result smaller with fonts of size < 30MB, it was observed to make a subroutinized result of a very large font (> 150MB) larger than without the option. This behavior can be explained as a sub-optimal assignment with a smaller set of subroutines after each iteration.
In this PR, the futile subroutine removal is re-implemented as a straight-forward logic. After the first subroutine assignment, detected single-use subroutines are inlined and removed along with zero-use. A call list is used to record subroutine calls made by each charstring or subroutine (previously, a subroutinized charstring/subroutine was generated immediately after assignment so calls were recorded only temporarily). After building call lists for all charstrings and (tentative) subroutines, futile subroutines are removed, remaining subroutines are renumbered (to fill gaps left by futile), then reordered by their actual use counts.
Although the new futile removal logic makes a difference with a very large font only, the new code appears to make subroutinized fonts often smaller than before due to the extra reordering step as a by-product.
tx
-no_futileoption has been deprecated. The new behavior is always enabled with+S.No test case is added for the original problem which is seen with a very large font as it is too much for a build check. An expected result
SourceCodeVar-Roman_CFF2_subrfor an existing test casetx_test.py::test_cff2_extracthas been updated with a new result.