diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -817,6 +817,7 @@ dependencies = [ "thiserror 2.0.21", "ttf-parser", "wasm-bindgen-test", + "write-fonts", ] [[package]] diff --git a/NOTICE b/NOTICE --- a/NOTICE +++ b/NOTICE @@ -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 diff --git a/crates/pdf/Cargo.toml b/crates/pdf/Cargo.toml --- a/crates/pdf/Cargo.toml +++ b/crates/pdf/Cargo.toml @@ -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" diff --git a/crates/pdf/README.md b/crates/pdf/README.md --- a/crates/pdf/README.md +++ b/crates/pdf/README.md @@ -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: diff --git a/crates/pdf/tests/colr.rs b/crates/pdf/tests/colr.rs new file mode 100644 --- /dev/null +++ b/crates/pdf/tests/colr.rs @@ -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 { + 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 { + 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)); +} diff --git a/vendor/krilla/README.craft.md b/vendor/krilla/README.craft.md --- a/vendor/krilla/README.craft.md +++ b/vendor/krilla/README.craft.md @@ -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. diff --git a/vendor/krilla/SOURCE.json b/vendor/krilla/SOURCE.json --- a/vendor/krilla/SOURCE.json +++ b/vendor/krilla/SOURCE.json @@ -337,7 +337,7 @@ { "path": "src/text/glyph/colr.rs", "upstream_sha256": "cde9bb33f7b87fe026aa9abe24dc375d8fbdce01a5d9aa6764f93fe6febef676", - "sha256": "cde9bb33f7b87fe026aa9abe24dc375d8fbdce01a5d9aa6764f93fe6febef676" + "sha256": "892351f0ad075306779a6bb239e1985c2931af367bdf826fff48aa4e58cc338c" }, { "path": "src/text/glyph/mod.rs", diff --git a/vendor/krilla/src/text/glyph/colr.rs b/vendor/krilla/src/text/glyph/colr.rs --- a/vendor/krilla/src/text/glyph/colr.rs +++ b/vendor/krilla/src/text/glyph/colr.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, surface: &mut Surface) { +fn interpret(instructions: Vec, 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, 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, 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, mode: CompositeMode, backdrop: Vec, + 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, mask: Vec, surface: &mut Surface) { +fn interpret_masked( + visible: Vec, + mask: Vec, + 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>, - stack: Vec>, + stack: Vec<(Vec, bool)>, + bounds: Option, layers: Vec, transforms: Vec, 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> { + pub fn finish(mut self) -> Option<(Vec, Option)> { 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) { - 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 {