From 2f8800328a7ab40a1a532a9075a76611c2d271ab Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 10 Jan 2024 13:59:11 +0000 Subject: [PATCH 01/12] Add a helper property --- Lib/glyphsLib/classes.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/glyphsLib/classes.py b/Lib/glyphsLib/classes.py index b0e736683..6528fd33f 100755 --- a/Lib/glyphsLib/classes.py +++ b/Lib/glyphsLib/classes.py @@ -3699,7 +3699,7 @@ def __repr__(self): return f'<{self.__class__.__name__} "{name}" ({parent})>' def __lt__(self, other): - if self.master and other.master and self.associatedMasterId == self.layerId: + if self.master and other.master and self._is_master_layer: return ( self.master.weightValue < other.master.weightValue or self.master.widthValue < other.master.widthValue @@ -3734,13 +3734,13 @@ def master(self): master = self.parent.parent.masterForId(self.associatedMasterId) return master + @property + def _is_master_layer(self): + return self.associatedMasterId == self.layerId + @property def name(self): - if ( - self.associatedMasterId - and self.associatedMasterId == self.layerId - and self.parent - ): + if self.associatedMasterId and self._is_master_layer and self.parent: master = self.parent.parent.masterForId(self.associatedMasterId) if master: return master.name From 59211297acf3ff5bcc82c2141dc1b2ab291eeb76 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 10 Jan 2024 13:59:38 +0000 Subject: [PATCH 02/12] Add the "intermediate layers" transformation --- .../builder/transformations/__init__.py | 4 + .../transformations/intermediate_layers.py | 135 ++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 Lib/glyphsLib/builder/transformations/__init__.py create mode 100644 Lib/glyphsLib/builder/transformations/intermediate_layers.py diff --git a/Lib/glyphsLib/builder/transformations/__init__.py b/Lib/glyphsLib/builder/transformations/__init__.py new file mode 100644 index 000000000..37e59870a --- /dev/null +++ b/Lib/glyphsLib/builder/transformations/__init__.py @@ -0,0 +1,4 @@ +from .intermediate_layers import resolve_intermediate_components + + +TRANSFORMATIONS = [resolve_intermediate_components] diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py new file mode 100644 index 000000000..3e47aa7ff --- /dev/null +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -0,0 +1,135 @@ +import logging +import uuid + +from fontTools.varLib.models import VariationModel, normalizeValue + +from glyphsLib.classes import GSLayer, GSNode, GSPath + + +logger = logging.getLogger(__name__) + + +def resolve_intermediate_components(font): + components_with_intermediate_layers = set() + all_intermediate_locations = set() + for glyph in font.glyphs: + for layer in glyph.layers: + if "coordinates" in layer.attributes: + # First, let's find glyphs with intermediate layers + # which have components which don't have intermediate layers + for shape in layer.components: + check_component_has_sparse_layer(font, shape, layer) + # Later we will check if everyone who uses me as a component + # has the same intermediate layers as I do + components_with_intermediate_layers.add(glyph.name) + all_intermediate_locations.add(tuple(layer.attributes["coordinates"])) + + # It's not clear to me that this is needed... (See #954) + + # for glyph in font.glyphs: + # # Just peek at first layer, assume compatibility + # if not any(c.name in components_with_intermediate_layers for c in glyph.layers[0].components): + # continue + # for location in all_intermediate_locations: + # if not any(list(location) == l.attributes.get("coordinates") for l in glyph.layers): + # import IPython; IPython.embed() + # logger.info("Adding intermediate layer at %s to %s", location, glyph.name) + # layer = GSLayer() + # layer.attributes["coordinates"] = location + # layer.layerId = str(uuid.uuid4()) + # layer.associatedMasterId = glyph.layers[0].associatedMasterId + # layer.name = glyph.layers[0].name + # for ix, shape in enumerate(glyph.layers[0].shapes): + # if isinstance(shape, GSPath): + # layer.shapes.append(shape.clone()) + # else: + # layer.shapes.append(shape.clone()) + # glyph.layers.append(layer) + + +def simple_variation_model(font): + tags = [axis.axisTag for axis in font.axes] + locations = [x.axes for x in font.masters] + limits = {tag: (min(x), max(x)) for tag, x in zip(tags, (zip(*locations)))} + master_locations = [] + for loc in locations: + this_loc = {} + for ix, axisTag in enumerate(tags): + axismin, axismax = limits[axisTag] + this_loc[axisTag] = normalizeValue(loc[ix], (axismin, axismin, axismax)) + master_locations.append(this_loc) + return VariationModel(master_locations, axisOrder=tags), limits + + +def check_component_has_sparse_layer(font, component, parent_layer): + tags = [axis.axisTag for axis in font.axes] + model, limits = simple_variation_model(font) + location = parent_layer.attributes["coordinates"] + normalized_location = { + axisTag: normalizeValue( + location[ix], (limits[axisTag][0], limits[axisTag][0], limits[axisTag][1]) + ) + for ix, axisTag in enumerate(tags) + } + componentglyph = component.component + for layer in componentglyph.layers: + if layer.layerId == parent_layer.layerId: + return + if ( + "coordinates" in layer.attributes + and layer.attributes["coordinates"] == location + ): + return + + # We'll add the appropriate intermediate layer to the component, that'll fix it + logger.info( + "Adding intermediate layer to %s to support %s %s", + componentglyph.name, + parent_layer.parent.name, + parent_layer.name, + ) + layer = GSLayer() + layer.attributes["coordinates"] = location + layer.layerId = str(uuid.uuid4()) + layer.associatedMasterId = parent_layer.associatedMasterId + layer.name = parent_layer.name + for ix, shape in enumerate(componentglyph.layers[0].shapes): + all_shapes = [l.shapes[ix] for l in componentglyph.layers if l._is_master_layer] + assert len(all_shapes) == len(font.masters) + if isinstance(shape, GSPath): + # We are making big assumptions about compatibility here + layer.shapes.append( + interpolate_path(all_shapes, model, normalized_location) + ) + else: + # We should really interpolate the transformation matrix here + check_component_has_sparse_layer(font, shape, parent_layer) + layer.shapes.append( + interpolate_component(all_shapes, model, normalized_location) + ) + componentglyph.layers.append(layer) + + +def interpolate_path(paths, model, location): + path = GSPath() + for master_nodes in zip(*[p.nodes for p in paths]): + node = GSNode() + node.type = master_nodes[0].type + node.smooth = master_nodes[0].smooth + xs = [n.position.x for n in master_nodes] + ys = [n.position.y for n in master_nodes] + node.position.x = model.interpolateFromMasters(location, xs) + node.position.y = model.interpolateFromMasters(location, ys) + path.nodes.append(node) + return path + + +def interpolate_component(components, model, location): + component = components[0].clone() + if all(c.transform == component.transform for c in components): + return component + transforms = [c.transform for c in components] + for element in range(6): + values = [t[element] for t in transforms] + component.transform[element] = model.interpolateFromMasters(location, values) + return component From ed9fc426e83a76f70cdbbecd79d0173c2358fd9e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 10 Jan 2024 13:59:45 +0000 Subject: [PATCH 03/12] Call transformations when building --- Lib/glyphsLib/builder/builders.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Lib/glyphsLib/builder/builders.py b/Lib/glyphsLib/builder/builders.py index 3beea4706..cae2b864a 100644 --- a/Lib/glyphsLib/builder/builders.py +++ b/Lib/glyphsLib/builder/builders.py @@ -28,6 +28,7 @@ FONT_CUSTOM_PARAM_PREFIX, ) from .axes import WEIGHT_AXIS_DEF, WIDTH_AXIS_DEF, find_base_style, class_to_value +from .transformations import TRANSFORMATIONS from glyphsLib.util import LoggerMixin @@ -86,7 +87,7 @@ def __init__( production, and unnecessary steps will be skipped. glyph_data -- A list of GlyphData. """ - self.font = font + self.font = preflight_glyphs(font) if ufo_module is None: import ufoLib2 as ufo_module @@ -411,6 +412,18 @@ def instance_data(self): ) +def preflight_glyphs(font): + # Run a set of transformations over a GSFont object to make + # it easier to convert to UFO; resolve all the "smart stuff". + + # We could "font = copy.deepcopy(font)" here, since we'll be + # modifying the original. But that's a pain and we won't do it + # until someone complains. + for transform in TRANSFORMATIONS: + transform(font) + return font + + def filter_instances_by_family(instances, family_name=None): """Yield instances whose 'familyName' custom parameter is equal to 'family_name'. From e2ddc755c96e8fe62e5cfc26ce90607446ecaa00 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 10 Jan 2024 14:52:13 +0000 Subject: [PATCH 04/12] Add a test --- tests/builder/preflight_test.py | 9 + tests/data/ComponentsWithIntermediates.glyphs | 359 ++++++++++++++++++ 2 files changed, 368 insertions(+) create mode 100644 tests/builder/preflight_test.py create mode 100644 tests/data/ComponentsWithIntermediates.glyphs diff --git a/tests/builder/preflight_test.py b/tests/builder/preflight_test.py new file mode 100644 index 000000000..389dd0928 --- /dev/null +++ b/tests/builder/preflight_test.py @@ -0,0 +1,9 @@ +import glyphsLib +from glyphsLib.builder.transformations import resolve_intermediate_components + + +def test_intermediates_with_components_without_intermediates(datadir): + font = glyphsLib.GSFont(str(datadir.join("ComponentsWithIntermediates.glyphs"))) + assert len(font.glyphs["A"].layers) != len(font.glyphs["Astroke"].layers) + resolve_intermediate_components(font) + assert len(font.glyphs["A"].layers) == len(font.glyphs["Astroke"].layers) diff --git a/tests/data/ComponentsWithIntermediates.glyphs b/tests/data/ComponentsWithIntermediates.glyphs new file mode 100644 index 000000000..dde9c5f8e --- /dev/null +++ b/tests/data/ComponentsWithIntermediates.glyphs @@ -0,0 +1,359 @@ +{ +.appVersion = "3227"; +.formatVersion = 3; +axes = ( +{ +name = Weight; +tag = wght; +} +); +date = "2021-10-04 15:53:11 +0000"; +familyName = ComponentsWithIntermediates; +fontMaster = ( +{ +axesValues = ( +90 +); +id = "9FF96064-8718-4A86-A5C8-73C592101F67"; +metricValues = ( +{ +over = 6; +pos = 760; +}, +{ +over = 12; +pos = 714; +}, +{ +over = 11; +pos = 536; +}, +{ +over = -15; +}, +{ +over = -16; +pos = -240; +}, +{ +over = -15; +}, +{ +} +); +name = Regular; +stemValues = ( +79, +90 +); +}, +{ +axesValues = ( +190 +); +iconName = Bold; +id = "0A718E2F-231D-484E-9433-16A5230D2CE8"; +metricValues = ( +{ +over = 6; +pos = 760; +}, +{ +over = 11; +pos = 714; +}, +{ +over = 10; +pos = 553; +}, +{ +over = -15; +}, +{ +over = -16; +pos = -240; +}, +{ +over = -15; +}, +{ +} +); +name = Bold; +stemValues = ( +158, +194 +); +userData = { +GSOffsetHorizontal = 16; +GSOffsetKeepCompatible = 1; +GSOffsetVertical = 9; +com.github.googlei18n.ufo2ft.filters = ( +{ +name = flattenComponents; +pre = 1; +} +); +}; +} +); +glyphs = ( +{ +category = Letter; +glyphname = A; +kernLeft = A.right; +kernRight = A.left; +layers = ( +{ +anchors = ( +{ +name = bottom; +pos = (363,0); +}, +{ +name = center; +pos = (363,358); +}, +{ +name = top; +pos = (363,714); +}, +{ +name = topright; +pos = (636,714); +} +); +layerId = "0A718E2F-231D-484E-9433-16A5230D2CE8"; +shapes = ( +{ +closed = 1; +nodes = ( +(726,0,l), +(490,717,l), +(233,717,l), +(0,0,l), +(212,0,l), +(248,134,l), +(480,134,l), +(515,0,l) +); +}, +{ +closed = 1; +nodes = ( +(440,292,l), +(288,292,l), +(319,409,ls), +(331,456,o), +(354,550,o), +(363,599,c), +(372,550,o), +(399,447,o), +(409,409,cs) +); +} +); +width = 726; +}, +{ +anchors = ( +{ +name = bottom; +pos = (324,0); +}, +{ +name = center; +pos = (319,358); +}, +{ +name = top; +pos = (318,714); +}, +{ +name = topright; +pos = (519,714); +} +); +layerId = "9FF96064-8718-4A86-A5C8-73C592101F67"; +shapes = ( +{ +closed = 1; +nodes = ( +(638,0,l), +(360,717,l), +(279,717,l), +(0,0,l), +(91,0,l), +(176,221,l), +(459,221,l), +(545,0,l) +); +}, +{ +closed = 1; +nodes = ( +(432,301,l), +(206,301,l), +(287,517,ls), +(295,540,o), +(308,583,o), +(318,624,c), +(325,599,o), +(346,533,o), +(352,517,cs) +); +} +); +width = 639; +} +); +script = latin; +subCategory = Uppercase; +unicode = 65; +}, +{ +category = Letter; +color = 10; +glyphname = Astroke; +layers = ( +{ +anchors = ( +{ +name = bottom; +pos = (324,0); +}, +{ +name = top; +pos = (318,714); +} +); +layerId = "9FF96064-8718-4A86-A5C8-73C592101F67"; +shapes = ( +{ +closed = 1; +nodes = ( +(432,760,l), +(142,-75,l), +(209,-75,l), +(499,760,l) +); +}, +{ +alignment = -1; +ref = A; +} +); +width = 639; +}, +{ +anchors = ( +{ +name = bottom; +pos = (345,0); +}, +{ +name = top; +pos = (345,714); +} +); +associatedMasterId = "9FF96064-8718-4A86-A5C8-73C592101F67"; +attr = { +coordinates = ( +151 +); +}; +background = { +shapes = ( +{ +closed = 1; +nodes = ( +(457.52,760,l), +(167.52,-75,l), +(265.84,-75,l), +(555.84,760,l) +); +} +); +}; +layerId = "05c7f3ac-c6d8-4d29-899b-2a10de32f922"; +name = "SemiBold (intermediate)"; +shapes = ( +{ +closed = 1; +nodes = ( +(446,760,l), +(156,-75,l), +(253,-75,l), +(543,760,l) +); +}, +{ +alignment = -1; +ref = A; +} +); +width = 690; +}, +{ +anchors = ( +{ +name = bottom; +pos = (363,0); +}, +{ +name = top; +pos = (363,714); +} +); +layerId = "0A718E2F-231D-484E-9433-16A5230D2CE8"; +shapes = ( +{ +closed = 1; +nodes = ( +(476,760,l), +(186,-75,l), +(307,-75,l), +(597,760,l) +); +}, +{ +alignment = -1; +ref = A; +} +); +width = 730; +} +); +script = latin; +subCategory = Uppercase; +unicode = 570; +} +); +metrics = ( +{ +type = ascender; +}, +{ +type = "cap height"; +}, +{ +type = "x-height"; +}, +{ +type = baseline; +}, +{ +type = descender; +}, +{ +filter = "case == 3"; +type = "x-height"; +}, +{ +type = "italic angle"; +} +); +unitsPerEm = 1000; +versionMajor = 2; +versionMinor = 13; +} From b817e439230ba52538989b2a8713ae5f045a8b78 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 10 Jan 2024 14:57:57 +0000 Subject: [PATCH 05/12] Delete this code for now but leave in git history This deals with components which have intermediate layers being used in glyphs which don't (see #954), but that seems to work for me, so maybe the code is not needed. --- .../transformations/intermediate_layers.py | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index 3e47aa7ff..21c574e05 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -24,28 +24,6 @@ def resolve_intermediate_components(font): components_with_intermediate_layers.add(glyph.name) all_intermediate_locations.add(tuple(layer.attributes["coordinates"])) - # It's not clear to me that this is needed... (See #954) - - # for glyph in font.glyphs: - # # Just peek at first layer, assume compatibility - # if not any(c.name in components_with_intermediate_layers for c in glyph.layers[0].components): - # continue - # for location in all_intermediate_locations: - # if not any(list(location) == l.attributes.get("coordinates") for l in glyph.layers): - # import IPython; IPython.embed() - # logger.info("Adding intermediate layer at %s to %s", location, glyph.name) - # layer = GSLayer() - # layer.attributes["coordinates"] = location - # layer.layerId = str(uuid.uuid4()) - # layer.associatedMasterId = glyph.layers[0].associatedMasterId - # layer.name = glyph.layers[0].name - # for ix, shape in enumerate(glyph.layers[0].shapes): - # if isinstance(shape, GSPath): - # layer.shapes.append(shape.clone()) - # else: - # layer.shapes.append(shape.clone()) - # glyph.layers.append(layer) - def simple_variation_model(font): tags = [axis.axisTag for axis in font.axes] From 9de5fdd50f7623b1e08a825f6d96eb6eb9e3bdcf Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 11 Jan 2024 16:54:58 +0000 Subject: [PATCH 06/12] Also interpolate width --- Lib/glyphsLib/builder/transformations/intermediate_layers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index 21c574e05..4c109a0ae 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -71,6 +71,8 @@ def check_component_has_sparse_layer(font, component, parent_layer): layer.layerId = str(uuid.uuid4()) layer.associatedMasterId = parent_layer.associatedMasterId layer.name = parent_layer.name + all_widths = [l.width for l in componentglyph.layers if l._is_master_layer] + layer.width = model.interpolateFromMasters(normalized_location, all_widths) for ix, shape in enumerate(componentglyph.layers[0].shapes): all_shapes = [l.shapes[ix] for l in componentglyph.layers if l._is_master_layer] assert len(all_shapes) == len(font.masters) From fa869047b6380c58a09c9e9bca92f41e08484f74 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Jan 2024 12:55:46 +0000 Subject: [PATCH 07/12] Move preflight_glyphs to .builder, call in to_..., option to maintain original --- Lib/glyphsLib/builder/__init__.py | 33 ++++++++++++++++++++++++++++--- Lib/glyphsLib/builder/builders.py | 19 ++++-------------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/Lib/glyphsLib/builder/__init__.py b/Lib/glyphsLib/builder/__init__.py index c52ccc530..f2c099af5 100644 --- a/Lib/glyphsLib/builder/__init__.py +++ b/Lib/glyphsLib/builder/__init__.py @@ -12,11 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import logging from glyphsLib import classes from .builders import UFOBuilder, GlyphsBuilder +from .transformations import TRANSFORMATIONS logger = logging.getLogger(__name__) @@ -34,11 +36,13 @@ def to_ufos( expand_includes=False, minimal=False, glyph_data=None, + preserve_original=False, ): """Take a GSFont object and convert it into one UFO per master. Takes in data as Glyphs.app-compatible classes, as documented at - https://docu.glyphsapp.com/ + https://docu.glyphsapp.com/. The input ``GSFont`` object is modified + unless ``preserve_original`` is true. If include_instances is True, also returns the parsed instance data. @@ -54,9 +58,14 @@ def to_ufos( If minimal is True, it is assumed that the UFOs will only be used in font production, and unnecessary steps (e.g. converting background layers) will be skipped. + + If preserve_original is True, this works on a copy of the font object + to avoid modifying the original object. """ + if preserve_original: + font = copy.deepcopy(font) builder = UFOBuilder( - font, + preflight_glyphs(font), ufo_module=ufo_module, family_name=family_name, propagate_anchors=propagate_anchors, @@ -89,6 +98,7 @@ def to_designspace( expand_includes=False, minimal=False, glyph_data=None, + preserve_original=False, ): """Take a GSFont object and convert it into a Designspace Document + UFOS. The UFOs are available as the attribute `font` of each SourceDescriptor of @@ -96,6 +106,8 @@ def to_designspace( ufos = [source.font for source in designspace.sources] + The input object is modified unless ``preserve_original`` is true. + The designspace and the UFOs are not written anywhere by default, they are all in-memory. If you want to write them to the disk, consider using the `filename` attribute of the DesignspaceDocument and of its @@ -111,9 +123,15 @@ def to_designspace( If generate_GDEF is True, write a `table GDEF {...}` statement in the UFO's features.fea, containing GlyphClassDef and LigatureCaretByPos. + + If preserve_original is True, this works on a copy of the font object + to avoid modifying the original object. """ + if preserve_original: + font = copy.deepcopy(font) + builder = UFOBuilder( - font, + preflight_glyphs(font), ufo_module=ufo_module, family_name=family_name, instance_dir=instance_dir, @@ -130,6 +148,15 @@ def to_designspace( return builder.designspace +def preflight_glyphs(font): + """Run a set of transformations over a GSFont object to make + it easier to convert to UFO; resolve all the "smart stuff".""" + + for transform in TRANSFORMATIONS: + transform(font) + return font + + def to_glyphs( ufos_or_designspace, glyphs_module=classes, diff --git a/Lib/glyphsLib/builder/builders.py b/Lib/glyphsLib/builder/builders.py index cae2b864a..3dc579d48 100644 --- a/Lib/glyphsLib/builder/builders.py +++ b/Lib/glyphsLib/builder/builders.py @@ -28,7 +28,6 @@ FONT_CUSTOM_PARAM_PREFIX, ) from .axes import WEIGHT_AXIS_DEF, WIDTH_AXIS_DEF, find_base_style, class_to_value -from .transformations import TRANSFORMATIONS from glyphsLib.util import LoggerMixin @@ -55,7 +54,9 @@ def __init__( """Create a builder that goes from Glyphs to UFO + designspace. Keyword arguments: - font -- The GSFont object to transform into UFOs + font -- The GSFont object to transform into UFOs. We expect this GSFont + object to have been pre-processed with + ``glyphsLib.builder.preflight_glyphs``. ufo_module -- A Python module to use to build UFO objects (you can pass a custom module that has the same classes as ufoLib2 or defcon to get instances of your own classes). Default: ufoLib2 @@ -87,7 +88,7 @@ def __init__( production, and unnecessary steps will be skipped. glyph_data -- A list of GlyphData. """ - self.font = preflight_glyphs(font) + self.font = font if ufo_module is None: import ufoLib2 as ufo_module @@ -412,18 +413,6 @@ def instance_data(self): ) -def preflight_glyphs(font): - # Run a set of transformations over a GSFont object to make - # it easier to convert to UFO; resolve all the "smart stuff". - - # We could "font = copy.deepcopy(font)" here, since we'll be - # modifying the original. But that's a pain and we won't do it - # until someone complains. - for transform in TRANSFORMATIONS: - transform(font) - return font - - def filter_instances_by_family(instances, family_name=None): """Yield instances whose 'familyName' custom parameter is equal to 'family_name'. From ce9dda4017b5d1dd19377e8775d9ce4553ab3844 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Jan 2024 12:56:58 +0000 Subject: [PATCH 08/12] Fix terminology, remove misleading comment --- .../builder/transformations/intermediate_layers.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index 4c109a0ae..6e1ac4aae 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -18,7 +18,7 @@ def resolve_intermediate_components(font): # First, let's find glyphs with intermediate layers # which have components which don't have intermediate layers for shape in layer.components: - check_component_has_sparse_layer(font, shape, layer) + ensure_component_has_sparse_layer(font, shape, layer) # Later we will check if everyone who uses me as a component # has the same intermediate layers as I do components_with_intermediate_layers.add(glyph.name) @@ -39,7 +39,7 @@ def simple_variation_model(font): return VariationModel(master_locations, axisOrder=tags), limits -def check_component_has_sparse_layer(font, component, parent_layer): +def ensure_component_has_sparse_layer(font, component, parent_layer): tags = [axis.axisTag for axis in font.axes] model, limits = simple_variation_model(font) location = parent_layer.attributes["coordinates"] @@ -82,8 +82,7 @@ def check_component_has_sparse_layer(font, component, parent_layer): interpolate_path(all_shapes, model, normalized_location) ) else: - # We should really interpolate the transformation matrix here - check_component_has_sparse_layer(font, shape, parent_layer) + ensure_component_has_sparse_layer(font, shape, parent_layer) layer.shapes.append( interpolate_component(all_shapes, model, normalized_location) ) From ea3fb1fa4d532918c44d8d0231c8b5e0b6fd8514 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Jan 2024 13:49:54 +0000 Subject: [PATCH 09/12] Compute and use default location when normalizing --- .../builder/transformations/intermediate_layers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index 6e1ac4aae..bc3d7f921 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -4,6 +4,7 @@ from fontTools.varLib.models import VariationModel, normalizeValue from glyphsLib.classes import GSLayer, GSNode, GSPath +from glyphsLib.builder.axes import get_regular_master logger = logging.getLogger(__name__) @@ -30,11 +31,14 @@ def simple_variation_model(font): locations = [x.axes for x in font.masters] limits = {tag: (min(x), max(x)) for tag, x in zip(tags, (zip(*locations)))} master_locations = [] + default_location = get_regular_master(font).axes for loc in locations: this_loc = {} for ix, axisTag in enumerate(tags): axismin, axismax = limits[axisTag] - this_loc[axisTag] = normalizeValue(loc[ix], (axismin, axismin, axismax)) + this_loc[axisTag] = normalizeValue( + loc[ix], (axismin, default_location[ix], axismax) + ) master_locations.append(this_loc) return VariationModel(master_locations, axisOrder=tags), limits From 0a340f21d3e0a3cdcf38f3acce364da9f6fdd348 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 25 Jan 2024 12:33:31 +0000 Subject: [PATCH 10/12] Use a glyph-level model for interpolation --- .../transformations/intermediate_layers.py | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index bc3d7f921..aefa51b0a 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -15,7 +15,7 @@ def resolve_intermediate_components(font): all_intermediate_locations = set() for glyph in font.glyphs: for layer in glyph.layers: - if "coordinates" in layer.attributes: + if layer._is_brace_layer(): # First, let's find glyphs with intermediate layers # which have components which don't have intermediate layers for shape in layer.components: @@ -23,12 +23,11 @@ def resolve_intermediate_components(font): # Later we will check if everyone who uses me as a component # has the same intermediate layers as I do components_with_intermediate_layers.add(glyph.name) - all_intermediate_locations.add(tuple(layer.attributes["coordinates"])) + all_intermediate_locations.add(tuple(layer._brace_coordinates())) -def simple_variation_model(font): +def variation_model(font, locations): tags = [axis.axisTag for axis in font.axes] - locations = [x.axes for x in font.masters] limits = {tag: (min(x), max(x)) for tag, x in zip(tags, (zip(*locations)))} master_locations = [] default_location = get_regular_master(font).axes @@ -45,8 +44,9 @@ def simple_variation_model(font): def ensure_component_has_sparse_layer(font, component, parent_layer): tags = [axis.axisTag for axis in font.axes] - model, limits = simple_variation_model(font) - location = parent_layer.attributes["coordinates"] + master_locations = [x.axes for x in font.masters] + _, limits = variation_model(font, master_locations) + location = tuple(parent_layer._brace_coordinates()) normalized_location = { axisTag: normalizeValue( location[ix], (limits[axisTag][0], limits[axisTag][0], limits[axisTag][1]) @@ -57,10 +57,7 @@ def ensure_component_has_sparse_layer(font, component, parent_layer): for layer in componentglyph.layers: if layer.layerId == parent_layer.layerId: return - if ( - "coordinates" in layer.attributes - and layer.attributes["coordinates"] == location - ): + if "coordinates" in layer.attributes and layer._brace_coordinates() == location: return # We'll add the appropriate intermediate layer to the component, that'll fix it @@ -75,20 +72,39 @@ def ensure_component_has_sparse_layer(font, component, parent_layer): layer.layerId = str(uuid.uuid4()) layer.associatedMasterId = parent_layer.associatedMasterId layer.name = parent_layer.name - all_widths = [l.width for l in componentglyph.layers if l._is_master_layer] - layer.width = model.interpolateFromMasters(normalized_location, all_widths) + # Create a glyph-level variation model for the component glyph, + # including any intermediate layers + interpolatable_layers = [] + locations = [] + for l in componentglyph.layers: + if l._is_brace_layer(): + locations.append(l.attributes["coordinates"]) + interpolatable_layers.append(l) + if l._is_master_layer: + locations.append(font.masters[l.associatedMasterId].axes) + interpolatable_layers.append(l) + glyph_level_model, _ = variation_model(font, locations) + + # Interpolate new layer width + all_widths = [l.width for l in interpolatable_layers] + layer.width = glyph_level_model.interpolateFromMasters( + normalized_location, all_widths + ) + + # Interpolate layer shapes for ix, shape in enumerate(componentglyph.layers[0].shapes): - all_shapes = [l.shapes[ix] for l in componentglyph.layers if l._is_master_layer] - assert len(all_shapes) == len(font.masters) + all_shapes = [l.shapes[ix] for l in interpolatable_layers] if isinstance(shape, GSPath): # We are making big assumptions about compatibility here layer.shapes.append( - interpolate_path(all_shapes, model, normalized_location) + interpolate_path(all_shapes, glyph_level_model, normalized_location) ) else: ensure_component_has_sparse_layer(font, shape, parent_layer) layer.shapes.append( - interpolate_component(all_shapes, model, normalized_location) + interpolate_component( + all_shapes, glyph_level_model, normalized_location + ) ) componentglyph.layers.append(layer) From a03e0a29d5be6918776d21885aecc59b075e869c Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 25 Jan 2024 12:34:13 +0000 Subject: [PATCH 11/12] Remove more leftover code for now --- .../builder/transformations/intermediate_layers.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index aefa51b0a..c5ec49700 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -11,8 +11,6 @@ def resolve_intermediate_components(font): - components_with_intermediate_layers = set() - all_intermediate_locations = set() for glyph in font.glyphs: for layer in glyph.layers: if layer._is_brace_layer(): @@ -20,10 +18,6 @@ def resolve_intermediate_components(font): # which have components which don't have intermediate layers for shape in layer.components: ensure_component_has_sparse_layer(font, shape, layer) - # Later we will check if everyone who uses me as a component - # has the same intermediate layers as I do - components_with_intermediate_layers.add(glyph.name) - all_intermediate_locations.add(tuple(layer._brace_coordinates())) def variation_model(font, locations): From 8b758e6f02aade6aed8b83cf8f61daeaaba87aea Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 25 Jan 2024 12:39:31 +0000 Subject: [PATCH 12/12] Use default location not axis min --- Lib/glyphsLib/builder/transformations/intermediate_layers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/glyphsLib/builder/transformations/intermediate_layers.py b/Lib/glyphsLib/builder/transformations/intermediate_layers.py index c5ec49700..0cf9749fc 100644 --- a/Lib/glyphsLib/builder/transformations/intermediate_layers.py +++ b/Lib/glyphsLib/builder/transformations/intermediate_layers.py @@ -41,9 +41,10 @@ def ensure_component_has_sparse_layer(font, component, parent_layer): master_locations = [x.axes for x in font.masters] _, limits = variation_model(font, master_locations) location = tuple(parent_layer._brace_coordinates()) + default_location = get_regular_master(font).axes normalized_location = { axisTag: normalizeValue( - location[ix], (limits[axisTag][0], limits[axisTag][0], limits[axisTag][1]) + location[ix], (limits[axisTag][0], default_location[ix], limits[axisTag][1]) ) for ix, axisTag in enumerate(tags) }