Issue354 fixes #467
Merged
Issue354 fixes #467
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
… lots of hints to CFF2. t2cCtx *h->stack.blendArgs was sized as 6 elements. The max number of operands in a T2 charstring (other than for the blend op) is actually 96, aka T2_Max_STEMS.
…hints". Also fixed bug in writing CFF2 In writing test for issue 354, discovered that tx was writing the CFF2 var store incorrectly when writing a CFF2: it was adding the var store offset twice during offset calculations.
miguelsousa
reviewed
Jul 11, 2018
Tests/tx_test.py
Outdated
| # tx defined an array of only 6 operants. | ||
| # This is encountered only when wrinting to a VF CFF2. | ||
| actual_path = runner(CMD + ['-o', 'cff2', '-f', 'cff2_vf.otf']) | ||
| actual_path = runner(CMD + ['-f', actual_path, '-o', 'dcf']) |
There was a problem hiding this comment.
I think you need to use -a option in this second call to runner
There was a problem hiding this comment.
Don't use actual_path twice, because in case of failure it may be less clear which one is
This is why I think adding tests is always a good thing. |
miguelsousa
reviewed
Jul 11, 2018
Tests/tx_test.py
Outdated
| # This is encountered only when wrinting to a VF CFF2. | ||
| actual_path = runner(CMD + ['-o', 'cff2', '-f', 'cff2_vf.otf']) | ||
| actual_path = runner(CMD + ['-f', actual_path, '-o', 'dcf']) | ||
| expected_path = _get_expected_path('cff2_vf.dcf.txt') |
There was a problem hiding this comment.
you did not include this file (cff2_vf.dcf.txt) in the commit
…any hints". Also fixed bug in writing CFF2 Prior change fixed issue for fonts without FDSelect, but not for fonts with FDSelect. Now works with both.
…ny hints". Added the expected output file, which I omitted from prior commits. Added modified OTF input file Changed test function per suggestions from msousa.
miguelsousa
requested changes
Jul 11, 2018
| } | ||
| if (font->size.VarStore > 0) { | ||
| font->offset.VarStore = h->offset.varStore; | ||
| } |
| if (h->size.FDSelect > 0) { | ||
| h->offset.FDSelect = offset; | ||
| offset += h->size.FDSelect; | ||
| } |
cffwrite.c Restore indent to tab, same as surrounding indentation, in several lines Update the expect output file Tests/tx_data/expected_output/cff2_vf.pfa to match updated input file cff2_vf.otf
fixed whitespace issues reported by flake8
miguelsousa
approved these changes
Jul 12, 2018
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.
Add test for issue 354 " Fails to dump a CFF2 VF font with many hints". Also fixed bug in writing CFF2
In writing test for issue 354, discovered that tx was writing the CFF2 var store incorrectly when writing a CFF2: it was adding the var store offset twice during offset calculations.