Skip to content

PDF import: Separation images keep their ink (#6314) - #6347

Merged
bflatastic merged 1 commit into
storytold:mainfrom
jmverges:fix/6314-pdf-separation-images
Nov 9, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
jmverges:fix/6314-pdf-separation-images

Conversation

@jmverges

@jmverges jmverges commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Fixes #6314. Follows #6321 (#6313), whose colorized image masks this reuses. Related: #3235 (spot inks in vector DeviceN colors and gradients), #1466 (colorizing grayscale images).

An image in a Separation color space (one ink, ISO 32000-1 §8.6.6.4) opened as an RGB image: its spot ink was lost (only the process plates in Separations Preview), and recolor.colors didn't list its color, so Recolor Artwork couldn't change it.

It now opens as a grayscale image of its tints (white where it lays no ink, black at 100 %, its Decode array applied) colorized by its ink at 100 %: the ink's spot swatch, a process plate's CMYK color, or Registration. That is the model #4431 added for colorized images.

How

  • import_image::separation_space and tint_gray (crates/pdf/src/import_image.rs): the image's [/Separation …] space, and its samples read as tints, the way import_image::cmyk reads a CMYK image's. Samples of 1, 2, 4, 8 and 16 bits, or one gray channel of the image's size from its JPEG, CCITT, JBIG2 or JPEG 2000 decoder. Samples that can't be read (data cut short, a decoder's output of another layout) leave the image in RGB, with a warning.
  • Colors::array_space (crates/pdf/src/import_color.rs): finds the image's space among those Colors::scan recorded, by kind, colorant names and tint transform. The page's walk counts paths, not images, so it can't name the ink.
  • Builder::separation_image: the tints, then the ink at 100 % through Colors::native, so the swatch, process plates and Registration work as they do for a fill. The ink is only marked used once the tints are read, so an image that stays RGB adds no swatch.
  • draw_image: a Separation image takes that path after the CMYK one, with its own mask as its opacity mask, as a CMYK image's is. The decision is kept per image (separation_paints), and the gray pixels have their own cache key, so an image that opens in RGB stays RGB when it is drawn again, and the tint transform isn't read again for each draw.

The ink None (which paints nothing), an ink whose color can't be told (an alternate space the importer doesn't model), images inside an alpha soft mask, inline images (their /CS names a resource) and DeviceN images open as before.

Two differences from the old RGB image, both from how VectorCraft draws a colorized image:

  • Where the image lays no ink, the art under it shows; the RGB image was opaque white there. On a white page it looks the same; over other art, the 0 % areas no longer knock it out.
  • On screen, a tint draws as the ink at 100 % at that coverage; the RGB image came from the tint transform, so mid tints can look slightly different. Printed plates and Separations Preview get the tint.

Docs: docs/file-format-parity.md (status line, the PDF row and the revision history).

How it was tested

crates/pdf/src/tests/colorize.rs:

  • a_separation_image_opens_colorized_by_its_ink: a 4 × 1 image of tints 0, 1/3, 2/3 and 1 opens as gray 255, 170, 85, 0, colorized by the SpotRed spot swatch at 100 %.
  • a_separation_image_reads_its_samples_and_decode_array: 1-bit with /Decode [1 0], 4-bit and 16-bit samples.
  • a_separation_jpeg_opens_colorized_by_its_ink: a one-component JPEG.
  • a_separation_image_of_a_process_plate_or_of_no_ink: /Black colorizes in CMYK black with no spot swatch; /None isn't colorized.
  • a_separation_image_keeps_its_soft_mask: its /SMask becomes its opacity mask.
  • a_separation_image_drawn_twice_is_colorized_twice: both draws are colorized, over the same pixels.
  • a_separation_image_that_cant_be_read_stays_rgb: data cut short, drawn twice, stays RGB both times, with one warning and no swatch for the ink.

All seven fail on main (the last one on its warning, which main doesn't give). The /None, JPEG and "stays RGB" tests also fail with the change but without, respectively, the None check, the decoder output accepted, and the ink resolved after the tints.

What I ran:

Checklist

  • cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm), apart from the one machine-specific vectorcraft-text test above.
  • No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
    unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
    messages. Errors are returned as Result and shown to the user
    (why and how).
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md). The test PDFs are written by hand from ISO 32000-1.
  • Every new asset has a row in ASSETS.md (no new assets).

Summary by CodeRabbit

  • New Features
    • PDF imports now preserve images that use Separation color spaces, retaining their grayscale tint values and applying the matching spot ink or recognized process-plate color.
    • Image masks continue to control opacity. Supported tint data includes multiple sample depths and Decode settings.
    • If tint data cannot be read or validated, the image remains uncolored rather than preventing the import from continuing.

An image in a Separation color space opened as an RGB image: its spot ink
was lost (no plate in Separations Preview), and Fill and Recolor Artwork
didn't reach its color.

It now opens as a grayscale image of its tints (white where it lays no
ink, black at 100 %, its Decode array applied; 1 to 16 bits a sample, or
one gray channel from its JPEG, CCITT, JBIG2 or JPEG 2000 decoder),
colorized by its ink at 100 %: the spot swatch, a process plate's CMYK
color, or Registration. Its mask becomes its opacity mask, as a CMYK
image's does. The ink is found by the image's color space among those the
file defines, once per image. The ink None, an ink whose color can't be
told, images inside an alpha soft mask, inline images and DeviceN images
open as before; samples that can't be read open in RGB with a warning, and
the ink gets no swatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

PDF import now recognizes Separation images, decodes tint samples into grayscale pixels, and applies the matching ink paint. It caches paint results by image key, preserves image masks as opacity masks, and falls back to existing image handling when Separation data cannot be read.

Changes: PDF Separation image import

Layer / File(s) Summary
Identify Separation spaces and decode tints
crates/pdf/src/import_color.rs, crates/pdf/src/import_image.rs, crates/pdf/src/tests/colorize.rs
The importer matches Separation color spaces to stored ink spaces and decodes supported one-component samples into grayscale tint pixels. Tests cover sample bit depths, Decode arrays, and JPEG decoding.
Apply ink paint during image import
crates/pdf/src/import/state.rs, crates/pdf/src/import/builder.rs, crates/pdf/src/import/device.rs, crates/pdf/src/tests/colorize.rs, docs/file-format-parity.md
Image import caches the resolved paint, applies it to grayscale pixels, and preserves masks as opacity masks. Tests cover process inks, /None, masks, repeated draws, and unreadable samples. The parity document describes the import behavior and fallback.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant draw_image
  participant separation_image
  participant ColorsArraySpace as Colors::array_space
  participant tint_gray
  participant separation_paints
  draw_image->>separation_paints: Look up paint by image key
  draw_image->>separation_image: Resolve ink paint and decode tint pixels
  separation_image->>ColorsArraySpace: Match color-space array
  separation_image->>tint_gray: Decode one-component samples
  separation_image-->>draw_image: Return paint and grayscale pixels
  draw_image->>separation_paints: Cache paint by image key
  draw_image->>draw_image: Store grayscale image and apply paint and opacity mask
Loading

Suggested reviewers: skylartaylor, dylanmtaylor

Merge Risk: Minimal · up to ca595

PDF import now keeps the ink of Separation images, with a fallback to RGB and a warning when the samples cannot be read. No actionable merge-blocking risk was found in the supplied changes.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly identifies the main change: preserving Separation-image ink during PDF import.
Description check Passed The description covers what and why, explains the implementation, reports tests and CI results with the machine-specific exception, and completes the checklist.
Linked Issues check Passed [#6314] Objective: an image in a Separation color space keeps its ink on PDF import, as a grayscale image of its tints colorized by its ink (issue's expected outcome, with a spot swatch as the example…
Out of Scope Changes check Passed Every changed area ties to #6314: builder.rs (separation_image and cache), device.rs (draw_image path), state.rs (separation_paints field), import_color.rs (Colors::array_space, which finds the ink's …
Docstring Coverage Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Full details: Linked Issues check

Explanation

[#6314] Objective: an image in a Separation color space keeps its ink on PDF import, as a grayscale image of its tints colorized by its ink (issue's expected outcome, with a spot swatch as the example). The PR implements this path. import_image::separation_space and tint_gray (crates/pdf/src/import_image.rs) read the samples as tints. Colors::array_space (crates/pdf/src/import_color.rs) finds the ink's space. Builder::separation_image (crates/pdf/src/import/builder.rs) returns the ink paint at 100 % with the tint pixels. draw_image (crates/pdf/src/import/device.rs) uses this path for Separation images, with the image mask as opacity mask. Covered by seven tests in crates/pdf/src/tests/colorize.rs, including a 4 × 1 spot-ink ramp that opens as gray 255, 170, 85, 0 colorized by SpotRed. The issue's reproduction (image.info reports Grayscale, recolor.colors lists the ink, plates include the spot) is stated as checked in the PR description. Both the issue's requirements and the PR description describe the same behavior. Unreadable samples and unsupported cases stay RGB as before, and that is a documented choice, not an unmet requirement. Assessment relies on the PR description, the whole-PR change summary and the docs diff. The code paths were not read line by line.

Full details: Out of Scope Changes check

Explanation

Every changed area ties to #6314: builder.rs (separation_image and cache), device.rs (draw_image path), state.rs (separation_paints field), import_color.rs (Colors::array_space, which finds the ink's space), import_image.rs (tint decoding and space detection), colorize.rs (tests for this behavior) and docs/file-format-parity.md. The docs diff adds only a status-line entry, a PDF-row sentence and a revision-history row for #6314. The docs row also keeps the existing #6313 and #6309 text, which predates this PR. No unrelated code or documentation change was found in the summary or the docs diff. Code changes outside the listed files were not inspected.

Full details: Docstring Coverage

Explanation

Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 6 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 2318ba0 and ca59598.

Files selected for processing: 7

  • crates/pdf/src/import/builder.rs
  • crates/pdf/src/import/device.rs
  • crates/pdf/src/import/state.rs
  • crates/pdf/src/import_color.rs
  • crates/pdf/src/import_image.rs
  • crates/pdf/src/tests/colorize.rs
  • docs/file-format-parity.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@eliets2 eliets2 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First-pass review notes (automated; combined from specialist angles: tests). Please treat as suggestions and verify before acting.

What the change does: Fixes #6314 by opening Separation color space images as grayscale tint images colorized by their ink at 100% — spot swatch, process-plate CMYK color, or Registration — so the ink is preserved for Recolor Artwork, with RGB fallback and a warning when tints cannot be read.

Logical units:

  • import_image::separation_space and tint_gray in crates/pdf/src/import_image.rs to read the Separation space and tint samples
  • Colors::array_space in crates/pdf/src/import_color.rs to resolve the ink by kind, colorant names, and tint transform
  • Builder::separation_image in crates/pdf/src/import/builder.rs to build the colorized image via Colors::native
  • draw_image in crates/pdf/src/import/device.rs with per-image separation_paints decision and separate gray cache key
  • Tests in crates/pdf/src/tests/colorize.rs and docs in docs/file-format-parity.md

Findings:

  • [tests] crates/pdf/src/import_color.rs Colors::array_space / crates/pdf/src/tests/colorize.rs: matching by tint transform is untested when two Separation spaces share the same ink name with different transforms; add a page with two such spaces and assert the image uses the transform from its own space.
  • [tests] crates/pdf/src/import/builder.rs Builder::separation_image: the unknown-ink fallback (array_space/native returning None) and Registration are claimed but have no test; add a Registration image that colorizes and an unmodelled alternate-space image that stays RGB with no swatch.
  • [tests] crates/pdf/src/import_image.rs tint_gray: the decoder-output mismatch branch (Some(_) => Err) is untested, only truncated data is tested in a_separation_image_that_cant_be_read_stays_rgb; add a case where the decoder returns RGB, wrong dimensions, or alpha and assert RGB fallback with one warning.
  • [tests] crates/pdf/src/import_image.rs tint_gray / a_separation_image_reads_its_samples_and_decode_array: 2-bit samples are claimed but not tested, and the 16-bit case uses only 0000/FFFF so a byte-order error passes; add 2-bit and mid-value 16-bit cases, including a width that needs row padding.
  • [tests] crates/pdf/src/import_image.rs tint_gray: the BitsPerComponent check and the zero/MAX_CMYK_PIXELS size cap have no test; add invalid-bit-depth and oversized-dimension cases asserting RGB fallback with a warning and no swatch.
  • [tests] crates/pdf/src/import/device.rs draw_image (!white guard) and separation_space: DeviceN, inline, and inside-a-soft-mask exclusions are claimed but untested; add negative tests that those images are not colorized.
  • [tests] crates/pdf/src/import_image.rs tint_gray Decode handling: only /Decode [1 0] is tested; add malformed Decode (wrong length or non-finite values, which should default to 0..1) and out-of-range Decode values that should clamp.
  • [tests] crates/pdf/src/import_image.rs tint_gray decoder support: CCITT, JBIG2 and JPEG 2000 are claimed in docs but only JPEG is tested; add at least one non-JPEG decoder case or remove the claim until tested.

Overlaps: Follows #6321 (#6313) and reuses its colorized image masks; related to #3235 and #1466.

Verdict: needs changes — 1) tint-transform disambiguation for same ink name is untested, 2) unknown-ink fallback and Registration colorization are untested, 3) decoder-output mismatch fallback is untested.

@bflatastic
bflatastic merged commit 9394a43 into storytold:main Nov 9, 2026
7 checks passed
@bflatastic

Copy link
Copy Markdown
Member

Merged, thanks @jmverges! It passed its CI checks.

bflatastic pushed a commit that referenced this pull request Nov 10, 2026
… inks (#6348)

An image in a DeviceN color space opened as an RGB image, losing its inks.

One of one colorant now opens as a Separation image does since #6347: a
grayscale image of its tints colorized by its ink. One of two to 32
colorants that are all process plates (the ink None may be among them)
opens as a CMYK image, each colorant's tint in its plate, its Decode array
applied (8 or 16 bits a sample). A DeviceN image that mixes a spot ink
with other colorants, such as a duotone, still opens as RGB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDF import: an image in a Separation color space opens as an RGB image and loses its spot ink

3 participants