Merge pull request #2887 from ham9zah/fix/arabic-alternate-glyph-highlight

Part of #394: required shaping is not an alternate glyph

crates/render/src/lib.rs

@@ -1823,10 +1823,12 @@ impl Renderer {
         if f.opts.highlight_substitutions {
             let setup = &f.doc.setup;
             for (on, path) in [
-                (setup.highlight_substituted_fonts, &g.substituted_fonts),
-                (setup.highlight_substituted_glyphs, &g.substituted_glyphs),
-                (f.opts.highlight_alternate_glyphs, &g.alternate_glyphs),
+                (setup.highlight_substituted_fonts, Some(&g.substituted_fonts)),
+                (setup.highlight_substituted_glyphs, Some(&g.substituted_glyphs)),
+                // Measured only when shown: it lays the text out again.
+                (f.opts.highlight_alternate_glyphs, f.opts.highlight_alternate_glyphs.then(|| g.alternate_glyphs(t))),
             ] {
+                let Some(path) = path else { continue };
                 if on && !path.elements().is_empty() {
                     ctx.set_transform(xf);
                     ctx.set_fill_rule(peniko::Fill::NonZero);
@@ -2254,9 +2256,13 @@ struct TextGeom {
     /// font lacks (drawn from a fallback font): Document Setup's substitution highlights.
     substituted_fonts: BezPath,
     substituted_glyphs: BezPath,
-    /// Boxes of glyphs OpenType features substituted (ligatures, contextual forms): Preferences ›
-    /// Highlight Alternate Glyphs.
-    alternate_glyphs: BezPath,
+    /// Glyphs OpenType may have substituted (a ligature, or not the face's cmap glyph for their
+    /// character), each with its box: what Preferences › Highlight Alternate Glyphs weighs
+    /// ([`TextGeom::alternate_glyphs`]).
+    alternate_candidates: Vec<AlternateCandidate>,
+    /// The boxes of the candidates that are alternates, measured the first time the preference
+    /// shows them.
+    alternate_glyphs: std::sync::OnceLock<BezPath>,
     /// Inline graphics placed by the layout.
     inlines: Vec<vectorcraft_text::InlineGlyph>,
 }
@@ -2279,10 +2285,32 @@ impl TextGeom {
         std::borrow::Cow::Owned(p)
     }
 
+    /// The boxes of the glyphs of `t` (the text this geometry is of) that optional OpenType features
+    /// chose (Preferences › Highlight Alternate Glyphs): candidates the text's required shaping
+    /// alone doesn't give ([`required_shaping`]). Arabic letters joining, Indic conjuncts and the
+    /// lam-alef ligature are what the script needs, not alternates. Measured once.
+    fn alternate_glyphs(&self, t: &TextObject) -> &BezPath {
+        self.alternate_glyphs.get_or_init(|| {
+            let mut out = BezPath::new();
+            if self.alternate_candidates.is_empty() {
+                return out;
+            }
+            let required = required_shaping(t);
+            for c in &self.alternate_candidates {
+                if !required.get(&(c.byte, c.len)).is_some_and(|glyphs| glyphs.contains(&(c.font_id, c.gid))) {
+                    out.extend(c.cell.iter());
+                }
+            }
+            out
+        })
+    }
+
     /// Approximate heap bytes, for the cache's budget.
     fn bytes(&self) -> usize {
         size_of::<Self>()
-            + [&self.all, &self.substituted_fonts, &self.substituted_glyphs, &self.alternate_glyphs].into_iter().map(lru::path_bytes).sum::<usize>()
+            + [&self.all, &self.substituted_fonts, &self.substituted_glyphs].into_iter().map(lru::path_bytes).sum::<usize>()
+            + self.alternate_candidates.iter().map(|c| size_of::<AlternateCandidate>() + lru::path_bytes(&c.cell)).sum::<usize>()
+            + self.alternate_glyphs.get().map_or(0, lru::path_bytes)
             + self.spans.len() * size_of::<(usize, usize, usize)>()
             + self.inlines.len() * size_of::<vectorcraft_text::InlineGlyph>()
     }
@@ -2294,6 +2322,35 @@ pub(crate) fn is_opentype_alternate(nominal_gid: u32, shaped_gid: u32, cluster_c
     shaped_gid != 0 && (cluster_chars > 1 || shaped_gid != nominal_gid)
 }
 
+/// A glyph [`is_opentype_alternate`] marks, in its cluster of the text (`byte`, `len`), with its
+/// box.
+#[derive(Debug)]
+struct AlternateCandidate {
+    byte: usize,
+    len: usize,
+    font_id: u32,
+    gid: u32,
+    cell: BezPath,
+}
+
+/// The glyphs `t` is shaped to by required shaping alone, by cluster (`byte`, `len`) → `(font,
+/// glyph)`: laid out again with every optional feature off (standard and contextual ligatures and
+/// alternates, and the features its characters turn on: stylistic sets, swashes, discretionary
+/// ligatures, figure styles…). What is left is what the script needs: `ccmp`, `locl`, `rlig`,
+/// Arabic's `init`, `medi`, `fina` and `isol`, the Indic and other complex scripts' required sets.
+fn required_shaping(t: &TextObject) -> std::collections::HashMap<(usize, usize), Vec<(u32, u32)>> {
+    let mut plain = t.clone();
+    for run in &mut plain.runs {
+        run.style.features = vec!["-liga".into(), "-calt".into()];
+    }
+    let layout = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &plain);
+    let mut glyphs: std::collections::HashMap<(usize, usize), Vec<(u32, u32)>> = std::collections::HashMap::new();
+    for g in &layout.glyphs {
+        glyphs.entry((g.byte, g.len)).or_default().push((g.font_id, g.gid));
+    }
+    glyphs
+}
+
 fn text_geom(t: &TextObject) -> TextGeom {
     text_geom_snapped(t, None)
 }
@@ -2334,7 +2391,7 @@ fn text_geom_snapped(t: &TextObject, snap: Option<Affine>) -> TextGeom {
             None => (true, None),
         })
         .collect();
-    let (mut substituted_fonts, mut substituted_glyphs, mut alternate_glyphs) = (BezPath::new(), BezPath::new(), BezPath::new());
+    let (mut substituted_fonts, mut substituted_glyphs, mut alternate_candidates) = (BezPath::new(), BezPath::new(), vec![]);
     let plain = t.plain_text();
     for g in &layout.glyphs {
         push(&mut all, g.run, &g.outline);
@@ -2356,14 +2413,23 @@ fn text_geom_snapped(t: &TextObject, snap: Option<Affine>) -> TextGeom {
         let Some(first) = cluster.chars().next() else { continue };
         let nominal = db.face_by_id(g.font_id).map(|f| f.glyph_for(first)).unwrap_or(0);
         if is_opentype_alternate(nominal, g.gid, cluster_chars) {
-            alternate_glyphs.extend(cell.iter());
+            alternate_candidates.push(AlternateCandidate { byte: g.byte, len: g.len, font_id: g.font_id, gid: g.gid, cell });
         }
     }
     // Underline and strikethrough bars, painted as their run's type is (#847).
     for (run, bar) in vectorcraft_text::decorations(&layout, db, t) {
         push(&mut all, run, &bar);
     }
-    TextGeom { all, spans, bounds: layout.bounds, substituted_fonts, substituted_glyphs, alternate_glyphs, inlines: layout.inlines }
+    TextGeom {
+        all,
+        spans,
+        bounds: layout.bounds,
+        substituted_fonts,
+        substituted_glyphs,
+        alternate_candidates,
+        alternate_glyphs: std::sync::OnceLock::new(),
+        inlines: layout.inlines,
+    }
 }
 
 /// Document Setup's and Preferences › Highlight Alternate Glyphs pink (screen only).

crates/render/src/tests_alternate.rs

@@ -40,3 +40,118 @@ fn highlight_alternate_glyphs_paints_pink_behind_a_ligature() {
     let pinkish = with.pixels.chunks(4).filter(|p| p[0] > 200 && p[1] < 180 && p[2] > 150 && p[2] < 230).count();
     assert!(pinkish > 10, "pink highlight cells behind the ligature ({pinkish} pixels)");
 }
+
+/// The number of glyph boxes Highlight Alternate Glyphs paints for `t`.
+fn alternates(t: &vectorcraft_doc::text::TextObject) -> usize {
+    text_geom(t).alternate_glyphs(t).elements().iter().filter(|e| matches!(e, kurbo::PathEl::MoveTo(_))).count()
+}
+
+/// An installed font that shapes Arabic with OpenType (as `vectorcraft-text`'s bidi tests pick
+/// one): Arial or Geeza Pro on macOS and Windows, Noto Sans Arabic on Linux. No font is kept in
+/// the repository, so the tests that need one say so and stop without it.
+fn arabic_style(test: &str) -> Option<vectorcraft_doc::text::CharStyle> {
+    let db = vectorcraft_text::FontDb::global();
+    let face = ["Arial", "Noto Sans Arabic", "Geeza Pro"]
+        .into_iter()
+        .filter_map(|name| db.face(name, "Regular"))
+        .find(|f| ['خ', 'ل', 'ا', 'م', 'ي'].iter().all(|c| f.covers(*c)));
+    match face {
+        Some(f) => {
+            eprintln!("ASSERTIONS RUN: {test}; Arabic font {} {}", f.family, f.style);
+            Some(vectorcraft_doc::text::CharStyle { size: 36.0, font_family: f.family.clone(), font_style: f.style.clone(), ..Default::default() })
+        }
+        None => {
+            eprintln!("SKIPPED: {test}; no installed font covers Arabic");
+            None
+        }
+    }
+}
+
+fn text(s: &str, style: vectorcraft_doc::text::CharStyle) -> vectorcraft_doc::text::TextObject {
+    vectorcraft_doc::text::TextObject::point(vectorcraft_geom::Point::ZERO, s, style)
+}
+
+/// The glyphs of `t` by cluster, as the layout shapes them.
+fn glyphs(t: &vectorcraft_doc::text::TextObject) -> Vec<(usize, usize, u32)> {
+    vectorcraft_text::layout(vectorcraft_text::FontDb::global(), t).glyphs.iter().map(|g| (g.byte, g.len, g.gid)).collect()
+}
+
+/// #394: Arabic letters take their joining forms (`init`, `medi`, `fina`, `isol`) in every word:
+/// required shaping, not alternates, so ordinary Arabic text shows no pink.
+#[test]
+fn arabic_joining_forms_are_not_alternates() {
+    let Some(style) = arabic_style("arabic_joining_forms_are_not_alternates") else { return };
+    let t = text("خلطة مبتكرة من اللومي الحساوي", style);
+    // The words are shaped: most glyphs are not their letter's cmap glyph.
+    assert!(!text_geom(&t).alternate_candidates.is_empty(), "the shaper joins the letters");
+    assert_eq!(alternates(&t), 0);
+}
+
+/// #394: the lam-alef ligature is required (`rlig`), so it isn't highlighted either.
+#[test]
+fn lam_alef_is_not_an_alternate() {
+    let Some(style) = arabic_style("lam_alef_is_not_an_alternate") else { return };
+    for s in ["لا", "سلام", "الاسم"] {
+        assert_eq!(alternates(&text(s, style.clone())), 0, "{s}");
+    }
+}
+
+/// #394: an optional feature the characters turn on (a stylistic set, swash, stylistic or
+/// discretionary alternate…) is highlighted on the glyphs it changes, and only on those.
+#[test]
+fn optional_features_on_arabic_flag_only_the_glyphs_they_change() {
+    let db = vectorcraft_text::FontDb::global();
+    let s = "خلطة مبتكرة من اللومي الحساوي";
+    // Arabic fonts that may have optional features; the first installed one with a feature that
+    // changes a glyph of the text.
+    let fonts = ["Amiri", "El Messiri", "Noto Naskh Arabic", "Scheherazade New", "Arial", "Noto Sans Arabic", "Geeza Pro"];
+    let tags = ["swsh", "salt", "dlig", "ss01", "ss02", "ss03", "ss04", "ss05", "ss06", "ss07", "ss08", "ss09", "ss10"];
+    let found = fonts.into_iter().filter_map(|name| db.face(name, "Regular")).filter(|f| f.covers('خ') && f.covers('ل')).find_map(|f| {
+        let style = vectorcraft_doc::text::CharStyle { size: 36.0, font_family: f.family.clone(), font_style: f.style.clone(), ..Default::default() };
+        let plain = glyphs(&text(s, style.clone()));
+        tags.into_iter().find_map(|tag| {
+            let t = text(s, vectorcraft_doc::text::CharStyle { features: vec![tag.into()], ..style.clone() });
+            let changed = glyphs(&t).iter().filter(|g| !plain.contains(g)).count();
+            (changed > 0).then(|| (f.family.clone(), tag, t, changed))
+        })
+    });
+    let Some((family, tag, t, changed)) = found else {
+        eprintln!(
+            "SKIPPED: optional_features_on_arabic_flag_only_the_glyphs_they_change; no installed Arabic font has an optional feature for this text"
+        );
+        return;
+    };
+    eprintln!("ASSERTIONS RUN: optional_features_on_arabic_flag_only_the_glyphs_they_change; {family} with {tag}, {changed} glyphs changed");
+    assert_eq!(alternates(&t), changed, "{family} {tag}: the glyphs it changed");
+}
+
+/// Latin keeps its highlight: the `ffi` of "office" is a standard ligature (`liga`).
+#[test]
+fn a_latin_standard_ligature_is_still_an_alternate() {
+    let style = vectorcraft_doc::text::CharStyle { size: 48.0, font_family: "Source Serif 4".into(), ..Default::default() };
+    let t = text("office", style.clone());
+    let shaped = glyphs(&t);
+    assert!(shaped.iter().any(|(_, len, _)| *len > 1), "Source Serif 4 ligates ffi: {shaped:?}");
+    assert!(alternates(&t) >= 1);
+    // Without ligatures nothing is substituted.
+    assert_eq!(alternates(&text("office", vectorcraft_doc::text::CharStyle { features: vec!["-liga".into()], ..style })), 0);
+}
+
+/// #394: in a font the app ships (Source Serif 4), an optional feature turned on for the
+/// characters is highlighted on exactly the glyphs it changes, the others staying clear.
+#[test]
+fn an_optional_feature_flags_only_the_glyphs_it_changes() {
+    let style = vectorcraft_doc::text::CharStyle { size: 36.0, font_family: "Source Serif 4".into(), ..Default::default() };
+    // No ligature in it, so the default features substitute nothing.
+    let s = "Quay 1234 agy";
+    assert_eq!(alternates(&text(s, style.clone())), 0);
+    let plain = glyphs(&text(s, style.clone()));
+    let tags = ["onum", "smcp", "salt", "swsh", "ss01", "ss02", "ss03", "ss04", "ordn", "sups"];
+    let found = tags.into_iter().find_map(|tag| {
+        let t = text(s, vectorcraft_doc::text::CharStyle { features: vec![tag.into()], ..style.clone() });
+        let changed = glyphs(&t).iter().filter(|g| !plain.contains(g)).count();
+        (changed > 0 && changed < plain.len()).then_some((tag, t, changed))
+    });
+    let (tag, t, changed) = found.expect("Source Serif 4 has an optional feature that changes some of these glyphs");
+    assert_eq!(alternates(&t), changed, "{tag}: the glyphs it changed");
+}

crates/render/src/tests_fontchange.rs

@@ -93,10 +93,11 @@ fn variable_font_settings_are_not_highlighted_as_substituted_glyphs() {
     vectorcraft_text::FontDb::global().add_font(two_axis_font().unwrap());
     let font_axes = [("wght".to_string(), 650.0)].into_iter().collect();
     let style = CharStyle { size: 40.0, font_family: TWO_AXIS_FAMILY.into(), font_axes, ..Default::default() };
-    let g = text_geom(&TextObject::point(Point::new(10.0, 60.0), "lIn", style));
+    let t = TextObject::point(Point::new(10.0, 60.0), "lIn", style);
+    let g = text_geom(&t);
     assert!(!g.all.elements().is_empty());
     assert!(g.substituted_glyphs.elements().is_empty() && g.substituted_fonts.elements().is_empty());
-    assert!(g.alternate_glyphs.elements().is_empty());
+    assert!(g.alternate_glyphs(&t).elements().is_empty());
 }
 
 /// Type at 80% opacity, in Multiply and with a drop shadow redraws in a font that becomes