mirror of
https://github.com/storytold/photocraft.git
synced 2026-11-11 00:11:46 +00:00
fix(text): #8247 colour glyph review findings (fan-out graphs, hidden glyphs, CMYK, clips, radial)
Works through the five CodeRabbit threads on d9102e7e0a:
- A COLR graph that reuses both children of every composite was walked
exponentially often (skrifa 0.44 only limits depth and cycles on the
current path), stalling rendering and measuring. photocraft-text moves to
skrifa 0.47, already in the lockfile, which caps a walk at 16384 paints;
such a glyph falls back to its plain outline like a cycle does.
- Glyphs hidden by a transparent text colour are skipped before their graph
is walked or painted, so they can't spend the budget and drop the
visible text.
- In CMYK (and other non-RGB) documents, pixels in the text colour alone
keep its own components instead of a round trip through sRGB, as plain
glyphs do (a K-only black stays K only).
- Nested clips intersect by the smaller coverage, so a glyph clipped by its
own outline twice keeps its anti-aliased edge.
- A radial gradient's shared point of touching circles (a = b = c = 0) is
painted with the largest t instead of dividing 0 by 0.
Each fix has a test that fails without it.
Co-Authored-By: adi-1108 <adi-1108@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5ac21d0166
commit
4e22c92889
5 files changed
+148
-7
No files matched your search
Generated
+1
-1
@@ -5767,7 +5767,7 @@ dependencies = [
|
||||
"photocraft-psd",
|
||||
"photocraft-raster",
|
||||
"rayon",
|
||||
"skrifa 0.44.0",
|
||||
"skrifa 0.47.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -18,7 +18,7 @@ photocraft-psd = { workspace = true }
|
||||
# No `system` feature: fontique's system backend links fontconfig on Linux. We scan font
|
||||
# directories ourselves (native) and always register the bundled fonts (native + web).
|
||||
parley = { version = "0.11", default-features = false, features = ["std"] }
|
||||
skrifa = "0.44"
|
||||
skrifa = "0.47"
|
||||
flate2 = { workspace = true }
|
||||
|
||||
# ICU dictionary data for Japanese word boundaries and complex-script (Thai, Lao, Khmer, Burmese)
|
||||
|
||||
@@ -164,6 +164,16 @@ impl Paint {
|
||||
let c = px * px + py * py - r0 * r0;
|
||||
let ok = |t: f32| t.is_finite() && r0 + t * dr >= 0.0;
|
||||
if a.abs() < 1e-6 {
|
||||
if b == 0.0 && c == 0.0 {
|
||||
// Every circle passes through p (the shared point of touching circles),
|
||||
// so the largest t wins: where shrinking circles reach radius 0, or the
|
||||
// end of the colour line when they grow. Equal circles paint nothing.
|
||||
return match dr.partial_cmp(&0.0) {
|
||||
Some(std::cmp::Ordering::Less) => Some(line.at(-r0 / dr)),
|
||||
Some(std::cmp::Ordering::Greater) => line.stops.last().map(|s| s.1),
|
||||
_ => None,
|
||||
};
|
||||
}
|
||||
let t = c / (2.0 * b);
|
||||
return ok(t).then(|| line.at(t));
|
||||
}
|
||||
@@ -372,7 +382,9 @@ impl<'a, F: FnMut(Shape, &Xform, &mut Coverage)> Painter<'a, F> {
|
||||
if self.clips.len() > base { self.clips.last() } else { None }
|
||||
}
|
||||
|
||||
/// Rasterises `shape` into a clip mask, intersected with the clip around it in this layer.
|
||||
/// Rasterises `shape` into a clip mask, intersected with the clip around it in this layer:
|
||||
/// the smaller coverage per pixel, so a glyph clipped by its own outline again (or by a box
|
||||
/// around it) keeps its anti-aliased edge instead of squaring it.
|
||||
fn push_shape(&mut self, shape: Shape) {
|
||||
// A mask is kept at one f32 per pixel; while it is built, the coverage accumulator and
|
||||
// the finished vector are live side by side.
|
||||
@@ -396,7 +408,7 @@ impl<'a, F: FnMut(Shape, &Xform, &mut Coverage)> Painter<'a, F> {
|
||||
let mode = self.antialias;
|
||||
c.iter_mut().for_each(|v| *v = shape_antialias_coverage(*v, mode));
|
||||
if let Some(outer) = self.layer_clip() {
|
||||
c.iter_mut().zip(outer).for_each(|(v, o)| *v *= o);
|
||||
c.iter_mut().zip(outer).for_each(|(v, o)| *v = v.min(*o));
|
||||
}
|
||||
self.clips.push(c);
|
||||
}
|
||||
@@ -820,6 +832,35 @@ mod tests {
|
||||
assert!(p.at(2.0, 0.5).is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn touching_circles_paint_their_shared_point() {
|
||||
// Circles grow from radius 0 at the origin to the right, all through the origin: there
|
||||
// the largest t is unbounded, the end of the colour line (it used to divide 0 by 0).
|
||||
let p = Paint::Radial { c0: (0.0, 0.0), r0: 0.0, cd: (1.0, 0.0), dr: 1.0, line: line(Extend::Pad) };
|
||||
assert_eq!(p.at(0.0, 0.0), Some(BLUE));
|
||||
assert!(p.at(1.0, 0.0).is_some());
|
||||
// Shrinking circles, all through (2, 0): the largest t is where the radius reaches 0.
|
||||
let p = Paint::Radial { c0: (0.0, 0.0), r0: 2.0, cd: (2.0, 0.0), dr: -2.0, line: line(Extend::Pad) };
|
||||
assert_eq!(p.at(2.0, 0.0), Some(BLUE));
|
||||
// Equal circles in the same place: no cone, nothing painted, not even on the circle.
|
||||
let p = Paint::Radial { c0: (0.0, 0.0), r0: 1.0, cd: (0.0, 0.0), dr: 0.0, line: line(Extend::Pad) };
|
||||
assert_eq!(p.at(1.0, 0.0), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_glyph_clipped_by_its_own_outline_keeps_its_edge() {
|
||||
// The same half-covered outline twice: the edge stays half opaque, not a quarter.
|
||||
let mut p =
|
||||
Painter::new(1, 1, |_: Shape, m: &Xform, c: &mut Coverage| rect_to(c, m, 0.0, 0.0, 0.5, 1.0), Xform::IDENTITY, ink(), AntiAlias::Smooth, PLENTY);
|
||||
p.push_clip_glyph(GlyphId::new(1));
|
||||
p.push_clip_glyph(GlyphId::new(1));
|
||||
p.fill(solid(0));
|
||||
p.pop_clip();
|
||||
p.pop_clip();
|
||||
let a = p.finish().unwrap_or_default().first().map_or(0.0, |c| c[3]);
|
||||
assert!((a - 0.5).abs() < 1e-5, "{a}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sweep_gradient_turns_clockwise_from_the_start_angle() {
|
||||
// skrifa's angles: clockwise, i.e. 360° minus the font's counter-clockwise ones.
|
||||
|
||||
@@ -305,3 +305,90 @@ fn colour_glyphs_keep_extended_text_colours_in_float_documents() {
|
||||
let int = pixels(&layout_with("H", font, color), PixelFormat::RGBA8);
|
||||
assert!(int.iter().filter(|p| p[3] > 0.01).all(|p| p[0] == 1.0 && p[1] == 0.0));
|
||||
}
|
||||
|
||||
/// A COLR v1 table whose `base` paints `paint` (format bytes, offsets relative to each table).
|
||||
fn colr_v1(base: u16, paint: &[u8]) -> Vec<u8> {
|
||||
let mut t = Vec::new();
|
||||
for v in [1u16, 0] {
|
||||
t.extend_from_slice(&v.to_be_bytes());
|
||||
}
|
||||
t.extend_from_slice(&0u32.to_be_bytes());
|
||||
t.extend_from_slice(&0u32.to_be_bytes());
|
||||
t.extend_from_slice(&0u16.to_be_bytes());
|
||||
t.extend_from_slice(&34u32.to_be_bytes());
|
||||
for _ in 0..4 {
|
||||
t.extend_from_slice(&0u32.to_be_bytes());
|
||||
}
|
||||
t.extend_from_slice(&1u32.to_be_bytes());
|
||||
t.extend_from_slice(&base.to_be_bytes());
|
||||
t.extend_from_slice(&10u32.to_be_bytes());
|
||||
t.extend_from_slice(paint);
|
||||
t
|
||||
}
|
||||
|
||||
/// `depth` PaintComposites, each with the next one as both its source and its backdrop, over
|
||||
/// `base` filled with palette entry `entry`: 2^depth fills from a few dozen bytes.
|
||||
fn fan_out(base: u16, depth: usize, entry: u16) -> Vec<u8> {
|
||||
let mut p = Vec::new();
|
||||
for _ in 0..depth {
|
||||
// PaintComposite (format 32): source 8 bytes in, SrcOver, backdrop 8 bytes in.
|
||||
p.extend_from_slice(&[32, 0, 0, 8, 3, 0, 0, 8]);
|
||||
}
|
||||
// PaintGlyph (format 10) of `base`, then PaintSolid (format 2), alpha 1.
|
||||
p.extend_from_slice(&[10, 0, 0, 6]);
|
||||
p.extend_from_slice(&base.to_be_bytes());
|
||||
p.push(2);
|
||||
p.extend_from_slice(&entry.to_be_bytes());
|
||||
p.extend_from_slice(&(1i16 << 14).to_be_bytes());
|
||||
p
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_colour_glyph_that_fans_out_draws_its_outline() {
|
||||
// A small graph that reuses both children of every composite would be walked 65536 times
|
||||
// over (for the bounds, the ink and the paint); skrifa stops after a fixed number of paints
|
||||
// and the glyph keeps its plain outline, as with a cycle.
|
||||
let inter = fonts::INTER_REGULAR.as_slice();
|
||||
let h = gid(inter, 'H');
|
||||
let font = with_tables(inter, &[(*b"COLR", colr_v1(h, &fan_out(h, 16, 0))), (*b"CPAL", cpal(&[RED]))]);
|
||||
let green = Color::rgb(0.0, 1.0, 0.0);
|
||||
let px = pixels(&layout_with("H", font, green), PixelFormat::RGBA8);
|
||||
let solid: Vec<_> = px.iter().filter(|p| p[3] > 0.99).collect();
|
||||
assert!(solid.len() > 100, "{} pixels", solid.len());
|
||||
assert!(solid.iter().all(|p| p[1] > 0.98 && p[0] < 0.02), "{:?}", solid.first());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_hidden_colour_glyph_does_not_spend_the_visible_texts_budget() {
|
||||
// A huge colour H at opacity 0 whose paint alone would overrun the pixel budget, then a
|
||||
// plain I in green: the H paints nothing anyway, and the I still shows.
|
||||
let inter = fonts::INTER_REGULAR.as_slice();
|
||||
let h = gid(inter, 'H');
|
||||
let font = with_tables(inter, &[(*b"COLR", colr_v1(h, &fan_out(h, 11, 0))), (*b"CPAL", cpal(&[RED]))]);
|
||||
let style = |color| CharStyle { size_pt: 600.0, color, ..Default::default() };
|
||||
let runs = vec![TextRun { len: 1, style: style(Color::TRANSPARENT) }, TextRun { len: 1, style: style(Color::rgb(0.0, 1.0, 0.0)) }];
|
||||
let layer = TextLayer { text: "HI".into(), font_family: "Inter".into(), runs, ..Default::default() };
|
||||
let mut l = TextEngine::new().layout(&layer, 72.0);
|
||||
for f in &mut l.faces {
|
||||
f.font = parley::FontData::new(parley::fontique::Blob::new(Arc::new(font.clone())), 0);
|
||||
}
|
||||
let px = pixels(&l, PixelFormat::RGBA8);
|
||||
let green = px.iter().filter(|p| p[3] > 0.99 && p[1] > 0.98 && p[0] < 0.02).count();
|
||||
assert!(green > 1000, "{green} green pixels");
|
||||
assert!(px.iter().all(|p| p[3] < 0.01 || p[0] < 0.02), "the hidden H showed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn colour_glyph_layers_in_the_text_colour_keep_its_cmyk_components() {
|
||||
// In a CMYK document a K-only text colour stays K only in a 0xFFFF layer, as in plain text,
|
||||
// instead of coming back from sRGB as a four-colour black.
|
||||
let inter = fonts::INTER_REGULAR.as_slice();
|
||||
let h = gid(inter, 'H');
|
||||
let font = with_tables(inter, &[(*b"COLR", colr_v0(h, &[(h, 0xFFFF)])), (*b"CPAL", cpal(&[RED]))]);
|
||||
let k = Color { mode: ColorMode::Cmyk, c: [0.0, 0.0, 0.0, 1.0], alpha: 1.0 };
|
||||
let r = rasterize(&layout_with("H", font, k), &Affine::IDENTITY, PixelFormat::CMYKA8, AntiAlias::Sharp);
|
||||
let data = r.surface.read_region(r.rect);
|
||||
let solid: Vec<&[f32; 5]> = data.as_chunks::<5>().0.iter().filter(|p| p[4] > 0.99).collect();
|
||||
assert!(solid.len() > 100, "{} pixels", solid.len());
|
||||
assert!(solid.iter().all(|p| p[0] < 0.01 && p[1] < 0.01 && p[2] < 0.01 && p[3] > 0.99), "{:?}", solid.first());
|
||||
}
|
||||
@@ -376,6 +376,12 @@ fn paint_color_glyphs(
|
||||
let keep_range = |v: f32| if format.sample == SampleType::F32 && v.is_finite() { v } else { v.clamp(0.0, 1.0) };
|
||||
for g in &layout.glyphs {
|
||||
let (Some(st), Some(face)) = (layout.styles.get(g.style as usize), layout.faces.get(g.face as usize)) else { continue };
|
||||
// A glyph the text colour's opacity hides paints nothing; walking it would only spend the
|
||||
// budget the visible text needs (as the plain passes skip it, `visible_color_passes`).
|
||||
let opacity = st.color.alpha.clamp(0.0, 1.0);
|
||||
if opacity.is_nan() || opacity <= 0.0 {
|
||||
continue;
|
||||
}
|
||||
let Ok(font) = skrifa::FontRef::from_index(face.font.data.as_ref(), face.font.index) else { continue };
|
||||
let coords: Vec<NormalizedCoord> = face.coords.iter().map(|&c| NormalizedCoord::from_bits(c)).collect();
|
||||
let Some(color) = color_glyph(&font, GlyphId::new(g.id), &coords) else { continue };
|
||||
@@ -398,6 +404,9 @@ fn paint_color_glyphs(
|
||||
let palettes = font.color_palettes();
|
||||
let palette = palettes.get(0);
|
||||
let [r, gr, bl] = st.color.to_rgb();
|
||||
// Pixels in the text colour alone keep its own components, as plain glyphs do (a CMYK
|
||||
// black stays K only instead of coming back from sRGB as a four-colour black).
|
||||
let native = (format.mode != ColorMode::Rgb).then(|| color_in(format, &st.color));
|
||||
let outline = |shape: Shape, m: &Xform, s: &mut Coverage| shape_into(&font, &coords, shape, unit.mul(m), &to_window, bend, s);
|
||||
let ink = Ink { palette: palette.as_ref().map_or(&[][..], |p| p.colors()), foreground: [r, gr, bl, 1.0] };
|
||||
let mut painter = Painter::new(ww, wh, outline, to_window.mul(&unit), ink, antialias, limits);
|
||||
@@ -406,13 +415,17 @@ fn paint_color_glyphs(
|
||||
}
|
||||
// Compositing the window into the text is one more pass over it.
|
||||
limits.work = painter.work_left().checked_sub((ww as u64).checked_mul(wh as u64)?)?;
|
||||
let opacity = st.color.alpha.clamp(0.0, 1.0);
|
||||
for (i, s) in painter.finish()?.iter().enumerate() {
|
||||
let a = s[3] * opacity;
|
||||
let o = ((wy0 + i / ww) * w + wx0 + i % ww) * stride;
|
||||
let (true, Some(px)) = (a > 0.0, acc.get_mut(o..o + stride)) else { continue };
|
||||
let rgb = [s[0] / s[3], s[1] / s[3], s[2] / s[3]].map(keep_range);
|
||||
let comps = if format.mode == ColorMode::Rgb { rgb.to_vec() } else { photocraft_raster::from_rgba(&opaque, [rgb[0], rgb[1], rgb[2], 1.0]) };
|
||||
let straight = [s[0] / s[3], s[1] / s[3], s[2] / s[3]];
|
||||
let rgb = straight.map(keep_range);
|
||||
let comps = match &native {
|
||||
None => rgb.to_vec(),
|
||||
Some(c) if straight.iter().zip([r, gr, bl]).all(|(v, f)| (v - f).abs() < 1e-5) => c.iter().copied().map(keep_range).collect(),
|
||||
Some(_) => photocraft_raster::from_rgba(&opaque, [rgb[0], rgb[1], rgb[2], 1.0]),
|
||||
};
|
||||
let keep = 1.0 - a;
|
||||
for (p, c) in px.iter_mut().zip(&comps).take(n) {
|
||||
*p = c * a + *p * keep;
|
||||
|
||||
Reference in new issue
Block a user