From c562d0c86c1fff10b752fc15c022f98507389e0a Mon Sep 17 00:00:00 2001 From: Jany Belluz Date: Tue, 23 Apr 2024 09:09:54 +0100 Subject: [PATCH 1/3] Add test --- tests/data/IntermediateLayerWidth.glyphs | 241 +++++++++++++++++++++++ tests/special_layer_width_test.py | 45 ++++- 2 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 tests/data/IntermediateLayerWidth.glyphs diff --git a/tests/data/IntermediateLayerWidth.glyphs b/tests/data/IntermediateLayerWidth.glyphs new file mode 100644 index 000000000..7a5b51580 --- /dev/null +++ b/tests/data/IntermediateLayerWidth.glyphs @@ -0,0 +1,241 @@ +{ +.appVersion = "3259"; +.formatVersion = 3; +DisplayStrings = ( +a +); +axes = ( +{ +name = Weight; +tag = wght; +}, +{ +name = ROUND; +tag = ROND; +} +); +date = "2024-04-22 16:44:09 +0000"; +familyName = "New Font"; +fontMaster = ( +{ +axesValues = ( +400, +0 +); +id = "m-400-0"; +metricValues = ( +{ +over = 16; +pos = 800; +}, +{ +over = -16; +}, +{ +over = -16; +pos = -200; +}, +{ +over = 16; +pos = 700; +}, +{ +over = 16; +pos = 500; +}, +{ +} +); +name = Regular; +}, +{ +axesValues = ( +700, +0 +); +iconName = SemiBold; +id = "m-700-0"; +metricValues = ( +{ +over = 16; +pos = 800; +}, +{ +over = -16; +}, +{ +over = -16; +pos = -200; +}, +{ +over = 16; +pos = 700; +}, +{ +over = 16; +pos = 500; +}, +{ +} +); +name = Bold; +}, +{ +axesValues = ( +400, +1 +); +customParameters = ( +{ +name = "Link Metrics With Master"; +value = "m-400-0"; +} +); +id = "m-400-1"; +metricValues = ( +{ +over = 16; +pos = 800; +}, +{ +over = -16; +}, +{ +over = -16; +pos = -200; +}, +{ +over = 16; +pos = 700; +}, +{ +over = 16; +pos = 500; +}, +{ +} +); +name = "Regular ROUND"; +}, +{ +axesValues = ( +700, +1 +); +customParameters = ( +{ +name = "Link Metrics With Master"; +value = "m-700-0"; +} +); +iconName = SemiBold; +id = "m-700-1"; +metricValues = ( +{ +over = 16; +pos = 800; +}, +{ +over = -16; +}, +{ +over = -16; +pos = -200; +}, +{ +over = 16; +pos = 700; +}, +{ +over = 16; +pos = 500; +}, +{ +} +); +name = "Bold ROUND"; +} +); +glyphs = ( +{ +glyphname = a; +lastChange = "2024-04-23 07:38:25 +0000"; +layers = ( +{ +layerId = "m-400-1"; +width = 100; +}, +{ +layerId = "m-400-0"; +width = 100; +}, +{ +layerId = "m-700-1"; +width = 300; +}, +{ +associatedMasterId = "m-400-0"; +attr = { +coordinates = ( +500, +0 +); +}; +layerId = "m-500-0"; +name = "Intermediate 500 0"; +width = 200; +}, +{ +layerId = "m-700-0"; +width = 300; +}, +{ +associatedMasterId = "m-400-1"; +attr = { +coordinates = ( +500, +1 +); +}; +layerId = "m-500-1"; +name = "Intermediate 500 1"; +width = 200; +} +); +unicode = 97; +}, +{ +glyphname = space; +layers = ( +{ +layerId = "m-400-1"; +width = 200; +} +); +unicode = 32; +} +); +metrics = ( +{ +type = ascender; +}, +{ +type = baseline; +}, +{ +type = descender; +}, +{ +type = "cap height"; +}, +{ +type = "x-height"; +}, +{ +type = "italic angle"; +} +); +unitsPerEm = 1000; +versionMajor = 1; +versionMinor = 0; +} diff --git a/tests/special_layer_width_test.py b/tests/special_layer_width_test.py index 2f36af461..18e3cc4d3 100644 --- a/tests/special_layer_width_test.py +++ b/tests/special_layer_width_test.py @@ -16,7 +16,7 @@ import os -from glyphsLib import load_to_ufos +from glyphsLib import load_to_ufos, GSFont, to_designspace def glyphs_file_path(): @@ -42,3 +42,46 @@ def test_substitution_layer_width(): assert masters[1]["B"].width == 600 assert masters[0]["B.BRACKET.varAlt01"].width == 510 assert masters[1]["B.BRACKET.varAlt01"].width == 610 + + +def test_intermediate_layer_width_with_metrics_source_on_parent(): + """This checks that "intermediate layers", a.k.a. "brace layers", do not + incorrectly inherit an irrelevant width from their parent layer. + + Scenario in the test file: + + - Glyph /a + - Regular (400, 0): advance width = 100 + - Intermediate layer {500, 0}: advance width = 200 + - Bold (700, 0): advance width = 300 + - Regular ROUND (400, 1); advance width = 100 + * This master layer has a "metricsSource" pointing to Regular (400, 0) + to ensure the widths are consistent. + - Intermediate layer {500, 1}: advance width = 200 + * This intermediate layer is under consideration for the test. + - Bold ROUND (700, 1): advance width = 300 + + Previously, the advance width of the intermediate layer `{500, 1}` would be + forcibly taken from the `metricsSource` of the master layer `Regular ROUND + (400, 1)` to which the intermediate layer `{500, 1}` was attached, and so it + would get 100 instead of 200. + + With this patch, the advance width of the layer `{500, 1}` will not be + changed, because the the layer `{500, 1}` does not define itself a + `metricsSource`. + + See https://github.com/googlefonts/glyphsLib/pull/985 + """ + test_path = os.path.join( + os.path.dirname(__file__), "data", "IntermediateLayerWidth.glyphs" + ) + font = GSFont(test_path) + doc = to_designspace(font) + for intermediate_round in doc.sources: + if intermediate_round.getFullDesignLocation(doc) == {"Weight": 500, "ROUND": 1}: + break + else: + assert False, "Can't find intermediate layer in the desigspace" + assert ( + intermediate_round.font.layers[intermediate_round.layerName]["a"].width == 200 + ) From 0e588735e119071771306db3eed04fde63d877c8 Mon Sep 17 00:00:00 2001 From: schriftgestalt Date: Sat, 10 Feb 2024 16:33:41 +0100 Subject: [PATCH 2/3] No width linking of brace layers it was getting it form the master they are attached too and that is supposed to have a different width --- Lib/glyphsLib/builder/glyph.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/Lib/glyphsLib/builder/glyph.py b/Lib/glyphsLib/builder/glyph.py index 24748a797..ed5ae68f3 100644 --- a/Lib/glyphsLib/builder/glyph.py +++ b/Lib/glyphsLib/builder/glyph.py @@ -250,22 +250,20 @@ def effective_width(layer, glyph): # The width may be taken from another master via the customParameters # 'Link Metrics With Master' or 'Link Metrics With First Master'. font = glyph.parent - master = font.masters[layer.associatedMasterId or layer.layerId] - metrics_source = master.metricsSource - if metrics_source is None: - width = layer.width - else: - metric_layer = font.glyphs[glyph.name].layers[metrics_source.id] - if metric_layer: - width = metric_layer.width - if layer.width != width: - logger.debug( - f"{layer.parent.name}: Applying width from master " - f"'{metrics_source.id}': {layer.width} -> {width}" - ) - else: - width = None - return width + master = font.masters[layer.layerId] + if master: + metrics_source = master.metricsSource + if metrics_source: + metric_layer = font.glyphs[glyph.name].layers[metrics_source.id] + if metric_layer: + width = metric_layer.width + if layer.width != width: + logger.debug( + f"{layer.parent.name}: Applying width from master " + f"'{metrics_source.id}': {layer.width} -> {width}" + ) + return width + return layer.width def to_ufo_glyph_color(self, ufo_glyph, layer, glyph, do_color_layers=True): From db710a4f3ad9bb47321694b87cc5bb51481a43e4 Mon Sep 17 00:00:00 2001 From: Jany Belluz Date: Tue, 23 Apr 2024 09:32:50 +0100 Subject: [PATCH 3/3] Fix lint --- tests/special_layer_width_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/special_layer_width_test.py b/tests/special_layer_width_test.py index 18e3cc4d3..1bf96ed66 100644 --- a/tests/special_layer_width_test.py +++ b/tests/special_layer_width_test.py @@ -81,7 +81,7 @@ def test_intermediate_layer_width_with_metrics_source_on_parent(): if intermediate_round.getFullDesignLocation(doc) == {"Weight": 500, "ROUND": 1}: break else: - assert False, "Can't find intermediate layer in the desigspace" + raise AssertionError("Can't find intermediate layer in the desigspace") assert ( intermediate_round.font.layers[intermediate_round.layerName]["a"].width == 200 )