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

[cffLib.specializer] generalizeProgram returns invalid charstring program? #1975

Closed
anthrotype opened this issue May 28, 2020 · 1 comment · Fixed by #2750
Closed

[cffLib.specializer] generalizeProgram returns invalid charstring program? #1975

anthrotype opened this issue May 28, 2020 · 1 comment · Fixed by #2750
Assignees

Comments

@anthrotype
Copy link
Member

anthrotype commented May 28, 2020

I wanted to use the cffLib.specializer to generalize some CFF2 charstrings, to produce some test fonts.
I did something like this:

import sys
from fontTools import ttLib
from fontTools.cffLib import specializer

font = ttLib.TTFont(sys.argv[1])
cff = font["CFF2"].cff
top_dict = cff.topDictIndex[0]
charstrings = top_dict.CharStrings
for glyph_name in font.getGlyphOrder():
    cs = charstrings[glyph_name]
    cs.decompile()
    cs.program = specializer.generalizeProgram(cs.program, getNumRegions=cs.getNumRegions)
font.save(sys.argv[2])

However, upon saving the font, when the font-bounding-box recalculation tries to call the charstrings' draw() method, it fails with an error like this:

Traceback (most recent call last):
  [...]
  File "/Users/clupo/Github/fonttools/Lib/fontTools/misc/psCharStrings.py", line 982, in draw
    extractor.execute(self)
  File "/Users/clupo/Github/fonttools/Lib/fontTools/misc/psCharStrings.py", line 300, in execute
    rv = handler(index)
  File "/Users/clupo/Github/fonttools/Lib/fontTools/misc/psCharStrings.py", line 563, in op_rmoveto
    self.rMoveTo(self.popallWidth())
  File "/Users/clupo/Github/fonttools/Lib/fontTools/misc/psCharStrings.py", line 474, in popallWidth
    assert self.defaultWidthX is not None, "CFF2 CharStrings must not have an initial width value"
AssertionError: CFF2 CharStrings must not have an initial width value

If I try to save the TTFont with recalcBBox=False, then the resulting font does not render at all (it's all blank), and OTS also rejects it (ERROR: CFF2: Failed validating CharStrings INDEX).

Upon inspecting the original and generalized programs, I see that the number of arguments for the blend operators seems incorrect. I think it's missing the last argument that OT spec calls numberOfBlends (ie. the one which tells the operator following blend how may operands that will be left on the stack after blending).
E.g. in the glyph below, the first line is the original specialised program, the second is the generalized one after our generalizeProgram function:

Original program:  96 -12 -20 1 blend hmoveto 432 74 120 1 blend 660 -432 -74 -120 1 blend -660 hlineto 48 32 101 164 45 72 2 blend rmoveto 102 176 64 106 -47 -76 -75 -120 -17 -28 -6 -10 4 blend rlineto 4 hlineto 62 -106 100 -176 -16 -26 6 10 -46 -74 75 120 4 blend rlineto -332 126 204 1 blend hlineto 166 334 -64 -104 -9 -14 2 blend rmoveto -56 92 -94 168 10 16 3 4 47 76 -82 -132 4 blend rlineto 302 -113 -182 1 blend hlineto -94 -168 -54 -92 47 76 82 132 9 14 -3 -4 4 blend rlineto -4 hlineto -176 -292 9 14 25 40 2 blend rmoveto 536 -124 -200 1 blend vlineto 154 -270 -154 -266 -45 -72 63 102 45 72 61 98 4 blend rlineto 354 -17 -26 1 blend hmoveto -152 266 152 270 44 70 -61 -98 -44 -70 -63 -102 4 blend rlineto -536 124 200 1 blend vlineto

Generalized program:  96 -12 -20 blend 0 rmoveto 432 74 120 blend 0 rlineto 0 660 rlineto -432 -74 -120 blend 0 rlineto 0 -660 rlineto 48 101 164 blend 32 45 72 blend rmoveto 102 -47 -76 blend 176 -75 -120 blend rlineto 64 -17 -28 blend 106 -6 -10 blend rlineto 4 0 rlineto 62 -16 -26 blend -106 6 10 blend rlineto 100 -46 -74 blend -176 75 120 blend rlineto -332 126 204 blend 0 rlineto 166 -64 -104 blend 334 -9 -14 blend rmoveto -56 10 16 blend 92 3 4 blend rlineto -94 47 76 blend 168 -82 -132 blend rlineto 302 -113 -182 blend 0 rlineto -94 47 76 blend -168 82 132 blend rlineto -54 9 14 blend -92 -3 -4 blend rlineto -4 0 rlineto -176 9 14 blend -292 25 40 blend rmoveto 0 536 -124 -200 blend rlineto 154 -45 -72 blend -270 63 102 blend rlineto -154 45 72 blend -266 61 98 blend rlineto 354 -17 -26 blend 0 rmoveto -152 44 70 blend 266 -61 -98 blend rlineto 152 -44 -70 blend 270 -63 -102 blend rlineto 0 -536 124 200 blend rlineto

I uploaded a test font (a subsetted copy of SourceSansVariable-Roman.otf, which I had previously run through fonttools subset --desubroutinize --no-hinting) and a script to reproduce the issue:

generalize-cff-issue.zip

$ python generalize.py SourceSansVariable-Roman.subset.otf SourceSansVariable-Roman.subset.gen.otf

I spent some time poking at the code, and eventually managed to produce a generalized version of the test font that seems to render and passes sanitization. I applied the following patch:

diff --git a/Lib/fontTools/cffLib/specializer.py b/Lib/fontTools/cffLib/specializer.py
index 1d2f4b73..8507b332 100644
--- a/Lib/fontTools/cffLib/specializer.py
+++ b/Lib/fontTools/cffLib/specializer.py
@@ -340,6 +340,12 @@ def generalizeCommands(commands, ignoreErrors=False):
                                result.append(('', [op]))
                        else:
                                raise
+
+       for i in range(len(result)):
+               op, args = result[i]
+               if any(isinstance(arg, list) for arg in args):
+                       result[i] = op, _convertToBlendCmds(args)
+
        return result

 def generalizeProgram(program, getNumRegions=None, **kwargs):

It seems that _convertToBlendCmds function puts back the final numberOfBlends argument, that was stripped by _convertBlendOpToArgs early on inside generalizeCommands function.

However, I'm not sure this is the right fix. Also the roundtrip tests fail after this.
It would be nice if @behdad or some of the Adobe folks could have a look.

anthrotype added a commit to anthrotype/fonttools that referenced this issue Jun 1, 2020
This currently fails with
AssertionError: CFF2 CharStrings must not have an initial width value

possibly related to fonttools#1975
@behdad
Copy link
Member

behdad commented Jun 3, 2020

I do see the problem now. When CFF2 was being added to this module, I didn't fully understand the design. I thought they addressed it, but now I see it didn't correctly.

What's happening is that in general, the form of commands is always the same format, and generalize and specialize return data in the same format. But with this change, the result of generalize uses a different format, and then specialize expects that and converts it back to the original format. That's not good. Solving it is tricky. I'm still thinking about it.

behdad added a commit that referenced this issue Aug 16, 2022
behdad added a commit that referenced this issue Aug 16, 2022
behdad added a commit that referenced this issue Aug 16, 2022
behdad added a commit that referenced this issue Aug 16, 2022
behdad added a commit that referenced this issue Aug 16, 2022
behdad added a commit that referenced this issue Aug 17, 2022
@behdad behdad closed this as completed in 61160fe Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants