Merge pull request #7679 from dotCooCoo/fix/colr-empty-fill-keeps-clip

Text: a COLR fill that paints nothing keeps the clip around the glyph after it

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).

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::<Vec<_>>()) };
+    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::<Vec<_>>()) };
+    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::<Vec<_>>()) };
+    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]