From c1971284ceed92c351c5ebbc8a3f263e0cb7ea35 Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:34:57 +0300 Subject: [PATCH] Type: required lam-alef is not an alternate in fonts that put it in liga (#394) Highlight Alternate Glyphs compares each glyph with required shaping alone, which turns liga and calt off. Older Arabic fonts (such as the legacy Arial of the Microsoft core fonts) put the mandatory lam-alef ligature in liga instead of rlig, so every lam-alef was highlighted. A lam + alef cluster (any alef form, marks allowed) is now never an alternate candidate, whatever feature the font uses. Adds a test font built in code (lam-alef under liga or rlig) and host-independent regressions. Also two equivalent one-line repairs strict Clippy needs on main: a nonminimal_bool in the Path Precision Tools inspect check and is_none() in a gradient panel test. --- crates/engine/src/cmd/addons/pathtools.rs | 2 +- crates/render/src/tests/alternate.rs | 36 ++++++++++++++- crates/render/src/text_geom.rs | 16 ++++++- crates/text/src/test_fonts.rs | 50 +++++++++++++++++++++ crates/ui-egui/src/panels/gradient/tests.rs | 2 +- docs/type-parity.md | 5 ++- 6 files changed, 104 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/addons/pathtools.rs b/crates/engine/src/cmd/addons/pathtools.rs index 4e78680ae..47dbe7335 100644 --- a/crates/engine/src/cmd/addons/pathtools.rs +++ b/crates/engine/src/cmd/addons/pathtools.rs @@ -355,7 +355,7 @@ fn inspect(s: &mut Session, p: &Value) -> Result { let id = checked_id(s, p.get("id").unwrap_or(&Value::Null), C)?; let (x, y) = (number_or(p, "x", f64::NAN, C)?, number_or(p, "y", f64::NAN, C)?); let tolerance = number_or(p, "tolerance", 3.0, C)?; - if !(x.is_finite() && y.is_finite()) || !(tolerance >= 0.0 && tolerance.is_finite()) { + if !(x.is_finite() && y.is_finite() && tolerance >= 0.0 && tolerance.is_finite()) { return Err(bad(C, "give `x` and `y` (and a `tolerance` of 0 or more)")); } let path = s.doc()?.doc.node(id).and_then(|n| n.path_data()).ok_or_else(|| bad(C, "`id` is not a path"))?; diff --git a/crates/render/src/tests/alternate.rs b/crates/render/src/tests/alternate.rs index ad39ed065..8ef690330 100644 --- a/crates/render/src/tests/alternate.rs +++ b/crates/render/src/tests/alternate.rs @@ -118,7 +118,9 @@ fn arabic_joining_forms_are_not_alternates() { assert_eq!(alternates(&t), 0); } -/// #394: the lam-alef ligature is required (`rlig`), so it isn't highlighted either. +/// #394: the lam-alef ligature is required (`rlig`), so it isn't highlighted either. With an +/// older font that files it under `liga` (as the legacy Arial of Microsoft's core fonts does) it +/// is still required: [`lam_alef_in_liga_is_not_an_alternate`] checks that without an OS font. #[test] fn lam_alef_is_not_an_alternate() { // Looking up an installed font loads it, which changes the fonts. @@ -129,6 +131,38 @@ fn lam_alef_is_not_an_alternate() { } } +/// #394: a font whose lam-alef ligature is in `liga` (older Arabic fonts) or `rlig` gets no +/// highlight on it: the ligature is required by the characters, not chosen by an optional +/// feature. Built in code, so it runs whatever fonts the machine has. +#[test] +fn lam_alef_in_liga_is_not_an_alternate() { + use vectorcraft_text::test_fonts::{lam_alef_family, lam_alef_font}; + let _adding = crate::tests_fontchange::adding_fonts(); + let db = vectorcraft_text::FontDb::global(); + for feature in [b"liga", b"rlig"] { + db.add_font(lam_alef_font(feature).unwrap()); + let style = vectorcraft_doc::text::CharStyle { size: 36.0, font_family: lam_alef_family(feature).into(), ..Default::default() }; + for s in ["\u{0644}\u{0627}", "\u{0644}\u{0623}", "\u{0628}\u{0644}\u{0627}", "\u{0644}\u{064E}\u{0627}"] { + let t = text(s, style.clone()); + // The font's ligature glyph stands for the pair. + assert!(glyphs(&t).iter().any(|g| g.1 > 2), "{}: {s} is shaped to one ligature glyph", lam_alef_family(feature)); + assert_eq!(alternates(&t), 0, "{}: {s}", lam_alef_family(feature)); + } + } +} + +/// Only a lam followed by an alef (marks between or after allowed) is the required lam-alef. +#[test] +fn only_lam_alef_clusters_are_the_required_ligature() { + for s in ["\u{0644}\u{0627}", "\u{0644}\u{0622}", "\u{0644}\u{0623}", "\u{0644}\u{0625}", "\u{0644}\u{064E}\u{0627}", "\u{0644}\u{0627}\u{064B}"] + { + assert!(crate::text_geom::is_lam_alef(s), "{s:?}"); + } + for s in ["", "\u{0644}", "\u{0627}\u{0644}", "\u{0644}\u{0628}", "\u{0628}\u{0644}\u{0627}", "\u{0644}\u{0627}\u{0627}", "fi", "\u{064E}"] { + assert!(!crate::text_geom::is_lam_alef(s), "{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] diff --git a/crates/render/src/text_geom.rs b/crates/render/src/text_geom.rs index 1408b397b..bf058d75f 100644 --- a/crates/render/src/text_geom.rs +++ b/crates/render/src/text_geom.rs @@ -48,7 +48,8 @@ impl TextGeom { /// 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. + /// lam-alef ligature are what the script needs, not alternates (the lam-alef ligature also when + /// a font files it under `liga`: [`is_lam_alef`]). Measured once. pub(crate) fn alternate_glyphs(&self, t: &TextObject) -> &BezPath { self.alternate_glyphs.get_or_init(|| { let mut out = BezPath::new(); @@ -84,6 +85,17 @@ pub(crate) fn is_opentype_alternate(nominal_gid: u32, shaped_gid: u32, cluster_c shaped_gid != 0 && (cluster_chars > 1 || shaped_gid != nominal_gid) } +/// Whether `cluster` is a lam followed by an alef (plain, with madda, or with hamza above or below), +/// with any Arabic marks between or after them: the lam-alef ligature, which the Unicode +/// Standard's Arabic chapter treats as obligatory, not a stylistic choice. Fonts made before +/// `rlig` was common put it in `liga`, which [`required_shaping`] turns off, so it is required +/// by its characters whatever feature the font files it under. +pub(crate) fn is_lam_alef(cluster: &str) -> bool { + let is_mark = |c: &char| matches!(c, '\u{0610}'..='\u{061A}' | '\u{064B}'..='\u{065F}' | '\u{0670}' | '\u{06D6}'..='\u{06DC}' | '\u{06DF}'..='\u{06E4}' | '\u{06E7}' | '\u{06E8}' | '\u{06EA}'..='\u{06ED}'); + let mut letters = cluster.chars().filter(|c| !is_mark(c)); + matches!((letters.next(), letters.next(), letters.next()), (Some('\u{0644}'), Some('\u{0622}' | '\u{0623}' | '\u{0625}' | '\u{0627}'), None)) +} + /// A glyph [`is_opentype_alternate`] marks, in its cluster of the text (`byte`, `len`), with its /// box. #[derive(Debug)] @@ -181,7 +193,7 @@ pub(crate) fn text_geom_snapped(t: &TextObject, snap: Option) -> TextGeo let cluster_chars = cluster.chars().count(); 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) { + if is_opentype_alternate(nominal, g.gid, cluster_chars) && !is_lam_alef(cluster) { alternate_candidates.push(AlternateCandidate { byte: g.byte, len: g.len, font_id: g.font_id, gid: g.gid, cell }); } } diff --git a/crates/text/src/test_fonts.rs b/crates/text/src/test_fonts.rs index 76f6603ff..449221f9f 100644 --- a/crates/text/src/test_fonts.rs +++ b/crates/text/src/test_fonts.rs @@ -343,6 +343,56 @@ pub fn half_form_font(half: bool) -> Option> { Some(builder.build()) } +/// The family of [`lam_alef_font`] whose lam-alef ligature is in `feature`. +pub fn lam_alef_family(feature: &[u8; 4]) -> &'static str { + match feature { + b"liga" => "Lamtest Liga", + b"rlig" => "Lamtest Rlig", + _ => "Lamtest Other", + } +} + +/// A test-only Arabic cmap over bundled Source Sans 3: lam uses l, alef I, alef with hamza above +/// J and beh b. The `GSUB` table's `arab` script has one ligature lookup, under `feature`, that +/// joins lam + alef into H and lam + alef with hamza above into K. Older fonts put the mandatory +/// lam-alef ligature in `liga` rather than `rlig`; this builds either, without an OS font. `None` +/// when it can't be built. +pub fn lam_alef_font(feature: &[u8; 4]) -> Option> { + use write_fonts::tables::cmap::Cmap; + use write_fonts::tables::gsub::{Gsub, Ligature, LigatureSet, LigatureSubstFormat1, SubstitutionLookup, SubstitutionLookupList}; + use write_fonts::tables::layout::{CoverageFormat1, Feature, FeatureList, FeatureRecord, LangSys, Lookup, Script, ScriptList, ScriptRecord}; + use write_fonts::types::GlyphId16; + + const BASE: &[u8] = include_bytes!("../../../assets/fonts/SourceSans3-Regular.ttf"); + let font = skrifa::FontRef::new(BASE).ok()?; + let charmap = font.charmap(); + let mapping = [('\u{0644}', 'l'), ('\u{0627}', 'I'), ('\u{0623}', 'J'), ('\u{0628}', 'b')]; + let mappings = mapping.into_iter().map(|(c, source)| Some((c, write_fonts::types::GlyphId::new(charmap.map(source)?.to_u32())))); + let cmap = Cmap::from_mappings(mappings.collect::>>()?).ok()?; + let family = lam_alef_family(feature); + let full = format!("{family} Regular"); + let postscript = format!("{}-Regular", family.replace(' ', "")); + let name = name_table(&[(1, family), (2, "Regular"), (4, &full), (6, &postscript)])?; + let mut builder = FontBuilder::new(); + builder.add_table(&cmap).ok()?; + builder.add_raw(Tag::new(b"name"), name); + let gid = |c| Some(GlyphId16::new(u16::try_from(charmap.map(c)?.to_u32()).ok()?)); + let coverage = CoverageFormat1::new(vec![gid('l')?]); + let ligatures = vec![Ligature::new(gid('H')?, vec![gid('I')?]), Ligature::new(gid('K')?, vec![gid('J')?])]; + let subst = LigatureSubstFormat1::new(coverage.into(), vec![LigatureSet::new(ligatures)]); + let features = FeatureList::new(vec![FeatureRecord::new(Tag::new(feature), Feature::new(None, vec![0]))]); + let lookups = SubstitutionLookupList::new(vec![SubstitutionLookup::Ligature(Lookup::new(Default::default(), vec![subst]))]); + let scripts = ScriptList::new(vec![ScriptRecord::new(Tag::new(b"arab"), Script::new(Some(LangSys::new(vec![0])), vec![]))]); + builder.add_table(&Gsub::new(scripts, features, lookups)).ok()?; + for r in font.table_directory.table_records() { + let tag = Tag::new(&r.tag().to_be_bytes()); + if !builder.contains(tag) { + builder.add_raw(tag, font.table_data(r.tag())?.as_bytes()); + } + } + Some(builder.build()) +} + /// The family of [`pair_kerning_font`]. pub const PAIR_KERNING_FAMILY: &str = "Pairtest Sans"; diff --git a/crates/ui-egui/src/panels/gradient/tests.rs b/crates/ui-egui/src/panels/gradient/tests.rs index 7d9dcbf9e..a3b7f5cfa 100644 --- a/crates/ui-egui/src/panels/gradient/tests.rs +++ b/crates/ui-egui/src/panels/gradient/tests.rs @@ -660,7 +660,7 @@ fn the_panel_menu_saves_deletes_and_hides_presets() { let mut p = Panel::new(); let texts = p.frame_of(&mut app, vec![], Modifiers::NONE, menu); let save = text_at(&texts, "Save Gradient Preset…").unwrap_or_else(|| panic!("{texts:?}")); - assert!(!app.session.active_saved_gradient_preset().is_some()); + assert!(app.session.active_saved_gradient_preset().is_none()); p.menu_click(&mut app, save); let d = app.ui.dialog.take().expect("the name dialog"); assert_eq!((d.fields["__command"].as_str(), d.fields["name"].as_str()), (Some("gradient.presets.save"), Some("Gradient Preset 1"))); diff --git a/docs/type-parity.md b/docs/type-parity.md index c4130f0e5..c0220b193 100644 --- a/docs/type-parity.md +++ b/docs/type-parity.md @@ -1,6 +1,6 @@ # Type parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-30 · **Change:** minor (Character panel menu › Show Snap to Glyph Options shows the Snap to Glyph section (Baseline, x-height, Glyph Bounds, Proximity Guides, Angular Guides, Anchor Point; `view.snapToGlyph.options`) (#4232); scores unchanged) · **Target:** Adobe Illustrator 2026 (30.x) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-30 · **Change:** minor (Highlight Alternate Glyphs leaves the required lam-alef ligature unhighlighted also in older Arabic fonts (such as legacy Arial) that put it in `liga` rather than `rlig` (#394); scores unchanged) · **Target:** Adobe Illustrator 2026 (30.x) Type is Illustrator's largest single area by use (weight 13 of 105 across "Type core" and "Type advanced" in [target-app-parity.md](target-app-parity.md)). This checklist lists the features one by one. Status is from the @@ -28,7 +28,7 @@ against Illustrator's public documentation; **estimated** unless noted. | Character and Paragraph panels, styles with overrides | done | Character-style Auto-leading resets, partial definitions, local leading overrides, override markers, Undo/Redo and native serialization have regression coverage; differing paragraph alignments leave the shared alignment buttons unselected (headless regressions, full integration CI and visual review pass); mixed-value display for other paragraph fields remains missing | | Character size and horizontal scale after object/group scaling | done | #1034: fields and setters compensate the stored affine; point size follows the transformed em-height axis, not the glyph ink bounds. Range edits, keyboard increments, leading and baseline shift use document points; rotation, reflection and shear remain editable. | | Preserve font sizes while scaling a group | missing | Separate enhancement; scaling still scales text with the other artwork. | -| OpenType panel, Glyphs panel, alternates | done | Highlight Alternate Glyphs preference (#394, off by default) paints pink behind OpenType substitutions on screen when on; stylistic sets ss01–ss20, stylistic and titling alternates, proportional and lining figures, superscript and subscript are character-style features (`text.setStyle {features}`, #1607) | +| OpenType panel, Glyphs panel, alternates | done | Highlight Alternate Glyphs preference (#394, off by default) paints pink behind OpenType substitutions on screen when on (required shaping such as Arabic joining forms and the lam-alef ligature, including a lam-alef older fonts file under `liga`, is not highlighted); stylistic sets ss01–ss20, stylistic and titling alternates, proportional and lining figures, superscript and subscript are character-style features (`text.setStyle {features}`, #1607) | | Font menu: samples, live preview, filters, favourites, recent families; font matching by any name; Find Font | done | #3515: recently applied families appear first, once each; missing families are omitted and the existing Number of Recent Fonts setting applies; #339; script filter: Japanese, Latin, Thai; the font scan on Linux also reads the folders fontconfig's configuration lists, such as TeX fonts under `/usr/share/texmf/fonts` (#1548, fixes #1529; not verified on Linux) | | Missing glyphs set in an installed font made for their script (Thai, Lao, Khmer, Myanmar first; then any font that has them) | done | Thai: Leelawadee UI, Tahoma, Thonburi, Sukhumvit Set, Noto Sans Thai, TLWG | | Full font-name matching (OpenType name ID 4) | done | #1258: loaded/cataloged faces and Missing Fonts searches; localized names and variable default faces; Windows font-service QA pending | @@ -71,6 +71,7 @@ Character Styles menu New/Redefine and footer New capture the first selected cha | Date | Change | Summary | |---|---|---| +| 2026-10-30 | minor | Highlight Alternate Glyphs leaves the required lam-alef ligature unhighlighted also in older Arabic fonts (such as legacy Arial) that put it in `liga` rather than `rlig` (#394); scores unchanged | | 2026-10-30 | minor | Character panel menu › Show Snap to Glyph Options shows the Snap to Glyph section (Baseline, x-height, Glyph Bounds, Proximity Guides, Angular Guides, Anchor Point; `view.snapToGlyph.options`) (#4232); scores unchanged | | 2026-10-29 | minor | Font fallback keeps a letter and its combining marks in one face; Vietnamese stacked diacritics (Ể, Ứ, Ự) attach in precomposed, decomposed and keyboard forms (#4491); scores and hours unchanged | | 2026-10-25 | minor | Threading ports: a double-click on a threaded port with the Selection tool breaks the thread there (`text.thread.break`); the story flows into the frames before the break; scores unchanged |