Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Move conversion to contours out of glyph order #568

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[WIP] Move conversion to contours out of glyph order #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8935568
Don't pointlessly block on glyph order.
rsheeter Nov 10, 2023
a25f885
Move some of the global glyph order work into per-glyph jobs. WIP.
rsheeter Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions fontbe/src/cmap.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading