diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 85778ac01..cade429e6 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/Lib/fontTools/colorLib/builder.py @@ -53,16 +53,17 @@ _ColorGlyphsV0Dict = Dict[str, Sequence[Tuple[str, int]]] MAX_PAINT_COLR_LAYER_COUNT = 255 _DEFAULT_ALPHA = VariableFloat(1.0) +_MAX_REUSE_LEN = 32 def _beforeBuildPaintRadialGradient(paint, source): # normalize input types (which may or may not specify a varIdx) x0 = convertTupleClass(VariableFloat, source["x0"]) - y0 = convertTupleClass(VariableFloat, source.get("y0", 0.0)) - r0 = convertTupleClass(VariableFloat, source.get("r0", 0.0)) - x1 = convertTupleClass(VariableFloat, source.get("x1", 0.0)) - y1 = convertTupleClass(VariableFloat, source.get("y1", 0.0)) - r1 = convertTupleClass(VariableFloat, source.get("r1", 0.0)) + y0 = convertTupleClass(VariableFloat, source["y0"]) + r0 = convertTupleClass(VariableFloat, source["r0"]) + x1 = convertTupleClass(VariableFloat, source["x1"]) + y1 = convertTupleClass(VariableFloat, source["y1"]) + r1 = convertTupleClass(VariableFloat, source["r1"]) # TODO apparently no builder_test confirms this works (?) @@ -373,11 +374,12 @@ def _split_color_glyphs_by_version( def _reuse_ranges(num_layers: int) -> Generator[Tuple[int, int], None, None]: # TODO feels like something itertools might have already for lbound in range(num_layers): - # TODO may want a max length to limit scope of search # Reuse of very large #s of layers is relatively unlikely # +2: we want sequences of at least 2 # otData handles single-record duplication - for ubound in range(lbound + 2, num_layers + 1): + for ubound in range( + lbound + 2, min(num_layers + 1, lbound + 2 + _MAX_REUSE_LEN) + ): yield (lbound, ubound) @@ -448,6 +450,10 @@ class LayerV1ListBuilder: # Convert maps seqs or whatever into typed objects layers = [self.buildPaint(l) for l in layers] + # No reason to have a colr layers with just one entry + if len(layers) == 1: + return layers[0], {} + # Look for reuse, with preference to longer sequences # This may make the layer list smaller found_reuse = True @@ -474,6 +480,7 @@ class LayerV1ListBuilder: break # The layer list is now final; if it's too big we need to tree it + is_tree = len(layers) > MAX_PAINT_COLR_LAYER_COUNT layers = _build_n_ary_tree(layers, n=MAX_PAINT_COLR_LAYER_COUNT) # We now have a tree of sequences with Paint leaves. @@ -494,12 +501,13 @@ class LayerV1ListBuilder: paint.FirstLayerIndex = len(self.layers) self.layers.extend(layers) - # Register our parts for reuse - # TODO what if we made ourselves a lovely little tree - for lbound, ubound in _reuse_ranges(len(layers)): - self.reusePool[self._as_tuple(layers[lbound:ubound])] = ( - lbound + paint.FirstLayerIndex - ) + # Register our parts for reuse provided we aren't a tree + # If we are a tree the leaves registered for reuse and that will suffice + if not is_tree: + for lbound, ubound in _reuse_ranges(len(layers)): + self.reusePool[self._as_tuple(layers[lbound:ubound])] = ( + lbound + paint.FirstLayerIndex + ) # we've fully built dest; empty source prevents generalized build from kicking in return paint, {} diff --git a/Lib/fontTools/colorLib/table_builder.py b/Lib/fontTools/colorLib/table_builder.py index 322673466..2ce893a94 100644 --- a/Lib/fontTools/colorLib/table_builder.py +++ b/Lib/fontTools/colorLib/table_builder.py @@ -12,7 +12,6 @@ from fontTools.ttLib.tables.otBase import ( ) from fontTools.ttLib.tables.otConverters import ( ComputedInt, - GlyphID, SimpleValue, Struct, Short, @@ -23,18 +22,6 @@ from fontTools.ttLib.tables.otConverters import ( ) -def _to_glyph_id(value): - assert isinstance(value, str), "Expected a glyph name" - return value - - -_CONVERTER_OVERRIDES = { - Short: int, - UShort: int, - GlyphID: _to_glyph_id, -} - - class BuildCallback(enum.Enum): """Keyed on (BEFORE_BUILD, class[, Format if available]). Receives (dest, source). @@ -107,10 +94,10 @@ class TableBuilder: self._callbackTable = callbackTable def _convert(self, dest, field, converter, value): - converter = _CONVERTER_OVERRIDES.get(type(converter), converter) - - enumClass = getattr(converter, "enumClass", None) tupleClass = getattr(converter, "tupleClass", None) + enumClass = getattr(converter, "enumClass", None) + simpleValueClass = getattr(converter, "valueClass", None) + if tupleClass: value = convertTupleClass(tupleClass, value) @@ -125,6 +112,9 @@ class TableBuilder: else: value = enumClass(value) + elif simpleValueClass: + value = simpleValueClass(value) + elif isinstance(converter, Struct): if converter.repeat: if _isNonStrSequence(value): diff --git a/Lib/fontTools/ttLib/tables/otConverters.py b/Lib/fontTools/ttLib/tables/otConverters.py index 1b278410b..6bf6c623d 100644 --- a/Lib/fontTools/ttLib/tables/otConverters.py +++ b/Lib/fontTools/ttLib/tables/otConverters.py @@ -216,6 +216,8 @@ class SimpleValue(BaseConverter): return self.fromString(attrs["value"]) class IntValue(SimpleValue): + valueClass = int + @staticmethod def fromString(value): return int(value, 0) @@ -295,6 +297,7 @@ class Tag(SimpleValue): writer.writeTag(value) class GlyphID(SimpleValue): + valueClass = str staticSize = 2 typecode = "H" def readArray(self, reader, font, tableDict, count): @@ -334,6 +337,7 @@ class NameID(UShort): class FloatValue(SimpleValue): + valueClass = float @staticmethod def fromString(value): return float(value) diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py index 95962f637..f9ffdd242 100644 --- a/Tests/colorLib/builder_test.py +++ b/Tests/colorLib/builder_test.py @@ -269,7 +269,9 @@ def test_buildPaintSolid_Alpha(): def test_buildPaintSolid_Variable(): - p = _buildPaint((ot.PaintFormat.PaintSolid, (3, builder.VariableFloat(0.5, varIdx=2)))) + p = _buildPaint( + (ot.PaintFormat.PaintSolid, (3, builder.VariableFloat(0.5, varIdx=2))) + ) assert p.Format == ot.PaintFormat.PaintSolid assert p.Color.PaletteIndex == 3 assert p.Color.Alpha.value == 0.5 @@ -1374,6 +1376,29 @@ class BuildCOLRTest(object): assert isinstance(colr.table, ot.COLR) assert colr.table.VarStore is None + def test_paint_one_colr_layers(self): + # A set of one layers should flip to just that layer + colr = builder.buildCOLR( + { + "a": ( + ot.PaintFormat.PaintColrLayers, + [ + ( + ot.PaintFormat.PaintGlyph, + (ot.PaintFormat.PaintSolid, 0), + "b", + ), + ], + ) + }, + ) + + assert len(colr.table.LayerV1List.Paint) == 0, "PaintColrLayers should be gone" + assert colr.table.BaseGlyphV1List.BaseGlyphCount == 1 + paint = colr.table.BaseGlyphV1List.BaseGlyphV1Record[0].Paint + assert paint.Format == ot.PaintFormat.PaintGlyph + assert paint.Paint.Format == ot.PaintFormat.PaintSolid + class TrickyRadialGradientTest: @staticmethod