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

Add width to MinimalGlyph object, so ufo2ft can access it when generating features #434

Merged
merged 4 commits into from
Sep 14, 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
41 changes: 30 additions & 11 deletions Lib/fontgoggles/compile/ufoCompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def compileUFOToFont(ufoPath):
if ".notdef" not in glyphOrder:
# We need a .notdef glyph, so let's make one.
glyphOrder.insert(0, ".notdef")
cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(glyphSet, ufoPath, ufo2=ufo2)
widths, cmap, revCmap, anchors = fetchGlyphInfo(glyphSet, ufoPath, ufo2=ufo2)
fb = FontBuilder(round(info.unitsPerEm))
fb.setupGlyphOrder(glyphOrder)
fb.setupCharacterMap(cmap)
Expand All @@ -46,7 +46,7 @@ def compileUFOToFont(ufoPath):
# changes.
ttFont["FGAx"] = newTable("FGAx")
ttFont["FGAx"].data = pickle.dumps(anchors)
ufo = MinimalFontObject(ufoPath, reader, revCmap, anchors)
ufo = MinimalFontObject(ufoPath, reader, widths, revCmap, anchors)
feaComp = FeatureCompiler(ufo, ttFont)
try:
feaComp.compile()
Expand All @@ -68,13 +68,15 @@ def compileUFOToPath(ufoPath, ttPath):
ttFont.save(ttPath, reorderTables=False)


_unicodeOrAnchorGLIFPattern = re.compile(rb'(<\s*(anchor|unicode)\s+([^>]+)>)')
_tagGLIFPattern = re.compile(rb'(<\s*(advance|anchor|unicode)\s+([^>]+)>)')
_ufo2AnchorPattern = re.compile(rb"<contour>\s+(<point\s+[^>]+move[^>]+name[^>]+>)\s+</contour>")
_unicodeAttributeGLIFPattern = re.compile(rb'hex\s*=\s*\"([0-9A-Fa-f]+)\"')
_widthAttributeGLIFPattern = re.compile(rb'width\s*=\s*\"([0-9A-Fa-f]+)\"')


def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=False):
def fetchGlyphInfo(glyphSet, ufoPath, glyphNames=None, ufo2=False):
# This seems about 2.3 times faster than reader.getCharacterMapping()
widths = {}
cmap = {} # unicode: glyphName
revCmap = {}
anchors = {} # glyphName: [(anchorName, x, y), ...]
Expand All @@ -86,12 +88,13 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
if b"<!--" in data:
# Fall back to proper parser, assuming this to be uncommon
# (This does not work for UFO 2)
unicodes, glyphAnchors = fetchUnicodesAndAnchors(data)
width, unicodes, glyphAnchors = fetchUnicodesAndAnchors(data)
else:
# Fast route with regex
width = None
unicodes = []
glyphAnchors = []
for rawElement, tag, rawAttributes in _unicodeOrAnchorGLIFPattern.findall(data):
for rawElement, tag, rawAttributes in _tagGLIFPattern.findall(data):
if tag == b"unicode":
m = _unicodeAttributeGLIFPattern.match(rawAttributes)
try:
Expand All @@ -101,11 +104,17 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
elif tag == b"anchor":
root = ET.fromstring(rawElement)
glyphAnchors.append(_parseAnchorAttrs(root.attrib))
elif tag == b"advance":
m = _widthAttributeGLIFPattern.search(rawAttributes)
if m is not None:
width = float(m.group(1))
if ufo2:
for rawElement in _ufo2AnchorPattern.findall(data):
root = ET.fromstring(rawElement)
glyphAnchors.append(_parseAnchorAttrs(root.attrib))

widths[glyphName] = width

uniqueUnicodes = []
for codePoint in unicodes:
if codePoint not in cmap:
Expand All @@ -127,7 +136,7 @@ def fetchCharacterMappingAndAnchors(glyphSet, ufoPath, glyphNames=None, ufo2=Fal
logger = logging.getLogger("fontgoggles.font.ufoFont")
logger.warning("Some code points in '%s' are assigned to multiple glyphs: %s",
ufoPath, dupMessage)
return cmap, revCmap, anchors
return widths, cmap, revCmap, anchors


def fetchUnicodesAndAnchors(glif):
Expand All @@ -136,7 +145,7 @@ def fetchUnicodesAndAnchors(glif):
"""
parser = FetchUnicodesAndAnchorsParser()
parser.parse(glif)
return parser.unicodes, parser.anchors
return parser.advanceWidth, parser.unicodes, parser.anchors


def _parseNumber(s):
Expand All @@ -158,6 +167,7 @@ class FetchUnicodesAndAnchorsParser(BaseGlifParser):
def __init__(self):
self.unicodes = []
self.anchors = []
self.advanceWidth = None
super().__init__()

def startElementHandler(self, name, attrs):
Expand All @@ -173,6 +183,8 @@ def startElementHandler(self, name, attrs):
pass
elif name == "anchor":
self.anchors.append(_parseAnchorAttrs(attrs))
elif name == "advance":
self.advanceWidth = _parseNumber(attrs.get("width"))
super().startElementHandler(name, attrs)


Expand All @@ -184,8 +196,9 @@ class MinimalFontObject:
# unicodes and anchors, and at the font level, only features, groups,
# kerning and lib are needed.

def __init__(self, ufoPath, reader, revCmap, anchors):
def __init__(self, ufoPath, reader, widths, revCmap, anchors):
self.path = ufoPath
self._widths = widths
self._revCmap = revCmap
self._anchors = anchors
self._glyphNames = set(reader.getGlyphSet().contents.keys())
Expand All @@ -212,15 +225,21 @@ def __getitem__(self, glyphName):
# TODO: should we even bother caching?
glyph = self._glyphs.get(glyphName)
if glyph is None:
glyph = MinimalGlyphObject(glyphName, self._revCmap.get(glyphName), self._anchors.get(glyphName, ()))
glyph = MinimalGlyphObject(
glyphName,
self._widths.get(glyphName, 0),
self._revCmap.get(glyphName),
self._anchors.get(glyphName, ()),
)
self._glyphs[glyphName] = glyph
return glyph


class MinimalGlyphObject:

def __init__(self, name, unicodes, anchors):
def __init__(self, name, width, unicodes, anchors):
self.name = name
self.width = width
self.unicodes = unicodes
self.anchors = [MinimalAnchorObject(name, x, y) for name, x, y in anchors]

Expand Down
10 changes: 6 additions & 4 deletions Lib/fontgoggles/font/ufoFont.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from .baseFont import BaseFont
from .glyphDrawing import GlyphDrawing, GlyphLayersDrawing
from ..compile.compilerPool import compileUFOToBytes
from ..compile.ufoCompiler import fetchCharacterMappingAndAnchors
from ..compile.ufoCompiler import fetchGlyphInfo
from ..misc.hbShape import HBShape
from ..misc.properties import cachedProperty

Expand Down Expand Up @@ -400,9 +400,11 @@ def getUpdateInfo(self):
changedGlyphNames = {glyphName for glyphName, mtime in prev.glyphModTimes ^ self.glyphModTimes}
deletedGlyphNames = {glyphName for glyphName in changedGlyphNames if glyphName not in self.glyphSet}

_, changedUnicodes, changedAnchors = fetchCharacterMappingAndAnchors(self.glyphSet,
self.reader.fs.getsyspath("/"),
changedGlyphNames - deletedGlyphNames)
_, _, changedUnicodes, changedAnchors = fetchGlyphInfo(
self.glyphSet,
self.reader.fs.getsyspath("/"),
changedGlyphNames - deletedGlyphNames,
)

# Within the changed glyphs, let's see if their anchors changed
for gn in changedGlyphNames:
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
include(features_test.fea)

table GDEF {
GlyphClassDef
[ A ],
[ ],
[ macroncmb ],
;
} GDEF;
8 changes: 5 additions & 3 deletions Tests/test_ufoCompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
import os
import pytest
from fontTools.ufoLib import UFOReader
from fontgoggles.compile.ufoCompiler import fetchCharacterMappingAndAnchors
from fontgoggles.compile.ufoCompiler import fetchGlyphInfo
from fontgoggles.compile.compilerPool import compileUFOToPath
from testSupport import getFontPath


def test_ufoCharacterMapping():
ufoPath = getFontPath("MutatorSansBoldWideMutated.ufo")
reader = UFOReader(ufoPath)
cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(reader.getGlyphSet(), ufoPath)
widths, cmap, revCmap, anchors = fetchGlyphInfo(reader.getGlyphSet(), ufoPath)
assert widths["A"] == 1290
assert widths["B"] == 1270
assert cmap[0x0041] == "A"
assert revCmap["A"] == [0x0041]
# MutatorSansBoldWideMutated.ufo/glyphs/A_.glif contains a commented-out <unicode>
Expand All @@ -28,7 +30,7 @@ def test_ufoCharacterMapping():
def test_ufoCharacterMapping_glyphNames():
ufoPath = getFontPath("MutatorSansBoldWideMutated.ufo")
reader = UFOReader(ufoPath)
cmap, revCmap, anchors = fetchCharacterMappingAndAnchors(reader.getGlyphSet(), ufoPath, ["A"])
widths, cmap, revCmap, anchors = fetchGlyphInfo(reader.getGlyphSet(), ufoPath, ["A"])
assert cmap[0x0041] == "A"
assert revCmap["A"] == [0x0041]
assert anchors == {"A": [("top", 645, 815)]}
Expand Down
4 changes: 2 additions & 2 deletions Tests/test_ufoState.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from fontTools.ufoLib import UFOReaderWriter
from fontTools.ufoLib.glifLib import Glyph
from fontgoggles.font.ufoFont import UFOState
from fontgoggles.compile.ufoCompiler import fetchCharacterMappingAndAnchors
from fontgoggles.compile.ufoCompiler import fetchGlyphInfo
from testSupport import getFontPath


Expand All @@ -13,7 +13,7 @@ def test_getUpdateInfo(tmpdir):
ufoPath = shutil.copytree(ufoSource, tmpdir / "test.ufo")
reader = UFOReaderWriter(ufoPath, validate=False)
glyphSet = reader.getGlyphSet()
cmap, unicodes, anchors = fetchCharacterMappingAndAnchors(glyphSet, ufoPath)
widths, cmap, unicodes, anchors = fetchGlyphInfo(glyphSet, ufoPath)

state = UFOState(reader, glyphSet, getUnicodesAndAnchors=lambda: (unicodes, anchors))

Expand Down