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.

Sign up for GitHub

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

WIP: Fix bracket glyph components self-referencing after roundtrip. #679

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

WIP: Fix bracket glyph components self-referencing after roundtrip. #679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
297b848
Fix self-referencing bracket glyph component after roundtrip + tests
tomarcher101 Apr 1, 2021
66b6356
Reverse small black formatting changes to reduce redundant commit diff
tomarcher101 Apr 5, 2021
6b5b617
Fix for flake8 linting
tomarcher101 Apr 5, 2021
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
4 changes: 4 additions & 0 deletions Lib/glyphsLib/builder/builders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,10 @@ def _copy_bracket_layers_to_ufo_glyphs(self, bracket_layer_map):
)
# swap components if base glyph contains matching bracket layers.
for comp in ufo_glyph.components:
if comp.baseGlyph == glyph_name:
# Do not create self referencing glyph components. See:
# tests\builder\designspace_roundtrip_test.py::test_roundtrip_self_ref
continue
bracket_comp_name = _bracket_glyph_name(
comp.baseGlyph, reverse, location
)
Expand Down
49 changes: 49 additions & 0 deletions tests/builder/designspace_roundtrip_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,52 @@ def test_default_master_roundtrips(ufo_module):
assert reg.copyGroups is True
assert reg.copyInfo is True
assert reg.copyLib is True


def test_roundtrip_self_ref(datadir, ufo_module):
"""This test is to solve the problem of roundtrips sometimes resulting in
broken ufos when working with bracket layers.

Starting with UFO: It's okay to have a bracket glyph that references its
base glyph as a component. However, on the glyphs.app side it's not okay
because the base and bracket glyphs are merged into a single glyph with
the bracket glyph now being a layer. Glyphsapp (2.6.1) does not like the
self referencing component, but that's the best we can do to preserve the
information.

The goal here is to make sure that the valid ufo we start with, is still
valid in ufo after roundtriping. We are not concerned with making
glyphs.app work properly at the intemediary stage.
"""
path = datadir / "BracketSelfReference/BracketSelfReference.designspace"
designspace = designspaceLib.DesignSpaceDocument.fromfile(path)

for source in designspace.sources:
font = ufo_module.Font(source.path)
assert "zero" in font
assert "zero.BRACKET.500" in font
assert "space" in font

# Check zero.BRACKET.500 component correct
assert font["zero.BRACKET.500"].components[0].baseGlyph == "zero"

gs_font = to_glyphs(designspace)

# Check that the "zero" glyph contains our bracket layers
glyph_obj = gs_font.glyphs["zero"]
layer_names = [layer.name for layer in glyph_obj.layers]
assert "Regular [500]" in layer_names
assert "Bold [500]" in layer_names

# Check that our bracket layers contain the base glyph as a component
bracket_layers = [layer for layer in glyph_obj.layers if "[500]" in layer.name]
for layer in bracket_layers:
component_names = [component.name for component in layer.components]
assert component_names == ["zero"]

designspace2 = to_designspace(gs_font, ufo_module=ufo_module)
ufos = [source.font for source in designspace2.sources]

# Check zero.BRACKET.500 does not reference itself after roundtrip
for ufo in ufos:
assert ufo["zero.BRACKET.500"].components[0].baseGlyph == "zero"
36 changes: 36 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/fontinfo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>ascender</key>
<integer>800</integer>
<key>capHeight</key>
<integer>700</integer>
<key>descender</key>
<integer>-200</integer>
<key>familyName</key>
<string>BracketSelfReference</string>
<key>italicAngle</key>
<integer>0</integer>
<key>openTypeHeadCreated</key>
<string>2019/07/25 10:20:06</string>
<key>openTypeOS2Type</key>
<array>
<integer>3</integer>
</array>
<key>postscriptUnderlinePosition</key>
<integer>-100</integer>
<key>postscriptUnderlineThickness</key>
<integer>50</integer>
<key>styleName</key>
<string>Bold</string>
<key>unitsPerEm</key>
<integer>1000</integer>
<key>versionMajor</key>
<integer>1</integer>
<key>versionMinor</key>
<integer>0</integer>
<key>xHeight</key>
<integer>500</integer>
</dict>
</plist>
12 changes: 12 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/glyphs/contents.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>space</key>
<string>space.glif</string>
<key>zero</key>
<string>zero.glif</string>
<key>zero.BRACKET.500</key>
<string>zero.B_R_A_C_K_E_T_.500.glif</string>
</dict>
</plist>
15 changes: 15 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/glyphs/layerinfo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>lib</key>
<dict>
<key>com.schriftgestaltung.layerId</key>
<string>48798C20-C9A0-4BB9-9A3E-BDB30805D9C9</string>
<key>com.schriftgestaltung.layerOrderInGlyph.space</key>
<integer>1</integer>
<key>com.schriftgestaltung.layerOrderInGlyph.zero</key>
<integer>0</integer>
</dict>
</dict>
</plist>
13 changes: 13 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/glyphs/space.glif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="space" format="2">
<advance width="600"/>
<unicode hex="0020"/>
<outline>
</outline>
<lib>
<dict>
<key>com.schriftgestaltung.Glyphs.lastChange</key>
<string>2019/07/25 10:21:14</string>
</dict>
</lib>
</glyph>
15 changes: 15 additions & 0 deletions ...ta/BracketSelfReference/BracketSelfReference-Bold.ufo/glyphs/zero.B_R_A_C_K_E_T_.500.glif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="zero.BRACKET.500" format="2">
<advance width="700"/>
<outline>
<component base="zero" xOffset="170"/>
</outline>
<lib>
<dict>
<key>com.schriftgestaltung.Glyphs._originalLayerName</key>
<string>Bold [500]</string>
<key>com.schriftgestaltung.Glyphs.lastChange</key>
<string>2021/03/31 15:27:05</string>
</dict>
</lib>
</glyph>
27 changes: 27 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/glyphs/zero.glif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="zero" format="2">
<advance width="500"/>
<unicode hex="0030"/>
<outline>
<contour>
<point x="248" y="0" type="curve" smooth="yes"/>
<point x="393" y="0"/>
<point x="510" y="157"/>
<point x="510" y="350" type="curve" smooth="yes"/>
<point x="510" y="543"/>
<point x="393" y="700"/>
<point x="248" y="700" type="curve" smooth="yes"/>
<point x="103" y="700"/>
<point x="-14" y="543"/>
<point x="-14" y="350" type="curve" smooth="yes"/>
<point x="-14" y="157"/>
<point x="103" y="0"/>
</contour>
</outline>
<lib>
<dict>
<key>com.schriftgestaltung.Glyphs.lastChange</key>
<string>2021/03/31 15:27:05</string>
</dict>
</lib>
</glyph>
10 changes: 10 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/layercontents.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<array>
<array>
<string>public.default</string>
<string>glyphs</string>
</array>
</array>
</plist>
67 changes: 67 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/lib.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.schriftgestaltung.appVersion</key>
<string>1352</string>
<key>com.schriftgestaltung.customParameter.GSFont.Axes</key>
<array>
<dict>
<key>Name</key>
<string>Weight</string>
<key>Tag</key>
<string>wght</string>
</dict>
<dict>
<key>Name</key>
<string>Width</string>
<key>Tag</key>
<string>wdth</string>
</dict>
</array>
<key>com.schriftgestaltung.customParameter.GSFont.DisplayStrings</key>
<array>
<string></string>
<string>0</string>
<string></string>
<string></string>
</array>
<key>com.schriftgestaltung.customParameter.GSFont.disablesAutomaticAlignment</key>
<false/>
<key>com.schriftgestaltung.customParameter.GSFont.useNiceNames</key>
<integer>1</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.customValue</key>
<integer>0</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.customValue1</key>
<integer>0</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.customValue2</key>
<integer>0</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.customValue3</key>
<integer>0</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.iconName</key>
<string></string>
<key>com.schriftgestaltung.customParameter.GSFontMaster.weightValue</key>
<integer>700</integer>
<key>com.schriftgestaltung.customParameter.GSFontMaster.widthValue</key>
<integer>100</integer>
<key>com.schriftgestaltung.fontMasterID</key>
<string>48798C20-C9A0-4BB9-9A3E-BDB30805D9C9</string>
<key>com.schriftgestaltung.fontMasterOrder</key>
<integer>1</integer>
<key>com.schriftgestaltung.keyboardIncrement</key>
<integer>1</integer>
<key>com.schriftgestaltung.weight</key>
<string>Bold</string>
<key>com.schriftgestaltung.weightValue</key>
<integer>700</integer>
<key>com.schriftgestaltung.width</key>
<string>Regular</string>
<key>com.schriftgestaltung.widthValue</key>
<integer>100</integer>
<key>public.glyphOrder</key>
<array>
<string>zero</string>
<string>space</string>
</array>
</dict>
</plist>
10 changes: 10 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Bold.ufo/metainfo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>creator</key>
<string>com.github.fonttools.ufoLib</string>
<key>formatVersion</key>
<integer>3</integer>
</dict>
</plist>
36 changes: 36 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Regular.ufo/fontinfo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>ascender</key>
<integer>800</integer>
<key>capHeight</key>
<integer>700</integer>
<key>descender</key>
<integer>-200</integer>
<key>familyName</key>
<string>BracketSelfReference</string>
<key>italicAngle</key>
<integer>0</integer>
<key>openTypeHeadCreated</key>
<string>2019/07/25 10:20:06</string>
<key>openTypeOS2Type</key>
<array>
<integer>3</integer>
</array>
<key>postscriptUnderlinePosition</key>
<integer>-100</integer>
<key>postscriptUnderlineThickness</key>
<integer>50</integer>
<key>styleName</key>
<string>Regular</string>
<key>unitsPerEm</key>
<integer>1000</integer>
<key>versionMajor</key>
<integer>1</integer>
<key>versionMinor</key>
<integer>0</integer>
<key>xHeight</key>
<integer>500</integer>
</dict>
</plist>
12 changes: 12 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Regular.ufo/glyphs/contents.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>space</key>
<string>space.glif</string>
<key>zero</key>
<string>zero.glif</string>
<key>zero.BRACKET.500</key>
<string>zero.B_R_A_C_K_E_T_.500.glif</string>
</dict>
</plist>
15 changes: 15 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Regular.ufo/glyphs/layerinfo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>lib</key>
<dict>
<key>com.schriftgestaltung.layerId</key>
<string>AC80BBED-896D-4DAA-A852-C79F9201ACE8</string>
<key>com.schriftgestaltung.layerOrderInGlyph.space</key>
<integer>0</integer>
<key>com.schriftgestaltung.layerOrderInGlyph.zero</key>
<integer>1</integer>
</dict>
</dict>
</plist>
13 changes: 13 additions & 0 deletions tests/data/BracketSelfReference/BracketSelfReference-Regular.ufo/glyphs/space.glif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="space" format="2">
<advance width="600"/>
<unicode hex="0020"/>
<outline>
</outline>
<lib>
<dict>
<key>com.schriftgestaltung.Glyphs.lastChange</key>
<string>2019/07/25 10:21:14</string>
</dict>
</lib>
</glyph>
15 changes: 15 additions & 0 deletions ...BracketSelfReference/BracketSelfReference-Regular.ufo/glyphs/zero.B_R_A_C_K_E_T_.500.glif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="zero.BRACKET.500" format="2">
<advance width="700"/>
<outline>
<component base="zero" xOffset="160"/>
</outline>
<lib>
<dict>
<key>com.schriftgestaltung.Glyphs._originalLayerName</key>
<string>Regular [500]</string>
<key>com.schriftgestaltung.Glyphs.lastChange</key>
<string>2021/03/31 15:27:05</string>
</dict>
</lib>
</glyph>
Loading