More fixes

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
+        if gc.page_excluded(i) {
-            .options
-            .page_ranges
-            .as_ref()
-            .is_some_and(|ranges| !ranges.includes_page_index(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<krilla::image::Image, Span>,
+    /// 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<Span>,
+    /// 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<Location, NamedDestination>,
@@ -327,27 +326,25 @@ pub(crate) fn handle_group(
 }
 
 /// Finish a krilla document and handle export errors.
-fn finish(document: Document, gc: GlobalContext) -> SourceResult<Vec<u8>> {
+fn finish(
-    let validator: krilla::configure::Validator = gc
+    document: Document,
-        .options
+    gc: GlobalContext,
-        .validator
+    configuration: Configuration,
-        .map(|v| v.into())
+) -> SourceResult<Vec<u8>> {
-        .unwrap_or(krilla::configure::Validator::None);
+    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})";
+                bail!(Span::detached(), "failed to process font {font_str}: {s}";
-                hint: "make sure the font is valid";
+                    hint: "make sure the font is valid";
-                hint: "this could also be a bug in the Typst compiler"
+                    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!("{} error:", validator.as_str());
-                let prefix =
-                    format!("validated export with {} failed:", validator.as_str());
 
                 let get_span = |loc: Option<krilla::surface::Location>| {
                     loc.map(|l| Span::from_raw(NonZeroU64::new(l).unwrap()))
@@ -357,27 +354,29 @@ fn finish(document: Document, gc: GlobalContext) -> SourceResult<Vec<u8>> {
                 let errors = ve.iter().map(|e| {
                     match e {
                         ValidationError::TooLongString => {
-                            error!(Span::detached(), "{prefix} a PDF string is longer \
+                            error!(Span::detached(), "{prefix} a PDF string is longer than \
-                        than 32767 characters";
+                            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 \
+                            error!(Span::detached(), "{prefix} a PDF dictionary has more than \
-                        more than 4095 entries";
+                            4095 entries";
                             hint: "try reducing the complexity of your document")
                         }
                         ValidationError::TooLargeFloat => {
-                            error!(Span::detached(), "{prefix} a PDF float number is larger than \
+                            error!(Span::detached(), "{prefix} a PDF floating point number is larger \
-                        the allowed limit";
+                            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<Vec<u8>> {
                             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) => {
+                        ValidationError::InvalidCodepointMapping(_, _, cp, loc) => {
-                            error!(get_span(*loc), "{prefix} the PDF contains \
+                            let code_point = cp.map(|c| format!("{:#06x}", c as u32));
-                        disallowed codepoints or is missing codepoint mappings";
+                            if let Some(cp) = code_point {
-                            hint: "make sure to not use the unicode characters 0x0, \
+                                let msg = if loc.is_some() {
-                            0xFEFF or 0xFFFE";
+                                    "the PDF contains text with" 
-                            hint: "for complex scripts like indic or arabic, it might \
+                                } else {
-                            not be possible to produce a compliant document")
+                                    "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) => {
+                        ValidationError::UnicodePrivateArea(_, _, c, loc) => {
-                            error!(get_span(*loc), "{prefix} the PDF contains characters from the \
+                            let code_point = format!("{:#06x}", *c as u32);
-                        Unicode private area";
+                            let msg = if loc.is_some() { "the PDF" } else { "the text" };
-                            hint: "remove the text containing codepoints \
+
-                            from the Unicode private area")
+                            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";
+                            let span = get_span(*loc);
-                                hint: "remove any transparency from your \
+                            let is_img = gc.image_spans.contains(&span);
-                                document (e.g. fills with opacity)";
+                            let hint1 = "export using a different standard \
-                                hint: "you might have to convert certain SVGs into a bitmap image if \
+                            that supports transparency";
-                                they contain transparency";
+
-                                hint: "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";
+                            let span = get_span(*loc);
-                                hint: "such images are not supported in this export mode"
+
-                            )
+                            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<Configuration> {
                     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}");
                 }
             }
         }

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::<EmbedElem>().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");
         }
     }
 

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<Arc<DynamicImage>>,
 }
 
-/// 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<Repr>);
 
@@ -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<krilla::image::Image> {
-    match raster.format() {
+    if let RasterFormat::Exchange(ExchangeFormat::Jpg) = raster.format() {
-        RasterFormat::Exchange(e) => match e {
+        let image_data: Arc<dyn AsRef<[u8]> + Send + Sync> =
-            ExchangeFormat::Jpg => {
+            Arc::new(raster.data().clone());
-                let image_data: Arc<dyn AsRef<[u8]> + Send + Sync> =
+        krilla::image::Image::from_jpeg(image_data.into(), interpolate)
-                    Arc::new(raster.data().clone());
+    } else {
-                krilla::image::Image::from_jpeg(image_data.into(), interpolate)
+        krilla::image::Image::from_custom(PdfImage::new(raster), interpolate)
-            }
-            _ => krilla::image::Image::from_custom(
-                PdfImage::new(raster),
-                interpolate,
-            ),
-        },
-        RasterFormat::Pixel(_) => krilla::image::Image::from_custom(
-            PdfImage::new(raster),
-            interpolate,
-        ),
     }
 }
 

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(
+                        Target::Destination(krilla::destination::Destination::Named(
-                            krilla::destination::Destination::Named(
+                            nd.clone(),
-                                nd.clone(),
+                        )),
-                            ),
-                        ),
                     )
                     .into(),
                 );

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;
             }
         }

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 => {
+        ColorSpace::Cmyk => (convert_cmyk(color).into(), 255),
-            (convert_cmyk(color).into(), 255)
+        // Convert all other colors in different colors spaces into RGB.
-        }
-        // 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(
+    cmyk::Color::new(components[0], components[1], components[2], components[3])
-        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<Stop> {
-    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<Stop> {
                 let ((c0, t0), (c1, t1)) = (window[0], window[1]);
 
                 // Precision:
-                // - On an even color, insert a stop every 90deg
+                // - On an even color, insert a stop every 90deg.
-                // - For a hue-based color space, insert 200 stops minimum
+                // - For a hue-based color space, insert 200 stops minimum.
-                // - On any other, insert 20 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() {

crates/typst-pdf/src/shape.rs

@@ -75,10 +75,8 @@ fn convert_geometry(geometry: &Geometry) -> Option<Path> {
             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
+                // krilla doesn't normally allow for negative dimensions, but
-                // Typst supports them, so we apply a transform if needed
+                // 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
                 let transform =
                     krilla::geom::Transform::from_scale(w.signum(), h.signum());
                 Rect::from_xywh(0.0, 0.0, w.abs(), h.abs())

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,
         );
     }

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::{
+use typst_library::visualize::{Curve, CurveItem, FillRule, LineCap, LineJoin};
-    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 {