From 2212af9653438b55104f37ed28f6762e578792f8 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 13:38:31 -0800 Subject: [PATCH] Text: a COLR fill that paints nothing keeps the clip around the glyph after it A fill in a glyph's clip marked the clip as filled before the fill was added. When the fill painted nothing (a palette index past the palette, or a radial gradient that draws no circle), the next glyph painted in the same clip became the clip's only child, and closing the clip kept that glyph on its own, so it drew outside the clip. The clip is now marked filled only when the fill adds the clip's first child, and a fill that adds nothing leaves the mark as it was. --- crates/text/src/colr.rs | 15 ++++++++---- crates/text/src/tests/colr.rs | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/text/src/colr.rs b/crates/text/src/colr.rs index 281ef8a65f..637dd9c243 100644 --- a/crates/text/src/colr.rs +++ b/crates/text/src/colr.rs @@ -508,12 +508,17 @@ impl ColorPainter for Tree<'_> { return; }; let Some(clip) = self.groups.get(at).and_then(|g| g.clip.clone()) else { return }; - if at + 1 == self.groups.len() - && let Some(g) = self.groups.last_mut() - { - g.flooded = g.children.is_empty(); - } + let innermost = at + 1 == self.groups.len(); + let before = self.groups.last().map_or(0, |g| g.children.len()); self.fill_path(&clip, &brush); + // Flooded only when this fill added the group's first child. A fill that paints nothing + // adds none and leaves the flag as it was, so the clip stays around what comes after it. + if innermost + && let Some(g) = self.groups.last_mut() + && g.children.len() > before + { + g.flooded = before == 0; + } } /// The glyph's outline filled, the brush moved by `brush_transform` (the glyph stays). diff --git a/crates/text/src/tests/colr.rs b/crates/text/src/tests/colr.rs index c6c4696f16..e59fb390c9 100644 --- a/crates/text/src/tests/colr.rs +++ b/crates/text/src/tests/colr.rs @@ -288,6 +288,49 @@ fn a_glyph_filled_with_one_colour_is_one_path() { assert_eq!(fill(only), Paint::solid(rgb(COLR_RED))); } +/// A fill that paints nothing (a palette index past the palette) followed by another glyph +/// inside the same glyph's clip keeps the clip: the second glyph is drawn clipped to the first, +/// not on its own. +#[test] +fn a_fill_that_paints_nothing_keeps_the_clip_around_the_glyph_after_it() { + use write_fonts::tables::colr::{BaseGlyphList, BaseGlyphPaint, Colr, LayerList}; + use write_fonts::types::GlyphId16; + let bytes = colr_font(false).unwrap(); + let read = skrifa::FontRef::new(&bytes).unwrap(); + let gid = |ch| GlyphId16::new(u16::try_from(read.charmap().map(ch).unwrap().to_u32()).unwrap()); + let mut colr = Colr::new(0, None, None, 0); + let paint = ColrPaint::glyph(ColrPaint::colr_layers(2, 0), gid('O')); + colr.base_glyph_list = Some(BaseGlyphList::new(1, vec![BaseGlyphPaint::new(gid(COLOUR_CHAR), paint)])).into(); + colr.layer_list = Some(LayerList::new(2, vec![solid(99), ColrPaint::glyph(solid(0), gid('I'))])).into(); + let font = crate::test_fonts::colr_font_with("Colrtest Empty Fill", &colr).unwrap(); + let root = art(&font, COLOUR_CHAR); + let [clipped] = children(&root) else { panic!("{:?}", children(&root).iter().map(|c| c.kind_label()).collect::>()) }; + assert!(matches!(clipped.kind, NodeKind::Group { clip: true, .. }), "the `O` still clips: {}", clipped.kind_label()); + let [clip, inner] = children(clipped) else { panic!("{:?}", children(clipped).iter().map(|c| c.kind_label()).collect::>()) }; + assert!(matches!(clip.kind, NodeKind::Path { clipping: true, .. }), "{}", clip.kind_label()); + assert_eq!(fill(inner), Paint::solid(rgb(COLR_RED))); +} + +/// A fill that paints nothing after a colour flooding the glyph leaves it that glyph filled: +/// one path, as without the empty fill. +#[test] +fn a_fill_that_paints_nothing_after_a_flood_keeps_one_path() { + use write_fonts::tables::colr::{BaseGlyphList, BaseGlyphPaint, Colr, LayerList}; + use write_fonts::types::GlyphId16; + let bytes = colr_font(false).unwrap(); + let read = skrifa::FontRef::new(&bytes).unwrap(); + let gid = |ch| GlyphId16::new(u16::try_from(read.charmap().map(ch).unwrap().to_u32()).unwrap()); + let mut colr = Colr::new(0, None, None, 0); + let paint = ColrPaint::glyph(ColrPaint::colr_layers(2, 0), gid('O')); + colr.base_glyph_list = Some(BaseGlyphList::new(1, vec![BaseGlyphPaint::new(gid(COLOUR_CHAR), paint)])).into(); + colr.layer_list = Some(LayerList::new(2, vec![solid(0), solid(99)])).into(); + let font = crate::test_fonts::colr_font_with("Colrtest Flood Then Empty", &colr).unwrap(); + let root = art(&font, COLOUR_CHAR); + let [only] = children(&root) else { panic!("{:?}", children(&root).iter().map(|c| c.kind_label()).collect::>()) }; + assert!(matches!(only.kind, NodeKind::Path { .. }), "{}", only.kind_label()); + assert_eq!(fill(only), Paint::solid(rgb(COLR_RED))); +} + /// A sweep glyph in a font without clip boxes, its base glyph without an outline (the space's, /// as COLRv1 fonts make them): its image is as large as what it paints. #[test]