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

```rust
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

- [x] A baseline JPEG decodes to its real pixel values, asserted against
      known colours, not buffer length.
- [x] A grayscale JPEG decodes.
- [x] A JPEG with chroma subsampling (4:2:0) decodes.
- [x] Progressive JPEG returns a typed refusal naming the format.
- [x] A truncated JPEG returns a typed error rather than partial garbage.
- [x] The stub can no longer exist: a test fails if a decode returns a
      uniformly zero buffer for a non-black image.
- [x] PNG decoding keeps working, with a colour-type test.
- [x] `downsample` reduces dimensions by an integer factor and averages
      pixels.
- [x] `downsample(1)` is the identity; `downsample(0)` is refused.
- [x] `decode_flate` and `decode_ascii_hex` round-trip their encoders over
      adversarial inputs.
- [x] Corpus fixtures: a real JPEG image XObject, a grayscale one, and a
      progressive one.
- [x] `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%.
- [x] `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.