Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert MATH plugin data into more usable form in UFO #980

Merged
merged 6 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Lib/glyphsLib/builder/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,8 @@
}

REVERSE_LANGUAGE_MAPPING = {v: k for v, k in LANGUAGE_MAPPING.items()}

GLYPHS_MATH_PREFIX = "com.nagwa.MATHPlugin."
GLYPHS_MATH_CONSTANTS_KEY = GLYPHS_MATH_PREFIX + "constants"
GLYPHS_MATH_VARIANTS_KEY = GLYPHS_MATH_PREFIX + "variants"
GLYPHS_MATH_EXTENDED_SHAPE_KEY = GLYPHS_MATH_PREFIX + "extendedShape"
4 changes: 2 additions & 2 deletions Lib/glyphsLib/builder/glyph.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ def to_ufo_glyph(self, ufo_glyph, layer, glyph, do_color_layers=True): # noqa:
self.to_ufo_guidelines(ufo_glyph, layer) # .guidelines
self.to_ufo_glyph_background(ufo_glyph, layer) # below
self.to_ufo_annotations(ufo_glyph, layer) # .annotations
self.to_ufo_glyph_user_data(ufo_font, glyph) # .user_data
self.to_ufo_layer_user_data(ufo_glyph, layer) # .user_data
self.to_ufo_smart_component_axes(ufo_glyph, glyph) # .components
self.to_ufo_glyph_user_data(ufo_font, ufo_glyph, glyph) # .user_data
self.to_ufo_layer_user_data(ufo_glyph, layer) # .user_data

# Optimization: profiling glyphs2ufo of NotoSans-MM.glyphs (6000 glyphs) on a Mac
# mini late 2014, Python 3.6.8, revealed that a whopping 17% of the time was spent
Expand Down
73 changes: 66 additions & 7 deletions Lib/glyphsLib/builder/user_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.


import logging
import os
import posixpath

Expand All @@ -28,8 +29,13 @@
LAYER_NAME_KEY,
GLYPH_USER_DATA_KEY,
NODE_USER_DATA_KEY,
GLYPHS_MATH_VARIANTS_KEY,
GLYPHS_MATH_EXTENDED_SHAPE_KEY,
GLYPHS_MATH_PREFIX,
)

logger = logging.getLogger(__name__)


def to_designspace_family_user_data(self):
if self.use_designspace:
Expand All @@ -56,10 +62,25 @@ def to_ufo_master_user_data(self, ufo, master):
ufo.data[filename] = bytes(data)


def to_ufo_glyph_user_data(self, ufo, glyph):
def to_ufo_glyph_user_data(self, ufo, ufo_glyph, glyph):
math_data = {
k: v for k, v in glyph.userData.items() if k.startswith(GLYPHS_MATH_PREFIX)
}
if math_data:
# Convert MATH userData to top-level keys and group them under the same
# key so that they are in a more usable/compact form.
if GLYPHS_MATH_EXTENDED_SHAPE_KEY in math_data:
ufo.lib.setdefault(GLYPHS_MATH_EXTENDED_SHAPE_KEY, []).append(glyph.name)
if GLYPHS_MATH_VARIANTS_KEY in math_data:
ufo_glyph.lib.setdefault(GLYPHS_MATH_VARIANTS_KEY, {}).update(
dict(math_data[GLYPHS_MATH_VARIANTS_KEY])
)
if self.minimal:
return
anthrotype marked this conversation as resolved.
Show resolved Hide resolved
other_data = {k: v for k, v in glyph.userData.items() if k not in math_data}
key = GLYPH_USER_DATA_KEY + "." + glyph.name
if glyph.userData:
ufo.lib[key] = dict(glyph.userData)
if other_data:
ufo.lib[key] = dict(other_data)


def to_ufo_layer_lib(self, master, ufo, ufo_layer):
Expand All @@ -84,9 +105,18 @@ def to_ufo_layer_lib(self, master, ufo, ufo_layer):

def to_ufo_layer_user_data(self, ufo_glyph, layer):
user_data = layer.userData
for key in user_data.keys():
math_data = {k: v for k, v in user_data.items() if k.startswith(GLYPHS_MATH_PREFIX)}
if math_data:
if GLYPHS_MATH_VARIANTS_KEY in math_data:
ufo_glyph.lib.setdefault(GLYPHS_MATH_VARIANTS_KEY, {}).update(
dict(math_data[GLYPHS_MATH_VARIANTS_KEY])
)
if self.minimal:
return
other_data = {k: v for k, v in user_data.items() if k not in math_data}
for key in other_data.keys():
if _user_data_has_no_special_meaning(key):
ufo_glyph.lib[key] = user_data[key]
ufo_glyph.lib[key] = other_data[key]


def to_ufo_node_user_data(self, ufo_glyph, node, user_data: dict):
Expand Down Expand Up @@ -124,8 +154,9 @@ def to_glyphs_family_user_data_from_ufo(self, ufo):
def to_glyphs_master_user_data(self, ufo, master):
"""Set the GSFontMaster userData from the UFO master-specific lib data."""
target_user_data = master.userData
special_math_keys = {GLYPHS_MATH_VARIANTS_KEY, GLYPHS_MATH_EXTENDED_SHAPE_KEY}
for key, value in ufo.lib.items():
if _user_data_has_no_special_meaning(key):
if _user_data_has_no_special_meaning(key) and key not in special_math_keys:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you should add the math keys to _user_data_has_no_special_meaning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_user_data_has_no_special_meaning is used in multiple places and in some of them we want to skip MATH key and in others we don’t.

target_user_data[key] = value

# Save UFO data files
Expand All @@ -144,6 +175,14 @@ def to_glyphs_glyph_user_data(self, ufo, glyph):
if key in ufo.lib:
glyph.userData = ufo.lib[key]

key = GLYPHS_MATH_EXTENDED_SHAPE_KEY
if key in ufo.lib and glyph.name in ufo.lib[key]:
glyph.userData[key] = True

key = GLYPHS_MATH_VARIANTS_KEY
if key in ufo.lib and glyph.name in ufo.lib[key]:
glyph.userData[key] = ufo.lib[key][glyph.name]


def to_glyphs_layer_lib(self, ufo_layer, master):
user_data = {}
Expand All @@ -168,7 +207,27 @@ def to_glyphs_layer_lib(self, ufo_layer, master):
def to_glyphs_layer_user_data(self, ufo_glyph, layer):
user_data = layer.userData
for key, value in ufo_glyph.lib.items():
if _user_data_has_no_special_meaning(key):
if key == GLYPHS_MATH_VARIANTS_KEY:
glyph_user_data = layer.parent.userData
for sub_key, sub_value in value.items():
if sub_key in {"vVariants", "hVariants"}:
if key not in glyph_user_data:
glyph_user_data[key] = {}
print(sub_value, glyph_user_data[key].get(sub_key))
if (
sub_key in glyph_user_data[key]
and glyph_user_data[key][sub_key] != sub_value
):
logger.warning(
f"Glyph '{layer.parent.name}' already has different "
f"'{sub_key}' in userData['{key}']. Overwriting it."
)
glyph_user_data[key][sub_key] = sub_value
else:
if key not in user_data:
user_data[key] = {}
user_data[key][sub_key] = sub_value
elif _user_data_has_no_special_meaning(key):
user_data[key] = value


Expand Down
84 changes: 84 additions & 0 deletions tests/builder/lib_and_user_data_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import os
from collections import OrderedDict

import pytest

from fontTools.designspaceLib import DesignSpaceDocument
from glyphsLib import classes
from glyphsLib.types import BinaryData
Expand All @@ -24,6 +26,9 @@
FONT_CUSTOM_PARAM_PREFIX,
UFO2FT_FEATURE_WRITERS_KEY,
DEFAULT_FEATURE_WRITERS,
GLYPHS_MATH_CONSTANTS_KEY,
GLYPHS_MATH_EXTENDED_SHAPE_KEY,
GLYPHS_MATH_VARIANTS_KEY,
)
from glyphsLib import to_glyphs, to_ufos, to_designspace

Expand Down Expand Up @@ -261,6 +266,85 @@ def test_glyph_user_data_into_ufo_lib():
assert font.glyphs["a"].userData["glyphUserDataKey"] == "glyphUserDataValue"


@pytest.mark.parametrize("minimal", [True, False])
def test_math_user_data_into_ufo_lib(datadir, minimal, caplog):
font = classes.GSFont(str(datadir.join("Math.glyphs")))

ufos = to_ufos(font, minimal=minimal)

math_glyphs = ["parenleft", "parenright"]

for ufo, master in zip(ufos, font.masters):
assert ufo.lib[GLYPHS_MATH_EXTENDED_SHAPE_KEY] == math_glyphs
assert (
ufo.lib[GLYPHS_MATH_CONSTANTS_KEY]
== master.userData[GLYPHS_MATH_CONSTANTS_KEY]
)
for name in math_glyphs:
for key in {"vVariants", "hVariants"}:
result = ufo[name].lib[GLYPHS_MATH_VARIANTS_KEY].get(key)
expected = font.glyphs[name].userData[GLYPHS_MATH_VARIANTS_KEY].get(key)
assert result == expected
for key in {"vAssembly", "hAssembly"}:
result = ufo[name].lib[GLYPHS_MATH_VARIANTS_KEY].get(key)
expected = (
font.glyphs[name]
.layers[master.id]
.userData[GLYPHS_MATH_VARIANTS_KEY]
.get(key)
)
assert result == expected

font2 = to_glyphs(ufos)
assert "already has different 'vVariants'" not in caplog.text

for master, ufo in zip(font2.masters, ufos):
assert font2.userData[GLYPHS_MATH_EXTENDED_SHAPE_KEY] is None
assert font2.userData[GLYPHS_MATH_CONSTANTS_KEY] is None
assert (
master.userData[GLYPHS_MATH_CONSTANTS_KEY]
== ufo.lib[GLYPHS_MATH_CONSTANTS_KEY]
)
for name in math_glyphs:
for key in {"vVariants", "hVariants"}:
result = font2.glyphs[name].userData[GLYPHS_MATH_VARIANTS_KEY].get(key)
expected = ufo[name].lib[GLYPHS_MATH_VARIANTS_KEY].get(key)
assert result == expected
for key in {"vAssembly", "hAssembly"}:
result = (
font2.glyphs[name]
.layers[master.id]
.userData[GLYPHS_MATH_VARIANTS_KEY]
.get(key)
)
expected = ufo[name].lib[GLYPHS_MATH_VARIANTS_KEY].get(key)
assert result == expected


@pytest.mark.parametrize("minimal", [True, False])
def test_math_user_data_into_ufo_lib_warn(datadir, minimal, caplog):
import copy

font = classes.GSFont(str(datadir.join("Math.glyphs")))

ufos = to_ufos(font, minimal=minimal)
ufos[0]["parenleft"] = copy.deepcopy(ufos[0]["parenleft"])
ufos[0]["parenleft"].lib[GLYPHS_MATH_VARIANTS_KEY]["vVariants"].pop()

assert (
ufos[0]["parenleft"].lib[GLYPHS_MATH_VARIANTS_KEY]["vVariants"]
!= ufos[1]["parenleft"].lib[GLYPHS_MATH_VARIANTS_KEY]["vVariants"]
)

to_glyphs(ufos)

assert any(record.levelname == "WARNING" for record in caplog.records)
assert (
"Glyph 'parenleft' already has different 'vVariants' in "
"userData['com.nagwa.MATHPlugin.variants']. Overwriting it." in caplog.text
)


def test_glif_lib_equivalent_to_layer_user_data(ufo_module):
ufo = ufo_module.Font()
# This glyph is in the `public.default` layer
Expand Down
Loading
Loading