From 0d2f442c359cc46ae3424f098a19aff4a4878d15 Mon Sep 17 00:00:00 2001 From: Laurenz Stampfl Date: Tue, 18 Mar 2025 16:46:38 +0100 Subject: [PATCH] More fixes --- crates/typst-pdf/src/convert.rs | 166 ++++++++++++++++++++------------ crates/typst-pdf/src/embed.rs | 6 +- crates/typst-pdf/src/image.rs | 34 +++---- crates/typst-pdf/src/link.rs | 10 +- crates/typst-pdf/src/outline.rs | 2 +- crates/typst-pdf/src/paint.rs | 35 +++---- crates/typst-pdf/src/shape.rs | 6 +- crates/typst-pdf/src/text.rs | 2 +- crates/typst-pdf/src/util.rs | 8 +- 9 files changed, 148 insertions(+), 121 deletions(-) diff --git a/crates/typst-pdf/src/convert.rs b/crates/typst-pdf/src/convert.rs index 527ebb07f..c3ff1a141 100644 --- a/crates/typst-pdf/src/convert.rs +++ b/crates/typst-pdf/src/convert.rs @@ -2,15 +2,15 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::num::NonZeroU64; use ecow::EcoVec; -use krilla::error::KrillaError; use krilla::annotation::Annotation; +use krilla::configure::{Configuration, ValidationError}; use krilla::destination::{NamedDestination, XyzDestination}; use krilla::embed::EmbedError; +use krilla::error::KrillaError; +use krilla::geom::PathBuilder; use krilla::page::{PageLabel, PageSettings}; use krilla::surface::Surface; use krilla::{Document, SerializeSettings}; -use krilla::configure::{Configuration, ValidationError}; -use krilla::geom::PathBuilder; use krilla_svg::render_svg_glyph; use typst_library::diag::{bail, error, SourceResult}; use typst_library::foundations::NativeElement; @@ -64,19 +64,14 @@ pub fn convert( document.set_outline(build_outline(&gc)); document.set_metadata(build_metadata(&gc)); - finish(document, gc) + finish(document, gc, configuration) } fn convert_pages(gc: &mut GlobalContext, document: &mut Document) -> SourceResult<()> { let mut skipped_pages = 0; for (i, typst_page) in gc.document.pages.iter().enumerate() { - if gc - .options - .page_ranges - .as_ref() - .is_some_and(|ranges| !ranges.includes_page_index(i)) - { + if gc.page_excluded(i) { // Don't export this page. skipped_pages += 1; continue; @@ -216,8 +211,12 @@ pub(crate) struct GlobalContext<'a> { // if it appears in the document multiple times. We just store the // first appearance, though. pub(crate) image_to_spans: HashMap, + /// The spans of all images that appear in the document. We use this so + /// we can give more accurate error messages. pub(crate) image_spans: HashSet, + /// The document to convert. pub(crate) document: &'a PagedDocument, + /// Options for PDF export. pub(crate) options: &'a PdfOptions<'a>, /// Mapping between locations in the document and named destinations. pub(crate) loc_to_named: HashMap, @@ -327,27 +326,25 @@ pub(crate) fn handle_group( } /// Finish a krilla document and handle export errors. -fn finish(document: Document, gc: GlobalContext) -> SourceResult> { - let validator: krilla::configure::Validator = gc - .options - .validator - .map(|v| v.into()) - .unwrap_or(krilla::configure::Validator::None); +fn finish( + document: Document, + gc: GlobalContext, + configuration: Configuration, +) -> SourceResult> { + let validator = configuration.validator(); match document.finish() { Ok(r) => Ok(r), Err(e) => match e { KrillaError::Font(f, s) => { let font_str = display_font(gc.fonts_backward.get(&f).unwrap()); - bail!(Span::detached(), "failed to process font {font_str} ({s})"; - hint: "make sure the font is valid"; - hint: "this could also be a bug in the Typst compiler" + bail!(Span::detached(), "failed to process font {font_str}: {s}"; + hint: "make sure the font is valid"; + hint: "the used font might be unsupported by Typst" ); } KrillaError::Validation(ve) => { - // We can only produce 1 error, so just take the first one. - let prefix = - format!("validated export with {} failed:", validator.as_str()); + let prefix = format!("{} error:", validator.as_str()); let get_span = |loc: Option| { loc.map(|l| Span::from_raw(NonZeroU64::new(l).unwrap())) @@ -357,27 +354,29 @@ fn finish(document: Document, gc: GlobalContext) -> SourceResult> { let errors = ve.iter().map(|e| { match e { ValidationError::TooLongString => { - error!(Span::detached(), "{prefix} a PDF string is longer \ - than 32767 characters"; + error!(Span::detached(), "{prefix} a PDF string is longer than \ + 32767 characters"; hint: "ensure title and author names are short enough") } - // Should in theory never occur, as krilla always trims font names + // Should in theory never occur, as krilla always trims font names. ValidationError::TooLongName => { - error!(Span::detached(), "{prefix} a PDF name is longer than 127 characters"; + error!(Span::detached(), "{prefix} a PDF name is longer than \ + 127 characters"; hint: "perhaps a font name is too long") } ValidationError::TooLongArray => { - error!(Span::detached(), "{prefix} a PDF array is longer than 8191 elements"; + error!(Span::detached(), "{prefix} a PDF array is longer than \ + 8191 elements"; hint: "this can happen if you have a very long text in a single line") } ValidationError::TooLongDictionary => { - error!(Span::detached(), "{prefix} a PDF dictionary has \ - more than 4095 entries"; + error!(Span::detached(), "{prefix} a PDF dictionary has more than \ + 4095 entries"; hint: "try reducing the complexity of your document") } ValidationError::TooLargeFloat => { - error!(Span::detached(), "{prefix} a PDF float number is larger than \ - the allowed limit"; + error!(Span::detached(), "{prefix} a PDF floating point number is larger \ + than the allowed limit"; hint: "try exporting using a higher PDF version") } ValidationError::TooManyIndirectObjects => { @@ -401,57 +400,104 @@ fn finish(document: Document, gc: GlobalContext) -> SourceResult> { let span = get_span(*loc); let font_str = display_font(gc.fonts_backward.get(&f).unwrap()); - error!(span, "{prefix} the text '{text}' cannot be displayed using {font_str}"; + error!(span, "{prefix} the text '{text}' cannot be displayed \ + using {font_str}"; hint: "try using a different font" ) } - ValidationError::InvalidCodepointMapping(_, _, _, loc) => { - error!(get_span(*loc), "{prefix} the PDF contains \ - disallowed codepoints or is missing codepoint mappings"; - hint: "make sure to not use the unicode characters 0x0, \ - 0xFEFF or 0xFFFE"; - hint: "for complex scripts like indic or arabic, it might \ - not be possible to produce a compliant document") + ValidationError::InvalidCodepointMapping(_, _, cp, loc) => { + let code_point = cp.map(|c| format!("{:#06x}", c as u32)); + if let Some(cp) = code_point { + let msg = if loc.is_some() { + "the PDF contains text with" + } else { + "the text contains" + }; + error!(get_span(*loc), "{prefix} {msg} the disallowed \ + codepoint {cp}") + } else { + // I think this code path is in theory unreachable, + // but just to be safe. + let msg = if loc.is_some() { "the PDF contains text with missing codepoints" } else { "the text was not mapped to a code point" }; + error!(get_span(*loc), "{prefix} {msg}"; + hint: "for complex scripts like indic or arabic, it might \ + not be possible to produce a compliant document") + } } - ValidationError::UnicodePrivateArea(_, _, _, loc) => { - error!(get_span(*loc), "{prefix} the PDF contains characters from the \ - Unicode private area"; - hint: "remove the text containing codepoints \ - from the Unicode private area") + ValidationError::UnicodePrivateArea(_, _, c, loc) => { + let code_point = format!("{:#06x}", *c as u32); + let msg = if loc.is_some() { "the PDF" } else { "the text" }; + + error!(get_span(*loc), "{prefix} {msg} contains the codepoint \ + {code_point}"; + hint: "codepoints from the Unicode private area are \ + forbidden in this export mode") } ValidationError::Transparency(loc) => { - error!(get_span(*loc), "{prefix} document contains transparency"; - hint: "remove any transparency from your \ - document (e.g. fills with opacity)"; - hint: "you might have to convert certain SVGs into a bitmap image if \ - they contain transparency"; - hint: "export using a different standard that supports transparency" - ) + let span = get_span(*loc); + let is_img = gc.image_spans.contains(&span); + let hint1 = "export using a different standard \ + that supports transparency"; + + if loc.is_some() { + if is_img { + error!(get_span(*loc), "{prefix} the image contains transparency"; + hint: "convert the image to a non-transparent one"; + hint: "you might have to convert SVGs into a \ + non-transparent bitmap image"; + hint: "{hint1}" + ) + } else { + error!(get_span(*loc), "{prefix} the used fill or stroke has \ + transparency"; + hint: "don't use colors with transparency in \ + this export mode"; + hint: "{hint1}" + ) + } + } else { + error!(get_span(*loc), "{prefix} the PDF contains transparency"; + hint: "convert any images with transparency into \ + non-transparent ones"; + hint: "don't use fills or strokes with transparent colors"; + hint: "{hint1}" + ) + } } ValidationError::ImageInterpolation(loc) => { - error!(get_span(*loc), "{prefix} the image has smooth interpolation"; - hint: "such images are not supported in this export mode" - ) + let span = get_span(*loc); + + if loc.is_some() { + error!(span, "{prefix} the image has smooth scaling"; + hint: "set the `scaling` attribute to `pixelated`") + } else { + error!(span, "{prefix} an image in the PDF has smooth scaling"; + hint: "set the `scaling` attribute of all images \ + to `pixelated`") + } } ValidationError::EmbeddedFile(e, s) => { + // We always set the span for embedded files, so it cannot be detached. let span = get_span(*s); match e { EmbedError::Existence => { error!(span, "{prefix} document contains an embedded file"; - hint: "embedded files are not supported in this export mode" + hint: "embedded files are not supported in this \ + export mode" ) } EmbedError::MissingDate => { error!(span, "{prefix} document date is missing"; - hint: "the document date needs to be set when embedding files" + hint: "the document date needs to be set when \ + embedding files" ) } EmbedError::MissingDescription => { - error!(span, "{prefix} file description is missing") + error!(span, "{prefix} the file description is missing") } EmbedError::MissingMimeType => { - error!(span, "{prefix} file mime type is missing") + error!(span, "{prefix} the file mime type is missing") } } } @@ -573,11 +619,11 @@ fn get_configuration(options: &PdfOptions) -> SourceResult { let s_string = v.as_str(); let h_message = format!( - "export using {} instead", + "export using version {} instead", v.recommended_version().as_str() ); - bail!(Span::detached(), "{pdf_string} is not compatible with standard {s_string}"; hint: "{h_message}"); + bail!(Span::detached(), "{pdf_string} is not compatible with {s_string}"; hint: "{h_message}"); } } } diff --git a/crates/typst-pdf/src/embed.rs b/crates/typst-pdf/src/embed.rs index 0d501184a..6ed65a2b6 100644 --- a/crates/typst-pdf/src/embed.rs +++ b/crates/typst-pdf/src/embed.rs @@ -6,7 +6,6 @@ use typst_library::diag::{bail, SourceResult}; use typst_library::foundations::{NativeElement, StyleChain}; use typst_library::layout::PagedDocument; use typst_library::pdf::{EmbedElem, EmbeddedFileRelationship}; -use typst_syntax::Span; pub(crate) fn embed_files( typst_doc: &PagedDocument, @@ -16,6 +15,7 @@ pub(crate) fn embed_files( for elem in &elements { let embed = elem.to_packed::().unwrap(); + let span = embed.span(); let derived_path = &embed.path.derived; let path = derived_path.to_string(); let mime_type = @@ -42,11 +42,11 @@ pub(crate) fn embed_files( association_kind, data: data.into(), compress: true, - location: Some(embed.span().into_raw().get()), + location: Some(span.into_raw().get()), }; if document.embed_file(file).is_none() { - bail!(Span::detached(), "attempted to embed file {derived_path} twice"); + bail!(span, "attempted to embed file {derived_path} twice"); } } diff --git a/crates/typst-pdf/src/image.rs b/crates/typst-pdf/src/image.rs index a43c72c89..f8ebbfa4b 100644 --- a/crates/typst-pdf/src/image.rs +++ b/crates/typst-pdf/src/image.rs @@ -33,6 +33,7 @@ pub(crate) fn handle_image( ImageKind::Raster(raster) => { let (exif_transform, new_size) = exif_transform(raster, size); surface.push_transform(&exif_transform.to_krilla()); + let image = match convert_raster(raster.clone(), interpolate) { None => bail!(span, "failed to process image"), Some(i) => i, @@ -72,7 +73,7 @@ struct Repr { actual_dynamic: OnceLock>, } -/// A wrapper around RasterImage so that we can implement `CustomImage`. +/// A wrapper around `RasterImage` so that we can implement `CustomImage`. #[derive(Clone)] struct PdfImage(Arc); @@ -96,7 +97,8 @@ impl Hash for PdfImage { impl CustomImage for PdfImage { fn color_channel(&self) -> &[u8] { - self.0.actual_dynamic + self.0 + .actual_dynamic .get_or_init(|| { let dynamic = self.0.raster.dynamic(); let channel_count = dynamic.color().channel_count(); @@ -115,10 +117,12 @@ impl CustomImage for PdfImage { } fn alpha_channel(&self) -> Option<&[u8]> { - self.0.alpha_channel + self.0 + .alpha_channel .get_or_init(|| { self.0.raster.dynamic().color().has_alpha().then(|| { - self.0.raster + self.0 + .raster .dynamic() .pixels() .map(|(_, _, Rgba([_, _, _, a]))| a) @@ -168,22 +172,12 @@ fn convert_raster( raster: RasterImage, interpolate: bool, ) -> Option { - match raster.format() { - RasterFormat::Exchange(e) => match e { - ExchangeFormat::Jpg => { - let image_data: Arc + Send + Sync> = - Arc::new(raster.data().clone()); - krilla::image::Image::from_jpeg(image_data.into(), interpolate) - } - _ => krilla::image::Image::from_custom( - PdfImage::new(raster), - interpolate, - ), - }, - RasterFormat::Pixel(_) => krilla::image::Image::from_custom( - PdfImage::new(raster), - interpolate, - ), + if let RasterFormat::Exchange(ExchangeFormat::Jpg) = raster.format() { + let image_data: Arc + Send + Sync> = + Arc::new(raster.data().clone()); + krilla::image::Image::from_jpeg(image_data.into(), interpolate) + } else { + krilla::image::Image::from_custom(PdfImage::new(raster), interpolate) } } diff --git a/crates/typst-pdf/src/link.rs b/crates/typst-pdf/src/link.rs index a8f95098d..d59cfe227 100644 --- a/crates/typst-pdf/src/link.rs +++ b/crates/typst-pdf/src/link.rs @@ -1,7 +1,7 @@ -use krilla::geom::Rect; use krilla::action::{Action, LinkAction}; use krilla::annotation::{LinkAnnotation, Target}; use krilla::destination::XyzDestination; +use krilla::geom::Rect; use typst_library::layout::{Abs, Point, Size}; use typst_library::model::Destination; @@ -65,11 +65,9 @@ pub(crate) fn handle_link( LinkAnnotation::new( rect, None, - Target::Destination( - krilla::destination::Destination::Named( - nd.clone(), - ), - ), + Target::Destination(krilla::destination::Destination::Named( + nd.clone(), + )), ) .into(), ); diff --git a/crates/typst-pdf/src/outline.rs b/crates/typst-pdf/src/outline.rs index f252d52d1..834476715 100644 --- a/crates/typst-pdf/src/outline.rs +++ b/crates/typst-pdf/src/outline.rs @@ -24,7 +24,7 @@ pub(crate) fn build_outline(gc: &GlobalContext) -> Outline { if !page_ranges .includes_page(gc.document.introspector.page(elem.location().unwrap())) { - // Don't bookmark headings in non-exported pages + // Don't bookmark headings in non-exported pages. continue; } } diff --git a/crates/typst-pdf/src/paint.rs b/crates/typst-pdf/src/paint.rs index bbdbfc114..70bf4d4a0 100644 --- a/crates/typst-pdf/src/paint.rs +++ b/crates/typst-pdf/src/paint.rs @@ -1,7 +1,7 @@ //! Convert paint types from typst to krilla. -use krilla::num::NormalizedF32; use krilla::color::{self, cmyk, luma, rgb}; +use krilla::num::NormalizedF32; use krilla::paint::{ Fill, LinearGradient, Pattern, RadialGradient, SpreadMethod, Stop, Stroke, StrokeDash, SweepGradient, @@ -79,7 +79,7 @@ fn convert_paint( Paint::Solid(c) => { let (c, a) = convert_solid(c); Ok((c.into(), a)) - }, + } Paint::Gradient(g) => Ok(convert_gradient(g, on_text, state, size)), Paint::Tiling(p) => convert_pattern(gc, p, on_text, surface, state), } @@ -91,10 +91,8 @@ fn convert_solid(color: &Color) -> (color::Color, u8) { let (c, a) = convert_luma(color); (c.into(), a) } - ColorSpace::Cmyk => { - (convert_cmyk(color).into(), 255) - } - // Convert all other colors in different colors spaces into RGB + ColorSpace::Cmyk => (convert_cmyk(color).into(), 255), + // Convert all other colors in different colors spaces into RGB. _ => { let (c, a) = convert_rgb(color); (c.into(), a) @@ -105,12 +103,7 @@ fn convert_solid(color: &Color) -> (color::Color, u8) { fn convert_cmyk(color: &Color) -> cmyk::Color { let components = color.to_space(ColorSpace::Cmyk).to_vec4_u8(); - cmyk::Color::new( - components[0], - components[1], - components[2], - components[3], - ) + cmyk::Color::new(components[0], components[1], components[2], components[3]) } fn convert_rgb(color: &Color) -> (rgb::Color, u8) { @@ -221,11 +214,11 @@ fn convert_gradient( (radial.into(), 255) } Gradient::Conic(conic) => { - // Correct the gradient's angle + // Correct the gradient's angle. let cx = size.x.to_f32() * conic.center.x.get() as f32; let cy = size.y.to_f32() * conic.center.y.get() as f32; let actual_transform = base_transform - // Adjust for the angle + // Adjust for the angle. .pre_concat(Transform::rotate_at( angle, Abs::pt(cx as f64), @@ -257,18 +250,18 @@ fn convert_gradient( } fn convert_gradient_stops(gradient: &Gradient) -> Vec { - let mut stops= vec![]; - + let mut stops = vec![]; + let use_cmyk = gradient.stops().iter().all(|s| s.color.space() == ColorSpace::Cmyk); let mut add_single = |color: &Color, offset: Ratio| { let (color, opacity) = if use_cmyk { (convert_cmyk(color).into(), 255) - } else { + } else { let (c, a) = convert_rgb(color); (c.into(), a) }; - + let opacity = NormalizedF32::new((opacity as f32) / 255.0).unwrap(); let offset = NormalizedF32::new(offset.get() as f32).unwrap(); let stop = Stop { offset, color, opacity }; @@ -314,9 +307,9 @@ fn convert_gradient_stops(gradient: &Gradient) -> Vec { let ((c0, t0), (c1, t1)) = (window[0], window[1]); // Precision: - // - On an even color, insert a stop every 90deg - // - For a hue-based color space, insert 200 stops minimum - // - On any other, insert 20 stops minimum + // - On an even color, insert a stop every 90deg. + // - For a hue-based color space, insert 200 stops minimum. + // - On any other, insert 20 stops minimum. let max_dt = if c0 == c1 { 0.25 } else if conic.space.hue_index().is_some() { diff --git a/crates/typst-pdf/src/shape.rs b/crates/typst-pdf/src/shape.rs index 2ab00c63f..c3a8ab265 100644 --- a/crates/typst-pdf/src/shape.rs +++ b/crates/typst-pdf/src/shape.rs @@ -75,10 +75,8 @@ fn convert_geometry(geometry: &Geometry) -> Option { let w = size.x.to_f32(); let h = size.y.to_f32(); let rect = if w < 0.0 || h < 0.0 { - // Skia doesn't normally allow for negative dimensions, but - // Typst supports them, so we apply a transform if needed - // Because this operation is expensive according to tiny-skia's - // docs, we prefer to not apply it if not needed + // krilla doesn't normally allow for negative dimensions, but + // Typst supports them, so we apply a transform if needed. let transform = krilla::geom::Transform::from_scale(w.signum(), h.signum()); Rect::from_xywh(0.0, 0.0, w.abs(), h.abs()) diff --git a/crates/typst-pdf/src/text.rs b/crates/typst-pdf/src/text.rs index 331b8cc44..1e2b15fb9 100644 --- a/crates/typst-pdf/src/text.rs +++ b/crates/typst-pdf/src/text.rs @@ -61,7 +61,7 @@ pub(crate) fn handle_text( font, text, size.to_f32(), - // TODO: What if only stroke? + // To prevent text from being embedded twice, we outline it instead if a stroke exists. true, ); } diff --git a/crates/typst-pdf/src/util.rs b/crates/typst-pdf/src/util.rs index aa122582c..7904f53fb 100644 --- a/crates/typst-pdf/src/util.rs +++ b/crates/typst-pdf/src/util.rs @@ -5,9 +5,7 @@ use krilla::geom::PathBuilder; use krilla::paint as kp; use typst_library::layout::{Abs, Point, Size, Transform}; use typst_library::text::Font; -use typst_library::visualize::{ - Curve, CurveItem, FillRule, LineCap, LineJoin, -}; +use typst_library::visualize::{Curve, CurveItem, FillRule, LineCap, LineJoin}; pub(crate) trait SizeExt { fn to_krilla(&self) -> kg::Size; @@ -97,13 +95,13 @@ impl AbsExt for Abs { } } -/// Display the font family and variant of a font. +/// Display the font family of a font. pub(crate) fn display_font(font: &Font) -> String { let font_family = &font.info().family; format!("{font_family}") } -/// Build a typst path using a path builder. +/// Convert a typst path to a krilla path. pub(crate) fn convert_path(path: &Curve, builder: &mut PathBuilder) { for item in &path.0 { match item {