nigig-org/REVIEWS/adr/0016-pdf-image-decode-surface.md

ADR 0016: the image decode surface — a fake JPEG decoder, downsampling, round-trips

• Status: Accepted
• Date: 2026-08-03
• Review item: NIGIG_PDF_FEATURE_PARITY_PLAN.md §1 Phase 3, the bullets ADR 0015 did not cover: "Image decode surface: PNG/JPEG image decoding for raster image XObjects and inline images … image downsampling for render performance", and the exit criterion's "round-trip encode tests where the format allows"
• Supersedes: nothing
• Related: ADR 0015 (filters — refused the generic DCTDecode filter and explicitly left the image path alone; this is that path)

Context

ADR 0015 closed the lossless filters and refused the image codecs at the generic filter boundary. It deliberately did not touch image.rs, noting that JPEG "is decoded on the image path". That claim deserved checking, and it does not hold.

The JPEG decoder is a stub that reports success

fn decode_jpeg_data(
    _data: &[u8],
    _pixels: &mut [u8],
    _width: u32,
    _height: u32,
    _num_components: u32,
) -> Option<()> {
    Some(())
}

Every argument is discarded. It writes nothing into pixels and returns Some(()), which the caller reads as success. read_jpeg_info allocates a zero-filled buffer, hands it to this function, and returns it as decoded pixels.

Probing an 8×8 baseline JPEG through the real ImageInfo path:

is_jpeg = true
decode_to_rgba -> 256 bytes, 64 non-zero
first 12: [0, 0, 0, 255, 0, 0, 0, 255, 0, 0, 0, 255]

256 bytes of pure black at full alpha. Not an error, not a None — a correctly sized, plausible-looking RGBA buffer that is entirely fabricated. Every JPEG in every PDF renders as a black rectangle and nothing reports it.

This is the exact defect class this codebase has removed repeatedly, and it is worse than the DCTDecode pass-through ADR 0015 fixed: that returned obviously-wrong bytes, this returns convincingly-wrong ones. The _-prefixed parameters are the tell — the signature was written to silence the unused warnings that would otherwise have announced the stub.

The coverage numbers agree: image.rs is at 14.2% line coverage, the lowest in the crate, with 617 lines never executed by any test.

Downsampling does not exist

The plan asks for it for render performance. Nothing in image.rs scales an image; a 4000×3000 scan is decoded and held at full size to fill a 200-pixel box.

There are no round-trip tests

encode_flate and encode_ascii_hex exist, with no test that decode(encode(x)) == x for any filter.

Decision

Implement a real baseline JPEG decoder

Baseline sequential DCT (SOF0), which is what PDF DCTDecode overwhelmingly carries: huffman tables, dequantisation, inverse DCT, upsampling, and YCbCr→RGB. Grayscale, YCbCr and CMYK/YCCK component counts, including the Adobe APP14 transform flag that decides whether a 4-component image is CMYK or YCCK.

Progressive JPEG (SOF2) is refused by name, not approximated. It is a substantially different coder, and a partial implementation would reproduce exactly the failure being fixed here.

No new dependency: this is a few hundred lines of well-specified arithmetic, and adding image or jpeg-decoder would pull a dependency tree into a crate that currently has one, on a target the team is already fighting to cross-compile.

Option is the wrong return type here, and it is why the stub survived

decode_to_rgba returns Option<Vec<u8>>, so "could not decode" and "decoded to nothing" are the same value, and a stub returning Some is indistinguishable from success. The image path gains a typed ImageDecodeError so a caller can tell why an image is missing — unsupported progressive JPEG, truncated data, unknown colour space — instead of getting None or, worse, black pixels.

Downsampling, exactly and only by integer factors

ImageInfo::downsample(factor) box-filters by an integer factor. Integer only, deliberately: arbitrary rescaling is a resampling-quality question belonging to the renderer, while the memory win — not holding a 12-megapixel scan to fill a thumbnail — comes almost entirely from the integer case.

Round-trip tests for the encoders that exist

encode_flate and encode_ascii_hex get decode(encode(x)) == x over adversarial inputs: empty, single byte, all-zero, all-0xFF, and random binary.

Non-negotiable rules

1. No decoder returns fabricated pixels. If it cannot decode, it says so with a typed error. A stub that returns Some(()) is forbidden.
2. Progressive JPEG is refused by name, never approximated.
3. No new external dependency for image decoding.
4. Decoded output is asserted against known pixel values, not against "a buffer of the right length" — the stub would have passed that.
5. Downsampling is integer-factor only, so no resampling policy is smuggled into the parser.

Merge criteria

• A baseline JPEG decodes to its real pixel values, asserted against known colours, not buffer length.
• A grayscale JPEG decodes.
• A JPEG with chroma subsampling (4:2:0) decodes.
• Progressive JPEG returns a typed refusal naming the format.
• A truncated JPEG returns a typed error rather than partial garbage.
• The stub can no longer exist: a test fails if a decode returns a uniformly zero buffer for a non-black image.
• PNG decoding keeps working, with a colour-type test.
• downsample reduces dimensions by an integer factor and averages pixels.
• downsample(1) is the identity; downsample(0) is refused.
• decode_flate and decode_ascii_hex round-trip their encoders over adversarial inputs.
• Corpus fixtures: a real JPEG image XObject, a grayscale one, and a progressive one.
• image.rs line coverage rises materially from 14.2%: now 32.9%, with the new jpeg.rs at 82.8% and the crate total 83.65% → 84.22%.
• TEST_TARGET=pdf ./tools/test-rust-clean.sh passes, rustfmt and clippy -D warnings clean.

All criteria met. TEST_TARGET=pdf went from 651 to 680 and TEST_TARGET=pdf-ui from 696 to 725.

Mutation-checked: reinstating the stub — returning a correctly sized zero-filled buffer — fails four tests, including a_decoded_jpeg_is_not_a_uniformly_zero_buffer, which exists solely to catch that shape.

The IDCT took three attempts, and the failures were informative

The first version was adapted from a hand-tuned integer kernel and had a scaling error. The symptom was diagnostic rather than random: greyscale decoded exactly (128 → 128) while colour came out a uniform 64 levels off. A constant offset across every channel is a scaling-factor mistake, not a coefficient one, so guessing at coefficients would never have found it.

Two further rounds of guess-and-check made it worse. The fix was to stop guessing: compute ground truth from the float reference in T.81 A.3.3 (a DC-only block of 512 must give every sample 64, plus the 128 level shift), then write the transform as a direct transcription of the separable form with an explicitly documented fixed-point scale. The cosine table is a const fn so it is computed once and cannot drift from the formula beside it, and the tests assert the DC-only and all-zero cases against the reference rather than against our own output.

Worth recording as a method note: when a numeric bug produces a uniform error, the constant is the bug. Iterating on the algorithm is wasted effort until the scale is derived rather than guessed.

Consequences

Positive. JPEG images in PDFs actually render. Given JPEG is the dominant image format in scanned and photographic PDFs, this is the difference between a viewer that shows documents and one that shows black rectangles where the content should be.

Negative. A few hundred lines of dense DCT arithmetic to maintain. Held to the baseline profile precisely to bound that.

Risk. A subtly wrong IDCT or dequantisation produces an image that looks almost right — harder to notice than black. Mitigated by asserting exact pixel values on fixtures whose expected colours are known by construction, and by a test that rejects a uniformly-zero decode.

Out of scope, deliberately: progressive JPEG, arithmetic-coded JPEG, 12-bit JPEG, JPEG encoding, ICC-aware colour conversion for JPEG (the existing colorspace.rs handles that once pixels exist), and non-integer downsampling.