From b246ea166abe2bd8230bc9abacda0d07e5f28d5e Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:48:05 +0100 Subject: [PATCH] #3692: COLR gradient stops and clip paths count against a per-glyph budget Finding 61: Recorder copied every gradient's stop list (and every clip glyph's outline) per op, capped only by op count, so a compact COLR graph reusing one long stop list could expand to billions of stops. Stops and path elements now share MAX_PAYLOAD (65,536) per glyph, charged before copying; past it the glyph falls back to its outline and no further clip outlines are built. The render cache charges each glyph's real ops, stops and path bytes (ops_bytes) instead of a flat 64 bytes per op. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/fonts/src/color.rs | 73 ++++++++++++++++++++++++++++++-- crates/render/src/color_glyph.rs | 2 +- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/fonts/src/color.rs b/crates/fonts/src/color.rs index 897f31c96..1f03641b6 100644 --- a/crates/fonts/src/color.rs +++ b/crates/fonts/src/color.rs @@ -18,6 +18,9 @@ use crate::FontFace; /// Upper bound on the ops one colour glyph may expand to. pub const MAX_OPS: usize = 20_000; +/// Upper bound on the gradient stops and clip-path elements one colour glyph may copy in total: a +/// paint graph can reference one long stop list or outline from every op. +pub const MAX_PAYLOAD: usize = 1 << 16; /// Upper bound on a bitmap glyph's width and height in pixels. pub const MAX_BITMAP_SIDE: u32 = 2048; @@ -175,12 +178,26 @@ pub fn bitmap_placement(bg: &skrifa::bitmap::BitmapGlyph<'_>, upem: f64) -> Opti finite.then_some(ColorBitmap { data, width: w, height: h, rect, ppem: bg.ppem_y }) } +/// Approximate heap bytes held by `ops`, counting each op's copied stops and path elements. +pub fn ops_bytes(ops: &[ColorOp]) -> usize { + let payload = |op: &ColorOp| match op { + ColorOp::PushClip(p) => p.elements().len().saturating_mul(std::mem::size_of::()), + ColorOp::Fill(ColorPaint::Linear { stops, .. } | ColorPaint::Radial { stops, .. } | ColorPaint::Sweep { stops, .. }) => { + stops.len().saturating_mul(std::mem::size_of::()) + } + _ => 0, + }; + ops.iter().fold(ops.len().saturating_mul(std::mem::size_of::()), |n, op| n.saturating_add(payload(op))) +} + /// A [`ColorPainter`] that records the paint calls as [`ColorOp`]s, resolving palette colours and -/// clip glyphs. Stops recording (and [`Recorder::finish`] gives None) past [`MAX_OPS`]. +/// clip glyphs. Stops recording (and [`Recorder::finish`] gives None) past [`MAX_OPS`] or +/// [`MAX_PAYLOAD`]. pub struct Recorder<'a, F: Fn(GlyphId) -> BezPath> { palette: &'a [[u8; 4]], outline: F, ops: Vec, + payload: usize, overflow: bool, } @@ -188,7 +205,7 @@ impl<'a, F: Fn(GlyphId) -> BezPath> Recorder<'a, F> { /// A recorder with palette `palette` (straight RGBA) and clip outlines (font units, y up) from /// `outline`. pub fn new(palette: &'a [[u8; 4]], outline: F) -> Self { - Recorder { palette, outline, ops: vec![], overflow: false } + Recorder { palette, outline, ops: vec![], payload: 0, overflow: false } } /// The recorded ops, None when the glyph was too big or its pushes and pops don't balance. @@ -218,6 +235,15 @@ impl<'a, F: Fn(GlyphId) -> BezPath> Recorder<'a, F> { self.ops.push(op); } + /// Charge `n` stops or path elements to the glyph's payload; false (and overflow) past the budget. + fn charge(&mut self, n: usize) -> bool { + self.payload = self.payload.saturating_add(n); + if self.payload > MAX_PAYLOAD { + self.overflow = true; + } + !self.overflow + } + fn color(&self, palette_index: u16, alpha: f32) -> GlyphColor { let alpha = if alpha.is_finite() { alpha.clamp(0.0, 1.0) } else { 1.0 }; if palette_index == 0xFFFF { @@ -249,8 +275,13 @@ impl BezPath> ColorPainter for Recorder<'_, F> { self.push(ColorOp::PopTransform); } fn push_clip_glyph(&mut self, glyph_id: GlyphId) { + if self.overflow { + return; + } let p = (self.outline)(glyph_id); - self.push(ColorOp::PushClip(p)); + if self.charge(p.elements().len()) { + self.push(ColorOp::PushClip(p)); + } } fn push_clip_box(&mut self, b: BoundingBox) { let r = Rect::new(b.x_min as f64, b.y_min as f64, b.x_max as f64, b.y_max as f64); @@ -260,6 +291,15 @@ impl BezPath> ColorPainter for Recorder<'_, F> { self.push(ColorOp::PopClip); } fn fill(&mut self, brush: Brush<'_>) { + let stops = match &brush { + Brush::Solid { .. } => 0, + Brush::LinearGradient { color_stops, .. } | Brush::RadialGradient { color_stops, .. } | Brush::SweepGradient { color_stops, .. } => { + color_stops.len() + } + }; + if !self.charge(stops) { + return; + } let paint = match brush { Brush::Solid { palette_index, alpha } => ColorPaint::Solid(self.color(palette_index, alpha)), Brush::LinearGradient { p0, p1, color_stops, extend } => { @@ -337,6 +377,33 @@ mod tests { assert!(r.finish().is_none()); } + #[test] + fn gradient_stops_and_clip_paths_share_a_payload_budget() { + // A few hundred fills reusing one long stop list would copy it per fill, far under MAX_OPS. + let stops = vec![ColorStop { offset: 0.5, palette_index: 0, alpha: 1.0 }; 4096]; + let fills = MAX_PAYLOAD / stops.len() + 1; + assert!(fills < MAX_OPS); + let outlines = std::cell::Cell::new(0); + let count = |g| { + outlines.set(outlines.get() + 1); + square(g) + }; + let mut r = Recorder::new(&[], count); + for _ in 0..fills { + r.fill(Brush::LinearGradient { p0: Default::default(), p1: Default::default(), color_stops: &stops, extend: Extend::Pad }); + } + assert!(r.ops.len() < fills, "stops past the budget are not copied"); + r.push_clip_glyph(GlyphId::new(1)); + r.pop_clip(); + assert_eq!(outlines.get(), 0, "no clip outline is built once the budget is spent"); + assert!(r.finish().is_none()); + // Within budget nothing changes, and the byte estimate counts the copied stops. + let mut r = Recorder::new(&[], square); + r.fill(Brush::LinearGradient { p0: Default::default(), p1: Default::default(), color_stops: &stops, extend: Extend::Pad }); + let ops = r.finish().unwrap(); + assert!(ops_bytes(&ops) >= stops.len() * std::mem::size_of::()); + } + fn bitmap(origin: skrifa::bitmap::Origin, data: skrifa::bitmap::BitmapData<'_>) -> skrifa::bitmap::BitmapGlyph<'_> { skrifa::bitmap::BitmapGlyph { data, diff --git a/crates/render/src/color_glyph.rs b/crates/render/src/color_glyph.rs index 8b169d9b6..9a77c1a7a 100644 --- a/crates/render/src/color_glyph.rs +++ b/crates/render/src/color_glyph.rs @@ -23,7 +23,7 @@ pub(crate) enum Drawable { impl Drawable { fn bytes(&self) -> usize { match self { - Drawable::Layers(ops) => ops.len().saturating_mul(64), + Drawable::Layers(ops) => wordcraft_fonts::color::ops_bytes(ops), Drawable::Bitmap { pixmap, .. } => pixmap.width() as usize * pixmap.height() as usize * 4, } }