From 938b29b96c6c06dcd7b0db4d116df24e890fca9d Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Sun, 27 Oct 2024 14:49:50 -0400 Subject: [PATCH 01/16] FIXUP: add File.is_variable --- Lib/gftools/builder/file.py | 6 ++++++ Lib/gftools/builder/recipeproviders/googlefonts.py | 6 +----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Lib/gftools/builder/file.py b/Lib/gftools/builder/file.py index 7191821a1..1d0b2d0b7 100644 --- a/Lib/gftools/builder/file.py +++ b/Lib/gftools/builder/file.py @@ -47,6 +47,12 @@ def is_designspace(self): def is_font_source(self): return self.is_glyphs or self.is_ufo or self.is_designspace + @cached_property + def is_variable(self) -> bool: + return (self.is_glyphs and len(self.gsfont.masters) > 1) or ( + self.is_designspace and len(self.designspace.sources) > 1 + ) + @cached_property def gsfont(self): if self.is_glyphs: diff --git a/Lib/gftools/builder/recipeproviders/googlefonts.py b/Lib/gftools/builder/recipeproviders/googlefonts.py index 4ab8a4961..dd8240d29 100644 --- a/Lib/gftools/builder/recipeproviders/googlefonts.py +++ b/Lib/gftools/builder/recipeproviders/googlefonts.py @@ -227,11 +227,7 @@ def build_all_variables(self): if not self.config.get("buildVariable", True): return for source in self.sources: - if ( - (source.is_glyphs and len(source.gsfont.masters) < 2) - or source.is_ufo - or (source.is_designspace and len(source.designspace.sources) < 2) - ): + if not source.is_variable: continue italic_ds = None if self.config["splitItalic"]: From 1b44e76e2a79ec73e286573e4831ee675a5bcae2 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Mon, 14 Oct 2024 16:29:12 -0400 Subject: [PATCH 02/16] [fontc] Initial support for fontc mode In the presence of a new CLI flag, the builder will now override certain operations so that they are driven by fontc. --- Lib/gftools/builder/__init__.py | 25 +++++++-- Lib/gftools/builder/operations/__init__.py | 53 ++++++++++++++----- .../builder/operations/fontc/__init__.py | 0 .../builder/operations/fontc/fontcBuildTTF.py | 6 +++ .../operations/fontc/fontcBuildVariable.py | 6 +++ 5 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 Lib/gftools/builder/operations/fontc/__init__.py create mode 100644 Lib/gftools/builder/operations/fontc/fontcBuildTTF.py create mode 100644 Lib/gftools/builder/operations/fontc/fontcBuildVariable.py diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index afdb9f20f..8ce88ae08 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -17,7 +17,7 @@ from ninja.ninja_syntax import Writer, escape_path from gftools.builder.file import File -from gftools.builder.operations import OperationBase, known_operations +from gftools.builder.operations import OperationBase, OperationRegistry from gftools.builder.operations.copy import Copy from gftools.builder.recipeproviders import get_provider from gftools.builder.schema import BASE_SCHEMA @@ -36,7 +36,7 @@ class GFBuilder: config: dict recipe: Recipe - def __init__(self, config: Union[dict, str]): + def __init__(self, config: Union[dict, str], use_fontc=False): if isinstance(config, str): parentpath = Path(config).resolve().parent with open(config, "r") as file: @@ -55,6 +55,14 @@ def __init__(self, config: Union[dict, str]): self._orig_config = yaml.dump(config) self.config = config + # TODO(colin) we also want to suppress instancing when running fontmake + # if we're using it to compare with fontc + if use_fontc: + # override config to turn not build instances if we're variable + if self.config.get("buildVariable", True): + self.config["buildStatic"] = False + + self.known_operations = OperationRegistry(use_fontc=use_fontc) self.writer = Writer(open("build.ninja", "w")) self.named_files = {} self.used_operations = set([]) @@ -156,9 +164,10 @@ def glyphs_to_ufo(self, source): def operation_step_to_object(self, step): operation = step.get("operation") or step.get("postprocess") - if operation not in known_operations: + cls = self.known_operations.get(operation) + if cls is None: raise ValueError(f"Unknown operation {operation}") - cls = known_operations[operation] + if operation not in self.used_operations: self.used_operations.add(operation) cls.write_rules(self.writer) @@ -381,6 +390,12 @@ def main(args=None): help="Just generate and output recipe from recipe builder", action="store_true", ) + parser.add_argument( + "--experimental-fontc", + help="Use fontc instead of fontmake", + action="store_true", + ) + parser.add_argument("config", help="Path to config file or source file", nargs="+") args = parser.parse_args(args) yaml_files = [] @@ -404,7 +419,7 @@ def main(args=None): raise ValueError("Only one config file can be given for now") config = args.config[0] - pd = GFBuilder(config) + pd = GFBuilder(config, use_fontc=args.experimental_fontc) if args.generate: config = pd.config config["recipe"] = pd.recipe diff --git a/Lib/gftools/builder/operations/__init__.py b/Lib/gftools/builder/operations/__init__.py index 0b0330735..7f5a2265b 100644 --- a/Lib/gftools/builder/operations/__init__.py +++ b/Lib/gftools/builder/operations/__init__.py @@ -8,6 +8,7 @@ import sys from os.path import dirname from tempfile import NamedTemporaryFile +from typing import Dict from gftools.builder.file import File from gftools.utils import shell_quote @@ -150,17 +151,43 @@ def variables(self): return vars -known_operations = {} +class OperationRegistry: + def __init__(self, use_fontc: bool): + self.known_operations = get_known_operations() + self.use_fontc = use_fontc -for mod in pkgutil.iter_modules([dirname(__file__)]): - imp = importlib.import_module("gftools.builder.operations." + mod.name) - classes = [ - (name, cls) - for name, cls in inspect.getmembers(sys.modules[imp.__name__], inspect.isclass) - if "OperationBase" not in name and issubclass(cls, OperationBase) - ] - if len(classes) > 1: - raise ValueError( - f"Too many classes in module gftools.builder.operations.{mod.name}" - ) - known_operations[mod.name] = classes[0][1] + def get(self, operation_name: str): + if self.use_fontc: + if operation_name == "buildVariable": + # if we import this at the top level it's a circular import error + from .fontc.fontcBuildVariable import FontcBuildVariable + + return FontcBuildVariable + if operation_name == "buildTTF": + from .fontc.fontcBuildTTF import FontcBuildTTF + + return FontcBuildTTF + + return self.known_operations.get(operation_name) + + +def get_known_operations() -> Dict[str, OperationBase]: + known_operations = {} + + for mod in pkgutil.iter_modules([dirname(__file__)]): + if "fontc" in mod.name: + continue + imp = importlib.import_module("gftools.builder.operations." + mod.name) + classes = [ + (name, cls) + for name, cls in inspect.getmembers( + sys.modules[imp.__name__], inspect.isclass + ) + if "OperationBase" not in name and issubclass(cls, OperationBase) + ] + if len(classes) > 1: + raise ValueError( + f"Too many classes in module gftools.builder.operations.{mod.name}" + ) + known_operations[mod.name] = classes[0][1] + return known_operations diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py new file mode 100644 index 000000000..d7de4a932 --- /dev/null +++ b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py @@ -0,0 +1,6 @@ +from gftools.builder.operations import OperationBase + + +class FontcBuildTTF(OperationBase): + description = "Build a TTF from a source file (with fontc)" + rule = "fontc -o $out $in" diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py new file mode 100644 index 000000000..92fd8e4ac --- /dev/null +++ b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py @@ -0,0 +1,6 @@ +from gftools.builder.operations import OperationBase + + +class FontcBuildVariable(OperationBase): + description = "Build a variable font from a source file (with fontc)" + rule = "fontc -o $out $in" From d28dada5a5f155e9b523a221f0ffac4be997674c Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 16 Oct 2024 10:42:37 -0400 Subject: [PATCH 03/16] [fontc] Rewrite fontmake args for fontc --- .../builder/operations/fontc/__init__.py | 44 +++++++++++++++++++ .../builder/operations/fontc/fontcBuildTTF.py | 6 +-- .../operations/fontc/fontcBuildVariable.py | 6 +-- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py index e69de29bb..2a6744b63 100644 --- a/Lib/gftools/builder/operations/fontc/__init__.py +++ b/Lib/gftools/builder/operations/fontc/__init__.py @@ -0,0 +1,44 @@ +from typing import List +from gftools.builder.operations import OperationBase + + +class FontcOperationBase(OperationBase): + @property + def variables(self): + vars = super().variables + args = vars.get("args") + if args: + vars["args"] = rewrite_fontmake_args_for_fontc(args) + + return vars + + +def rewrite_fontmake_args_for_fontc(args: str) -> str: + out_args = [] + arg_list = args.split() + # reverse so we can pop in order + arg_list.reverse() + while arg_list: + out_args.append(rewrite_one_arg(arg_list)) + return " ".join(out_args) + + +# remove next arg from the front of the list and return its fontc equivalent +def rewrite_one_arg(args: List[str]) -> str: + next_ = args.pop() + if next_ == "--filter": + filter_ = args.pop() + # this means 'retain filters defined in UFO', which... do we even support + # that in fontc? + if filter_ == "...": + pass + elif filter_ == "FlattenComponentsFilter": + return "--flatten-components" + elif filter_ == "DecomposeTransformedComponentsFilter": + return "--decompose-transformed-components" + else: + # glue the filter back together for better reporting below + next_ = f"{next_} {filter_}" + else: + raise ValueError(f"unknown fontmake arg '{next_}'") + return "" diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py index d7de4a932..96a6e268f 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py @@ -1,6 +1,6 @@ -from gftools.builder.operations import OperationBase +from gftools.builder.operations.fontc import FontcOperationBase -class FontcBuildTTF(OperationBase): +class FontcBuildTTF(FontcOperationBase): description = "Build a TTF from a source file (with fontc)" - rule = "fontc -o $out $in" + rule = "fontc -o $out $in $args" diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py index 92fd8e4ac..fddc833c9 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py @@ -1,6 +1,6 @@ -from gftools.builder.operations import OperationBase +from gftools.builder.operations.fontc import FontcOperationBase -class FontcBuildVariable(OperationBase): +class FontcBuildVariable(FontcOperationBase): description = "Build a variable font from a source file (with fontc)" - rule = "fontc -o $out $in" + rule = "fontc -o $out $in $args" From 365451b6e2bd2166eb12aeab308b510611fe9516 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 16 Oct 2024 11:36:17 -0400 Subject: [PATCH 04/16] [fontc] Add dummy otf builder --- Lib/gftools/builder/__init__.py | 6 ++++++ Lib/gftools/builder/operations/__init__.py | 4 +++- Lib/gftools/builder/operations/fontc/fontcBuildOTF.py | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 Lib/gftools/builder/operations/fontc/fontcBuildOTF.py diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index 8ce88ae08..a2c58fafd 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -58,9 +58,15 @@ def __init__(self, config: Union[dict, str], use_fontc=False): # TODO(colin) we also want to suppress instancing when running fontmake # if we're using it to compare with fontc if use_fontc: + self.config["buildWebfont"] = False # override config to turn not build instances if we're variable if self.config.get("buildVariable", True): self.config["buildStatic"] = False + # if the font doesn't explicitly request CFF, just built TT outlines + # if the font _only_ wants CFF outlines, we will try to build them + # ( but fail on fontc for now ) + elif self.config.get("buildTTF", True): + self.config["buildOTF"] = False self.known_operations = OperationRegistry(use_fontc=use_fontc) self.writer = Writer(open("build.ninja", "w")) diff --git a/Lib/gftools/builder/operations/__init__.py b/Lib/gftools/builder/operations/__init__.py index 7f5a2265b..5cd7da85e 100644 --- a/Lib/gftools/builder/operations/__init__.py +++ b/Lib/gftools/builder/operations/__init__.py @@ -166,7 +166,9 @@ def get(self, operation_name: str): if operation_name == "buildTTF": from .fontc.fontcBuildTTF import FontcBuildTTF - return FontcBuildTTF + if operation_name == "buildOTF": + from .fontc.fontcBuildOTF import FontcBuildOTF + return FontcBuildOTF return self.known_operations.get(operation_name) diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py new file mode 100644 index 000000000..732d5feb0 --- /dev/null +++ b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py @@ -0,0 +1,8 @@ +from gftools.builder.operations.fontc import FontcOperationBase + + +class FontcBuildTTF(FontcOperationBase): + description = "Build an OTF from a source file (with fontc)" + # the '--cff-outlines' flag does not exit in fontc, so this will + # error, which we want + rule = "fontc -o $out $in $args --cff-outlines" From b3723ae9e743c31ed9073230d437ed9f86b908ee Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 16 Oct 2024 11:54:29 -0400 Subject: [PATCH 05/16] [fontc] Skip instantiateUfo if running fontc --- Lib/gftools/builder/__init__.py | 2 ++ Lib/gftools/builder/recipeproviders/googlefonts.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index a2c58fafd..ebb28045b 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -58,6 +58,8 @@ def __init__(self, config: Union[dict, str], use_fontc=False): # TODO(colin) we also want to suppress instancing when running fontmake # if we're using it to compare with fontc if use_fontc: + # we stash this flag here to pass it down to the recipe provider + self.config["use_fontc"] = use_fontc self.config["buildWebfont"] = False # override config to turn not build instances if we're variable if self.config.get("buildVariable", True): diff --git a/Lib/gftools/builder/recipeproviders/googlefonts.py b/Lib/gftools/builder/recipeproviders/googlefonts.py index dd8240d29..7b68ffc37 100644 --- a/Lib/gftools/builder/recipeproviders/googlefonts.py +++ b/Lib/gftools/builder/recipeproviders/googlefonts.py @@ -323,7 +323,8 @@ def build_a_static(self, source: File, instance: InstanceDescriptor, output): steps = [ {"source": source.path}, ] - if not source.is_ufo: + # if we're running fontc we skip conversion to UFO + if not source.is_ufo and not self.config.get('use_fontc', False): instancename = instance.name if instancename is None: if not instance.familyName or not instance.styleName: From bea9f76068ef3c1cedb6962ee9cae3d182c8c72f Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 16 Oct 2024 12:00:01 -0400 Subject: [PATCH 06/16] [fontc] Add --experimental-simple-output cli option This does two things: - it takes a path, where all our final build products will be saved - it reduces the set of build targets to match what we would build with fontc --- Lib/gftools/builder/__init__.py | 38 +++++++++++++++---- Lib/gftools/builder/operations/__init__.py | 1 + .../builder/recipeproviders/googlefonts.py | 2 +- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index ebb28045b..b741765b8 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -15,6 +15,7 @@ from fontmake.font_project import FontProject from ninja import _program from ninja.ninja_syntax import Writer, escape_path +from typing import Union from gftools.builder.file import File from gftools.builder.operations import OperationBase, OperationRegistry @@ -36,7 +37,12 @@ class GFBuilder: config: dict recipe: Recipe - def __init__(self, config: Union[dict, str], use_fontc=False): + def __init__( + self, + config: Union[dict, str], + use_fontc=False, + simple_outputs=Union[Path, None], + ): if isinstance(config, str): parentpath = Path(config).resolve().parent with open(config, "r") as file: @@ -55,20 +61,24 @@ def __init__(self, config: Union[dict, str], use_fontc=False): self._orig_config = yaml.dump(config) self.config = config - # TODO(colin) we also want to suppress instancing when running fontmake - # if we're using it to compare with fontc - if use_fontc: + if use_fontc or simple_outputs: # we stash this flag here to pass it down to the recipe provider self.config["use_fontc"] = use_fontc self.config["buildWebfont"] = False + self.config["buildSmallCap"] = False # override config to turn not build instances if we're variable if self.config.get("buildVariable", True): self.config["buildStatic"] = False - # if the font doesn't explicitly request CFF, just built TT outlines + # 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 fail on fontc for now) (but is this even a thing?) elif self.config.get("buildTTF", True): self.config["buildOTF"] = False + if simple_outputs: + # we dump everything into one dir in this case + self.config["outputDir"] = str(simple_outputs) + self.config["ttDir"] = str(simple_outputs) + self.config["otDir"] = str(simple_outputs) self.known_operations = OperationRegistry(use_fontc=use_fontc) self.writer = Writer(open("build.ninja", "w")) @@ -175,7 +185,6 @@ def operation_step_to_object(self, step): cls = self.known_operations.get(operation) if cls is None: raise ValueError(f"Unknown operation {operation}") - if operation not in self.used_operations: self.used_operations.add(operation) cls.write_rules(self.writer) @@ -404,8 +413,17 @@ def main(args=None): action="store_true", ) + parser.add_argument( + "--experimental-simple-output", + help="generate a reduced set of targets, and copy them to the provided directory", + type=Path, + ) + parser.add_argument("config", help="Path to config file or source file", nargs="+") args = parser.parse_args(args) + if args.experimental_simple_output: + # get the abs path because we use cwd later and relative paths will break + args.experimental_simple_output = args.experimental_simple_output.absolute() yaml_files = [] source_files = [] for config in args.config: @@ -427,7 +445,11 @@ def main(args=None): raise ValueError("Only one config file can be given for now") config = args.config[0] - pd = GFBuilder(config, use_fontc=args.experimental_fontc) + pd = GFBuilder( + config, + use_fontc=args.experimental_fontc, + simple_outputs=args.experimental_simple_output, + ) if args.generate: config = pd.config config["recipe"] = pd.recipe diff --git a/Lib/gftools/builder/operations/__init__.py b/Lib/gftools/builder/operations/__init__.py index 5cd7da85e..029bc81c0 100644 --- a/Lib/gftools/builder/operations/__init__.py +++ b/Lib/gftools/builder/operations/__init__.py @@ -168,6 +168,7 @@ def get(self, operation_name: str): if operation_name == "buildOTF": from .fontc.fontcBuildOTF import FontcBuildOTF + return FontcBuildOTF return self.known_operations.get(operation_name) diff --git a/Lib/gftools/builder/recipeproviders/googlefonts.py b/Lib/gftools/builder/recipeproviders/googlefonts.py index 7b68ffc37..38b2e7d43 100644 --- a/Lib/gftools/builder/recipeproviders/googlefonts.py +++ b/Lib/gftools/builder/recipeproviders/googlefonts.py @@ -324,7 +324,7 @@ def build_a_static(self, source: File, instance: InstanceDescriptor, output): {"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): + if not source.is_ufo and not self.config.get("use_fontc", False): instancename = instance.name if instancename is None: if not instance.familyName or not instance.styleName: From 4a59df718e6f807de3cec12e38fb6857afd142cd Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Fri, 25 Oct 2024 13:28:14 -0400 Subject: [PATCH 07/16] [fontc] Pass path to fontc executable via CLI this requires an ugly global in order to expose this to the relevant operations, but that was the best solution I could come up with. --- Lib/gftools/builder/__init__.py | 9 +++++++-- Lib/gftools/builder/operations/fontc/__init__.py | 11 +++++++++++ Lib/gftools/builder/operations/fontc/fontcBuildOTF.py | 2 +- Lib/gftools/builder/operations/fontc/fontcBuildTTF.py | 2 +- .../builder/operations/fontc/fontcBuildVariable.py | 2 +- 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index b741765b8..80d656acc 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -9,6 +9,7 @@ from tempfile import NamedTemporaryFile, gettempdir from typing import Any, Dict, List, Union +from gftools.builder.operations.fontc import set_global_fontc_path import networkx as nx import strictyaml import yaml @@ -409,8 +410,8 @@ def main(args=None): ) parser.add_argument( "--experimental-fontc", - help="Use fontc instead of fontmake", - action="store_true", + help=f"Use fontc instead of fontmake. Argument is path to the fontc executable", + type=Path, ) parser.add_argument( @@ -424,6 +425,10 @@ def main(args=None): if args.experimental_simple_output: # get the abs path because we use cwd later and relative paths will break args.experimental_simple_output = args.experimental_simple_output.absolute() + if args.experimental_fontc: + fontc_path = Path(args.experimental_fontc).resolve() + assert fontc_path.is_file(), f"{fontc_path} is not a file" + set_global_fontc_path(Path(fontc_path)) yaml_files = [] source_files = [] for config in args.config: diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py index 2a6744b63..983d41bcc 100644 --- a/Lib/gftools/builder/operations/fontc/__init__.py +++ b/Lib/gftools/builder/operations/fontc/__init__.py @@ -1,11 +1,22 @@ +from pathlib import Path from typing import List from gftools.builder.operations import OperationBase +FONTC_PATH = "FONTC_PATH_NOT_SET" + + +# should only be called once, from main, before doing anything else. This is a +# relatively non-invasive way to smuggle this value into FontcOperationBase +def set_global_fontc_path(path: Path): + global FONTC_PATH + FONTC_PATH = path + class FontcOperationBase(OperationBase): @property def variables(self): vars = super().variables + vars["fontc_path"] = FONTC_PATH args = vars.get("args") if args: vars["args"] = rewrite_fontmake_args_for_fontc(args) diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py index 732d5feb0..cf4834db1 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py @@ -5,4 +5,4 @@ class FontcBuildTTF(FontcOperationBase): description = "Build an OTF from a source file (with fontc)" # the '--cff-outlines' flag does not exit in fontc, so this will # error, which we want - rule = "fontc -o $out $in $args --cff-outlines" + rule = "$fontc_path -o $out $in $args --cff-outlines" diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py index 96a6e268f..a28c450b5 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildTTF.py @@ -3,4 +3,4 @@ class FontcBuildTTF(FontcOperationBase): description = "Build a TTF from a source file (with fontc)" - rule = "fontc -o $out $in $args" + rule = "$fontc_path -o $out $in $args" diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py index fddc833c9..abe725046 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildVariable.py @@ -3,4 +3,4 @@ class FontcBuildVariable(FontcOperationBase): description = "Build a variable font from a source file (with fontc)" - rule = "fontc -o $out $in $args" + rule = f"$fontc_path -o $out $in $args" From fa3852da8783b2daf9084f22b36f80c1ade96f1e Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Fri, 25 Oct 2024 15:47:57 -0400 Subject: [PATCH 08/16] [fontc] Move fontc config logic into separate module --- Lib/gftools/builder/__init__.py | 41 ++++-------------------- Lib/gftools/builder/fontc.py | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 35 deletions(-) create mode 100644 Lib/gftools/builder/fontc.py diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index 80d656acc..931e97b26 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -9,7 +9,7 @@ from tempfile import NamedTemporaryFile, gettempdir from typing import Any, Dict, List, Union -from gftools.builder.operations.fontc import set_global_fontc_path +from gftools.builder.fontc import FontcArgs import networkx as nx import strictyaml import yaml @@ -41,8 +41,7 @@ class GFBuilder: def __init__( self, config: Union[dict, str], - use_fontc=False, - simple_outputs=Union[Path, None], + fontc_args=FontcArgs(None), ): if isinstance(config, str): parentpath = Path(config).resolve().parent @@ -61,27 +60,9 @@ def __init__( else: self._orig_config = yaml.dump(config) self.config = config + fontc_args.modify_config(self.config) - if use_fontc or simple_outputs: - # we stash this flag here to pass it down to the recipe provider - self.config["use_fontc"] = use_fontc - self.config["buildWebfont"] = False - self.config["buildSmallCap"] = False - # override config to turn not build instances if we're variable - if self.config.get("buildVariable", True): - self.config["buildStatic"] = False - # 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 self.config.get("buildTTF", True): - self.config["buildOTF"] = False - if simple_outputs: - # we dump everything into one dir in this case - self.config["outputDir"] = str(simple_outputs) - self.config["ttDir"] = str(simple_outputs) - self.config["otDir"] = str(simple_outputs) - - self.known_operations = OperationRegistry(use_fontc=use_fontc) + self.known_operations = OperationRegistry(use_fontc=fontc_args.use_fontc) self.writer = Writer(open("build.ninja", "w")) self.named_files = {} self.used_operations = set([]) @@ -422,13 +403,7 @@ def main(args=None): parser.add_argument("config", help="Path to config file or source file", nargs="+") args = parser.parse_args(args) - if args.experimental_simple_output: - # get the abs path because we use cwd later and relative paths will break - args.experimental_simple_output = args.experimental_simple_output.absolute() - if args.experimental_fontc: - fontc_path = Path(args.experimental_fontc).resolve() - assert fontc_path.is_file(), f"{fontc_path} is not a file" - set_global_fontc_path(Path(fontc_path)) + fontc_args = FontcArgs(args) yaml_files = [] source_files = [] for config in args.config: @@ -450,11 +425,7 @@ def main(args=None): raise ValueError("Only one config file can be given for now") config = args.config[0] - pd = GFBuilder( - config, - use_fontc=args.experimental_fontc, - simple_outputs=args.experimental_simple_output, - ) + pd = GFBuilder(config, fontc_args=fontc_args) if args.generate: config = pd.config config["recipe"] = pd.recipe diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py new file mode 100644 index 000000000..9a717126b --- /dev/null +++ b/Lib/gftools/builder/fontc.py @@ -0,0 +1,57 @@ +"""functionality for running fontc via gftools + +This mostly exists so that we can keep as much of the fontc logic in one place, +and not need to dirty up anything else. +""" + +from argparse import Namespace +from pathlib import Path +from typing import Union + +from gftools.builder.operations.fontc import set_global_fontc_path + + +class FontcArgs: + simple_output_path: Union[Path, None] + fontc_bin_path: Union[Path, None] + + # init with 'None' returns a default obj where everything is None + def __init__(self, args: Union[Namespace, None]) -> None: + if not args: + return None + self.simple_output_path = abspath(args.experimental_simple_output) + self.fontc_bin_path = abspath(args.experimental_fontc) + if self.fontc_bin_path: + if not self.fontc_bin_path.is_file(): + raise ValueError(f"fontc does not exist at {self.fontc_bin_path}") + set_global_fontc_path(self.fontc_bin_path) + + @property + def use_fontc(self) -> bool: + return self.fontc_bin_path is not None + + # update the config dictionary based on our special needs + def modify_config(self, config: dict): + if self.fontc_bin_path or self.simple_output_path: + # we stash this flag here to pass it down to the recipe provider + config["use_fontc"] = self.fontc_bin_path + config["buildWebfont"] = False + config["buildSmallCap"] = False + # override config to turn not build instances if we're variable + if config.get("buildVariable", True): + config["buildStatic"] = False + # 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): + config["buildOTF"] = False + if self.simple_output_path: + output_dir = str(self.simple_output_path) + # we dump everything into one dir in this case + config["outputDir"] = str(output_dir) + config["ttDir"] = str(output_dir) + config["otDir"] = str(output_dir) + + +def abspath(path: Union[Path, None]) -> Union[Path, None]: + return path.resolve() if path else None From b40e0b326f8aab1257a7a5d1b40ffc3331024a86 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Fri, 25 Oct 2024 15:58:29 -0400 Subject: [PATCH 09/16] [fontc] Add --experimental-single-source flag This instructs gftools to only build the single named source file. --- Lib/gftools/builder/__init__.py | 8 +++++++- Lib/gftools/builder/fontc.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index 931e97b26..9eca24080 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -7,7 +7,7 @@ from os import chdir from pathlib import Path from tempfile import NamedTemporaryFile, gettempdir -from typing import Any, Dict, List, Union +from typing import Any, Dict, List, Tuple, Union from gftools.builder.fontc import FontcArgs import networkx as nx @@ -401,6 +401,12 @@ def main(args=None): type=Path, ) + parser.add_argument( + "--experimental-single-source", + help="only compile the single named source file", + type=str, + ) + parser.add_argument("config", help="Path to config file or source file", nargs="+") args = parser.parse_args(args) fontc_args = FontcArgs(args) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index 9a717126b..0db9c3400 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -14,6 +14,7 @@ class FontcArgs: simple_output_path: Union[Path, None] fontc_bin_path: Union[Path, None] + single_source: Union[str, None] # init with 'None' returns a default obj where everything is None def __init__(self, args: Union[Namespace, None]) -> None: @@ -21,6 +22,7 @@ def __init__(self, args: Union[Namespace, None]) -> None: return None self.simple_output_path = abspath(args.experimental_simple_output) self.fontc_bin_path = abspath(args.experimental_fontc) + self.single_source = args.experimental_single_source if self.fontc_bin_path: if not self.fontc_bin_path.is_file(): raise ValueError(f"fontc does not exist at {self.fontc_bin_path}") @@ -51,6 +53,14 @@ def modify_config(self, config: dict): config["outputDir"] = str(output_dir) config["ttDir"] = str(output_dir) config["otDir"] = str(output_dir) + if self.single_source: + filtered_sources = [s for s in config["sources"] if self.single_source in s] + n_sources = len(filtered_sources) + if n_sources != 1: + raise ValueError( + f"--exerimental-single-source {self.single_source} must match exactly one of {config['sources']} (matched {n_sources}) " + ) + config["sources"] = filtered_sources def abspath(path: Union[Path, None]) -> Union[Path, None]: From 8e3c626e05a08816c6261a31b9e1916a0341260a Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Fri, 25 Oct 2024 17:17:09 -0400 Subject: [PATCH 10/16] [fontc] Code review and fixups --- Lib/gftools/builder/fontc.py | 16 +++++++++------- Lib/gftools/builder/operations/fontc/__init__.py | 9 +++++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index 0db9c3400..b96442279 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -1,7 +1,10 @@ """functionality for running fontc via gftools -This mostly exists so that we can keep as much of the fontc logic in one place, -and not need to dirty up anything else. +gftools has a few special flags that allow it to use fontc, an alternative +font compiler (https://github.com/googlefonts/fontc). + +This module exists to keep the logic related to fontc in one place, and not +dirty up everything else. """ from argparse import Namespace @@ -12,14 +15,13 @@ class FontcArgs: - simple_output_path: Union[Path, None] - fontc_bin_path: Union[Path, None] - single_source: Union[str, None] - # init with 'None' returns a default obj where everything is None def __init__(self, args: Union[Namespace, None]) -> None: if not args: - return None + self.simple_output_path = None + self.fontc_bin_path = None + self.single_source = None + return self.simple_output_path = abspath(args.experimental_simple_output) self.fontc_bin_path = abspath(args.experimental_fontc) self.single_source = args.experimental_single_source diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py index 983d41bcc..a0f06b6e5 100644 --- a/Lib/gftools/builder/operations/fontc/__init__.py +++ b/Lib/gftools/builder/operations/fontc/__init__.py @@ -2,21 +2,22 @@ from typing import List from gftools.builder.operations import OperationBase -FONTC_PATH = "FONTC_PATH_NOT_SET" +_FONTC_PATH = None # should only be called once, from main, before doing anything else. This is a # relatively non-invasive way to smuggle this value into FontcOperationBase def set_global_fontc_path(path: Path): - global FONTC_PATH - FONTC_PATH = path + global _FONTC_PATH + assert _FONTC_PATH is None, "set_global_fontc_path should only be called once" + _FONTC_PATH = path class FontcOperationBase(OperationBase): @property def variables(self): vars = super().variables - vars["fontc_path"] = FONTC_PATH + vars["fontc_path"] = _FONTC_PATH args = vars.get("args") if args: vars["args"] = rewrite_fontmake_args_for_fontc(args) From 1cf3b20496c16c0e40f50aa8130ceabd60d1d9e4 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Sun, 27 Oct 2024 14:53:07 -0400 Subject: [PATCH 11/16] [fontc] More careful check for variable source --- Lib/gftools/builder/fontc.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index b96442279..e2a92c521 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -11,6 +11,7 @@ from pathlib import Path from typing import Union +from gftools.builder.file import File from gftools.builder.operations.fontc import set_global_fontc_path @@ -36,13 +37,22 @@ def use_fontc(self) -> bool: # update the config dictionary based on our special needs def modify_config(self, config: dict): + if self.single_source: + filtered_sources = [s for s in config["sources"] if self.single_source in s] + n_sources = len(filtered_sources) + if n_sources != 1: + raise ValueError( + f"--exerimental-single-source {self.single_source} must match exactly one of {config['sources']} (matched {n_sources}) " + ) + config["sources"] = filtered_sources + if self.fontc_bin_path or self.simple_output_path: # we stash this flag here to pass it down to the recipe provider config["use_fontc"] = self.fontc_bin_path config["buildWebfont"] = False config["buildSmallCap"] = False # override config to turn not build instances if we're variable - if config.get("buildVariable", True): + if self.will_build_variable_font(config): config["buildStatic"] = False # 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 @@ -55,14 +65,14 @@ def modify_config(self, config: dict): config["outputDir"] = str(output_dir) config["ttDir"] = str(output_dir) config["otDir"] = str(output_dir) - if self.single_source: - filtered_sources = [s for s in config["sources"] if self.single_source in s] - n_sources = len(filtered_sources) - if n_sources != 1: - raise ValueError( - f"--exerimental-single-source {self.single_source} must match exactly one of {config['sources']} (matched {n_sources}) " - ) - config["sources"] = filtered_sources + + def will_build_variable_font(self, config: dict) -> bool: + # if config explicitly says dont build variable, believe it + if not config.get("buildVariable", True): + return False + + source = File(config["sources"][0]) + return source.is_variable def abspath(path: Union[Path, None]) -> Union[Path, None]: From 7a2be4d83f0631e71ffa1cbdf2a7b24c3bfe5100 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Sun, 27 Oct 2024 16:14:55 -0400 Subject: [PATCH 12/16] [fontc] Fixups: don't split italic, set vfDir --- Lib/gftools/builder/fontc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index e2a92c521..ad389e017 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -51,6 +51,7 @@ def modify_config(self, config: dict): config["use_fontc"] = self.fontc_bin_path config["buildWebfont"] = False config["buildSmallCap"] = False + config["splitItalic"] = False # override config to turn not build instances if we're variable if self.will_build_variable_font(config): config["buildStatic"] = False @@ -65,6 +66,7 @@ def modify_config(self, config: dict): config["outputDir"] = str(output_dir) config["ttDir"] = str(output_dir) config["otDir"] = str(output_dir) + config["vfDir"] = str(output_dir) def will_build_variable_font(self, config: dict) -> bool: # if config explicitly says dont build variable, believe it From ebad92951e6bfd8c72a547da1cee95c9708ba1df Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 30 Oct 2024 13:14:52 -0400 Subject: [PATCH 13/16] [fontc] Use unique names for ninja files This way if we are running multiple instances of gftools agains the same sources, we aren't overwriting each other's build files. --- Lib/gftools/builder/__init__.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/gftools/builder/__init__.py b/Lib/gftools/builder/__init__.py index 9eca24080..5c12c87c5 100644 --- a/Lib/gftools/builder/__init__.py +++ b/Lib/gftools/builder/__init__.py @@ -7,7 +7,8 @@ from os import chdir from pathlib import Path from tempfile import NamedTemporaryFile, gettempdir -from typing import Any, Dict, List, Tuple, Union +import time +from typing import Any, Dict, List, Union from gftools.builder.fontc import FontcArgs import networkx as nx @@ -63,7 +64,8 @@ def __init__( fontc_args.modify_config(self.config) self.known_operations = OperationRegistry(use_fontc=fontc_args.use_fontc) - self.writer = Writer(open("build.ninja", "w")) + self.ninja_file_name = f"build-{time.time_ns()}.ninja" + self.writer = Writer(open(self.ninja_file_name, "w")) self.named_files = {} self.used_operations = set([]) self.graph = nx.DiGraph() @@ -336,7 +338,9 @@ def walk_graph(self): def draw_graph(self): import pydot - dot = subprocess.run(["ninja", "-t", "graph"], capture_output=True) + dot = subprocess.run( + ["ninja", "-t", "graph", "-f", self.ninja_file_name], capture_output=True + ) graphs = pydot.graph_from_dot_data(dot.stdout.decode("utf-8")) targets = self.recipe.keys() if graphs and graphs[0]: @@ -362,7 +366,7 @@ def clean(self): if cleanUp == True: print("Cleaning up temporary files...") - for file in ["./build.ninja", "./.ninja_log"]: + for file in [self.ninja_file_name, "./.ninja_log"]: if os.path.exists(file): os.remove(file) @@ -444,4 +448,4 @@ def main(args=None): pd.draw_graph() if not args.no_ninja: atexit.register(pd.clean) - raise SystemExit(_program("ninja", [])) + raise SystemExit(_program("ninja", ["-f", pd.ninja_file_name])) From f14c7bc4e5e17b3262297c1f64d63a0ac717d4c0 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 31 Oct 2024 11:58:05 -0400 Subject: [PATCH 14/16] [fontc] Actually build static fonts with fontc --- Lib/gftools/builder/operations/__init__.py | 2 ++ Lib/gftools/builder/operations/fontc/fontcBuildOTF.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/gftools/builder/operations/__init__.py b/Lib/gftools/builder/operations/__init__.py index 029bc81c0..ca4d3cd05 100644 --- a/Lib/gftools/builder/operations/__init__.py +++ b/Lib/gftools/builder/operations/__init__.py @@ -166,6 +166,8 @@ def get(self, operation_name: str): if operation_name == "buildTTF": from .fontc.fontcBuildTTF import FontcBuildTTF + return FontcBuildTTF + if operation_name == "buildOTF": from .fontc.fontcBuildOTF import FontcBuildOTF diff --git a/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py index cf4834db1..8ca3c8523 100644 --- a/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py +++ b/Lib/gftools/builder/operations/fontc/fontcBuildOTF.py @@ -1,7 +1,7 @@ from gftools.builder.operations.fontc import FontcOperationBase -class FontcBuildTTF(FontcOperationBase): +class FontcBuildOTF(FontcOperationBase): description = "Build an OTF from a source file (with fontc)" # the '--cff-outlines' flag does not exit in fontc, so this will # error, which we want From 4d011a3da2bff2560dc20acaa5aeaa9fa452a084 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 31 Oct 2024 12:22:51 -0400 Subject: [PATCH 15/16] [fontc] Set --no-production-names --- Lib/gftools/builder/fontc.py | 4 ++++ Lib/gftools/builder/operations/fontc/__init__.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index ad389e017..9d951554f 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -52,6 +52,10 @@ def modify_config(self, config: dict): config["buildWebfont"] = False config["buildSmallCap"] = False config["splitItalic"] = False + # set --no-production-names, because it's easier to debug + extra_args = config.get("extraFontmakeArgs") or "" + extra_args += " --no-production-names" + 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 diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py index a0f06b6e5..e25cad2ae 100644 --- a/Lib/gftools/builder/operations/fontc/__init__.py +++ b/Lib/gftools/builder/operations/fontc/__init__.py @@ -51,6 +51,8 @@ def rewrite_one_arg(args: List[str]) -> str: else: # glue the filter back together for better reporting below next_ = f"{next_} {filter_}" + elif next_ == "--no-production-names": + return next_ else: raise ValueError(f"unknown fontmake arg '{next_}'") return "" From 31d7c7871796abdcaca25e98535abbcfe188b950 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 31 Oct 2024 15:21:02 -0400 Subject: [PATCH 16/16] [fontc] Set --drop-implied-oncurves --- Lib/gftools/builder/fontc.py | 2 +- Lib/gftools/builder/operations/fontc/__init__.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/gftools/builder/fontc.py b/Lib/gftools/builder/fontc.py index 9d951554f..5033248f8 100644 --- a/Lib/gftools/builder/fontc.py +++ b/Lib/gftools/builder/fontc.py @@ -54,7 +54,7 @@ def modify_config(self, config: dict): config["splitItalic"] = False # set --no-production-names, because it's easier to debug extra_args = config.get("extraFontmakeArgs") or "" - extra_args += " --no-production-names" + 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): diff --git a/Lib/gftools/builder/operations/fontc/__init__.py b/Lib/gftools/builder/operations/fontc/__init__.py index e25cad2ae..e3f0fb6dd 100644 --- a/Lib/gftools/builder/operations/fontc/__init__.py +++ b/Lib/gftools/builder/operations/fontc/__init__.py @@ -53,6 +53,9 @@ def rewrite_one_arg(args: List[str]) -> str: next_ = f"{next_} {filter_}" elif next_ == "--no-production-names": return next_ + elif next_ == "--drop-implied-oncurves": + # this is our default behaviour so no worries + return "" else: raise ValueError(f"unknown fontmake arg '{next_}'") return ""