From af1a0d1fe524ffb2718f5068df75fcd0863f9e02 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 15 Jan 2019 14:07:54 +0000 Subject: [PATCH 1/6] [subset] set emptied CFF charstrings to 'endchar' with --retain-gids Part of #1446 --- Lib/fontTools/subset/cff.py | 67 ++++++++++++++++++++----------------- Tests/subset/subset_test.py | 37 +++++++++++++++++++- 2 files changed, 73 insertions(+), 31 deletions(-) diff --git a/Lib/fontTools/subset/cff.py b/Lib/fontTools/subset/cff.py index 9a2b77e4d..71092c70c 100644 --- a/Lib/fontTools/subset/cff.py +++ b/Lib/fontTools/subset/cff.py @@ -104,37 +104,44 @@ def subset_glyphs(self, s): font = cff[fontname] cs = font.CharStrings - # Load all glyphs - for g in font.charset: - if g not in s.glyphs: continue - c, _ = cs.getItemAndSelector(g) - - if cs.charStringsAreIndexed: - indices = [i for i,g in enumerate(font.charset) if g in s.glyphs] - csi = cs.charStringsIndex - csi.items = [csi.items[i] for i in indices] - del csi.file, csi.offsets - if hasattr(font, "FDSelect"): - sel = font.FDSelect - # XXX We want to set sel.format to None, such that the - # most compact format is selected. However, OTS was - # broken and couldn't parse a FDSelect format 0 that - # happened before CharStrings. As such, always force - # format 3 until we fix cffLib to always generate - # FDSelect after CharStrings. - # https://github.com/khaledhosny/ots/pull/31 - #sel.format = None - sel.format = 3 - sel.gidArray = [sel.gidArray[i] for i in indices] - cs.charStrings = {g:indices.index(v) - for g,v in cs.charStrings.items() - if g in s.glyphs} + if s.options.retain_gids: + for g in cs.keys(): + if g in s.glyphs_emptied: + c = cs[g] + c.decompile() + c.program = ["endchar"] else: - cs.charStrings = {g:v - for g,v in cs.charStrings.items() - if g in s.glyphs} - font.charset = [g for g in font.charset if g in s.glyphs] - font.numGlyphs = len(font.charset) + # Load all glyphs + for g in font.charset: + if g not in s.glyphs: continue + c, _ = cs.getItemAndSelector(g) + + if cs.charStringsAreIndexed: + indices = [i for i,g in enumerate(font.charset) if g in s.glyphs] + csi = cs.charStringsIndex + csi.items = [csi.items[i] for i in indices] + del csi.file, csi.offsets + if hasattr(font, "FDSelect"): + sel = font.FDSelect + # XXX We want to set sel.format to None, such that the + # most compact format is selected. However, OTS was + # broken and couldn't parse a FDSelect format 0 that + # happened before CharStrings. As such, always force + # format 3 until we fix cffLib to always generate + # FDSelect after CharStrings. + # https://github.com/khaledhosny/ots/pull/31 + #sel.format = None + sel.format = 3 + sel.gidArray = [sel.gidArray[i] for i in indices] + cs.charStrings = {g:indices.index(v) + for g,v in cs.charStrings.items() + if g in s.glyphs} + else: + cs.charStrings = {g:v + for g,v in cs.charStrings.items() + if g in s.glyphs} + font.charset = [g for g in font.charset if g in s.glyphs] + font.numGlyphs = len(font.charset) return True # any(cff[fontname].numGlyphs for fontname in cff.keys()) diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py index 030caa136..cd8787b2c 100644 --- a/Tests/subset/subset_test.py +++ b/Tests/subset/subset_test.py @@ -475,7 +475,7 @@ class SubsetTest(unittest.TestCase): subset.main([fontpath, "--recalc-timestamp", "--output-file=%s" % subsetpath, "*"]) self.assertLess(modified, TTFont(subsetpath)['head'].modified) - def test_retain_gids(self): + def test_retain_gids_ttf(self): _, fontpath = self.compile_font(self.getpath("TestTTF-Regular.ttx"), ".ttf") font = TTFont(fontpath) @@ -507,6 +507,41 @@ class SubsetTest(unittest.TestCase): self.assertGreater(glyf["A"].numberOfContours, 0) self.assertEqual(glyf["B"].numberOfContours, 0) + def test_retain_gids_otf(self): + _, fontpath = self.compile_font(self.getpath("TestOTF-Regular.ttx"), ".otf") + font = TTFont(fontpath) + + self.assertEqual(font["hmtx"]["A"], (500, 132)) + self.assertEqual(font["hmtx"]["B"], (400, 132)) + + font["CFF "].cff[0].decompileAllCharStrings() + cs = font["CFF "].cff[0].CharStrings + self.assertGreater(len(cs["A"].program), 0) + self.assertGreater(len(cs["B"].program), 0) + + subsetpath = self.temp_path(".ttf") + subset.main( + [ + fontpath, + "--retain-gids", + "--output-file=%s" % subsetpath, + "--glyph-names", + "A", + ] + ) + subsetfont = TTFont(subsetpath) + + self.assertEqual(subsetfont.getGlyphOrder(), font.getGlyphOrder()) + + hmtx = subsetfont["hmtx"] + self.assertEqual(hmtx["A"], (500, 132)) + self.assertEqual(hmtx["B"], (0, 0)) + + subsetfont["CFF "].cff[0].decompileAllCharStrings() + cs = subsetfont["CFF "].cff[0].CharStrings + self.assertGreater(len(cs["A"].program), 0) + self.assertEqual(cs["B"].program, ["endchar"]) + if __name__ == "__main__": sys.exit(unittest.main()) From 336abfcaf066c00d82597a8466ff049a33881c0a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 15 Jan 2019 15:10:04 +0000 Subject: [PATCH 2/6] [subset] set charstring's effective width to 0 when --retain-gids seems to be the consensus, despite this may add a few bytes when the emptied glyph's width is different from PrivateDict.defaultWidthX --- Lib/fontTools/subset/cff.py | 40 ++++++++++++++++++++----------------- Tests/subset/subset_test.py | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Lib/fontTools/subset/cff.py b/Lib/fontTools/subset/cff.py index 71092c70c..b56a4430b 100644 --- a/Lib/fontTools/subset/cff.py +++ b/Lib/fontTools/subset/cff.py @@ -66,6 +66,25 @@ def closure_glyphs(self, s): s.glyphs.update(components) decompose = components +def _empty_charstring(font, glyphName, width=None): + c, fdSelectIndex = font.CharStrings.getItemAndSelector(glyphName) + if hasattr(font, 'FDArray') and font.FDArray is not None: + private = font.FDArray[fdSelectIndex].Private + else: + private = font.Private + dfltWdX = private.defaultWidthX + nmnlWdX = private.nominalWidthX + if width is None: + pen = NullPen() + c.draw(pen) # this will set the charstring's width + width = c.width + else: + c.decompile() + if width != dfltWdX: + c.program = [width - nmnlWdX, 'endchar'] + else: + c.program = ['endchar'] + @_add_method(ttLib.getTableClass('CFF ')) def prune_pre_subset(self, font, options): cff = self.cff @@ -75,19 +94,7 @@ def prune_pre_subset(self, font, options): if options.notdef_glyph and not options.notdef_outline: for fontname in cff.keys(): font = cff[fontname] - c, fdSelectIndex = font.CharStrings.getItemAndSelector('.notdef') - if hasattr(font, 'FDArray') and font.FDArray is not None: - private = font.FDArray[fdSelectIndex].Private - else: - private = font.Private - dfltWdX = private.defaultWidthX - nmnlWdX = private.nominalWidthX - pen = NullPen() - c.draw(pen) # this will set the charstring's width - if c.width != dfltWdX: - c.program = [c.width - nmnlWdX, 'endchar'] - else: - c.program = ['endchar'] + _empty_charstring(font, ".notdef") # Clear useless Encoding for fontname in cff.keys(): @@ -105,11 +112,8 @@ def subset_glyphs(self, s): cs = font.CharStrings if s.options.retain_gids: - for g in cs.keys(): - if g in s.glyphs_emptied: - c = cs[g] - c.decompile() - c.program = ["endchar"] + for g in s.glyphs_emptied: + _empty_charstring(font, g, width=0) else: # Load all glyphs for g in font.charset: diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py index cd8787b2c..521c93a77 100644 --- a/Tests/subset/subset_test.py +++ b/Tests/subset/subset_test.py @@ -540,7 +540,7 @@ class SubsetTest(unittest.TestCase): subsetfont["CFF "].cff[0].decompileAllCharStrings() cs = subsetfont["CFF "].cff[0].CharStrings self.assertGreater(len(cs["A"].program), 0) - self.assertEqual(cs["B"].program, ["endchar"]) + self.assertEqual(cs["B"].program, [-300, "endchar"]) if __name__ == "__main__": From 3e400c88280d87c16345e4f5d8320c70e5000c98 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 16 Jan 2019 13:31:35 +0000 Subject: [PATCH 3/6] don't add width and endchar for empty glyphs if it's CFF2 This also seems to fix https://github.com/fonttools/fonttools/issues/1448 --- Lib/fontTools/subset/cff.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/subset/cff.py b/Lib/fontTools/subset/cff.py index b56a4430b..59bd49780 100644 --- a/Lib/fontTools/subset/cff.py +++ b/Lib/fontTools/subset/cff.py @@ -66,8 +66,13 @@ def closure_glyphs(self, s): s.glyphs.update(components) decompose = components -def _empty_charstring(font, glyphName, width=None): +def _empty_charstring(font, glyphName, isCFF2, width=None): c, fdSelectIndex = font.CharStrings.getItemAndSelector(glyphName) + if isCFF2: + # CFF2 charstrings have no widths nor 'endchar' operators + c.decompile() + c.program = [] + return if hasattr(font, 'FDArray') and font.FDArray is not None: private = font.FDArray[fdSelectIndex].Private else: @@ -94,7 +99,7 @@ def prune_pre_subset(self, font, options): if options.notdef_glyph and not options.notdef_outline: for fontname in cff.keys(): font = cff[fontname] - _empty_charstring(font, ".notdef") + _empty_charstring(font, ".notdef", isCFF2=cff.major > 1) # Clear useless Encoding for fontname in cff.keys(): @@ -113,7 +118,7 @@ def subset_glyphs(self, s): if s.options.retain_gids: for g in s.glyphs_emptied: - _empty_charstring(font, g, width=0) + _empty_charstring(font, g, isCFF2=cff.major > 1, width=0) else: # Load all glyphs for g in font.charset: From 6b4474b2c452a767022b31235ab41d161fe173c3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 16 Jan 2019 16:01:12 +0000 Subject: [PATCH 4/6] [subset] actually, ignore the width of emptied charstrings basically, implies setting them to their defaultWidthX, which is the most efficient way to store these (unnecessary) piece of data. --- Lib/fontTools/subset/cff.py | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/Lib/fontTools/subset/cff.py b/Lib/fontTools/subset/cff.py index 59bd49780..2ac295113 100644 --- a/Lib/fontTools/subset/cff.py +++ b/Lib/fontTools/subset/cff.py @@ -66,29 +66,25 @@ def closure_glyphs(self, s): s.glyphs.update(components) decompose = components -def _empty_charstring(font, glyphName, isCFF2, width=None): +def _empty_charstring(font, glyphName, isCFF2, ignoreWidth=False): c, fdSelectIndex = font.CharStrings.getItemAndSelector(glyphName) - if isCFF2: + if isCFF2 or ignoreWidth: # CFF2 charstrings have no widths nor 'endchar' operators c.decompile() - c.program = [] - return - if hasattr(font, 'FDArray') and font.FDArray is not None: - private = font.FDArray[fdSelectIndex].Private + c.program = [] if isCFF2 else ['endchar'] else: - private = font.Private - dfltWdX = private.defaultWidthX - nmnlWdX = private.nominalWidthX - if width is None: + if hasattr(font, 'FDArray') and font.FDArray is not None: + private = font.FDArray[fdSelectIndex].Private + else: + private = font.Private + dfltWdX = private.defaultWidthX + nmnlWdX = private.nominalWidthX pen = NullPen() c.draw(pen) # this will set the charstring's width - width = c.width - else: - c.decompile() - if width != dfltWdX: - c.program = [width - nmnlWdX, 'endchar'] - else: - c.program = ['endchar'] + if c.width != dfltWdX: + c.program = [c.width - nmnlWdX, 'endchar'] + else: + c.program = ['endchar'] @_add_method(ttLib.getTableClass('CFF ')) def prune_pre_subset(self, font, options): @@ -118,7 +114,7 @@ def subset_glyphs(self, s): if s.options.retain_gids: for g in s.glyphs_emptied: - _empty_charstring(font, g, isCFF2=cff.major > 1, width=0) + _empty_charstring(font, g, isCFF2=cff.major > 1, ignoreWidth=True) else: # Load all glyphs for g in font.charset: From 1cde186172276320deeb1106949e18bfc5e8052a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 16 Jan 2019 16:02:07 +0000 Subject: [PATCH 5/6] subset_test: add test for --retain-gids and CFF2 table --- Tests/subset/subset_test.py | 40 ++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py index 521c93a77..72fee41b4 100644 --- a/Tests/subset/subset_test.py +++ b/Tests/subset/subset_test.py @@ -507,7 +507,7 @@ class SubsetTest(unittest.TestCase): self.assertGreater(glyf["A"].numberOfContours, 0) self.assertEqual(glyf["B"].numberOfContours, 0) - def test_retain_gids_otf(self): + def test_retain_gids_cff(self): _, fontpath = self.compile_font(self.getpath("TestOTF-Regular.ttx"), ".otf") font = TTFont(fontpath) @@ -519,7 +519,7 @@ class SubsetTest(unittest.TestCase): self.assertGreater(len(cs["A"].program), 0) self.assertGreater(len(cs["B"].program), 0) - subsetpath = self.temp_path(".ttf") + subsetpath = self.temp_path(".otf") subset.main( [ fontpath, @@ -540,7 +540,41 @@ class SubsetTest(unittest.TestCase): subsetfont["CFF "].cff[0].decompileAllCharStrings() cs = subsetfont["CFF "].cff[0].CharStrings self.assertGreater(len(cs["A"].program), 0) - self.assertEqual(cs["B"].program, [-300, "endchar"]) + self.assertEqual(cs["B"].program, ["endchar"]) + + def test_retain_gids_cff2(self): + fontpath = self.getpath("../../varLib/data/TestCFF2VF.otf") + font = TTFont(fontpath) + + self.assertEqual(font["hmtx"]["A"], (600, 31)) + self.assertEqual(font["hmtx"]["T"], (600, 41)) + + font["CFF2"].cff[0].decompileAllCharStrings() + cs = font["CFF2"].cff[0].CharStrings + self.assertGreater(len(cs["A"].program), 0) + self.assertGreater(len(cs["T"].program), 0) + + subsetpath = self.temp_path(".otf") + subset.main( + [ + fontpath, + "--retain-gids", + "--output-file=%s" % subsetpath, + "A", + ] + ) + subsetfont = TTFont(subsetpath) + + self.assertEqual(len(subsetfont.getGlyphOrder()), len(font.getGlyphOrder())) + + hmtx = subsetfont["hmtx"] + self.assertEqual(hmtx["A"], (600, 31)) + self.assertEqual(hmtx["glyph00002"], (0, 0)) + + subsetfont["CFF2"].cff[0].decompileAllCharStrings() + cs = subsetfont["CFF2"].cff[0].CharStrings + self.assertGreater(len(cs["A"].program), 0) + self.assertEqual(cs["glyph00002"].program, []) if __name__ == "__main__": From fe40af6d994bb65b498d0088974f6d054b6f8a4e Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 16 Jan 2019 16:10:13 +0000 Subject: [PATCH 6/6] minor --- Lib/fontTools/subset/cff.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/subset/cff.py b/Lib/fontTools/subset/cff.py index 2ac295113..96dc3210f 100644 --- a/Lib/fontTools/subset/cff.py +++ b/Lib/fontTools/subset/cff.py @@ -93,9 +93,10 @@ def prune_pre_subset(self, font, options): cff.fontNames = cff.fontNames[:1] if options.notdef_glyph and not options.notdef_outline: + isCFF2 = cff.major > 1 for fontname in cff.keys(): font = cff[fontname] - _empty_charstring(font, ".notdef", isCFF2=cff.major > 1) + _empty_charstring(font, ".notdef", isCFF2=isCFF2) # Clear useless Encoding for fontname in cff.keys(): @@ -113,8 +114,9 @@ def subset_glyphs(self, s): cs = font.CharStrings if s.options.retain_gids: + isCFF2 = cff.major > 1 for g in s.glyphs_emptied: - _empty_charstring(font, g, isCFF2=cff.major > 1, ignoreWidth=True) + _empty_charstring(font, g, isCFF2=isCFF2, ignoreWidth=True) else: # Load all glyphs for g in font.charset: