diff --git a/Cargo.toml b/Cargo.toml index ed4b7e042..b50c30a7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,3 +1,7 @@ +[patch.crates-io] +read-fonts = { git = 'https://github.com/googlefonts/fontations' } +write-fonts = { git = 'https://github.com/googlefonts/fontations' } + [workspace] members = [ diff --git a/fontbe/Cargo.toml b/fontbe/Cargo.toml index 6d1b89346..b99347966 100644 --- a/fontbe/Cargo.toml +++ b/fontbe/Cargo.toml @@ -18,6 +18,7 @@ serde = {version = "1.0", features = ["derive"] } serde_yaml = "0.9.14" thiserror = "1.0.37" +kurbo = { version = "0.9.0" } ordered-float = { version = "3.4.0", features = ["serde"] } indexmap = "1.9.2" @@ -26,11 +27,12 @@ env_logger = "0.9.0" parking_lot = "0.12.1" -read-fonts = "0.0.5" -write-fonts = "0.0.5" fea-rs = "0.2.0" smol_str = "0.1.18" +read-fonts = "0.0.5" +write-fonts = "0.0.5" + [dev-dependencies] diff = "0.1.12" ansi_term = "0.12.1" diff --git a/fontbe/src/error.rs b/fontbe/src/error.rs index 942d0002a..ac4d79645 100644 --- a/fontbe/src/error.rs +++ b/fontbe/src/error.rs @@ -1,7 +1,9 @@ -use std::io; +use std::{fmt::Display, io}; use fea_rs::compile::error::{BinaryCompilationError, CompilerError}; +use fontdrasil::types::GlyphName; use thiserror::Error; +use write_fonts::tables::glyf::BadKurbo; #[derive(Debug, Error)] pub enum Error { @@ -11,4 +13,30 @@ pub enum Error { FeaAssembleError(#[from] BinaryCompilationError), #[error("Fea compilation failure")] FeaCompileError(#[from] CompilerError), + #[error("'{0}' {1}")] + GlyphError(GlyphName, GlyphProblem), + #[error("'{0}' {1:?}")] + KurboError(GlyphName, BadKurbo), +} + +#[derive(Debug)] +pub enum GlyphProblem { + InconsistentComponents, + InconsistentPathElements, + HasComponentsAndPath, + MissingDefault, +} + +impl Display for GlyphProblem { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let message = match self { + GlyphProblem::HasComponentsAndPath => "has components *and* paths", + GlyphProblem::InconsistentComponents => { + "has different components at different points in designspace" + } + GlyphProblem::InconsistentPathElements => "has interpolation-incompatible paths", + GlyphProblem::MissingDefault => "has no default master", + }; + f.write_str(message) + } } diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 445714af7..944634f94 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -13,9 +13,9 @@ use fea_rs::{ }; use fontir::ir::Features; use log::{debug, error, trace, warn}; -use write_fonts::FontBuilder; use fontdrasil::orchestration::Work; +use write_fonts::FontBuilder; use crate::{ error::Error, @@ -74,7 +74,11 @@ impl Display for NotSupportedError { } impl FeatureWork { - fn compile(&self, features: &Features, glyph_order: GlyphMap) -> Result { + fn compile( + &self, + features: &Features, + glyph_order: GlyphMap, + ) -> Result, Error> { let root_path = if let Features::File(file) = features { OsString::from(file) } else { diff --git a/fontbe/src/glyphs.rs b/fontbe/src/glyphs.rs new file mode 100644 index 000000000..c349a8fb1 --- /dev/null +++ b/fontbe/src/glyphs.rs @@ -0,0 +1,229 @@ +//! 'glyf' Glyph binary compilation + +use std::collections::{BTreeSet, HashMap, HashSet}; + +use fontdrasil::{orchestration::Work, types::GlyphName}; +use fontir::{coords::NormalizedLocation, ir}; +use kurbo::{Affine, BezPath, PathEl}; +use log::{error, info, trace, warn}; + +use write_fonts::tables::glyf::SimpleGlyph; + +use crate::{ + error::{Error, GlyphProblem}, + orchestration::{BeWork, Context}, +}; + +struct GlyphWork { + glyph_name: GlyphName, +} + +pub fn create_glyph_work(glyph_name: GlyphName) -> Box { + Box::new(GlyphWork { glyph_name }) +} + +impl Work for GlyphWork { + fn exec(&self, context: &Context) -> Result<(), Error> { + trace!("BE glyph work for {}", self.glyph_name); + + let static_metadata = context.ir.get_static_metadata(); + let var_model = &static_metadata.variation_model; + let default_location = var_model.default_location(); + let ir_glyph = &*context.ir.get_glyph_ir(&self.glyph_name); + let glyph: CheckedGlyph = ir_glyph.try_into()?; + + // TODO refine (submodel) var model if glyph locations is a subset of var model locations + + // TODO do we want to write to ensure we don't lose interpolability? + match glyph { + CheckedGlyph::Composite { + components, + transforms, + } => { + warn!( + "'{}': composite glyphs not implemented yet; uses {:?} {:?}", + ir_glyph.name, components, transforms + ); + } + CheckedGlyph::Contour { contours } => { + if contours.get(default_location).is_none() { + eprintln!("{}", self.glyph_name); + for contour in contours.iter() { + eprintln!(" {:?}", contour.0); + } + } + let path = contours.get(default_location).ok_or_else(|| { + Error::GlyphError(ir_glyph.name.clone(), GlyphProblem::MissingDefault) + })?; + let base_glyph = SimpleGlyph::from_kurbo(path) + .map_err(|e| Error::KurboError(self.glyph_name.clone(), e))?; + info!("'{}' base is {}", ir_glyph.name, path.to_svg()); + context.set_glyph(ir_glyph.name.clone(), base_glyph); + } + } + + Ok(()) + } +} + +/// An [ir::Glyph] that has been confirmed to maintain invariants: +/// +///
    +///
  • Components are consistent across the design space
    • +///
  • Paths are interpolation compatible
    • +///
+enum CheckedGlyph { + Composite { + components: Vec, + transforms: HashMap<(GlyphName, NormalizedLocation), Affine>, + }, + Contour { + contours: HashMap, + }, +} + +impl TryFrom<&ir::Glyph> for CheckedGlyph { + type Error = Error; + + fn try_from(glyph: &ir::Glyph) -> Result { + // every instance must have consistent component glyphs + let components: HashSet> = glyph + .sources + .values() + .map(|s| s.components.iter().map(|c| c.base.clone()).collect()) + .collect(); + if components.len() > 1 { + return Err(Error::GlyphError( + glyph.name.clone(), + GlyphProblem::InconsistentComponents, + )); + } + + // every instance must have consistent path element types + let path_els: HashSet = glyph + .sources + .values() + .map(|s| { + s.contours + .iter() + .map(|c| c.elements().iter().map(path_el_type).collect::()) + .collect() + }) + .collect(); + if path_els.len() > 1 { + return Err(Error::GlyphError( + glyph.name.clone(), + GlyphProblem::InconsistentPathElements, + )); + } + let mut components = components.into_iter().next().unwrap_or_default(); + let path_els = path_els.into_iter().next().unwrap_or_default(); + trace!( + "'{}' consistent: components '{:?}', paths '{}'", + glyph.name, + components, + path_els + ); + + // TEMPORARY HACKERY; real fix is to hoist the contour into a new glyph + if !components.is_empty() && !path_els.is_empty() { + error!( + "'{}' components discarded due to use of both outline *and* component", + glyph.name + ); + components = BTreeSet::new(); + } + + if !components.is_empty() && !path_els.is_empty() { + return Err(Error::GlyphError( + glyph.name.clone(), + GlyphProblem::HasComponentsAndPath, + )); + } + + // TEMPORARY HACKERY; real fix is to cu2qu in an interpolation friendly manner + if path_els.contains('C') { + error!("'{}' outline discarded due to use of cubics", glyph.name); + let contours = glyph + .sources + .keys() + .map(|location| (location.clone(), BezPath::new())) + .collect(); + return Ok(CheckedGlyph::Contour { contours }); + } + + // All is well, build the result + Ok(if components.is_empty() { + let contours = glyph + .sources + .iter() + .map(|(location, instance)| { + if instance.contours.len() > 1 { + trace!( + "Merging {} contours to form '{}' at {:?}", + instance.contours.len(), + glyph.name, + location + ); + } + let mut path = instance.contours.first().cloned().unwrap_or_default(); + for contour in instance.contours.iter().skip(1) { + for el in contour.elements() { + path.push(*el); + } + } + (location.clone(), path) + }) + .collect(); + CheckedGlyph::Contour { contours } + } else { + // Stable ordering is nice + let mut components: Vec<_> = components.iter().cloned().collect(); + components.sort(); + + let transforms = glyph + .sources + .iter() + .flat_map(|(location, instance)| { + instance + .components + .iter() + .map(|c| ((c.base.clone(), location.clone()), c.transform)) + }) + .collect(); + CheckedGlyph::Composite { + components, + transforms, + } + }) + } +} + +fn path_el_type(el: &PathEl) -> &'static str { + match el { + PathEl::MoveTo(..) => "M", + PathEl::LineTo(..) => "L", + PathEl::QuadTo(..) => "Q", + PathEl::CurveTo(..) => "C", + PathEl::ClosePath => "Z", + } +} + +struct GlyphMergeWork {} + +pub fn create_glyph_merge_work() -> Box { + Box::new(GlyphMergeWork {}) +} + +impl Work for GlyphMergeWork { + fn exec(&self, context: &Context) -> Result<(), Error> { + let static_metadata = context.ir.get_static_metadata(); + + error!( + "TODO merge {} glyphs in glyph order => final result", + static_metadata.glyph_order.len() + ); + + Ok(()) + } +} diff --git a/fontbe/src/lib.rs b/fontbe/src/lib.rs index b2e334768..f0c7a114e 100644 --- a/fontbe/src/lib.rs +++ b/fontbe/src/lib.rs @@ -1,4 +1,5 @@ pub mod error; pub mod features; +pub mod glyphs; pub mod orchestration; pub mod paths; diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index 248a50d3b..624f33a72 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -1,6 +1,11 @@ //! Helps coordinate the graph execution for BE -use std::{collections::HashSet, fs, path::Path, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + fs, + path::Path, + sync::Arc, +}; use fontdrasil::{ orchestration::{AccessControlList, Work, MISSING_DATA}, @@ -8,7 +13,10 @@ use fontdrasil::{ }; use fontir::orchestration::{Context as FeContext, WorkId as FeWorkIdentifier}; use parking_lot::RwLock; -use write_fonts::FontBuilder; +use read_fonts::{FontData, FontRead}; +//use read_fonts::{tables::glyf::SimpleGlyph, FontRead, FontData}; +use write_fonts::from_obj::FromTableRef; +use write_fonts::{dump_table, tables::glyf::SimpleGlyph, FontBuilder}; use crate::{error::Error, paths::Paths}; @@ -84,6 +92,10 @@ pub struct Context { // work results we've completed or restored from disk // We create individual caches so we can return typed results from get fns features: Arc>>>>, + + // TODO: variations + // TODO: components + glyphs: Arc>>>, } impl Context { @@ -100,21 +112,23 @@ impl Context { ir: Arc::from(ir.read_only()), acl: AccessControlList::read_only(), features: Arc::from(RwLock::new(None)), + glyphs: Arc::from(RwLock::new(HashMap::new())), } } pub fn copy_for_work( &self, - work_id: WorkId, dependencies: Option>, + write_access: Arc bool + Send + Sync>, ) -> Context { Context { emit_ir: self.emit_ir, emit_debug: self.emit_debug, paths: self.paths.clone(), ir: self.ir.clone(), - acl: AccessControlList::read_write(dependencies.unwrap_or_default(), work_id.into()), + acl: AccessControlList::read_write(dependencies.unwrap_or_default(), write_access), features: self.features.clone(), + glyphs: self.glyphs.clone(), } } } @@ -167,4 +181,36 @@ impl Context { self.maybe_persist(&self.paths.target_file(&id), &font); self.set_cached_features(font); } + + fn set_cached_glyph(&self, glyph_name: GlyphName, glyph: SimpleGlyph) { + let mut wl = self.glyphs.write(); + wl.insert(glyph_name, Arc::from(glyph)); + } + + pub fn get_glyph(&self, glyph_name: &GlyphName) -> Arc { + let id = WorkId::Glyph(glyph_name.clone()); + self.acl.check_read_access(&id.clone().into()); + { + let rl = self.glyphs.read(); + if let Some(glyph) = rl.get(glyph_name) { + return glyph.clone(); + } + } + + // Vec[u8] => read type => write type == all the right type + let glyph = self.restore(&self.paths.target_file(&id)); + let glyph = read_fonts::tables::glyf::SimpleGlyph::read(FontData::new(&glyph)).unwrap(); + let glyph = SimpleGlyph::from_table_ref(&glyph); + + self.set_cached_glyph(glyph_name.clone(), glyph); + let rl = self.glyphs.read(); + rl.get(glyph_name).expect(MISSING_DATA).clone() + } + + pub fn set_glyph(&self, glyph_name: GlyphName, glyph: SimpleGlyph) { + let id = WorkId::Glyph(glyph_name.clone()); + self.acl.check_write_access(&id.clone().into()); + self.maybe_persist(&self.paths.target_file(&id), &dump_table(&glyph).unwrap()); + self.set_cached_glyph(glyph_name, glyph); + } } diff --git a/fontbe/src/paths.rs b/fontbe/src/paths.rs index 380a36ddd..3e832dbb0 100644 --- a/fontbe/src/paths.rs +++ b/fontbe/src/paths.rs @@ -33,8 +33,12 @@ impl Paths { &self.debug_dir } + pub fn glyph_dir(&self) -> &Path { + &self.glyph_dir + } + fn glyph_file(&self, name: &str) -> PathBuf { - self.glyph_dir.join(glyph_file(name, ".ttf")) + self.glyph_dir.join(glyph_file(name, ".glyph")) } pub fn target_file(&self, id: &WorkId) -> PathBuf { diff --git a/fontc/src/main.rs b/fontc/src/main.rs index a474665ae..42a3039d6 100644 --- a/fontc/src/main.rs +++ b/fontc/src/main.rs @@ -4,12 +4,14 @@ use std::{ fs, io::{self, Write}, path::{Path, PathBuf}, + sync::Arc, }; use clap::Parser; use crossbeam_channel::{Receiver, TryRecvError}; use fontbe::{ features::FeatureWork, + glyphs::{create_glyph_merge_work, create_glyph_work}, orchestration::{AnyWorkId, Context as BeContext, WorkId as BeWorkIdentifier}, paths::Paths as BePaths, }; @@ -17,8 +19,12 @@ use fontc::{ work::{AnyContext, AnyWork, AnyWorkError}, Error, }; -use fontdrasil::types::GlyphName; +use fontdrasil::{ + orchestration::{access_none, access_one}, + types::GlyphName, +}; use fontir::{ + glyph::create_finalize_static_metadata_work, orchestration::{Context as FeContext, WorkId as FeWorkIdentifier}, paths::Paths as IrPaths, source::{DeleteWork, Input, Source}, @@ -98,6 +104,7 @@ fn require_dir(dir: &Path) -> Result { if !dir.exists() { fs::create_dir(dir)? } + debug!("require_dir {:?}", dir); Ok(dir.to_path_buf()) } @@ -153,7 +160,7 @@ impl ChangeDetector { self.current_inputs.static_metadata != self.prev_inputs.static_metadata || !self .ir_paths - .target_file(&FeWorkIdentifier::StaticMetadata) + .target_file(&FeWorkIdentifier::InitStaticMetadata) .is_file() } @@ -209,23 +216,60 @@ impl ChangeDetector { } } -fn add_static_metadata_ir_job( +fn add_init_static_metadata_ir_job( change_detector: &mut ChangeDetector, workload: &mut Workload, ) -> Result<(), Error> { if change_detector.static_metadata_ir_change() { + let id: AnyWorkId = FeWorkIdentifier::InitStaticMetadata.into(); + let write_access = access_one(id.clone()); workload.insert( - FeWorkIdentifier::StaticMetadata.into(), + id, Job { work: change_detector .ir_source .create_static_metadata_work(&change_detector.current_inputs)? .into(), dependencies: HashSet::new(), + write_access, + }, + ); + } else { + workload.mark_success(FeWorkIdentifier::InitStaticMetadata.into()); + } + Ok(()) +} + +fn add_finalize_static_metadata_ir_job( + change_detector: &mut ChangeDetector, + workload: &mut Workload, +) -> Result<(), Error> { + if change_detector.static_metadata_ir_change() { + let mut dependencies: HashSet<_> = change_detector + .glyphs_changed() + .iter() + .map(|gn| FeWorkIdentifier::Glyph(gn.clone()).into()) + .collect(); + dependencies.insert(FeWorkIdentifier::InitStaticMetadata.into()); + + // Grant write to any glyph including ones we've never seen before so job can create them + let write_access = Arc::new(|an_id: &AnyWorkId| { + matches!( + an_id, + AnyWorkId::Fe(FeWorkIdentifier::Glyph(..)) + | AnyWorkId::Fe(FeWorkIdentifier::FinalizeStaticMetadata) + ) + }); + workload.insert( + FeWorkIdentifier::FinalizeStaticMetadata.into(), + Job { + work: create_finalize_static_metadata_work().into(), + dependencies, + write_access, }, ); } else { - workload.mark_success(FeWorkIdentifier::StaticMetadata.into()); + workload.mark_success(FeWorkIdentifier::FinalizeStaticMetadata.into()); } Ok(()) } @@ -235,14 +279,17 @@ fn add_feature_ir_job( workload: &mut Workload, ) -> Result<(), Error> { if change_detector.feature_ir_change() { + let id: AnyWorkId = FeWorkIdentifier::Features.into(); + let write_access = access_one(id.clone()); workload.insert( - FeWorkIdentifier::Features.into(), + id, Job { work: change_detector .ir_source .create_feature_ir_work(&change_detector.current_inputs)? .into(), dependencies: HashSet::new(), + write_access, }, ); } else { @@ -256,14 +303,17 @@ fn add_feature_be_job( workload: &mut Workload, ) -> Result<(), Error> { if change_detector.feature_be_change() { + let id: AnyWorkId = BeWorkIdentifier::Features.into(); + let write_access = access_one(id.clone()); workload.insert( - BeWorkIdentifier::Features.into(), + id, Job { work: FeatureWork::create().into(), dependencies: HashSet::from([ - FeWorkIdentifier::StaticMetadata.into(), + FeWorkIdentifier::FinalizeStaticMetadata.into(), FeWorkIdentifier::Features.into(), ]), + write_access, }, ); } else { @@ -285,6 +335,7 @@ fn add_glyph_ir_jobs( Job { work: DeleteWork::create(path).into(), dependencies: HashSet::new(), + write_access: access_none(), }, ); } @@ -297,14 +348,83 @@ fn add_glyph_ir_jobs( for (glyph_name, work) in glyphs_changed.iter().zip(glyph_work) { let id = FeWorkIdentifier::Glyph(glyph_name.clone()); let work = work.into(); - let dependencies = HashSet::from([FeWorkIdentifier::StaticMetadata.into()]); + let dependencies = HashSet::from([FeWorkIdentifier::InitStaticMetadata.into()]); - workload.insert(id.into(), Job { work, dependencies }); + let id: AnyWorkId = id.into(); + let write_access = access_one(id.clone()); + workload.insert( + id, + Job { + work, + dependencies, + write_access, + }, + ); } Ok(()) } +fn add_glyph_merge_be_job( + change_detector: &mut ChangeDetector, + workload: &mut Workload, +) -> Result<(), Error> { + let glyphs_changed = change_detector.glyphs_changed(); + + // If no glyph has changed there isn't a lot of merging to do + if !glyphs_changed.is_empty() { + let mut dependencies: HashSet<_> = glyphs_changed + .iter() + .map(|gn| BeWorkIdentifier::Glyph(gn.clone()).into()) + .collect(); + dependencies.insert(FeWorkIdentifier::FinalizeStaticMetadata.into()); + + let id: AnyWorkId = BeWorkIdentifier::GlyphMerge.into(); + let write_access = access_one(id.clone()); + workload.insert( + id, + Job { + work: create_glyph_merge_work().into(), + dependencies, + write_access, + }, + ); + } else { + workload.mark_success(BeWorkIdentifier::GlyphMerge.into()); + } + Ok(()) +} + +fn add_glyph_be_job(workload: &mut Workload, fe_root: &FeContext, glyph_name: GlyphName) { + let glyph_ir = fe_root.get_glyph_ir(&glyph_name); + + // To build a glyph we need it's components, plus static metadata + let mut dependencies: HashSet<_> = glyph_ir + .sources + .values() + .flat_map(|s| &s.components) + .map(|c| AnyWorkId::Fe(FeWorkIdentifier::Glyph(c.base.clone()))) + .collect(); + dependencies.insert(FeWorkIdentifier::FinalizeStaticMetadata.into()); + + let id = AnyWorkId::Be(BeWorkIdentifier::Glyph(glyph_name.clone())); + + // this job should already be a dependency of the glyph merge; if not terrible things will happen + if !workload.is_dependency(&BeWorkIdentifier::GlyphMerge.into(), &id) { + panic!("BE glyph '{glyph_name}' is being built but not participating in glyph merge",); + } + + let write_access = access_one(id.clone()); + workload.insert( + id, + Job { + work: create_glyph_work(glyph_name).into(), + dependencies, + write_access, + }, + ); +} + struct ChangeDetector { ir_paths: IrPaths, ir_source: Box, @@ -357,6 +477,14 @@ impl Workload { fn insert(&mut self, id: AnyWorkId, job: Job) { self.jobs_pending.insert(id, job); + self.job_count += 1; + } + + fn is_dependency(&mut self, id: &AnyWorkId, dep: &AnyWorkId) -> bool { + self.jobs_pending + .get(id) + .map(|job| job.dependencies.contains(dep)) + .unwrap_or(false) } fn mark_success(&mut self, id: AnyWorkId) { @@ -387,8 +515,6 @@ impl Workload { } fn exec(mut self, fe_root: FeContext, be_root: BeContext) -> Result<(), Error> { - self.job_count = self.jobs_pending.len(); - // Async work will send us it's ID on completion let (send, recv) = crossbeam_channel::unbounded::<(AnyWorkId, Result<(), AnyWorkError>)>(); @@ -402,8 +528,13 @@ impl Workload { trace!("Start {:?}", id); let job = self.jobs_pending.remove(&id).unwrap(); let work = job.work; - let work_context = - AnyContext::for_work(&fe_root, &be_root, &id, job.dependencies); + let work_context = AnyContext::for_work( + &fe_root, + &be_root, + &id, + job.dependencies, + job.write_access, + ); let send = send.clone(); scope.spawn(move |_| { @@ -416,7 +547,14 @@ impl Workload { // Block for things to phone home to say they are done // Then complete everything that has reported since our last check - self.read_completions(&recv, RecvType::Blocking)?; + let successes = self.read_completions(&recv, RecvType::Blocking)?; + + // When a glyph finishes register BE work for it + for success in successes { + if let AnyWorkId::Fe(FeWorkIdentifier::Glyph(glyph_name)) = success { + add_glyph_be_job(&mut self, &fe_root, glyph_name); + } + } } Ok::<(), Error>(()) })?; @@ -439,7 +577,8 @@ impl Workload { &mut self, recv: &Receiver<(AnyWorkId, Result<(), AnyWorkError>)>, initial_read: RecvType, - ) -> Result<(), Error> { + ) -> Result, Error> { + let mut successes = Vec::new(); let mut opt_complete = match initial_read { RecvType::Blocking => match recv.recv() { Ok(completed) => Some(completed), @@ -455,7 +594,13 @@ impl Workload { }; while let Some((completed_id, result)) = opt_complete.take() { if !match result { - Ok(..) => self.success.insert(completed_id.clone()), + Ok(..) => { + let inserted = self.success.insert(completed_id.clone()); + if inserted { + successes.push(completed_id.clone()); + } + inserted + } Err(e) => { error!("{:?} failed {}", completed_id, e); self.error.push((completed_id.clone(), format!("{e}"))); @@ -479,7 +624,7 @@ impl Workload { if !self.error.is_empty() { return Err(Error::TasksFailed(self.error.clone())); } - Ok(()) + Ok(successes) } } @@ -487,20 +632,24 @@ impl Workload { struct Job { // The actual task work: AnyWork, - // Things that must happen before we execute + // Things that must happen before we execute. Our task can read these things. dependencies: HashSet, + // Things our job needs write access to + write_access: Arc bool + Send + Sync>, } fn create_workload(change_detector: &mut ChangeDetector) -> Result { let mut workload = Workload::new(); // FE: f(source) => IR - add_static_metadata_ir_job(change_detector, &mut workload)?; + add_init_static_metadata_ir_job(change_detector, &mut workload)?; add_feature_ir_job(change_detector, &mut workload)?; add_glyph_ir_jobs(change_detector, &mut workload)?; + add_finalize_static_metadata_ir_job(change_detector, &mut workload)?; // BE: f(IR) => binary add_feature_be_job(change_detector, &mut workload)?; + add_glyph_merge_be_job(change_detector, &mut workload)?; Ok(workload) } @@ -531,6 +680,7 @@ fn main() -> Result<(), Error> { fs::remove_dir_all(be_paths.debug_dir()).map_err(Error::IoError)?; } require_dir(be_paths.debug_dir())?; + require_dir(be_paths.glyph_dir())?; let config = Config::new(args, build_dir)?; let prev_inputs = init(&config).map_err(Error::IoError)?; @@ -580,8 +730,10 @@ mod tests { use tempfile::tempdir; use crate::{ - add_feature_be_job, add_feature_ir_job, add_glyph_ir_jobs, add_static_metadata_ir_job, - finish_successfully, init, require_dir, Args, ChangeDetector, Config, Workload, + add_feature_be_job, add_feature_ir_job, add_finalize_static_metadata_ir_job, + add_glyph_be_job, add_glyph_ir_jobs, add_glyph_merge_be_job, + add_init_static_metadata_ir_job, finish_successfully, init, require_dir, Args, + ChangeDetector, Config, Workload, }; fn testdata_dir() -> PathBuf { @@ -670,6 +822,7 @@ mod tests { let ir_paths = IrPaths::new(build_dir); let be_paths = BePaths::new(build_dir); require_dir(ir_paths.glyph_ir_dir()).unwrap(); + require_dir(be_paths.glyph_dir()).unwrap(); let config = Config::new(args, build_dir.to_path_buf()).unwrap(); let prev_inputs = init(&config).unwrap(); @@ -680,11 +833,14 @@ mod tests { let mut workload = Workload::new(); - add_static_metadata_ir_job(&mut change_detector, &mut workload).unwrap(); + add_init_static_metadata_ir_job(&mut change_detector, &mut workload).unwrap(); + add_finalize_static_metadata_ir_job(&mut change_detector, &mut workload).unwrap(); add_glyph_ir_jobs(&mut change_detector, &mut workload).unwrap(); add_feature_ir_job(&mut change_detector, &mut workload).unwrap(); add_feature_be_job(&mut change_detector, &mut workload).unwrap(); + add_glyph_merge_be_job(&mut change_detector, &mut workload).unwrap(); + // Try to do the work // As we currently don't stress dependencies just run one by one // This will likely need to change when we start doing things like glyphs with components @@ -721,12 +877,17 @@ mod tests { let id = &launchable[0]; let job = workload.jobs_pending.remove(id).unwrap(); - let context = AnyContext::for_work(&fe_root, &be_root, id, job.dependencies); + let context = + AnyContext::for_work(&fe_root, &be_root, id, job.dependencies, job.write_access); job.work.exec(context).unwrap(); assert!( workload.success.insert(id.clone()), "We just did {id:?} a second time?" ); + + if let AnyWorkId::Fe(FeWorkIdentifier::Glyph(glyph_name)) = id { + add_glyph_be_job(&mut workload, &fe_root, glyph_name.clone()); + } } finish_successfully(change_detector).unwrap(); result.work_completed = workload.success.difference(&pre_success).cloned().collect(); @@ -741,11 +902,15 @@ mod tests { let result = compile(build_dir, "wght_var.designspace"); assert_eq!( HashSet::from([ - FeWorkIdentifier::StaticMetadata.into(), + FeWorkIdentifier::InitStaticMetadata.into(), + FeWorkIdentifier::FinalizeStaticMetadata.into(), FeWorkIdentifier::Features.into(), FeWorkIdentifier::Glyph("bar".into()).into(), FeWorkIdentifier::Glyph("plus".into()).into(), BeWorkIdentifier::Features.into(), + BeWorkIdentifier::Glyph("bar".into()).into(), + BeWorkIdentifier::Glyph("plus".into()).into(), + BeWorkIdentifier::GlyphMerge.into(), ]), result.work_completed ); @@ -782,7 +947,11 @@ mod tests { let result = compile(build_dir, "wght_var.designspace"); assert_eq!( - HashSet::from([FeWorkIdentifier::Glyph("bar".into()).into(),]), + HashSet::from([ + FeWorkIdentifier::Glyph("bar".into()).into(), + BeWorkIdentifier::Glyph("bar".into()).into(), + BeWorkIdentifier::GlyphMerge.into(), + ]), result.work_completed ); } diff --git a/fontc/src/work.rs b/fontc/src/work.rs index b8439b362..4b042c57f 100644 --- a/fontc/src/work.rs +++ b/fontc/src/work.rs @@ -2,7 +2,7 @@ //! //! Basically enums that can be a FeWhatever or a BeWhatever. -use std::{collections::HashSet, fmt::Display}; +use std::{collections::HashSet, fmt::Display, sync::Arc}; use fontbe::{ error::Error as BeError, @@ -78,20 +78,21 @@ impl AnyContext { be_root: &BeContext, id: &AnyWorkId, dependencies: HashSet, + write_access: Arc bool + Send + Sync>, ) -> AnyContext { match id { - AnyWorkId::Be(id) => { - AnyContext::Be(be_root.copy_for_work(id.clone(), Some(dependencies))) + AnyWorkId::Be(..) => { + AnyContext::Be(be_root.copy_for_work(Some(dependencies), write_access)) } - AnyWorkId::Fe(id) => AnyContext::Fe( + AnyWorkId::Fe(..) => AnyContext::Fe( fe_root.copy_for_work( - id.clone(), Some( dependencies .into_iter() .map(|a| a.unwrap_fe().clone()) .collect(), ), + Arc::new(move |id| (write_access)(&AnyWorkId::Fe(id.clone()))), ), ), } diff --git a/fontdrasil/src/orchestration.rs b/fontdrasil/src/orchestration.rs index 660dfeded..f8a3b45a2 100644 --- a/fontdrasil/src/orchestration.rs +++ b/fontdrasil/src/orchestration.rs @@ -1,6 +1,6 @@ //! Common parts of work orchestration. -use std::{collections::HashSet, fmt::Debug, hash::Hash}; +use std::{collections::HashSet, fmt::Debug, hash::Hash, sync::Arc}; pub const MISSING_DATA: &str = "Missing data, dependency management failed us?"; @@ -22,9 +22,8 @@ pub struct AccessControlList where I: Eq + Hash + Debug, { - // If present, the one and only key you are allowed to write to - // None means we're read-only - write_mask: Option, + // Returns true if you can write to the provided id + write_access: Arc bool + Send + Sync>, // If present, what you can access through this context // Intent is root has None, task-specific Context only allows access to dependencies @@ -34,20 +33,44 @@ where impl AccessControlList { pub fn read_only() -> AccessControlList { AccessControlList { - write_mask: None, + write_access: Arc::new(|_| false), read_mask: None, } } - pub fn read_write(read: HashSet, write: I) -> AccessControlList { + pub fn read_write( + read: HashSet, + write_access: Arc bool + Send + Sync>, + ) -> AccessControlList { AccessControlList { - write_mask: Some(write), + write_access, read_mask: Some(read), } } } +pub fn access_one( + id: I, +) -> Arc bool + Send + Sync> { + Arc::new(move |id2| id == *id2) +} + +pub fn access_none() -> Arc bool + Send + Sync> { + Arc::new(move |_| false) +} + impl AccessControlList { + pub fn check_read_access_to_any(&self, ids: &[I]) { + if self.read_mask.is_none() { + return; + } + let read_mask = self.read_mask.as_ref().unwrap(); + + if !ids.iter().any(|id| read_mask.contains(id)) { + panic!("Illegal access to {ids:?}"); + } + } + pub fn check_read_access(&self, id: &I) { if !self .read_mask @@ -55,12 +78,18 @@ impl AccessControlList { .map(|mask| mask.contains(id)) .unwrap_or(true) { - panic!("Illegal access"); + panic!("Illegal access to {id:?}"); + } + } + + pub fn check_write_access_to_any(&self, ids: &[I]) { + if !ids.iter().any(|id| (self.write_access)(id)) { + panic!("Illegal access to {ids:?}"); } } pub fn check_write_access(&self, id: &I) { - if self.write_mask.as_ref() != Some(id) { + if !(self.write_access)(id) { panic!("Illegal access to {id:?}"); } } diff --git a/fontir/src/glyph.rs b/fontir/src/glyph.rs new file mode 100644 index 000000000..449b91cf9 --- /dev/null +++ b/fontir/src/glyph.rs @@ -0,0 +1,82 @@ +//! IR glyph processing. +//! +//! Notably includes splitting glyphs with contours and components into one new glyph with +//! the contours and one updated glyph with no contours that references the new gyph as a component. + +use fontdrasil::orchestration::Work; +use kurbo::Affine; +use log::debug; + +use crate::{ + error::WorkError, + ir::Component, + orchestration::{Context, IrWork}, +}; + +pub fn create_finalize_static_metadata_work() -> Box { + Box::new(FinalizeStaticMetadataWork {}) +} + +struct FinalizeStaticMetadataWork {} + +impl Work for FinalizeStaticMetadataWork { + fn exec(&self, context: &Context) -> Result<(), WorkError> { + // We should now have access to *all* the glyph IR + // Some of it may need to be massaged to produce BE glyphs + // In particular, glyphs with both paths and components need to push the path into a component + let current_metadata = context.get_static_metadata(); + let mut new_glyph_order = current_metadata.glyph_order.clone(); + + // Glyphs with paths and components need to push their paths to a new glyph that is a component + for glyph_to_split in current_metadata + .glyph_order + .iter() + .map(|gn| context.get_glyph_ir(gn)) + .filter(|glyph| { + glyph + .sources + .values() + .any(|inst| !inst.components.is_empty() && !inst.contours.is_empty()) + }) + { + debug!( + "Splitting '{0}' because it has contours and components", + glyph_to_split.name + ); + + // Make a glyph with just the contours by erasing the components from it + let mut contours = (*glyph_to_split).clone(); + contours.sources.iter_mut().for_each(|(_, inst)| { + inst.components.clear(); + }); + + // Find a free name for the contour glyph + let mut i = 0; + let mut contour_glyph_name = glyph_to_split.name.clone(); + while new_glyph_order.contains(&contour_glyph_name) { + contour_glyph_name = format!("{}.{}", glyph_to_split.name.as_str(), i).into(); + i += 1; + } + contours.name = contour_glyph_name.clone(); + + // Save the contour glyph IR and add it to glyph order + context.set_glyph_ir(contours.name.clone(), contours); + new_glyph_order.insert(contour_glyph_name.clone()); + + // Use the contour glyph as a component in the original glyph, erasing it's contours + let mut components = (*glyph_to_split).clone(); + components.sources.iter_mut().for_each(|(_, inst)| { + inst.contours.clear(); + inst.components.push(Component { + base: contour_glyph_name.clone(), + transform: Affine::IDENTITY, + }); + }); + + // Update the original glyph + context.set_glyph_ir(components.name.clone(), components); + } + + Ok(()) + } +} diff --git a/fontir/src/lib.rs b/fontir/src/lib.rs index 3cd8cb917..7792a8fe5 100644 --- a/fontir/src/lib.rs +++ b/fontir/src/lib.rs @@ -1,5 +1,6 @@ pub mod coords; pub mod error; +pub mod glyph; pub mod ir; pub mod orchestration; pub mod paths; diff --git a/fontir/src/orchestration.rs b/fontir/src/orchestration.rs index 493ab2d04..9ee26dcd8 100644 --- a/fontir/src/orchestration.rs +++ b/fontir/src/orchestration.rs @@ -22,9 +22,15 @@ use crate::{error::WorkError, ir, paths::Paths, source::Input}; // Meant to be small and cheap to copy around. #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum WorkId { - StaticMetadata, + /// Build the initial static metadata. + InitStaticMetadata, Glyph(GlyphName), GlyphIrDelete, + /// Update static metadata based on what we learned from IR + /// + /// Notably, IR glyphs with both components and paths may split into multiple + /// BE glyphs. + FinalizeStaticMetadata, Features, } @@ -84,10 +90,14 @@ impl Context { } } - pub fn copy_for_work(&self, work_id: WorkId, dependencies: Option>) -> Context { + pub fn copy_for_work( + &self, + dependencies: Option>, + write_access: Arc bool + Send + Sync>, + ) -> Context { self.copy(AccessControlList::read_write( dependencies.unwrap_or_default(), - work_id, + write_access, )) } @@ -133,23 +143,23 @@ impl Context { } pub fn get_static_metadata(&self) -> Arc { - let id = WorkId::StaticMetadata; - self.acl.check_read_access(&id); + let ids = [WorkId::InitStaticMetadata, WorkId::FinalizeStaticMetadata]; + self.acl.check_read_access_to_any(&ids); { let rl = self.static_metadata.read(); if rl.is_some() { return rl.as_ref().unwrap().clone(); } } - self.set_cached_static_metadata(self.restore(&self.paths.target_file(&id))); + self.set_cached_static_metadata(self.restore(&self.paths.target_file(&ids[0]))); let rl = self.static_metadata.read(); rl.as_ref().expect(MISSING_DATA).clone() } pub fn set_static_metadata(&self, ir: ir::StaticMetadata) { - let id = WorkId::StaticMetadata; - self.acl.check_write_access(&id); - self.maybe_persist(&self.paths.target_file(&id), &ir); + let ids = [WorkId::InitStaticMetadata, WorkId::FinalizeStaticMetadata]; + self.acl.check_write_access_to_any(&ids); + self.maybe_persist(&self.paths.target_file(&ids[0]), &ir); self.set_cached_static_metadata(ir); } diff --git a/fontir/src/paths.rs b/fontir/src/paths.rs index 4acc1407f..c36a0785b 100644 --- a/fontir/src/paths.rs +++ b/fontir/src/paths.rs @@ -43,7 +43,8 @@ impl Paths { pub fn target_file(&self, id: &WorkId) -> PathBuf { match id { - WorkId::StaticMetadata => self.build_dir.join("static_metadata.yml"), + WorkId::InitStaticMetadata => self.build_dir.join("static_metadata.yml"), + WorkId::FinalizeStaticMetadata => self.build_dir.join("static_metadata.yml"), WorkId::Glyph(name) => self.glyph_ir_file(name.as_str()), WorkId::GlyphIrDelete => self.build_dir.join("delete.yml"), WorkId::Features => self.build_dir.join("features.yml"), diff --git a/fontir/src/variations.rs b/fontir/src/variations.rs index f4dc18eea..e0d37d2a0 100644 --- a/fontir/src/variations.rs +++ b/fontir/src/variations.rs @@ -25,6 +25,8 @@ const ONE: OrderedFloat = OrderedFloat(1.0); /// See `class VariationModel` in #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct VariationModel { + default: NormalizedLocation, + // TODO: why isn't this a Map // All Vec's have same length and items at the same index refer to the same master // Which sounds a *lot* like we should have a Vec or Map of Some Struct. @@ -47,6 +49,10 @@ impl VariationModel { axis_order: Vec, ) -> Result { let axes = axis_order.iter().collect::>(); + let default = axes + .iter() + .map(|axis_name| (axis_name.to_string(), NormalizedCoord::new(ZERO))) + .collect(); let mut expanded_locations = HashSet::new(); for mut location in locations.into_iter() { @@ -84,6 +90,7 @@ impl VariationModel { let delta_weights = delta_weights(&locations, &influence); Ok(VariationModel { + default, locations, influence, delta_weights, @@ -92,6 +99,7 @@ impl VariationModel { pub fn empty() -> Self { VariationModel { + default: NormalizedLocation::new(), locations: Vec::new(), influence: Vec::new(), delta_weights: Vec::new(), @@ -101,6 +109,10 @@ impl VariationModel { pub fn locations(&self) -> impl Iterator { self.locations.iter() } + + pub fn default_location(&self) -> &NormalizedLocation { + &self.default + } } /// Gryffindor! diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs index ad42e45ea..960be8747 100644 --- a/glyphs2fontir/src/source.rs +++ b/glyphs2fontir/src/source.rs @@ -307,7 +307,7 @@ mod tests { path::{Path, PathBuf}, }; - use fontdrasil::types::GlyphName; + use fontdrasil::{orchestration::access_one, types::GlyphName}; use fontir::{ coords::{ CoordConverter, DesignCoord, NormalizedCoord, NormalizedLocation, UserCoord, @@ -397,7 +397,7 @@ mod tests { #[test] fn static_metadata_ir() { let (source, context) = context_for(glyphs3_dir().join("WghtVar.glyphs")); - let task_context = context.copy_for_work(WorkId::StaticMetadata, None); + let task_context = context.copy_for_work(None, access_one(WorkId::InitStaticMetadata)); source .create_static_metadata_work(&context.input) .unwrap() @@ -424,7 +424,7 @@ mod tests { fn static_metadata_ir_multi_axis() { // Caused index out of bounds due to transposed master and value indices let (source, context) = context_for(glyphs2_dir().join("BadIndexing.glyphs")); - let task_context = context.copy_for_work(WorkId::StaticMetadata, None); + let task_context = context.copy_for_work(None, access_one(WorkId::InitStaticMetadata)); source .create_static_metadata_work(&context.input) .unwrap() @@ -435,7 +435,7 @@ mod tests { #[test] fn loads_axis_mappings_from_glyphs2() { let (source, context) = context_for(glyphs2_dir().join("OpszWghtVar_AxisMappings.glyphs")); - let task_context = context.copy_for_work(WorkId::StaticMetadata, None); + let task_context = context.copy_for_work(None, access_one(WorkId::InitStaticMetadata)); source .create_static_metadata_work(&context.input) .unwrap() @@ -488,7 +488,7 @@ mod tests { fn build_static_metadata(glyphs_file: PathBuf) -> (impl Source, Context) { let (source, context) = context_for(glyphs_file); - let task_context = context.copy_for_work(WorkId::StaticMetadata, None); + let task_context = context.copy_for_work(None, access_one(WorkId::InitStaticMetadata)); source .create_static_metadata_work(&context.input) .unwrap() @@ -509,8 +509,8 @@ mod tests { .unwrap(); for work in work_items.iter() { let task_context = context.copy_for_work( - WorkId::Glyph(glyph_name.clone()), - Some(HashSet::from([WorkId::StaticMetadata])), + Some(HashSet::from([WorkId::InitStaticMetadata])), + access_one(WorkId::Glyph(glyph_name.clone())), ); work.exec(&task_context)?; }