From 7e6d31569fd6a77db8add6ab90bb3007c621b8b7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 17:32:31 -0700 Subject: [PATCH 1/8] [cffLib.specializer] Adjust stack use calculation See comment. --- Lib/fontTools/cffLib/specializer.py | 12 +++++++++++- Tests/cffLib/specializer_test.py | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/cffLib/specializer.py b/Lib/fontTools/cffLib/specializer.py index e61ad26f8..11e9e578e 100644 --- a/Lib/fontTools/cffLib/specializer.py +++ b/Lib/fontTools/cffLib/specializer.py @@ -764,7 +764,17 @@ def specializeCommands( # Make sure the stack depth does not exceed (maxstack - 1), so # that subroutinizer can insert subroutine calls at any point. - if new_op and _argsStackUse(args1) + _argsStackUse(args2) < maxstack: + # + # The assumption is that args1, and args2, each individually + # can be successfully processed without a stack overflow. + # When combined, the stack depth to consider would be the + # number of items in args1 plus what it takes to build args2 + # on top of args1, which will be already on the stack as + # len(args1) items. + # + # It's unfortunate that _argsStackUse() is O(n) in the number + # of args, but it's not a big deal hopefully. + if new_op and len(args1) + _argsStackUse(args2) < maxstack: commands[i - 1] = (new_op, args1 + args2) del commands[i] diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index 08c137ada..aad3bfdfb 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -599,6 +599,20 @@ class CFFSpecializeProgramTest: stack_use = charstr_stack_use(specialized, getNumRegions=getNumRegions) assert maxStack - numRegions < stack_use < maxStack + def test_maxstack_blends2(self): + # See if two long blend sequences are merged into one + numRegions = 400 + numOps = 2 + getNumRegions = lambda iv: numRegions + blend_one = " ".join([str(i) for i in range(1 + numRegions)] + ["1", "blend"]) + operands = " ".join([blend_one] * 6) + operator = "rrcurveto" + charstr = " ".join([operands, operator] * numOps) + specialized = charstr_specialize( + charstr, getNumRegions=getNumRegions, maxstack=maxStack + ) + assert specialized.index("rrcurveto") == len(specialized) - len("rrcurveto") + class CFF2VFTestSpecialize(DataFilesHandler): def test_blend_round_trip(self): From cfba1f995faa4f8acff2423f519e0ddf8594b90e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 18:43:03 -0700 Subject: [PATCH 2/8] [cffLib.specializer] Make command-merging linear again The consideration for blends had made it into O(n^2). Make it linear again. Speeds up Tests/cffLib/specializer_test.py::CFFSpecializeProgramTest::test_maxstack_blends 3x for me. --- Lib/fontTools/cffLib/specializer.py | 18 +++++++----------- Tests/cffLib/specializer_test.py | 4 ++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/Lib/fontTools/cffLib/specializer.py b/Lib/fontTools/cffLib/specializer.py index 11e9e578e..ce7bbcebc 100644 --- a/Lib/fontTools/cffLib/specializer.py +++ b/Lib/fontTools/cffLib/specializer.py @@ -715,6 +715,7 @@ def specializeCommands( continue # 5. Combine adjacent operators when possible, minding not to go over max stack size. + stackUse = _argsStackUse(commands[-1][1]) if commands else 0 for i in range(len(commands) - 1, 0, -1): op1, args1 = commands[i - 1] op2, args2 = commands[i] @@ -764,19 +765,14 @@ def specializeCommands( # Make sure the stack depth does not exceed (maxstack - 1), so # that subroutinizer can insert subroutine calls at any point. - # - # The assumption is that args1, and args2, each individually - # can be successfully processed without a stack overflow. - # When combined, the stack depth to consider would be the - # number of items in args1 plus what it takes to build args2 - # on top of args1, which will be already on the stack as - # len(args1) items. - # - # It's unfortunate that _argsStackUse() is O(n) in the number - # of args, but it's not a big deal hopefully. - if new_op and len(args1) + _argsStackUse(args2) < maxstack: + args1StackUse = _argsStackUse(args1) + combinedStackUse = max(args1StackUse, len(args1) + stackUse) + if new_op and combinedStackUse < maxstack: commands[i - 1] = (new_op, args1 + args2) del commands[i] + stackUse = combinedStackUse + else: + stackUse = args1StackUse # 6. Resolve any remaining made-up operators into real operators. for i in range(len(commands)): diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index aad3bfdfb..ffc85b51c 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -599,8 +599,8 @@ class CFFSpecializeProgramTest: stack_use = charstr_stack_use(specialized, getNumRegions=getNumRegions) assert maxStack - numRegions < stack_use < maxStack - def test_maxstack_blends2(self): - # See if two long blend sequences are merged into one + def test_maxstack_commands(self): + # See if two commands with deep blends are merged into one numRegions = 400 numOps = 2 getNumRegions = lambda iv: numRegions From b54936400e5b0f37d5ff830da50722901dff9399 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 19:23:00 -0700 Subject: [PATCH 3/8] [cffLib.specializer_test] Do less work No need to generalizeFirst these. The tests are in the general form. --- Tests/cffLib/specializer_test.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index ffc85b51c..0ee51fd1e 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -594,7 +594,10 @@ class CFFSpecializeProgramTest: charstr = " ".join([operands, operator] * numOps) expected = charstr specialized = charstr_specialize( - charstr, getNumRegions=getNumRegions, maxstack=maxStack + charstr, + getNumRegions=getNumRegions, + maxstack=maxStack, + generalizeFirst=False, ) stack_use = charstr_stack_use(specialized, getNumRegions=getNumRegions) assert maxStack - numRegions < stack_use < maxStack @@ -609,7 +612,10 @@ class CFFSpecializeProgramTest: operator = "rrcurveto" charstr = " ".join([operands, operator] * numOps) specialized = charstr_specialize( - charstr, getNumRegions=getNumRegions, maxstack=maxStack + charstr, + getNumRegions=getNumRegions, + maxstack=maxStack, + generalizeFirst=False, ) assert specialized.index("rrcurveto") == len(specialized) - len("rrcurveto") From 7457f8ac52f03555273ec9bfee13e3bb5f5d55af Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 19:29:13 -0700 Subject: [PATCH 4/8] [specializer_test] Speed up test No need to test 2000 ops. Just 600, anything higher than the max stack depth (514) is as good... --- Tests/cffLib/specializer_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index 0ee51fd1e..e945709ca 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -586,7 +586,7 @@ class CFFSpecializeProgramTest: # maxstack CFF2=513, specializer uses up to 512 def test_maxstack_blends(self): numRegions = 15 - numOps = 2000 + numOps = 600 getNumRegions = lambda iv: numRegions blend_one = " ".join([str(i) for i in range(1 + numRegions)] + ["1", "blend"]) operands = " ".join([blend_one] * 6) From 6f37252e857c854f99f75550eeb1b33f14b4728d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 19:38:11 -0700 Subject: [PATCH 5/8] [specializer_test] Remove unused variable --- Tests/cffLib/specializer_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index e945709ca..0e522064a 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -592,7 +592,6 @@ class CFFSpecializeProgramTest: operands = " ".join([blend_one] * 6) operator = "rrcurveto" charstr = " ".join([operands, operator] * numOps) - expected = charstr specialized = charstr_specialize( charstr, getNumRegions=getNumRegions, From 4e2968462a10572d59a46c51834c74f0d9425484 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 19:44:50 -0700 Subject: [PATCH 6/8] [specializer_test] Simplify tests No need to go to string and back... --- Tests/cffLib/specializer_test.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Tests/cffLib/specializer_test.py b/Tests/cffLib/specializer_test.py index 0e522064a..ebe702fee 100644 --- a/Tests/cffLib/specializer_test.py +++ b/Tests/cffLib/specializer_test.py @@ -23,9 +23,7 @@ def charstr_specialize(charstr, **kwargs): return programToString(specializeProgram(stringToProgram(charstr), **kwargs)) -def charstr_stack_use(charstr, getNumRegions=None): - program = stringToProgram(charstr) - +def program_stack_use(program, getNumRegions=None): vsindex = None maxStack = 0 stack = [] @@ -588,17 +586,17 @@ class CFFSpecializeProgramTest: numRegions = 15 numOps = 600 getNumRegions = lambda iv: numRegions - blend_one = " ".join([str(i) for i in range(1 + numRegions)] + ["1", "blend"]) - operands = " ".join([blend_one] * 6) + blend_one = [i for i in range(1 + numRegions)] + [1, "blend"] + operands = blend_one * 6 operator = "rrcurveto" - charstr = " ".join([operands, operator] * numOps) - specialized = charstr_specialize( - charstr, + program = (operands + [operator]) * numOps + specialized = specializeProgram( + program, getNumRegions=getNumRegions, maxstack=maxStack, generalizeFirst=False, ) - stack_use = charstr_stack_use(specialized, getNumRegions=getNumRegions) + stack_use = program_stack_use(specialized, getNumRegions=getNumRegions) assert maxStack - numRegions < stack_use < maxStack def test_maxstack_commands(self): @@ -606,17 +604,17 @@ class CFFSpecializeProgramTest: numRegions = 400 numOps = 2 getNumRegions = lambda iv: numRegions - blend_one = " ".join([str(i) for i in range(1 + numRegions)] + ["1", "blend"]) - operands = " ".join([blend_one] * 6) + blend_one = [i for i in range(1 + numRegions)] + [1, "blend"] + operands = blend_one * 6 operator = "rrcurveto" - charstr = " ".join([operands, operator] * numOps) - specialized = charstr_specialize( - charstr, + program = (operands + [operator]) * numOps + specialized = specializeProgram( + program, getNumRegions=getNumRegions, maxstack=maxStack, generalizeFirst=False, ) - assert specialized.index("rrcurveto") == len(specialized) - len("rrcurveto") + assert specialized.index("rrcurveto") == len(specialized) - 1 class CFF2VFTestSpecialize(DataFilesHandler): From 751d1383afbe2f4836d0cfc79a1dc115d7a9a53e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 20:09:51 -0700 Subject: [PATCH 7/8] [specializer] Reuse list len()'s --- Lib/fontTools/cffLib/specializer.py | 58 +++++++++++++++++------------ 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/Lib/fontTools/cffLib/specializer.py b/Lib/fontTools/cffLib/specializer.py index ce7bbcebc..63c563dcb 100644 --- a/Lib/fontTools/cffLib/specializer.py +++ b/Lib/fontTools/cffLib/specializer.py @@ -80,8 +80,9 @@ def programToCommands(program, getNumRegions=None): numBlendArgs = numBlends * numSourceFonts + 1 # replace first blend op by a list of the blend ops. stack[-numBlendArgs:] = [stack[-numBlendArgs:]] - lenBlendStack += numBlends + len(stack) - 1 - lastBlendIndex = len(stack) + lenStack = len(stack) + lenBlendStack += numBlends + lenStack - 1 + lastBlendIndex = lenStack # if a blend op exists, this is or will be a CFF2 charstring. continue @@ -153,9 +154,10 @@ def commandsToProgram(commands): def _everyN(el, n): """Group the list el into groups of size n""" - if len(el) % n != 0: + l = len(el) + if l % n != 0: raise ValueError(el) - for i in range(0, len(el), n): + for i in range(0, l, n): yield el[i : i + n] @@ -218,9 +220,10 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def hhcurveto(args): - if len(args) < 4 or len(args) % 4 > 1: + l = len(args) + if l < 4 or l % 4 > 1: raise ValueError(args) - if len(args) % 2 == 1: + if l % 2 == 1: yield ("rrcurveto", [args[1], args[0], args[2], args[3], args[4], 0]) args = args[5:] for args in _everyN(args, 4): @@ -228,9 +231,10 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def vvcurveto(args): - if len(args) < 4 or len(args) % 4 > 1: + l = len(args) + if l < 4 or l % 4 > 1: raise ValueError(args) - if len(args) % 2 == 1: + if l % 2 == 1: yield ("rrcurveto", [args[0], args[1], args[2], args[3], 0, args[4]]) args = args[5:] for args in _everyN(args, 4): @@ -238,11 +242,12 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def hvcurveto(args): - if len(args) < 4 or len(args) % 8 not in {0, 1, 4, 5}: + l = len(args) + if l < 4 or l % 8 not in {0, 1, 4, 5}: raise ValueError(args) last_args = None - if len(args) % 2 == 1: - lastStraight = len(args) % 8 == 5 + if l % 2 == 1: + lastStraight = l % 8 == 5 args, last_args = args[:-5], args[-5:] it = _everyN(args, 4) try: @@ -262,11 +267,12 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def vhcurveto(args): - if len(args) < 4 or len(args) % 8 not in {0, 1, 4, 5}: + l = len(args) + if l < 4 or l % 8 not in {0, 1, 4, 5}: raise ValueError(args) last_args = None - if len(args) % 2 == 1: - lastStraight = len(args) % 8 == 5 + if l % 2 == 1: + lastStraight = l % 8 == 5 args, last_args = args[:-5], args[-5:] it = _everyN(args, 4) try: @@ -286,7 +292,8 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def rcurveline(args): - if len(args) < 8 or len(args) % 6 != 2: + l = len(args) + if l < 8 or l % 6 != 2: raise ValueError(args) args, last_args = args[:-2], args[-2:] for args in _everyN(args, 6): @@ -295,7 +302,8 @@ class _GeneralizerDecombinerCommandsMap(object): @staticmethod def rlinecurve(args): - if len(args) < 8 or len(args) % 2 != 0: + l = len(args) + if l < 8 or l % 2 != 0: raise ValueError(args) args, last_args = args[:-6], args[-6:] for args in _everyN(args, 2): @@ -330,8 +338,9 @@ def _convertBlendOpToArgs(blendList): # comprehension. See calling context args = args[:-1] - numRegions = len(args) // numBlends - 1 - if not (numBlends * (numRegions + 1) == len(args)): + l = len(args) + numRegions = l // numBlends - 1 + if not (numBlends * (numRegions + 1) == l): raise ValueError(blendList) defaultArgs = [[arg] for arg in args[:numBlends]] @@ -726,9 +735,10 @@ def specializeCommands( if op1 == op2: new_op = op1 else: - if op2 == "rrcurveto" and len(args2) == 6: + l = len(args2) + if op2 == "rrcurveto" and l == 6: new_op = "rlinecurve" - elif len(args2) == 2: + elif l == 2: new_op = "rcurveline" elif (op1, op2) in {("rlineto", "rlinecurve"), ("rrcurveto", "rcurveline")}: @@ -783,9 +793,11 @@ def specializeCommands( continue if op[2:] == "curveto" and op[:2] not in {"rr", "hh", "vv", "vh", "hv"}: + l = len(args) + op0, op1 = op[:2] if (op0 == "r") ^ (op1 == "r"): - assert len(args) % 2 == 1 + assert l % 2 == 1 if op0 == "0": op0 = "h" if op1 == "0": @@ -796,9 +808,9 @@ def specializeCommands( op1 = _negateCategory(op0) assert {op0, op1} <= {"h", "v"}, (op0, op1) - if len(args) % 2: + if l % 2: if op0 != op1: # vhcurveto / hvcurveto - if (op0 == "h") ^ (len(args) % 8 == 1): + if (op0 == "h") ^ (l % 8 == 1): # Swap last two args order args = args[:-2] + args[-1:] + args[-2:-1] else: # hhcurveto / vvcurveto From a8462a65c5b857056a7f1218d38eca481f61e6e9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 12 Nov 2024 20:17:50 -0700 Subject: [PATCH 8/8] [specializer] Use "is None" --- Lib/fontTools/cffLib/specializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/cffLib/specializer.py b/Lib/fontTools/cffLib/specializer.py index 63c563dcb..5fddcb67d 100644 --- a/Lib/fontTools/cffLib/specializer.py +++ b/Lib/fontTools/cffLib/specializer.py @@ -377,7 +377,7 @@ def generalizeCommands(commands, ignoreErrors=False): raise func = getattr(mapping, op, None) - if not func: + if func is None: result.append((op, args)) continue try: