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

Disable JSON structure when using fontc #1067

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
9 changes: 7 additions & 2 deletions Lib/gftools/builder/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def edge_with_operation(node, operation):
class GFBuilder:
config: dict
recipe: Recipe
fontc_args: FontcArgs
simoncozens marked this conversation as resolved.
Show resolved Hide resolved

def __init__(
self,
Expand All @@ -60,6 +61,7 @@ def __init__(
else:
self._orig_config = yaml.dump(config)
self.config = config
self.fontc_args = fontc_args
fontc_args.modify_config(self.config)

self.known_operations = OperationRegistry(use_fontc=fontc_args.use_fontc)
Expand Down Expand Up @@ -150,14 +152,17 @@ def glyphs_to_ufo(self, source):
glyph_data = self.config.get("glyphData")
if glyph_data is not None:
glyph_data = glyph_data
ufo_structure = "json"
if self.fontc_args.use_fontc:
ufo_structure = "package"
FontProject().run_from_glyphs(
str(source.resolve()),
**{
"output": ["ufo"],
"output_dir": directory,
"master_dir": directory,
"designspace_path": output,
"ufo_structure": "json",
"ufo_structure": ufo_structure,
"glyph_data": glyph_data,
},
)
Expand Down Expand Up @@ -338,7 +343,7 @@ def draw_graph(self):
import pydot

dot = subprocess.run(
["ninja", "-t", "graph", "-f", self.ninja_file_name], capture_output=True
["ninja", "-f", self.ninja_file_name, "-t", "graph"], capture_output=True
)
graphs = pydot.graph_from_dot_data(dot.stdout.decode("utf-8"))
targets = self.recipe.keys()
Expand Down
5 changes: 1 addition & 4 deletions Lib/gftools/builder/fontc.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,10 @@ def modify_config(self, config: dict):
extra_args = config.get("extraFontmakeArgs") or ""
extra_args += " --no-production-names --drop-implied-oncurves"
config["extraFontmakeArgs"] = extra_args
# override config to turn not build instances if we're variable
if self.will_build_variable_font(config):
config["buildStatic"] = False
Copy link
Member

Choose a reason for hiding this comment

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

Are you accomplishing this another way, or are you building statics? The current fontc mode is very much limited in focus to supporting the crater project, which lets us run both compilers and compare outputs, and I would like to avoid doing extra work in that case because it already takes quite a long time to run.

Maybe we could keep this, but have it be gated on the --fontc-experimental-single-source argument?

# if the font doesn't explicitly request CFF, just build TT outlines
# if the font _only_ wants CFF outlines, we will try to build them
# ( but fail on fontc for now) (but is this even a thing?)
elif config.get("buildTTF", True):
if config.get("buildTTF", True):
config["buildOTF"] = False
if self.simple_output_path:
output_dir = str(self.simple_output_path)
Expand Down
10 changes: 10 additions & 0 deletions Lib/gftools/builder/operations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ def get(self, operation_name: str):

return FontcBuildOTF

if operation_name == "addSubset":
from .fontc.fontcAddSubset import FontcAddSubset

return FontcAddSubset

if operation_name == "instantiateUfo":
from .fontc.fontcInstantiateUfo import FontcInstantiateUFO

return FontcInstantiateUFO

return self.known_operations.get(operation_name)


Expand Down
43 changes: 43 additions & 0 deletions Lib/gftools/builder/operations/fontc/fontcAddSubset.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import os
from tempfile import NamedTemporaryFile, TemporaryDirectory

import yaml

from gftools.builder.file import File
from gftools.builder.operations import OperationBase


class FontcAddSubset(OperationBase):
description = "Add a subset from another font"
rule = "gftools-add-ds-subsets $args -y $yaml -o $out $in"

def validate(self):
# Ensure there is a new name
if not self.first_source.is_font_source:
raise ValueError("%s is not a font source file" % self.first_source)
if "subsets" not in self.original:
raise ValueError("No subsets defined")

def convert_dependencies(self, graph):
self._target = TemporaryDirectory() # Stow object
self._orig = NamedTemporaryFile(delete=False, mode="w")
yaml.dump(self.original["subsets"], self._orig)
self._orig.close()

@property
def targets(self):
if "directory" in self.original:
target = self.original["directory"]
else:
target = self._target.name
dspath = os.path.join(
target, self.first_source.basename.rsplit(".", 1)[0] + ".designspace"
)
return [File(dspath)]

@property
def variables(self):
return {
"yaml": self._orig.name,
"args": self.original.get("args"),
}
70 changes: 70 additions & 0 deletions Lib/gftools/builder/operations/fontc/fontcInstantiateUfo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from pathlib import Path
from typing import List
from gftools.builder.file import File
from gftools.builder.operations import FontmakeOperationBase
import glyphsLib
import os
import gftools.builder
from functools import cached_property
from glyphsLib.builder import UFOBuilder
from ninja.ninja_syntax import Writer, escape_path
from fontTools.designspaceLib import InstanceDescriptor


class FontcInstantiateUFO(FontmakeOperationBase):
description = "Create instance UFOs from a Glyphs or designspace file"
rule = 'fontmake -i "$instance_name" -o ufo $fontmake_type $in $args'

def validate(self):
# Ensure there is an instance name
if "instance_name" not in self.original:
raise ValueError("No instance name specified")
# Ensure the instance is defined in the font
desired = self.original["instance_name"]
if "target" not in self.original and not self.relevant_instance:
raise ValueError(
f"Instance {desired} not found in {self.first_source.path}"
)

@cached_property
def relevant_instance(self):
desired = self.original["instance_name"]
relevant_instance = [
i
for i in self.first_source.instances
if i.name == desired or i.familyName + " " + i.styleName == desired
]
if len(relevant_instance) == 0:
return None
return relevant_instance[0]

@property
def instance_dir(self):
return Path(self.first_source.path).parent / "instance_ufos"

@property
def targets(self):
if "target" in self.original:
return [File(self.original["target"])]
instance = self.relevant_instance
assert instance is not None
assert instance.filename is not None
# if self.first_source.is_glyphs:
return [File(str(self.instance_dir / (os.path.basename(instance.filename))))]
# return [ File(instance.filename) ]

@property
def variables(self):
vars = super().variables
if self.first_source.is_glyphs:
vars["args"] += f"--instance-dir {escape_path(str(self.instance_dir))}"
else:
vars["args"] += f"--output-dir {escape_path(str(self.instance_dir))}"
vars["instance_name"] = self.original["instance_name"]
if self.original.get("glyphData") is not None:
for glyphData in self.original["glyphData"]:
vars["args"] += f" --glyph-data {escape_path(glyphData)}"
return vars

def set_target(self, target: File):
raise ValueError("Cannot set target on InstantiateUFO")
3 changes: 1 addition & 2 deletions Lib/gftools/builder/recipeproviders/googlefonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ def build_a_static(self, source: File, instance: InstanceDescriptor, output):
steps = [
{"source": source.path},
]
# if we're running fontc we skip conversion to UFO
if not source.is_ufo and not self.config.get("use_fontc", False):
simoncozens marked this conversation as resolved.
Show resolved Hide resolved
if not source.is_ufo:
instancename = instance.name
if instancename is None:
if not instance.familyName or not instance.styleName:
Expand Down
2 changes: 1 addition & 1 deletion Lib/gftools/builder/recipeproviders/noto.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ def build_a_static(self, source, instance, output):
"instance_name": instance.name,
"target": "full-designspace/instance_ufos/"
+ os.path.basename(instance.filename)
+ ".json",
+ ("" if self.builder.fontc_args.use_fontc else ".json"),
simoncozens marked this conversation as resolved.
Show resolved Hide resolved
},
{
"operation": "buildTTF" if output == "ttf" else "buildOTF",
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,6 @@ extend-exclude = '''
.*_pb2.py # exclude autogenerated Protocol Buffer files anywhere in the project
)
'''

[tool.pytest.ini_options]
filterwarnings = 'ignore::DeprecationWarning'