Repository navigation
Conversation
…2269) Export all images, Save Image As and Word export (all through pdfcraft_create::image_file) converted colours their own way: CMYK with (1 - c)(1 - k) instead of the renderer's CMYK profile (cyan ink came out (0, 255, 255) where the page draws (0, 174, 239), magenta 115 levels off, black ink pure black), and ICCBased images without their profile (an Adobe RGB or Display P3 image as if it were sRGB). JPEGs with CMYK samples or an ICC profile the file doesn't carry were written as is, so viewers and Word showed those in other colours too. The renderer's own conversion is now public (vendored hayro-interpret patch 39: color::device_cmyk_to_srgb and color::IccToSrgb), wrapped by pdfcraft-render (cmyk_to_srgb, IccProfile, panics caught), and image export converts through it: DeviceCMYK, ICCBased grey/RGB/CMYK, indexed palettes over them (converted once), 16-bit and inverted CMYK, and /Matte colours. CMYK JPEGs, and JPEGs drawn through an ICC profile the file doesn't carry itself, are written as PNG in the page's colours; a JPEG that carries the page's profile (inverted, for CMYK) is still written unchanged. Still missing: CalGray/CalRGB images are written as device grey/RGB and Lab images are skipped.
mtlcl410
left a comment
mtlcl410
left a comment
There was a problem hiding this comment.
I tested the head on Windows 11 x64 (debug) with doc_export_all_images. I compared the page render's centre pixel with what a colour-managed viewer shows for the exported file (Pillow + LittleCMS, converting any embedded ICC profile to sRGB). These are JPEG cases the PR body's table doesn't list:
| PDF image | main: page → exported file |
this PR: page → exported file |
|---|---|---|
/DeviceRGB, plain JPEG |
(30,160,60) → JPEG, no ICC, (30,160,60) | same |
[/ICCBased AdobeRGB], JPEG without a profile |
(0,161,46) → JPEG, no ICC, (30,160,60) | (0,161,46) → PNG, (0,161,46) ✔ |
[/ICCBased AdobeRGB], JPEG carrying the same AdobeRGB profile |
(0,161,46) → JPEG + ICC, (0,161,47) | same, passed through ✔ |
/DeviceRGB, JPEG carrying an AdobeRGB profile (APP2) |
(30,160,60) → JPEG + ICC, (0,161,47) | same: (0,161,47) |
The last row is the gap I'd point out:
- The page draws the samples as DeviceRGB and ignores the profile inside the JPEG stream.
- The JPEG is written unchanged under the "grey or RGB in a device space" rule.
- A viewer then applies the JPEG's own APP2 profile, so the exported picture shows different colours from the page.
This isn't a regression (main does the same), but it is the case the PR's rule ("a JPEG is still written unchanged when the file shows the page's colours") is meant to exclude. Producers that embed camera or Adobe RGB JPEGs as /DeviceRGB do this routinely.
Two ways to close it:
- Pass a device-space JPEG through only when its stream has no ICC APP2 segment, or one that is sRGB.
- Otherwise drop the APP2 segments when copying, which keeps the file a JPEG and lossless.
Everything else I tried behaves as described. My earlier render-side encoding probes (CMYK JPEG with and without an inverting /Decode, ICC CMYK raw and JPEG) agree between page and renderer, which is what this PR now reuses.
…age ignores The page draws a DeviceGray or DeviceRGB JPEG's samples as they are, but a JPEG file can carry an ICC profile of its own (an Adobe RGB APP2 from a camera or editor), which viewers apply: the exported file showed other colours than the page. Such a JPEG is now written without its APP2 ICC_PROFILE segments, every other byte as it was, so it stays a lossless JPEG. If the segments before the first scan can't be read, the file is written as before.
|
Thanks, that's a real gap, and a common one, as you say. I went with your second option, so the file stays a lossless JPEG. b6a497f writes a DeviceGray or DeviceRGB JPEG without its APP2
It fails on the previous head. |
|
Confirmed on b6a497f with the same probe. The |
This branch has not been deployed
2 participants
What this changes
Fixes #2269.
The issue doesn't include the comparison image, so I built a synthetic PDF for each common kind of image. For each one I compared the centre pixel the page renderer draws with the same pixel in the exported file. Export all images, Save Image As and Word export all write images through
pdfcraft_create::image_file, so they had the same problem.Two things were wrong:
(1 − c)(1 − k). The page converts it through the renderer's CMYK profile. Cyan ink came out as (0, 255, 255) where the page draws (0, 174, 239). Magenta was up to 115 levels off, and black ink came out pure black.ICCBasedprofiles: an Adobe RGB or Display P3 image came out as if it were sRGB, about 30 levels off.Some JPEGs were also written as they are, so viewers and Word showed them in other colours: CMYK JPEGs, and JPEGs drawn through an ICC profile the file doesn't carry.
The fix reuses the renderer's own conversion rather than writing a second one:
Vendored hayro-interpret patch (39).
color::device_cmyk_to_srgbandcolor::IccToSrgbmake public the conversions the renderer already uses for DeviceCMYK andICCBased. Both check that input and output hold the same number of pixels.pdfcraft-renderwrapper.pdfcraft_render::cmyk_to_srgbandIccProfilewrap them, catching panics like the rest of the crate. This keeps hayro behindpdfcraft-render, the bootstrap rule (ADR-0004).pdfcraft-create(extract.rs). Image export now depends onpdfcraft-render(L4 → L3, whichxtask layersallows) and converts through it:ICCBasedgrey, RGB and CMYK;/Mattecolours, now converted like the image, the way the renderer does.JPEGs written as they are. A JPEG is still written unchanged when the file shows the page's colours:
ICC_PROFILEsegments): the page ignores that profile, and a viewer would apply it. Every other byte is kept, so it stays a lossless JPEG;CMYK JPEGs, and JPEGs drawn through a profile the file doesn't carry, are now decoded and written as PNG in the page's colours.
Cases tested (renderer centre pixel against export, before → after)
/Decode [1 0 1 0 1 0]on RGB/Decode/Decode [1 0 …](written as is)ICCBasedAdobe RGB, Display P3ICCBasedCMYK; CMYK JPEG underICCBasedICCBasedAdobe RGB, profile not in the file (written as is)/MatteICCBasedwith a sRGB or grey gamma 2.2 profileCalGray/CalRGB and Lab images need more of the renderer's conversions made public (they are built from their dictionaries). They are left for a follow-up to keep this change focused, and the parity notes say so.
How you verified it
cargo fmt --check-D warningson the crates I changed:cargo clippy -p pdfcraft-create -p pdfcraft-render --all-targets -- -D warningscargo test -p pdfcraft-create: 34 passed;cargo test -p pdfcraft-render: 302 passed, plus 4 insearchand 1 ignored injbig2_invariants, as before.cargo xtask layers(OK, 37 crates),cargo xtask assets(ok),cargo xtask parity(ok)Tests. All of them compare against the page renderer as the oracle:
create::tests::exported_images_show_the_colours_the_page_draws: draws and exports 17 kinds of image and checks the colours agree within one level.create::tests::jpegs_that_carry_the_profile_the_page_uses_are_written_unchanged: the JPEG round trip, and the cases that still need converting.create::tests::device_jpegs_are_written_without_the_profile_the_page_ignores(second commit, after review): a DeviceRGB or DeviceGray JPEG carrying an ICC profile is written as the same JPEG without it. The cases are a whole profile, a profile split into two chunks, and a profile next to an APP2 that isn't one, which stays. A JPEG without a profile stays byte for byte.create::tests::damaged_and_mismatched_icc_profiles_export_as_the_page_draws_them: garbage, truncated and empty profiles, and profiles with a different number of components than/N. The export follows the page and nothing panics.cmyk_and_ycck_jpegs_export_in_the_colours_the_page_shows(Some images are exported as negative #1640) now checks the page's cyan, not pure (0, 255, 255).render::colour::tests::{cmyk,icc}_samples_convert_to_the_colours_pages_drawcover patch 39.I checked that the tests fail before the fix by reverting it in place:
main'sextract.rs. The four create tests go red:extract.rsreverted.device_jpegs_are_written_without_the_profile_the_page_ignoresgoes red at its first case: the exported JPEG still carries the Adobe RGB profile.Not run locally: the wasm check and the workspace-wide sweep (the disk is shared; CI runs both). Word export calls the same
image_file, atcrates/engine/src/compare.rs:195, and I didn't add an engine-level test for it.Never-crash
No new
unwrap/expect/panic!/unreachable!/todo!outside tests.No indexing with input-derived positions:
get;Buffers stay bounded by
MAX_PIXELSas before. The decoded samples are freed before the conversion, so peak memory doesn't grow.No
unsafe.Untrusted ICC profiles now reach moxcms from export, as they already did from the renderer. The wrapper catches panics, and lengths are checked before every conversion. Unreadable or mismatched profiles fall back as the page does: the device space of
/Ncomponents.Provenance
ATTRIBUTION.tomlchange.Security
Parity and reach
parity/acrobat-features.toml:create.export-all-images: notes and evidence updated; the notes now say what is still missing (CalGray/CalRGB written as device spaces, Lab skipped);edit.image-extract: notes and evidence updated.doc_export_all_images,image_saveanddoc_export_officeall go through the changed path.vendor/README.mdlists patch (39) and its tests;ROADMAP.mdprogress log anddocs/roadmap.mdrevision history updated (estimates unchanged).Anything a reviewer should look at first
shown_as_drawn.color.rs, right afterCMYK_TRANSFORM. M11: Output Preview — separations, ink coverage, overprint simulation and output-intent soft proofing #2770 adds a large block just before that static, so whichever of the two lands second may need a trivial merge there, and in the one-line hayro-interpret row ofvendor/README.md.extract.rs. Whichever lands second needs a small, mechanical rebase. The two don't overlap in behaviour: Decode there, colour conversion here.