From e992953474d4db131061627168a4ba13acf69961 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 30 Oct 2020 19:27:54 +0000 Subject: [PATCH] swap xy and yx fields in COLRv1 Affine2x3 struct following the latest draft See discussion at https://github.com/googlefonts/colr-gradients-spec/pull/85 --- Lib/fontTools/colorLib/builder.py | 28 +++++++++++++++------------- Lib/fontTools/ttLib/tables/otData.py | 12 ++++++------ Tests/colorLib/builder_test.py | 14 +++++++------- Tests/ttLib/tables/C_O_L_R_test.py | 6 +++--- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 410f2b787..002d4e0bd 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/Lib/fontTools/colorLib/builder.py @@ -449,19 +449,21 @@ def buildPaintLinearGradient( return self -def buildAffine2x3( - xx: _ScalarInput, - xy: _ScalarInput, - yx: _ScalarInput, - yy: _ScalarInput, - dx: _ScalarInput, - dy: _ScalarInput, -) -> ot.Affine2x3: +def buildAffine2x3(transform: _AffineTuple) -> ot.Affine2x3: + if len(transform) != 6: + raise ValueError(f"Expected 6-tuple of floats, found: {transform!}") self = ot.Affine2x3() - locs = locals() - for attr in ("xx", "xy", "yx", "yy", "dx", "dy"): - value = locs[attr] - setattr(self, attr, _to_variable_f16dot16_float(value)) + # COLRv1 Affine2x3 uses the same column-major order to serialize a 2D + # Affine Transformation as the one used by fontTools.misc.transform. + # However, for historical reasons, the labels 'xy' and 'yx' are swapped. + # Their fundamental meaning is the same though. + # COLRv1 Affine2x3 follows the names found in FreeType and Cairo. + # In all case, the second element in the 6-tuple correspond to the + # y-part of the x basis vector, and the third to the x-part of the y + # basis vector. + # See https://github.com/googlefonts/colr-gradients-spec/pull/85 + for i, attr in enumerate(("xx", "yx", "xy", "yy", "dx", "dy")): + setattr(self, attr, _to_variable_f16dot16_float(transform[i])) return self @@ -517,7 +519,7 @@ def buildPaintTransform(transform: _AffineInput, paint: _PaintInput) -> ot.Paint self = ot.Paint() self.Format = int(ot.Paint.Format.PaintTransform) if not isinstance(transform, ot.Affine2x3): - transform = buildAffine2x3(*transform) + transform = buildAffine2x3(transform) self.Transform = transform self.Paint = buildPaint(paint) return self diff --git a/Lib/fontTools/ttLib/tables/otData.py b/Lib/fontTools/ttLib/tables/otData.py index 360bb58f3..6a1b7808e 100755 --- a/Lib/fontTools/ttLib/tables/otData.py +++ b/Lib/fontTools/ttLib/tables/otData.py @@ -1588,12 +1588,12 @@ otData = [ ]), ('Affine2x3', [ - ('VarFixed', 'xx', None, None, ''), - ('VarFixed', 'xy', None, None, ''), - ('VarFixed', 'yx', None, None, ''), - ('VarFixed', 'yy', None, None, ''), - ('VarFixed', 'dx', None, None, ''), - ('VarFixed', 'dy', None, None, ''), + ('VarFixed', 'xx', None, None, 'x-part of x basis vector'), + ('VarFixed', 'yx', None, None, 'y-part of x basis vector'), + ('VarFixed', 'xy', None, None, 'x-part of y basis vector'), + ('VarFixed', 'yy', None, None, 'y-part of y basis vector'), + ('VarFixed', 'dx', None, None, 'Translation in x direction'), + ('VarFixed', 'dy', None, None, 'Translation in y direction'), ]), ('ColorIndex', [ diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py index 35eb1c6a6..ac0193b49 100644 --- a/Tests/colorLib/builder_test.py +++ b/Tests/colorLib/builder_test.py @@ -285,10 +285,10 @@ def test_buildColorLine(): def test_buildAffine2x3(): - matrix = builder.buildAffine2x3(1.5, 0, 0.5, 2.0, 1.0, -3.0) + matrix = builder.buildAffine2x3((1.5, 0, 0.5, 2.0, 1.0, -3.0)) assert matrix.xx == builder.VariableFloat(1.5) - assert matrix.xy == builder.VariableFloat(0.0) - assert matrix.yx == builder.VariableFloat(0.5) + assert matrix.yx == builder.VariableFloat(0.0) + assert matrix.xy == builder.VariableFloat(0.5) assert matrix.yy == builder.VariableFloat(2.0) assert matrix.dx == builder.VariableFloat(1.0) assert matrix.dy == builder.VariableFloat(-3.0) @@ -458,7 +458,7 @@ def test_buildPaintColrSlice(): def test_buildPaintTransform(): paint = builder.buildPaintTransform( - transform=builder.buildAffine2x3(1, 2, 3, 4, 5, 6), + transform=builder.buildAffine2x3((1, 2, 3, 4, 5, 6)), paint=builder.buildPaintGlyph( glyph="a", paint=builder.buildPaintSolid(paletteIndex=0, alpha=1.0), @@ -467,8 +467,8 @@ def test_buildPaintTransform(): assert paint.Format == ot.Paint.Format.PaintTransform assert paint.Transform.xx.value == 1.0 - assert paint.Transform.xy.value == 2.0 - assert paint.Transform.yx.value == 3.0 + assert paint.Transform.yx.value == 2.0 + assert paint.Transform.xy.value == 3.0 assert paint.Transform.yy.value == 4.0 assert paint.Transform.dx.value == 5.0 assert paint.Transform.dy.value == 6.0 @@ -488,8 +488,8 @@ def test_buildPaintTransform(): assert paint.Format == ot.Paint.Format.PaintTransform assert paint.Transform.xx.value == 1.0 - assert paint.Transform.xy.value == 0.0 assert paint.Transform.yx.value == 0.0 + assert paint.Transform.xy.value == 0.0 assert paint.Transform.yy.value == 0.3333 assert paint.Transform.dx.value == 10 assert paint.Transform.dy.value == 10 diff --git a/Tests/ttLib/tables/C_O_L_R_test.py b/Tests/ttLib/tables/C_O_L_R_test.py index adaa9e383..7d0ec7bed 100644 --- a/Tests/ttLib/tables/C_O_L_R_test.py +++ b/Tests/ttLib/tables/C_O_L_R_test.py @@ -308,8 +308,8 @@ COLR_V1_XML = [ " ", " ", ' ', - ' ', - ' ', + ' ', + ' ', ' ', ' ', ' ', @@ -338,8 +338,8 @@ COLR_V1_XML = [ " ", " ", ' ', - ' ', ' ', + ' ', ' ', ' ', ' ',