collective
Browse / github.com/storytold/craft-libs / patch
Krilla: COLR glyphs take their palette's colours, without panicking
Krilla: COLR glyphs take their palette's colours, without panicking Backports the three upstream Krilla changes to `src/text/glyph/colr.rs` since 0.8.2, unchanged, from commit [`acd6891f`](https://github.com/LaurenzV/krilla/commit/acd6891f7435d3272274989faa0cb12b79203258): - [#436](https://github.com/LaurenzV/krilla/pull/436): colours are looked up in palette 0 by their colour record index; 0.8.2 indexed the whole record array and panicked past its end. - [#437](https://github.com/LaurenzV/krilla/pull/437): colour glyphs without bounds are rejected, and unclipped fills stay within the glyph's bounds; 0.8.2 panicked on a fill without a clip. - [#438](https://g
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- lehni (imported from pull request #231, open)
- Base
b1401e33b105ca2861575ede90f67f4d54449001- Size
- +230 −61 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
b40a96df5a60e460354b206d
How it went
No other operator has reported using it yet.
Diff
raw · Cargo.lock NOTICE crates/pdf/Cargo.toml crates/pdf/README.md crates/pdf/tests/colr.rs vendor/krilla/README.craft.md vendor/krilla/SOURCE.json vendor/krilla/src/text/glyph/colr.rs
@@ -817,6 +817,7 @@ dependencies = [ "thiserror 2.0.21", "ttf-parser", "wasm-bindgen-test",+ "write-fonts", ] [[package]]@@ -91,7 +91,7 @@ The optional writer embeds Compact ICC Profiles by Clinton Ingram See licenses/pdf-writer/LICENSE-ICC.txt and ATTRIBUTION.md for sources and hashes. The writer adds no font files. Its four existing ICC profiles are retained under vendor/krilla/icc. -Krilla is vendored with a native unwind-cleanup patch; see vendor/krilla/README.craft.md+Krilla is vendored with a native unwind-cleanup patch and an upstream COLR backport; see vendor/krilla/README.craft.md and vendor/krilla/SOURCE.json. Its upstream MIT/Apache-2.0 licences and CC0 profiles are unchanged. Optional PDF reader@@ -33,6 +33,7 @@ hayro-interpret = { version = "=0.7.0", default-features = false } hayro-cmap = "=0.1.0" kurbo = "0.13" ttf-parser = "=0.25.1"+write-fonts = "=0.48.1" [target.'cfg(target_arch = "wasm32")'.dev-dependencies] wasm-bindgen-test = "0.3"@@ -103,7 +103,7 @@ that supply shaped glyphs do not need that shaper. Adapters that call `draw_text DeckCraft, also need `write-simple-text` when adopting this revision. `write-pdf` adds embedded-PDF support independently of text shaping. -The writer pins Krilla 0.8.2, with a local unwind-cleanup patch documented in+The writer pins Krilla 0.8.2, with a local unwind-cleanup patch and an upstream COLR backport documented in [`vendor/krilla/README.craft.md`](../../vendor/krilla/README.craft.md). Successful serialization uses the same backend version as before extraction. It provides: new file mode 100644@@ -0,0 +1,131 @@+//! COLR colour glyphs, drawn from a minimal synthetic font built in the test.+#![cfg(feature = "write")]+#![allow(clippy::unwrap_used, clippy::panic)]++use backend::geom::Point;+use backend::page::PageSettings;+use backend::text::{GlyphId, KrillaGlyph};+use craft_pdf::write::{self, Metadata, WriteError, Writer, backend};+use kurbo::{Rect, Shape};+use write_fonts::FontBuilder;+use write_fonts::tables::colr::{BaseGlyph, BaseGlyphList, BaseGlyphPaint, Colr, Layer, Paint};+use write_fonts::tables::cpal::{ColorRecord, Cpal};+use write_fonts::tables::glyf::{GlyfLocaBuilder, Glyph, SimpleGlyph};+use write_fonts::tables::head::{Flags, Head, MacStyle};+use write_fonts::tables::hhea::Hhea;+use write_fonts::tables::hmtx::{Hmtx, LongMetric};+use write_fonts::tables::loca::LocaFormat;+use write_fonts::tables::maxp::Maxp;+use write_fonts::types::{F2Dot14, FWord, Fixed, GlyphId16, LongDateTime, UfWord};++/// COLRv0: layers with palette entries 0 and 1.+const IN_RANGE: u16 = 1;+/// COLRv0: a second layer with a palette entry the font doesn't have, and an outline.+const OUT_OF_RANGE: u16 = 2;+/// COLRv1: a solid paint that no glyph or clip box bounds, and an outline.+const UNBOUNDED: u16 = 3;+/// COLRv1: palette entry 0 with an alpha above 1.+const OVER_OPAQUE: u16 = 4;+const LEFT: u16 = 5;+const RIGHT: u16 = 6;++const BLUE: &str = "0 0 1 rg";+const GREEN: &str = "0 1 0 rg";+const RED: &str = "1 0 0 rg";+/// The outline of the invalid colour glyphs, a square 123 units wide:+const OUTLINE: &str = "0 0 m 123 0 l 123 700 l";++/// A font whose only palette starts at colour record 2: records 0 and 1 are red and+/// must not be used, palette entries 0 and 1 are blue and green.+fn colr_font() -> Vec<u8> {+ let square = |x0, x1| Glyph::Simple(SimpleGlyph::from_bezpath(&Rect::new(x0, 0.0, x1, 700.0).to_path(0.1)).unwrap());+ let glyphs = [Glyph::Empty, Glyph::Empty, square(0.0, 123.0), square(0.0, 123.0), Glyph::Empty, square(0.0, 250.0), square(250.0, 500.0)];+ let mut glyf_loca = GlyfLocaBuilder::new();+ for glyph in &glyphs {+ glyf_loca.add_glyph(glyph).unwrap();+ }+ let (glyf, loca, loca_format) = glyf_loca.build();+ let index_to_loc_format = if loca_format == LocaFormat::Long { 1 } else { 0 };+ let date = LongDateTime::new(0);+ let head = Head::new(Fixed::ONE, 0, Flags::empty(), 1000, date, date, 0, 0, 500, 700, MacStyle::empty(), 8, index_to_loc_format);+ let num_glyphs = glyphs.len() as u16;+ let hhea = Hhea {+ ascender: FWord::new(800),+ descender: FWord::new(-200),+ advance_width_max: UfWord::new(600),+ number_of_h_metrics: num_glyphs,+ ..Default::default()+ };+ let hmtx = Hmtx::new(glyphs.iter().map(|_| LongMetric::new(600, 0)).collect(), vec![]);++ let id = GlyphId16::new;+ let layers = [(LEFT, 0), (RIGHT, 1), (LEFT, 0), (RIGHT, 7)];+ let mut colr = Colr::new(+ 2,+ Some(vec![BaseGlyph::new(id(IN_RANGE), 0, 2), BaseGlyph::new(id(OUT_OF_RANGE), 2, 2)]),+ Some(layers.iter().map(|&(glyph, index)| Layer::new(id(glyph), index)).collect()),+ layers.len() as u16,+ );+ let paints = vec![+ BaseGlyphPaint::new(id(UNBOUNDED), Paint::solid(0, F2Dot14::ONE)),+ BaseGlyphPaint::new(id(OVER_OPAQUE), Paint::glyph(Paint::solid(0, F2Dot14::from_f32(1.5)), id(LEFT))),+ ];+ colr.base_glyph_list = BaseGlyphList::new(paints.len() as u32, paints).into();+ let red = ColorRecord::new(0, 0, 255, 255);+ let records = vec![red.clone(), red, ColorRecord::new(255, 0, 0, 255), ColorRecord::new(0, 255, 0, 255)];+ let cpal = Cpal::new(2, 1, records.len() as u16, Some(records), vec![2]);++ let mut builder = FontBuilder::new();+ builder.add_table(&head).unwrap();+ builder.add_table(&hhea).unwrap();+ builder.add_table(&Maxp::new(num_glyphs)).unwrap();+ builder.add_table(&hmtx).unwrap();+ builder.add_table(&glyf).unwrap();+ builder.add_table(&loca).unwrap();+ builder.add_table(&colr).unwrap();+ builder.add_table(&cpal).unwrap();+ builder.build()+}++/// Export one glyph of the font, uncompressed, behind the writer's unwind guard.+fn export(glyph: u16) -> Result<String, WriteError> {+ write::guard(|| {+ let font = write::font(&colr_font(), 0, None)?;+ let settings = backend::SerializeSettings { compress_content_streams: false, ..Default::default() };+ let mut writer = Writer::new(settings, Metadata::default());+ let mut page = writer.document_mut().start_page_with(PageSettings::from_wh(100.0, 100.0).unwrap());+ let mut surface = page.surface();+ let glyphs = [KrillaGlyph::new(GlyphId::new(u32::from(glyph)), 0.6, 0.0, 0.0, 0.0, 0..1, None)];+ surface.draw_glyphs(Point::from_xy(10.0, 80.0), &glyphs, font, "x", 50.0, false);+ surface.finish();+ page.finish();+ Ok(String::from_utf8_lossy(&writer.finish()?).into_owned())+ })+}++#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]+#[cfg_attr(not(target_arch = "wasm32"), test)]+fn colr_layers_take_their_colours_from_the_palette_offset() {+ let pdf = export(IN_RANGE).unwrap();+ assert!(pdf.contains(BLUE), "palette entry 0 is blue");+ assert!(pdf.contains(GREEN), "palette entry 1 is green");+ assert!(!pdf.contains(RED), "the records before the palette are not used");+}++#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]+#[cfg_attr(not(target_arch = "wasm32"), test)]+fn invalid_colr_glyphs_fall_back_to_their_outline() {+ for glyph in [OUT_OF_RANGE, UNBOUNDED] {+ let pdf = export(glyph).unwrap();+ assert!(pdf.contains(OUTLINE), "invalid colour glyph {glyph} draws its outline");+ for colour in [BLUE, GREEN, RED] {+ assert!(!pdf.contains(colour), "invalid colour glyph {glyph} draws no layer");+ }+ }+}++#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]+#[cfg_attr(not(target_arch = "wasm32"), test)]+fn colr_alpha_above_one_is_opaque() {+ assert!(export(OVER_OPAQUE).unwrap().contains(BLUE));+}@@ -1,4 +1,4 @@-# Local Krilla 0.8.2 patch+# Local Krilla 0.8.2 patches This is the crates.io Krilla 0.8.2 package (MIT OR Apache-2.0), upstream commit `3ffdf0588cf98050aad6edba51ca70162e1fb5b5`. `SOURCE.json` records each retained@@ -26,3 +26,18 @@ The shared writer's native subprocess regression covers the previously aborting cleanup and a successful fresh export afterward. Native and WASM export tests cover the unchanged successful path. WASM aborts, aborting panic configurations and process-wide allocation failures remain outside unwind recovery.++## COLR backport++`src/text/glyph/colr.rs` is replaced by its upstream version from Krilla commit+[`acd6891f`](https://github.com/LaurenzV/krilla/commit/acd6891f7435d3272274989faa0cb12b79203258),+which applies the only three upstream changes to that file since 0.8.2, unchanged:+[#436](https://github.com/LaurenzV/krilla/pull/436) (`d29cec50`) looks colours up in palette 0+through its colour record index and rejects out-of-range palette indices, which 0.8.2 indexed+into the whole record array, ignoring the palette offset and panicking past its end;+[#437](https://github.com/LaurenzV/krilla/pull/437) (`dc97b994`) rejects colour glyphs that+no glyph or clip box bounds and fills unclipped paints within the glyph's bounds, where+0.8.2 panicked on a fill without a clip; [#438](https://github.com/LaurenzV/krilla/pull/438)+(`acd6891f`) clamps paint alpha to 0–1, which 0.8.2 rejected. A rejected colour glyph falls+back to its outline. `crates/pdf/tests/colr.rs` covers each change with a font built in the+test. Drop this backport when a Krilla release includes these commits.@@ -337,7 +337,7 @@ { "path": "src/text/glyph/colr.rs", "upstream_sha256": "cde9bb33f7b87fe026aa9abe24dc375d8fbdce01a5d9aa6764f93fe6febef676",- "sha256": "cde9bb33f7b87fe026aa9abe24dc375d8fbdce01a5d9aa6764f93fe6febef676"+ "sha256": "892351f0ad075306779a6bb239e1985c2931af367bdf826fff48aa4e58cc338c" }, { "path": "src/text/glyph/mod.rs",@@ -1,9 +1,8 @@ use skrifa::color::{Brush, ColorPainter, ColorStop, CompositeMode}; use skrifa::outline::DrawSettings; use skrifa::raw::types::BoundingBox;-use skrifa::raw::TableProvider; use skrifa::MetadataProvider;-use tiny_skia_path::{Path, PathBuilder};+use tiny_skia_path::{Path, PathBuilder, Rect}; use crate::geom::Transform; use crate::graphics::blend::BlendMode;@@ -45,24 +44,28 @@ pub(crate) fn draw_glyph( colr_glyph .paint(font.location_ref(), &mut colr_canvas) .ok()?;- let instructions = colr_canvas.finish()?;+ let (instructions, bounds) = colr_canvas.finish()?;+ let Some(bounds) = bounds.filter(|rect| !rect.is_empty()) else {+ return Some(());+ };+ let bounds = PathBuilder::from_rect(bounds); surface.push_transform(&Transform::from_scale(1.0, -1.0));- interpret(instructions, surface);+ interpret(instructions, &bounds, surface); surface.pop(); Some(()) } // Interpret the glyph bytecode-fn interpret(instructions: Vec<Instruction>, surface: &mut Surface) {+fn interpret(instructions: Vec<Instruction>, bounds: &Path, surface: &mut Surface) { for instruction in instructions { match instruction { Instruction::Layer(mode, instructions) => { let blend = mode.to_blend_mode().unwrap_or(BlendMode::Normal); surface.push_blend_mode(blend); surface.push_isolated();- interpret(instructions, surface);+ interpret(instructions, bounds, surface); surface.pop(); surface.pop(); }@@ -70,9 +73,9 @@ fn interpret(instructions: Vec<Instruction>, surface: &mut Surface) { source, mode, backdrop,- } => interpret_composite(source, mode, backdrop, surface),+ } => interpret_composite(source, mode, backdrop, bounds, surface), Instruction::Filled(fill, mut clips) => {- let filled = clips.split_off(clips.len() - 1);+ let filled = clips.pop().unwrap_or_else(|| bounds.clone()); let num_clips = clips.len(); for path in clips {@@ -85,7 +88,7 @@ fn interpret(instructions: Vec<Instruction>, surface: &mut Surface) { surface.set_fill(Some(*fill)); surface.set_stroke(None); - surface.draw_path(&crate::geom::Path(filled[0].clone()));+ surface.draw_path(&crate::geom::Path(filled)); surface.set_fill(old_fill); surface.set_stroke(old_stroke);@@ -102,48 +105,49 @@ fn interpret_composite( source: Vec<Instruction>, mode: CompositeMode, backdrop: Vec<Instruction>,+ bounds: &Path, surface: &mut Surface, ) { match mode { CompositeMode::Clear => {} CompositeMode::Src => { surface.push_isolated();- interpret(source, surface);+ interpret(source, bounds, surface); surface.pop(); } CompositeMode::Dest => { surface.push_isolated();- interpret(backdrop, surface);+ interpret(backdrop, bounds, surface); surface.pop(); } CompositeMode::SrcOver => { surface.push_isolated();- interpret(backdrop, surface);- interpret(source, surface);+ interpret(backdrop, bounds, surface);+ interpret(source, bounds, surface); surface.pop(); } CompositeMode::DestOver => { surface.push_isolated();- interpret(source, surface);- interpret(backdrop, surface);+ interpret(source, bounds, surface);+ interpret(backdrop, bounds, surface); surface.pop(); }- CompositeMode::SrcIn => interpret_masked(source, backdrop, surface),- CompositeMode::DestIn => interpret_masked(backdrop, source, surface),+ CompositeMode::SrcIn => interpret_masked(source, backdrop, bounds, surface),+ CompositeMode::DestIn => interpret_masked(backdrop, source, bounds, surface), _ => { let Some(blend) = mode.to_blend_mode() else { surface.push_isolated();- interpret(backdrop, surface);- interpret(source, surface);+ interpret(backdrop, bounds, surface);+ interpret(source, bounds, surface); surface.pop(); return; }; surface.push_isolated();- interpret(backdrop, surface);+ interpret(backdrop, bounds, surface); surface.push_blend_mode(blend); surface.push_isolated();- interpret(source, surface);+ interpret(source, bounds, surface); surface.pop(); surface.pop(); surface.pop();@@ -151,17 +155,22 @@ fn interpret_composite( } } -fn interpret_masked(visible: Vec<Instruction>, mask: Vec<Instruction>, surface: &mut Surface) {+fn interpret_masked(+ visible: Vec<Instruction>,+ mask: Vec<Instruction>,+ bounds: &Path,+ surface: &mut Surface,+) { let mask_stream = { let mut builder = surface.stream_builder(); let mut mask_surface = builder.surface();- interpret(mask, &mut mask_surface);+ interpret(mask, bounds, &mut mask_surface); mask_surface.finish(); builder.finish() }; surface.push_mask(Mask::new(mask_stream, MaskType::Alpha));- interpret(visible, surface);+ interpret(visible, bounds, surface); surface.pop(); } @@ -170,7 +179,8 @@ struct ColrBuilder { font: Font, context_color: rgb::Color, clips: Vec<Vec<Path>>,- stack: Vec<Vec<Instruction>>,+ stack: Vec<(Vec<Instruction>, bool)>,+ bounds: Option<Rect>, layers: Vec<CompositeMode>, transforms: Vec<Transform>, error: bool,@@ -220,7 +230,8 @@ impl ColrBuilder { Self { font, context_color,- stack: vec![vec![]],+ stack: vec![(vec![], true)],+ bounds: None, transforms: vec![Transform::identity()], clips: vec![vec![]], layers: vec![],@@ -228,14 +239,31 @@ impl ColrBuilder { } } - pub fn finish(mut self) -> Option<Vec<Instruction>> {+ pub fn finish(mut self) -> Option<(Vec<Instruction>, Option<Rect>)> { if self.error { return None;- } else if let Some(instructions) = self.stack.pop() {- return Some(instructions); } - None+ let (instructions, bounded) = self.stack.pop()?;+ // A color glyph needs to be bounded to be considered valid.+ bounded.then_some((instructions, self.bounds))+ }++ fn push_clip(&mut self, path: Path) {+ let Some(mut clips) = self.clips.last().cloned() else {+ self.error = true;+ return;+ };+ self.bounds = match self.bounds {+ Some(bounds) => bounds.join(&path.bounds()),+ None => Some(path.bounds()),+ };+ if self.bounds.is_none() {+ self.error = true;+ return;+ }+ clips.push(path);+ self.clips.push(clips); } } @@ -245,14 +273,12 @@ impl ColrBuilder { palette_index: u16, alpha: f32, ) -> Option<(rgb::Color, NormalizedF32)> {+ let alpha = alpha.clamp(0.0, 1.0);+ if palette_index != u16::MAX {- let color = self- .font- .font_ref()- .cpal()- .ok()?- .color_records_array()?- .ok()?[palette_index as usize];+ let palettes = self.font.font_ref().color_palettes();+ let palette = palettes.get(0)?;+ let color = palette.colors().get(usize::from(palette_index))?; Some(( rgb::Color::new(color.red, color.green, color.blue),@@ -321,11 +347,6 @@ impl ColorPainter for ColrBuilder { } fn push_clip_glyph(&mut self, glyph_id: skrifa::GlyphId) {- let Some(mut old) = self.clips.last().cloned() else {- self.error = true;- return;- };- let mut glyph_builder = OutlineBuilder::new(); let outline_glyphs = self.font.outline_glyphs(); let Some(outline_glyph) = outline_glyphs.get(glyph_id) else {@@ -349,17 +370,10 @@ impl ColorPainter for ColrBuilder { return; }; - old.push(path);-- self.clips.push(old);+ self.push_clip(path); } fn push_clip_box(&mut self, clip_box: BoundingBox<f32>) {- let Some(mut old) = self.clips.last().cloned() else {- self.error = true;- return;- };- let mut path_builder = PathBuilder::new(); path_builder.move_to(clip_box.x_min, clip_box.y_min); path_builder.line_to(clip_box.x_min, clip_box.y_max);@@ -374,9 +388,7 @@ impl ColorPainter for ColrBuilder { self.error = true; return; };- old.push(path);-- self.clips.push(old);+ self.push_clip(path); } fn pop_clip(&mut self) {@@ -533,28 +545,30 @@ impl ColorPainter for ColrBuilder { return; }; - let Some(stack) = self.stack.last_mut() else {+ let Some((stack, bounded)) = self.stack.last_mut() else { self.error = true; return; }; + *bounded &= !clips.is_empty(); stack.push(Instruction::Filled(Box::new(fill), clips)); } } fn push_layer(&mut self, composite_mode: CompositeMode) { self.layers.push(composite_mode);- self.stack.push(vec![]);+ self.stack.push((vec![], true)); } fn pop_layer(&mut self) {- let (Some(composite), Some(mut instructions)) = (self.layers.pop(), self.stack.pop())+ let (Some(composite), Some((mut instructions, bounded))) =+ (self.layers.pop(), self.stack.pop()) else { self.error = true; return; }; - let Some(stack) = self.stack.last_mut() else {+ let Some((stack, parent_bounded)) = self.stack.last_mut() else { self.error = true; return; };@@ -562,6 +576,13 @@ impl ColorPainter for ColrBuilder { // See the skrifa code to see how it handles composite operations. // https://github.com/googlefonts/fontations/blob/fd0178ce4ea48301a7f06b35a6fce9879c102292/skrifa/src/color/traversal.rs#L566-L593 // Everything is wrapped in a `SrcOver` base layer.+ *parent_bounded = match composite {+ CompositeMode::Clear => true,+ CompositeMode::Src | CompositeMode::SrcOut => bounded,+ CompositeMode::Dest | CompositeMode::DestOut => *parent_bounded,+ CompositeMode::SrcIn | CompositeMode::DestIn => bounded || *parent_bounded,+ _ => bounded && *parent_bounded,+ }; if composite == CompositeMode::SrcOver { if let Some(Instruction::Layer(_, _)) = instructions.last() { let Some(Instruction::Layer(mode, source)) = instructions.pop() else {