Type: reuse harakat ownership shaping metadata

Reuse resolved features and borrowed GDEF classes for mark ownership,
skip the character-level pass without eligible mark glyphs, and reuse
paint-only boundary classification. Compare non-paint style attributes
without cloning while preserving the previous PartialEq behavior.

Add deterministic font variants and a field-by-field style oracle
to preserve glyph geometry, conservative ownership and NaN handling.
This commit is contained in:
Ahmed Gomaa
2026-11-08 06:55:03 +02:00
parent d8f8ff0222
commit f3e5cfad26
3 changed files with 254 additions and 37 deletions
+92 -25
View File
@@ -14,9 +14,8 @@
//! and exporters read the glyph's run as they always did, so they need no change. The text itself
//! is never changed.
use skrifa::GlyphId;
use skrifa::raw::TableProvider;
use skrifa::raw::tables::gdef::GlyphClassDef;
use skrifa::raw::tables::layout::ClassDef;
use vectorcraft_doc::CharStyle;
use crate::fontdb::FontFace;
@@ -42,19 +41,89 @@ pub(crate) fn is_harakat(c: char) -> bool {
/// and change no glyph, advance or position. Stricter than [`crate::shape`]'s `shapes_alike`
/// (which also ignores baseline shift, tracking, underline, …).
pub(crate) fn paint_only(a: &CharStyle, b: &CharStyle) -> bool {
let mut b = b.clone();
b.fill = a.fill.clone();
b.stroke = a.stroke.clone();
b.stroke_width = a.stroke_width;
b.overprint_fill = a.overprint_fill;
b.overprint_stroke = a.overprint_stroke;
b.stroke_cap = a.stroke_cap;
b.stroke_join = a.stroke_join;
b.stroke_miter_limit = a.stroke_miter_limit;
b.stroke_dash = a.stroke_dash.clone();
b.stroke_align = a.stroke_align;
b.style_name = a.style_name.clone();
*a == b
// Exhaustive: a new attribute must explicitly keep or relax this boundary rule.
let CharStyle {
font_family,
font_style,
font_version,
font_axes,
size,
leading,
tracking,
kerning,
optical_kerning,
baseline_shift,
h_scale,
v_scale,
rotation,
underline,
strikethrough,
all_caps,
features,
position,
small_caps,
char_align,
char_direction,
kashidas,
proportional_metrics,
tate_chu_yoko,
tcy_up_down,
tcy_left_right,
standard_vertical_roman,
digits,
font_height_reference,
no_break,
language,
glyph_alternate,
fill,
stroke,
stroke_width,
stroke_miter_limit,
stroke_dash,
overprint_fill: _,
overprint_stroke: _,
stroke_cap: _,
stroke_join: _,
stroke_align: _,
style_name: _,
} = a;
// Previously b's paint was replaced with a's before comparing the whole styles. Keep
// that PartialEq behavior for non-reflexive paint values (NaN), without cloning paint.
let original_paint = (&a.fill, &a.stroke, &a.stroke_width, &a.stroke_miter_limit, &a.stroke_dash);
let normalized_paint = (fill, stroke, stroke_width, stroke_miter_limit, stroke_dash);
original_paint == normalized_paint
&& font_family == &b.font_family
&& font_style == &b.font_style
&& font_version == &b.font_version
&& font_axes == &b.font_axes
&& size == &b.size
&& leading == &b.leading
&& tracking == &b.tracking
&& kerning == &b.kerning
&& optical_kerning == &b.optical_kerning
&& baseline_shift == &b.baseline_shift
&& h_scale == &b.h_scale
&& v_scale == &b.v_scale
&& rotation == &b.rotation
&& underline == &b.underline
&& strikethrough == &b.strikethrough
&& all_caps == &b.all_caps
&& features == &b.features
&& position == &b.position
&& small_caps == &b.small_caps
&& char_align == &b.char_align
&& char_direction == &b.char_direction
&& kashidas == &b.kashidas
&& proportional_metrics == &b.proportional_metrics
&& tate_chu_yoko == &b.tate_chu_yoko
&& tcy_up_down == &b.tcy_up_down
&& tcy_left_right == &b.tcy_left_right
&& standard_vertical_roman == &b.standard_vertical_roman
&& digits == &b.digits
&& font_height_reference == &b.font_height_reference
&& no_break == &b.no_break
&& language == &b.language
&& glyph_alternate == &b.glyph_alternate
}
/// Is the run boundary at byte `at` of `text`, between styles `before` and `after`, one that
@@ -90,17 +159,15 @@ pub(crate) fn harakat_ranges(text: &str, range: std::ops::Range<usize>) -> Vec<s
/// Does `face`'s GDEF GlyphClassDef name glyph `gid` as class 3 (mark)? False when the face has
/// no GDEF, no class def, or the class is unavailable (typical of AAT/`morx` paths).
#[cfg(test)]
pub(crate) fn is_gdef_mark(face: &FontFace, gid: u32) -> bool {
let Some(font) = face.skrifa() else {
return false;
};
let Ok(gdef) = font.gdef() else {
return false;
};
let Some(Ok(classes)) = gdef.glyph_class_def() else {
return false;
};
GlyphClassDef::new(classes.get(GlyphId::new(gid))) == GlyphClassDef::Mark
mark_classes(face).as_ref().is_some_and(|classes| classes.get(skrifa::GlyphId::new(gid)) == 3)
}
/// Borrow the face's mark classification once for a shaping pass. No GDEF, missing classes
/// or malformed tables keep the conservative cluster-start ownership.
pub(crate) fn mark_classes(face: &FontFace) -> Option<ClassDef<'_>> {
face.skrifa()?.gdef().ok()?.glyph_class_def()?.ok()
}
/// Each glyph's own character, from the shaping `by_char` at character cluster level of the
+22 -12
View File
@@ -1048,8 +1048,11 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
}
// Harakat styled apart from their letters (paint only): shaped with them, each glyph then
// takes its own character's run ([`crate::harakat`]).
let styled_marks =
boundaries().any(|b| run_at(b - 1).1 != run_at(b).1 && crate::harakat::paint_only_mark_boundary(text, b, run_at(b - 1).1, run_at(b).1));
let mark_boundaries: Vec<usize> = boundaries()
.filter(|&b| run_at(b - 1).1 != run_at(b).1 && crate::harakat::paint_only_mark_boundary(text, b, run_at(b - 1).1, run_at(b).1))
.collect();
// Source run starts are sorted. Reuse their classification when checking cluster splits.
let styled_marks = !mark_boundaries.is_empty();
let char_level = std::cell::Cell::new(false);
// Shape the segment as if all of it were in `st`, with the features `more` on top.
@@ -1064,9 +1067,12 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
let Shaped { raw, proportional, vpal, chars } = &mut shaped;
let hb_shaped = face.hb().map(|hb| {
let shaper = face.shaper.shaper(&hb).instance(face.instance.as_ref()).build();
// The baseline and character-level pass use exactly the same resolved features.
// Alternate positioning passes still resolve their own features, with `more` last.
let with_more = |resolved: Vec<Feature>| resolved.into_iter().chain(more.iter().cloned()).collect::<Vec<_>>();
let along_features = with_more(feats.resolve(st));
// gid, cluster, x and y advance, x and y offset.
let shape = |feats: &[Feature], direction: Direction| {
let feats: Vec<Feature> = feats.iter().chain(more).cloned().collect();
let mut buf = UnicodeBuffer::new();
for (i, c) in text_seg.char_indices() {
let cl = (range.start + i) as u32;
@@ -1100,8 +1106,8 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
}
// A buffer whose characters name no script reads as `UNKNOWN`: its plan has none.
let script = Some(buf.script()).filter(|s| *s != harfrust::script::UNKNOWN);
let plan = face.shape_plan(&shaper, buf.direction(), script, language.as_ref(), &feats);
let gb = shaper.shape(buf, ShapeOptions::new().features(&feats).plan(Some(&plan)));
let plan = face.shape_plan(&shaper, buf.direction(), script, language.as_ref(), feats);
let gb = shaper.shape(buf, ShapeOptions::new().features(feats).plan(Some(&plan)));
gb.glyph_infos()
.iter()
.zip(gb.glyph_positions())
@@ -1109,21 +1115,25 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
.collect::<Vec<_>>()
};
let along = if level.is_rtl() { Direction::RightToLeft } else { Direction::LeftToRight };
*raw = shape(&feats.resolve(st), along).into_iter().map(|(g, c, xa, _, xo, yo)| (g, c, xa, xo, yo)).collect();
*raw = shape(&along_features, along).into_iter().map(|(g, c, xa, _, xo, yo)| (g, c, xa, xo, yo)).collect();
let raw = &*raw;
let same_glyphs = |v: &[(u32, u32, i32, i32, i32, i32)]| v.len() == raw.len() && v.iter().zip(raw).all(|(a, b)| a.0 == b.0);
// The same shaping at character level names each glyph's own character (kept only when
// it sets the same glyphs alike, each character in its glyph's cluster).
if styled_marks {
if let Some(classes) = styled_marks.then(|| crate::harakat::mark_classes(face)).flatten()
// Without a mark glyph, no glyph can take a later character's paint. Avoid
// reshaping; the original clusters already give the conservative ownership.
&& raw.iter().any(|g| classes.get(skrifa::GlyphId::new(g.0)) == 3)
{
char_level.set(true);
let by_char = shape(&feats.resolve(st), along);
let by_char = shape(&along_features, along);
char_level.set(false);
*chars = crate::harakat::own_chars(raw, &by_char, range.end, text, |gid| crate::harakat::is_gdef_mark(face, gid));
*chars = crate::harakat::own_chars(raw, &by_char, range.end, text, |gid| classes.get(skrifa::GlyphId::new(gid)) == 3);
}
if feats.proportional(st) {
// `palt` is positioning only: the same glyphs set on their full widths tell which
// advances it changed (kerning and the rest change both alike).
let full = shape(&feats.resolve_fixed_width(st), along);
let full = shape(&with_more(feats.resolve_fixed_width(st)), along);
if same_glyphs(&full) {
*proportional = full.iter().zip(raw).map(|(a, b)| a.2 != b.2).collect();
}
@@ -1134,7 +1144,7 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
// and without either tell how it moves each and changes its advance down the column
// (y up, advances negative). Which advances `vpal` itself changed comes from setting
// them with it alone.
let ttb = |proportional, kerning| shape(&feats.resolve_vertical(st, proportional, kerning), Direction::TopToBottom);
let ttb = |proportional, kerning| shape(&with_more(feats.resolve_vertical(st, proportional, kerning)), Direction::TopToBottom);
let (with, without, alone) = (ttb(true, true), ttb(false, false), ttb(true, false));
if same_glyphs(&with) && same_glyphs(&without) && same_glyphs(&alone) {
*vpal = with
@@ -1237,7 +1247,7 @@ fn shape_segment(text: &str, context: &Range<usize>, seg: &Segment, feats: &OtFe
// Except at harakat styled apart only in paint ([`crate::harakat`]).
let inside_clusters: Vec<usize> = boundaries()
.filter(|&b| cluster_starts.binary_search(&b).is_err() && run_at(b - 1).1 != run_at(b).1)
.filter(|&b| !crate::harakat::paint_only_mark_boundary(text, b, run_at(b - 1).1, run_at(b).1))
.filter(|b| mark_boundaries.binary_search(b).is_err())
.collect();
if !inside_clusters.is_empty() {
return shape_apart(inside_clusters, out);
+140
View File
@@ -152,6 +152,146 @@ fn paint_only_differences_are_told_apart() {
assert_eq!(harakat_ranges("اللَّهِ", 0.."اللَّهِ".len()), [6..10, 12..14], "shadda and fatha together");
}
/// The allocation-free comparison must keep the previous comparison's complete contract,
/// including attributes that affect layout rather than shaping and non-reflexive file values.
#[test]
fn paint_only_keeps_every_nonpaint_attribute_and_partial_equality() {
use vectorcraft_doc::{CharAlign, CharPosition, Dash, Digits, FontHeightReference, GlyphChoice, LineCap, LineJoin, ParaDirection, StrokeAlign};
fn previous(a: &CharStyle, b: &CharStyle) -> bool {
let mut b = b.clone();
b.fill = a.fill.clone();
b.stroke = a.stroke.clone();
b.stroke_width = a.stroke_width;
b.overprint_fill = a.overprint_fill;
b.overprint_stroke = a.overprint_stroke;
b.stroke_cap = a.stroke_cap;
b.stroke_join = a.stroke_join;
b.stroke_miter_limit = a.stroke_miter_limit;
b.stroke_dash = a.stroke_dash.clone();
b.stroke_align = a.stroke_align;
b.style_name = a.style_name.clone();
*a == b
}
fn check(a: &CharStyle, b: &CharStyle, want: bool, name: &str) {
for (left, right) in [(a, b), (b, a)] {
assert_eq!(paint_only(left, right), previous(left, right), "{name}: old contract");
}
assert_eq!(paint_only(a, b), want, "{name}");
}
let a = CharStyle {
fill: Paint::Pattern { pattern: "named pattern".repeat(8), xf: Affine::IDENTITY },
stroke: Paint::Solid { color: Color::BLACK, swatch: Some("global swatch".repeat(8)), tint: 0.5 },
stroke_dash: Some(Dash { pattern: vec![2.0, 3.0, 4.0, 5.0], offset: 0.5, align_corners: true }),
style_name: Some("named character style".repeat(8)),
..style(FAMILY)
};
macro_rules! field {
($field:ident, $value:expr, $want:expr) => {{
let mut b = a.clone();
b.$field = $value;
check(&a, &b, $want, stringify!($field));
}};
}
field!(font_family, "other family".into(), false);
field!(font_style, "Bold".into(), false);
field!(font_version, Some("other version".into()), false);
field!(font_axes, [("wght".into(), 700.0)].into(), false);
field!(size, 13.0, false);
field!(leading, Some(60.0), false);
field!(tracking, 20.0, false);
field!(kerning, Some(0.0), false);
field!(optical_kerning, true, false);
field!(baseline_shift, 1.0, false);
field!(h_scale, 90.0, false);
field!(v_scale, 90.0, false);
field!(rotation, 3.0, false);
field!(underline, true, false);
field!(strikethrough, true, false);
field!(all_caps, true, false);
field!(features, vec!["-liga".into()], false);
field!(position, CharPosition::Superscript(Default::default()), false);
field!(small_caps, Some(80.0), false);
field!(char_align, CharAlign::EmBoxTop, false);
field!(char_direction, Some(ParaDirection::RightToLeft), false);
field!(kashidas, false, false);
field!(proportional_metrics, true, false);
field!(tate_chu_yoko, true, false);
field!(tcy_up_down, 1.0, false);
field!(tcy_left_right, 1.0, false);
field!(standard_vertical_roman, true, false);
field!(digits, Digits::Hindi, false);
field!(font_height_reference, FontHeightReference::CapHeight, false);
field!(no_break, true, false);
field!(language, Some("ar".into()), false);
field!(glyph_alternate, Some(GlyphChoice { ch: 'a', gid: 1, font: "other".into(), version: String::new() }), false);
field!(fill, Paint::None, true);
field!(stroke, Paint::None, true);
field!(stroke_width, 2.0, true);
field!(overprint_fill, true, true);
field!(overprint_stroke, true, true);
field!(stroke_cap, LineCap::Round, true);
field!(stroke_join, LineJoin::Round, true);
field!(stroke_miter_limit, 2.0, true);
field!(stroke_dash, None, true);
field!(stroke_align, StrokeAlign::Outside, true);
field!(style_name, None, true);
// Only the first style's ignored fields survive the old normalization. A NaN in the
// second style's paint is ignored, but reversing the arguments must still return false.
field!(fill, Paint::solid(Color::Gray { k: f32::NAN }), true);
field!(stroke, Paint::solid(Color::Gray { k: f32::NAN }), true);
field!(stroke_width, f64::NAN, true);
field!(stroke_miter_limit, f64::NAN, true);
field!(stroke_dash, Some(Dash { pattern: vec![f64::NAN], offset: 0.0, align_corners: false }), true);
field!(tracking, f64::NAN, false);
}
/// A face without usable mark classification keeps cluster-start paint, without changing
/// its glyphs or positioning when a harakat's paint is edited. All variants are in-memory.
#[test]
fn synthetic_harakat_keep_geometry_with_and_without_gdef_marks() {
use skrifa::raw::TableProvider;
use write_fonts::FontBuilder;
use write_fonts::tables::gdef::Gdef;
use write_fonts::tables::layout::ClassDefFormat1;
use write_fonts::types::{GlyphId16, Tag};
let bytes = crate::test_fonts::lam_alef_font(b"rlig").unwrap();
let font = skrifa::FontRef::new(&bytes).unwrap();
for variant in ["marks", "no GDEF", "no class definition", "base classes"] {
let mut builder = FontBuilder::new();
for r in font.table_directory.table_records() {
let tag = Tag::new(&r.tag().to_be_bytes());
if variant == "marks" || tag != Tag::new(b"GDEF") {
builder.add_raw(tag, font.table_data(r.tag()).unwrap().as_bytes());
}
}
if variant == "no class definition" {
builder.add_table(&Gdef::new(None, None, None, None)).unwrap();
} else if variant == "base classes" {
let classes = ClassDefFormat1::new(GlyphId16::new(0), vec![1; usize::from(font.maxp().unwrap().num_glyphs())]);
builder.add_table(&Gdef::new(Some(classes.into()), None, None, None)).unwrap();
}
let db = FontDb::with_font_dirs(vec![]);
assert_eq!(db.add_font(builder.build()), 1, "{variant}");
let family = crate::test_fonts::lam_alef_family(b"rlig");
let plain = text_with_harakat("بَبَبَ", family, None);
let coloured = text_with_harakat("بَبَبَ", family, Some(red()));
let before = layout(&db, &plain);
let after = layout(&db, &coloured);
assert!(!before.glyphs.is_empty(), "{variant}");
assert_eq!(geometry(&before), geometry(&after), "{variant}");
assert_eq!(coloured.plain_text(), plain.plain_text(), "{variant}");
if variant == "marks" {
assert!(painted(&coloured, &after, &red()) > 0, "{variant}: harakat own their paint");
} else {
assert!(after.glyphs.iter().all(|g| run_of(&coloured, g).style.fill != red()), "{variant}: cluster-start paint");
}
}
}
/// A range is normalised as `edit::style_range` takes one: clamped, each end moved back to the
/// start of its character, put in order. Never a silent empty result for a usable range.
#[test]