Skip to content

Commit

Permalink
Move some of the global glyph order work into per-glyph jobs. WIP.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rsheeter
rsheeter committed Nov 29, 2023
1 parent 8935568 commit a25f885
Show file tree
Hide file tree
Showing 15 changed files with 503 additions and 178 deletions.
15 changes: 9 additions & 6 deletions fontbe/src/cmap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ impl Work<Context, AnyWorkId, Error> for CmapWork {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::GlyphOrder) | AnyWorkId::Fe(FeWorkId::Glyph(..))
)
}))
Access::Custom(
"Cmap::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::GlyphOrder) | AnyWorkId::Fe(FeWorkId::Glyph(..))
)
}),
)
}

/// Generate [cmap](https://learn.microsoft.com/en-us/typography/opentype/spec/cmap)
Expand Down
40 changes: 23 additions & 17 deletions fontbe/src/glyphs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,26 +853,32 @@ impl Work<Context, AnyWorkId, Error> for GlyfLocaWork {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
)
}))
Access::Custom(
"GlyfLoca::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
)
}),
)
}

fn write_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Be(WorkId::Glyf)
| AnyWorkId::Be(WorkId::Loca)
| AnyWorkId::Be(WorkId::LocaFormat)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
)
}))
Access::Custom(
"GlyfLoca::Write",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Be(WorkId::Glyf)
| AnyWorkId::Be(WorkId::Loca)
| AnyWorkId::Be(WorkId::LocaFormat)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
)
}),
)
}

fn also_completes(&self) -> Vec<AnyWorkId> {
Expand Down
19 changes: 11 additions & 8 deletions fontbe/src/gvar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@ impl Work<Context, AnyWorkId, Error> for GvarWork {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GvarFragment(..))
)
}))
Access::Custom(
"Gvar::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GvarFragment(..))
)
}),
)
}

/// Generate [gvar](https://learn.microsoft.com/en-us/typography/opentype/spec/gvar)
Expand Down
19 changes: 11 additions & 8 deletions fontbe/src/hvar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,17 @@ impl Work<Context, AnyWorkId, Error> for HvarWork {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
)
}))
Access::Custom(
"Hvar::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
)
}),
)
}

/// Generate [HVAR](https://learn.microsoft.com/en-us/typography/opentype/spec/HVAR)
Expand Down
38 changes: 19 additions & 19 deletions fontbe/src/metrics_and_limits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,28 +199,28 @@ impl Work<Context, AnyWorkId, Error> for MetricAndLimitWork {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::GlobalMetrics)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
| AnyWorkId::Be(WorkId::Head)
)
}))
Access::Custom(
"MetricsAndLimits::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::GlobalMetrics)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Be(WorkId::GlyfFragment(..))
| AnyWorkId::Be(WorkId::Head)
)
}),
)
}

fn write_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Be(WorkId::Hmtx)
| AnyWorkId::Be(WorkId::Hhea)
| AnyWorkId::Be(WorkId::Maxp)
| AnyWorkId::Be(WorkId::Head)
)
}))
Access::set(HashSet::from([
AnyWorkId::Be(WorkId::Hmtx),
AnyWorkId::Be(WorkId::Hhea),
AnyWorkId::Be(WorkId::Maxp),
AnyWorkId::Be(WorkId::Head),
]))
}

fn also_completes(&self) -> Vec<AnyWorkId> {
Expand Down
29 changes: 16 additions & 13 deletions fontbe/src/os2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -797,19 +797,22 @@ impl Work<Context, AnyWorkId, Error> for Os2Work {
}

fn read_access(&self) -> Access<AnyWorkId> {
Access::Custom(Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Fe(FeWorkId::GlobalMetrics)
| AnyWorkId::Be(WorkId::Hhea)
| AnyWorkId::Be(WorkId::Hmtx)
| AnyWorkId::Be(WorkId::Gpos)
| AnyWorkId::Be(WorkId::Gsub)
)
}))
Access::Custom(
"Os2::Read",
Arc::new(|id| {
matches!(
id,
AnyWorkId::Fe(FeWorkId::Glyph(..))
| AnyWorkId::Fe(FeWorkId::StaticMetadata)
| AnyWorkId::Fe(FeWorkId::GlyphOrder)
| AnyWorkId::Fe(FeWorkId::GlobalMetrics)
| AnyWorkId::Be(WorkId::Hhea)
| AnyWorkId::Be(WorkId::Hmtx)
| AnyWorkId::Be(WorkId::Gpos)
| AnyWorkId::Be(WorkId::Gsub)
)
}),
)
}

/// Generate [OS/2](https://learn.microsoft.com/en-us/typography/opentype/spec/os2)
Expand Down
57 changes: 55 additions & 2 deletions fontc/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ use fontbe::{
};

use fontdrasil::types::GlyphName;
use fontir::{glyph::create_glyph_order_work, source::DeleteWork};
use fontir::{
glyph::{create_glyph_order_work, create_glyph_touchup_work},
source::DeleteWork,
};

use fontbe::orchestration::Context as BeContext;
use fontbe::paths::Paths as BePaths;
Expand Down Expand Up @@ -209,8 +212,9 @@ fn add_glyph_ir_jobs(workload: &mut Workload) -> Result<(), Error> {
.change_detector
.ir_source()
.create_glyph_ir_work(&glyphs_changed, workload.change_detector.current_inputs())?;
for work in glyph_work {
for (work, glyph_name) in glyph_work.into_iter().zip(glyphs_changed) {
workload.add(work.into(), true);
workload.add(create_glyph_touchup_work(glyph_name).into(), true);
}

Ok(())
Expand Down Expand Up @@ -670,6 +674,11 @@ mod tests {
.iter()
.map(|n| FeWorkIdentifier::Glyph((*n).into()).into()),
);
expected.extend(
glyphs
.iter()
.map(|n| FeWorkIdentifier::GlyphTouchup((*n).into()).into()),
);
expected.extend(
glyphs
.iter()
Expand Down Expand Up @@ -747,6 +756,7 @@ mod tests {
assert_eq!(
vec![
AnyWorkId::Fe(FeWorkIdentifier::Glyph("bar".into())),
AnyWorkId::Fe(FeWorkIdentifier::GlyphTouchup("bar".into())),
FeWorkIdentifier::Anchor("bar".into()).into(),
BeWorkIdentifier::Features.into(),
BeWorkIdentifier::Cmap.into(),
Expand Down Expand Up @@ -2503,4 +2513,47 @@ mod tests {
// We had a bug where it was 2
assert_eq!(1, mark_base_lookups(&gpos).len());
}

#[test]
fn compile_nested_components_prefer_simple() {
let temp_dir = tempdir().unwrap();
let build_dir = temp_dir.path();
let mut args = Args::for_test(build_dir, "glyphs3/ComponentComponents.glyphs");
args.prefer_simple_glyphs = true;
let result = compile(Args::for_test(
build_dir,
"glyphs3/ComponentComponents.glyphs",
));

// Our deeply nested component w/contours should have become simple
let glyph = result.fe_context.glyphs.get(&FeWorkIdentifier::Glyph(
"component_component_and_contour".into(),
));
let instance = glyph.default_instance();
assert_eq!(
(instance.contours.is_empty(), instance.components.is_empty()),
(false, true),
"{instance:?} should have only contours"
);
}

#[test]
fn compile_nested_components_prefer_components() {
let temp_dir = tempdir().unwrap();
let build_dir = temp_dir.path();
let mut args = Args::for_test(build_dir, "glyphs3/ComponentComponents.glyphs");
args.prefer_simple_glyphs = false;
let result = compile(args);

// Our deeply nested component w/contours should have had it's contours hoisted out
let glyph = result.fe_context.glyphs.get(&FeWorkIdentifier::Glyph(
"component_component_and_contour".into(),
));
let instance = glyph.default_instance();
assert_eq!(
(instance.contours.is_empty(), instance.components.is_empty()),
(true, false),
"{instance:?} should have only components"
);
}
}
2 changes: 2 additions & 0 deletions fontc/src/timing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ fn short_name(id: &AnyWorkId) -> &'static str {
AnyWorkId::Fe(FeWorkIdentifier::Features) => "fea",
AnyWorkId::Fe(FeWorkIdentifier::GlobalMetrics) => "metrics",
AnyWorkId::Fe(FeWorkIdentifier::Glyph(..)) => "glyph",
AnyWorkId::Fe(FeWorkIdentifier::GlyphTouchup(..)) => "touchup",
AnyWorkId::Fe(FeWorkIdentifier::GlyphIrDelete(..)) => "rm ir",
AnyWorkId::Fe(FeWorkIdentifier::GlyphOrder) => "glyphorder",
AnyWorkId::Fe(FeWorkIdentifier::Kerning) => "kern",
Expand Down Expand Up @@ -159,6 +160,7 @@ fn color(id: &AnyWorkId) -> &'static str {
AnyWorkId::Fe(FeWorkIdentifier::Features) => "#e18707",
AnyWorkId::Fe(FeWorkIdentifier::GlobalMetrics) => "gray",
AnyWorkId::Fe(FeWorkIdentifier::Glyph(..)) => "#830356",
AnyWorkId::Fe(FeWorkIdentifier::GlyphTouchup(..)) => "#720345",
AnyWorkId::Fe(FeWorkIdentifier::GlyphIrDelete(..)) => "gray",
AnyWorkId::Fe(FeWorkIdentifier::GlyphOrder) => "gray",
AnyWorkId::Fe(FeWorkIdentifier::Kerning) => "gray",
Expand Down
7 changes: 3 additions & 4 deletions fontc/src/work.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,9 @@ impl AnyContext {
AnyWorkId::Be(..) => AnyContext::Be(
be_root.copy_for_work(read_access.unwrap_be(), write_access.unwrap_be()),
),
AnyWorkId::Fe(..) => AnyContext::Fe(fe_root.copy_for_work(
Access::custom(move |id: &WorkId| read_access.check(&AnyWorkId::Fe(id.clone()))),
Access::custom(move |id: &WorkId| write_access.check(&AnyWorkId::Fe(id.clone()))),
)),
AnyWorkId::Fe(..) => AnyContext::Fe(
fe_root.copy_for_work(read_access.unwrap_fe(), write_access.unwrap_fe()),
),
AnyWorkId::InternalTiming(..) => {
panic!("Should never create a context for internal timing")
}
Expand Down
Loading

0 comments on commit a25f885

Please sign in to comment.