Implement better handling of color spaces

crates/krilla/src/content.rs

@@ -14,7 +14,7 @@ use skrifa::GlyphId;
 use tiny_skia_path::Size;
 use tiny_skia_path::{NormalizedF32, Path, PathSegment, Point, Rect, Transform};
 
-use crate::color::Color;
+use crate::color::{Color, ColorSpace};
 use crate::font::{Font, Glyph, GlyphUnits};
 use crate::graphics_state::GraphicsStates;
 #[cfg(feature = "raster-images")]
@@ -32,7 +32,7 @@ use crate::paint::{InnerPaint, Paint};
 use crate::path::{Fill, FillRule, LineCap, LineJoin, Stroke};
 use crate::resource;
 use crate::resource::{Resource, ResourceDictionaryBuilder};
-use crate::serialize::SerializeContext;
+use crate::serialize::{MaybeDeviceColorSpace, SerializeContext};
 use crate::stream::Stream;
 use crate::tagging::ContentTag;
 use crate::util::{calculate_stroke_bbox, LineCapExt, LineJoinExt, NameExt, RectExt, TransformExt};
@@ -48,6 +48,14 @@ pub(crate) struct ContentBuilder {
     pub(crate) active_marked_content: bool,
 }
 
+/// Stores either a device-specific color space,
+/// or the name of a different colorspace (e.g. ICCBased) stored in
+/// the current resource dictionary
+enum ContentColorSpace {
+    Device,
+    Named(String),
+}
+
 impl ContentBuilder {
     pub(crate) fn new(root_transform: Transform) -> Self {
         Self {
@@ -653,7 +661,7 @@ impl ContentBuilder {
         opacity: NormalizedF32,
         sc: &mut SerializeContext,
         mut set_pattern_fn: impl FnMut(&mut Content, String),
-        mut set_solid_fn: impl FnMut(&mut Content, String, Color),
+        mut set_solid_fn: impl FnMut(&mut Content, ContentColorSpace, Color),
     ) {
         let pattern_transform = |transform: Transform| -> Transform {
             transform.post_concat(self.cur_transform_with_root_transform())
@@ -667,10 +675,8 @@ impl ContentBuilder {
                 if let Some((color, opacity)) = gradient_props.single_stop_color() {
                     // Write gradients with one stop as a solid color fill.
                     content_builder.set_fill_opacity(opacity);
-                    let color_space = color.color_space(sc);
-                    let color_space_resource = content_builder
-                        .rd_builder
-                        .register_resource(sc.register_colorspace(color_space));
+                    let cs = color.color_space(sc);
+                    let color_space_resource = Self::cs_to_content_cs(content_builder, sc, cs);
                     set_solid_fn(&mut content_builder.content, color_space_resource, color);
                 } else {
                     let shading_mask =
@@ -706,9 +712,7 @@ impl ContentBuilder {
         match &paint.0 {
             InnerPaint::Color(c) => {
                 let cs = c.color_space(sc);
-                let color_space_resource = self
-                    .rd_builder
-                    .register_resource(sc.register_colorspace(cs));
+                let color_space_resource = Self::cs_to_content_cs(self, sc, cs);
                 set_solid_fn(&mut self.content, color_space_resource, *c);
             }
             InnerPaint::LinearGradient(lg) => {
@@ -744,6 +748,21 @@ impl ContentBuilder {
         }
     }
 
+    fn cs_to_content_cs(
+        content_builder: &mut ContentBuilder,
+        sc: &mut SerializeContext,
+        cs: ColorSpace,
+    ) -> ContentColorSpace {
+        match sc.register_colorspace(cs) {
+            MaybeDeviceColorSpace::ColorSpace(s) => {
+                ContentColorSpace::Named(content_builder.rd_builder.register_resource(s))
+            }
+            MaybeDeviceColorSpace::DeviceGray => ContentColorSpace::Device,
+            MaybeDeviceColorSpace::DeviceRgb => ContentColorSpace::Device,
+            MaybeDeviceColorSpace::DeviceCMYK => ContentColorSpace::Device,
+        }
+    }
+
     fn content_set_fill_properties(
         &mut self,
         bounds: Rect,
@@ -755,9 +774,26 @@ impl ContentBuilder {
             content.set_fill_pattern(None, color_space.to_pdf_name());
         }
 
-        fn set_solid_fn(content: &mut Content, color_space: String, color: Color) {
-            content.set_fill_color_space(color_space.to_pdf_name());
-            content.set_fill_color(color.to_pdf_color());
+        fn set_solid_fn(content: &mut Content, color_space: ContentColorSpace, color: Color) {
+            match color_space {
+                ContentColorSpace::Device => match color {
+                    Color::Rgb(r) => {
+                        let comps = r.to_pdf_color();
+                        content.set_fill_rgb(comps[0], comps[1], comps[2]);
+                    }
+                    Color::Luma(l) => {
+                        content.set_fill_gray(l.to_pdf_color());
+                    }
+                    Color::Cmyk(c) => {
+                        let comps = c.to_pdf_color();
+                        content.set_fill_cmyk(comps[0], comps[1], comps[2], comps[3]);
+                    }
+                },
+                ContentColorSpace::Named(n) => {
+                    content.set_fill_color_space(n.to_pdf_name());
+                    content.set_fill_color(color.to_pdf_color());
+                }
+            }
         }
 
         self.content_set_fill_stroke_properties(
@@ -781,9 +817,26 @@ impl ContentBuilder {
             content.set_stroke_pattern(None, color_space.to_pdf_name());
         }
 
-        fn set_solid_fn(content: &mut Content, color_space: String, color: Color) {
-            content.set_stroke_color_space(color_space.to_pdf_name());
-            content.set_stroke_color(color.to_pdf_color());
+        fn set_solid_fn(content: &mut Content, color_space: ContentColorSpace, color: Color) {
+            match color_space {
+                ContentColorSpace::Device => match color {
+                    Color::Rgb(r) => {
+                        let comps = r.to_pdf_color();
+                        content.set_stroke_rgb(comps[0], comps[1], comps[2]);
+                    }
+                    Color::Luma(l) => {
+                        content.set_stroke_gray(l.to_pdf_color());
+                    }
+                    Color::Cmyk(c) => {
+                        let comps = c.to_pdf_color();
+                        content.set_stroke_cmyk(comps[0], comps[1], comps[2], comps[3]);
+                    }
+                },
+                ContentColorSpace::Named(n) => {
+                    content.set_stroke_color_space(n.to_pdf_name());
+                    content.set_stroke_color(color.to_pdf_color());
+                }
+            }
         }
 
         self.content_set_fill_stroke_properties(

crates/krilla/src/object/color.rs

@@ -73,9 +73,9 @@ pub(crate) enum Color {
 impl Color {
     pub(crate) fn to_pdf_color(self) -> Vec<f32> {
         match self {
-            Color::Rgb(rgb) => rgb.to_pdf_color().into_iter().collect::<Vec<_>>(),
-            Color::Luma(l) => l.to_pdf_color().into_iter().collect::<Vec<_>>(),
-            Color::Cmyk(cmyk) => cmyk.to_pdf_color().into_iter().collect::<Vec<_>>(),
+            Color::Rgb(rgb) => rgb.to_pdf_color().to_vec(),
+            Color::Luma(l) => vec![l.to_pdf_color()],
+            Color::Cmyk(cmyk) => cmyk.to_pdf_color().to_vec(),
         }
     }
 
@@ -118,8 +118,8 @@ pub mod luma {
             Self::new(255)
         }
 
-        pub(crate) fn to_pdf_color(self) -> impl IntoIterator<Item = f32> {
-            [self.0 as f32 / 255.0]
+        pub(crate) fn to_pdf_color(self) -> f32 {
+            self.0 as f32 / 255.0
         }
 
         pub(crate) fn color_space(no_device_cs: bool) -> ColorSpace {
@@ -160,7 +160,7 @@ pub mod cmyk {
             Color(cyan, magenta, yellow, black)
         }
 
-        pub(crate) fn to_pdf_color(self) -> impl IntoIterator<Item = f32> {
+        pub(crate) fn to_pdf_color(self) -> [f32; 4] {
             [
                 self.0 as f32 / 255.0,
                 self.1 as f32 / 255.0,
@@ -229,8 +229,8 @@ pub mod rgb {
             Self::new(255, 255, 255)
         }
 
-        pub(crate) fn to_pdf_color(self) -> impl IntoIterator<Item = f32> {
-            vec![
+        pub(crate) fn to_pdf_color(self) -> [f32; 3] {
+            [
                 self.0 as f32 / 255.0,
                 self.1 as f32 / 255.0,
                 self.2 as f32 / 255.0,

crates/krilla/src/object/shading_function.rs

@@ -1,22 +1,23 @@
 //! Shading functions.
 
 use std::hash::{Hash, Hasher};
+use std::ops::DerefMut;
 use std::sync::Arc;
 
 use bumpalo::Bump;
 use pdf_writer::types::{FunctionShadingType, PostScriptOp};
-use pdf_writer::{Chunk, Finish, Name, Ref};
+use pdf_writer::{Chunk, Dict, Finish, Name, Ref};
 use tiny_skia_path::{NormalizedF32, Point, Rect, Transform};
 
-use crate::color::luma;
+use crate::color::{luma, ColorSpace, DEVICE_CMYK, DEVICE_GRAY, DEVICE_RGB};
 use crate::object::color::Color;
 use crate::object::{Cacheable, ChunkContainerFn, Resourceable};
 use crate::paint::SpreadMethod;
 use crate::paint::{LinearGradient, RadialGradient, SweepGradient};
 use crate::resource;
 use crate::resource::Resource;
-use crate::serialize::SerializeContext;
-use crate::util::{RectExt, RectWrapper};
+use crate::serialize::{MaybeDeviceColorSpace, SerializeContext};
+use crate::util::{NameExt, RectExt, RectWrapper};
 use crate::validation::ValidationError;
 
 #[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)]
@@ -271,18 +272,14 @@ fn serialize_postscript_shading(
     let cs = if use_opacities {
         luma::Color::color_space(sc.serialize_settings().no_device_cs)
     } else {
-        // Note: This means for example if the user provides a linear RGB stop as the first
-        // and sRGB as the remaining ones, the whole gradient will
-        // use linear RGB.
         post_script_gradient.stops[0].color.color_space(sc)
     };
 
     let mut shading = chunk.function_shading(root_ref);
     shading.shading_type(FunctionShadingType::Function);
 
-    shading
-        .insert(Name(b"ColorSpace"))
-        .primitive(sc.register_colorspace(cs).get_ref());
+    set_colorspace(sc, cs, shading.deref_mut());
+
     // Write the identity matrix, because ghostscript has a bug where
     // it thinks the entry is mandatory.
     shading.matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0]);
@@ -293,6 +290,17 @@ fn serialize_postscript_shading(
     shading.finish();
 }
 
+fn set_colorspace(sc: &mut SerializeContext, cs: ColorSpace, target: &mut Dict) {
+    let pdf_cs = target.insert(Name(b"ColorSpace"));
+
+    match sc.register_colorspace(cs) {
+        MaybeDeviceColorSpace::DeviceGray => pdf_cs.primitive(DEVICE_GRAY.to_pdf_name()),
+        MaybeDeviceColorSpace::DeviceRgb => pdf_cs.primitive(DEVICE_RGB.to_pdf_name()),
+        MaybeDeviceColorSpace::DeviceCMYK => pdf_cs.primitive(DEVICE_CMYK.to_pdf_name()),
+        MaybeDeviceColorSpace::ColorSpace(cs) => pdf_cs.primitive(cs.get_ref()),
+    }
+}
+
 fn serialize_axial_radial_shading(
     sc: &mut SerializeContext,
     chunk: &mut Chunk,
@@ -317,9 +325,8 @@ fn serialize_axial_radial_shading(
     } else {
         shading.shading_type(FunctionShadingType::Axial);
     }
-    shading
-        .insert(Name(b"ColorSpace"))
-        .primitive(sc.register_colorspace(cs).get_ref());
+
+    set_colorspace(sc, cs, shading.deref_mut());
 
     shading.anti_alias(radial_axial_gradient.anti_alias);
     shading.function(function_ref);

crates/krilla/src/object/xobject.rs

@@ -4,13 +4,13 @@ use pdf_writer::{Chunk, Finish, Name, Ref};
 use std::ops::DerefMut;
 use tiny_skia_path::Rect;
 
-use crate::color::rgb;
+use crate::color::{rgb, DEVICE_RGB};
 use crate::object::{Cacheable, ChunkContainerFn, Resourceable};
 use crate::resource;
 use crate::resource::Resource;
-use crate::serialize::SerializeContext;
+use crate::serialize::{MaybeDeviceColorSpace, SerializeContext};
 use crate::stream::{FilterStreamBuilder, Stream};
-use crate::util::{RectExt, RectWrapper};
+use crate::util::{NameExt, RectExt, RectWrapper};
 use crate::validation::ValidationError;
 
 #[derive(Debug, Hash, Eq, PartialEq)]
@@ -87,7 +87,14 @@ impl Cacheable for XObject {
 
             if self.transparency_group_color_space {
                 let cs = rgb::Color::rgb_color_space(sc.serialize_settings().no_device_cs);
-                transparency.pair(Name(b"CS"), sc.register_colorspace(cs).get_ref());
+                let pdf_cs = transparency.insert(Name(b"CS"));
+
+                match sc.register_colorspace(cs) {
+                    MaybeDeviceColorSpace::DeviceRgb => pdf_cs.primitive(DEVICE_RGB.to_pdf_name()),
+                    // Can only be SRGB
+                    MaybeDeviceColorSpace::ColorSpace(cs) => pdf_cs.primitive(cs.get_ref()),
+                    _ => unreachable!(),
+                }
             }
 
             transparency.finish();

crates/krilla/src/serialize.rs

@@ -15,20 +15,19 @@ use skrifa::raw::TableProvider;
 use tiny_skia_path::Size;
 
 use crate::chunk_container::ChunkContainer;
-use crate::color::{ColorSpace, ICCBasedColorSpace, ICCProfile, DEVICE_CMYK};
+use crate::color::{ColorSpace, ICCBasedColorSpace, ICCProfile};
 use crate::destination::{NamedDestination, XyzDestination};
 use crate::error::{KrillaError, KrillaResult};
 use crate::font::{Font, FontInfo};
 #[cfg(feature = "raster-images")]
 use crate::image::Image;
 use crate::metadata::Metadata;
-use crate::object::color::{DEVICE_GRAY, DEVICE_RGB};
 use crate::object::font::cid_font::CIDFont;
 use crate::object::font::type3_font::Type3FontMapper;
 use crate::object::font::{FontContainer, FontIdentifier};
 use crate::object::outline::Outline;
 use crate::object::page::{InternalPage, PageLabelContainer};
-use crate::object::{Cacheable, ChunkContainerFn, Resourceable};
+use crate::object::{Cacheable, Resourceable};
 use crate::page::PageLabel;
 use crate::resource;
 use crate::resource::Resource;
@@ -165,6 +164,13 @@ enum StructParentElement {
     Annotation(usize, usize),
 }
 
+pub(crate) enum MaybeDeviceColorSpace {
+    DeviceRgb,
+    DeviceGray,
+    DeviceCMYK,
+    ColorSpace(resource::ColorSpace),
+}
+
 /// The serializer context is more or less the core piece of krilla. It is passed around
 /// throughout pretty much the whole conversion process, and contains all mutable state
 /// that is needed when writing a PDF file. This includes for example:
@@ -513,47 +519,20 @@ impl SerializeContext {
         }
     }
 
-    pub(crate) fn register_colorspace(&mut self, cs: ColorSpace) -> resource::ColorSpace {
-        macro_rules! cs {
-            ($name:ident, $color_name:expr) => {
-                #[derive(Copy, Clone, Hash)]
-                struct $name;
-
-                impl Cacheable for $name {
-                    fn chunk_container(&self) -> ChunkContainerFn {
-                        Box::new(|cc| &mut cc.color_spaces)
-                    }
-
-                    fn serialize(self, _: &mut SerializeContext, root_ref: Ref) -> Chunk {
-                        let mut chunk = Chunk::new();
-                        chunk
-                            .indirect(root_ref)
-                            .primitive(Name($color_name.as_bytes()));
-                        chunk
-                    }
-                }
-
-                impl Resourceable for $name {
-                    type Resource = resource::ColorSpace;
-                }
-            };
-        }
-
-        cs!(DeviceRgb, DEVICE_RGB);
-        cs!(DeviceGray, DEVICE_GRAY);
-        cs!(DeviceCmyk, DEVICE_CMYK);
-
+    pub(crate) fn register_colorspace(&mut self, cs: ColorSpace) -> MaybeDeviceColorSpace {
         match cs {
-            ColorSpace::Srgb => self.register_resourceable(ICCBasedColorSpace(
-                self.serialize_settings.pdf_version.rgb_icc(),
+            ColorSpace::Srgb => MaybeDeviceColorSpace::ColorSpace(self.register_resourceable(
+                ICCBasedColorSpace(self.serialize_settings.pdf_version.rgb_icc()),
             )),
-            ColorSpace::Luma => self.register_resourceable(ICCBasedColorSpace(
-                self.serialize_settings.pdf_version.grey_icc(),
+            ColorSpace::Luma => MaybeDeviceColorSpace::ColorSpace(self.register_resourceable(
+                ICCBasedColorSpace(self.serialize_settings.pdf_version.grey_icc()),
             )),
-            ColorSpace::Cmyk(cs) => self.register_resourceable(cs),
-            ColorSpace::DeviceGray => self.register_resourceable(DeviceGray),
-            ColorSpace::DeviceRgb => self.register_resourceable(DeviceRgb),
-            ColorSpace::DeviceCmyk => self.register_resourceable(DeviceCmyk),
+            ColorSpace::Cmyk(cs) => {
+                MaybeDeviceColorSpace::ColorSpace(self.register_resourceable(cs))
+            }
+            ColorSpace::DeviceGray => MaybeDeviceColorSpace::DeviceGray,
+            ColorSpace::DeviceRgb => MaybeDeviceColorSpace::DeviceRgb,
+            ColorSpace::DeviceCmyk => MaybeDeviceColorSpace::DeviceCMYK,
         }
     }
 }