From 84240b96635eb4ca3c17f1ebb31662a16752fa0e Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 13 Feb 2025 12:47:55 -0500 Subject: [PATCH] [fea-rs] Handle edge case in chain context compilation The way format 1 subtables work means they don't support input classes, _unless_ those classes contain a single glyph, in which case we can pretend they aren't classes. --- fea-rs/src/common.rs | 9 +++ fea-rs/src/compile/lookups/contextual.rs | 18 +++-- .../good/chain_sub_format_choice.fea | 13 +++ .../good/chain_sub_format_choice.ttx | 79 +++++++++++++++++++ 4 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.fea create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.ttx diff --git a/fea-rs/src/common.rs b/fea-rs/src/common.rs index 33807da75..fc99e660d 100644 --- a/fea-rs/src/common.rs +++ b/fea-rs/src/common.rs @@ -88,6 +88,15 @@ impl GlyphOrClass { } } + /// If this is a glyph or a class with exactly one, return it. + pub(crate) fn single_glyph(&self) -> Option { + match self { + GlyphOrClass::Glyph(gid) => Some(*gid), + GlyphOrClass::Class(class) if class.len() == 1 => class.iter().next(), + _ => None, + } + } + pub(crate) fn iter(&self) -> impl Iterator + '_ { let mut idx = 0; std::iter::from_fn(move || { diff --git a/fea-rs/src/compile/lookups/contextual.rs b/fea-rs/src/compile/lookups/contextual.rs index e5b2d59ac..e8599004a 100644 --- a/fea-rs/src/compile/lookups/contextual.rs +++ b/fea-rs/src/compile/lookups/contextual.rs @@ -325,8 +325,10 @@ impl ContextBuilder { self.rules.iter().any(ContextRule::is_chain_rule) } - fn has_glyph_classes(&self) -> bool { - self.rules.iter().any(ContextRule::has_glyph_classes) + fn has_glyph_classes_with_more_than_one_glyph(&self) -> bool { + self.rules + .iter() + .any(ContextRule::has_glyph_classes_with_more_than_one_glyph) } /// If the input sequence can be represented as a class def, return it @@ -345,13 +347,13 @@ impl ContextBuilder { } fn format_1_coverage(&self) -> Option { - if self.has_glyph_classes() { + if self.has_glyph_classes_with_more_than_one_glyph() { return None; } Some( self.rules .iter() - .map(|rule| rule.first_input_sequence_item().to_glyph().unwrap()) + .map(|rule| rule.first_input_sequence_item().single_glyph().unwrap()) .collect::(), ) } @@ -360,7 +362,7 @@ impl ContextBuilder { let coverage = self.format_1_coverage()?.build(); let mut rule_sets = HashMap::<_, Vec<_>>::new(); for rule in &self.rules { - let key = rule.first_input_sequence_item().to_glyph().unwrap(); + let key = rule.first_input_sequence_item().single_glyph().unwrap(); let seq_lookups = rule.lookup_records(in_gpos); let rule = write_layout::SequenceRule::new( rule.context @@ -404,12 +406,12 @@ impl ContextRule { !self.backtrack.is_empty() || !self.lookahead.is_empty() } - fn has_glyph_classes(&self) -> bool { + fn has_glyph_classes_with_more_than_one_glyph(&self) -> bool { self.backtrack .iter() .chain(self.lookahead.iter()) .chain(self.context.iter().map(|(glyphs, _)| glyphs)) - .any(|x| x.is_class()) + .any(|x| x.len() > 1) } fn first_input_sequence_item(&self) -> &GlyphOrClass { @@ -525,7 +527,7 @@ impl ChainContextBuilder { let mut rule_sets = HashMap::<_, Vec<_>>::new(); for rule in &self.0.rules { - let key = rule.first_input_sequence_item().to_glyph().unwrap(); + let key = rule.first_input_sequence_item().single_glyph().unwrap(); let seq_lookups = rule.lookup_records(in_gpos); let rule = write_layout::ChainedSequenceRule::new( rule.backtrack.iter().flat_map(|cls| cls.iter()).collect(), diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.fea b/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.fea new file mode 100644 index 000000000..09c7ebabd --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.fea @@ -0,0 +1,13 @@ + +feature ccmp { + @one = [a]; + @two = [b]; + + # this should compile to format 1, because the class has only a single glyph + lookup test { + sub @one' @two by @two; + sub @two @one' by @two; + } test; + +} ccmp; + diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.ttx b/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.ttx new file mode 100644 index 000000000..0df9fcd82 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/chain_sub_format_choice.ttx @@ -0,0 +1,79 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +