nigig-org/REVIEWS/adr/0029-pdf-mesh-shadings.md

ADR 0029: mesh shadings — the code was written, and it was wrong

• Status: Accepted
• Date: 2026-08-18
• Review item: NIGIG_PDF_FEATURE_PARITY_PLAN.md §1 Phase 7, "Shading: … + mesh (free-form/lattice/Coons)"
• Supersedes: the mesh half of ADR 0028
• Related: ADR 0016 (the JPEG decoder that was a stub), ADR 0028 (shadings), ADR 0021 (enumerate criteria from the plan)

Context

ADR 0028 shipped parse_mesh and triangulate for shading types 4–7, and said so honestly: "mesh types 4–7 written but unproven — shading.rs is at 68% and the uncovered part is exactly parse_mesh." It named the risk in its own words:

the position image.rs was in before ADR 0016 found the JPEG decoder was a stub.

That was the right diagnosis. Writing the fixtures found three real bugs in code that compiled, had no warnings, and would have been reported as a completed feature.

Bug 1 — type 5 has no flag field

parse_mesh read bits_per_flag bits at the start of every record for all four types. A lattice-form (type 5) stream carries no flags at all; /VerticesPerRow is what delimits it. Reading 8 phantom bits shifted every subsequent vertex by one byte, so the coordinates decoded to arbitrary points inside the decode range — plausible numbers, in the right ballpark, entirely wrong. a_lattice_mesh_has_no_per_vertex_flag is that test.

Bug 2 — patches carry four colours, not one per control point

Types 6 and 7 are patches, not vertices: one flag, then 12 (Coons) or 16 (tensor) control points carrying no colour of their own, then four corner colours. The old loop read a colour after every point — 12 colours where the stream had 4 — consuming three times too many components and running off the end of the stream, which the None-on-truncation path then silently swallowed as "the mesh ended".

The old code also treated a control point as a vertex and then took indices 0, 3, 6 and 9 of a 12-vertex window, so even when it did not run short the corners were read from the wrong offsets.

Bug 3 — flag 0 cleared the strip on every vertex

Found by the fixture, in new code, before it shipped: a flag-0 triangle is three vertices and the second and third also carry flag 0, their flags ignored per §8.7.4.5.5. Acting on them cleared the strip each time and produced no triangles at all.

Also missing outright: a parsed mesh had no way to produce a colour. color_at_point returned None for ShadingGeometry::Mesh, so a mesh that parsed perfectly still painted nothing — indistinguishable, from the caller's side, from one that failed.

Decision

One reader per type, not one loop with conditionals

parse_mesh now branches on the shading type up front. The four encodings differ in what a record is — a flagged padded vertex, an unflagged unpadded vertex, or a patch — and the previous attempt to express that as a single loop with two if shading_type == adjustments is precisely how bugs 1 and 2 got in.

Shared edges are honoured, not treated as new patches

Types 6 and 7 with flag 1–3 share an edge and two colours with the previous patch and therefore encode four fewer points and two fewer colours. Treating a continuation as a fresh patch does not merely draw one patch wrong: it desynchronises the rest of the stream. previous carries the corners forward and the flag selects which edge (table 85).

Gouraud interpolation by barycentric coordinates

MeshTriangle::color_at returns the interpolated corner colours, or None outside the triangle — barycentric coordinates are both the inside test and the weights, so computing them any other way does the same arithmetic twice. Shading::color_at_point now walks the triangles, later ones winning, which matches the painting order of the vertex stream.

None outside is the ADR 0028 rule again: black is a colour a mesh can legitimately produce, so "no colour here" cannot be spelled black.

Coons flattening is still an approximation, and still says so

Two triangles across the four corners loses the curvature of the patch edges. is_approximate reports it. This ADR does not fix that, and does not claim to: a_coons_patch_reads_four_corner_colours_not_one_per_control_point asserts the flag is set.

Consequences

• Mesh shadings decode correctly and can be sampled. shading.rs coverage rises from 68%; the uncovered region was exactly the code this ADR fixed.
• Coons and tensor patches are drawn as flat quadrilaterals. A patch with strongly curved edges will show straight ones. Recorded, not hidden.
• The corpus gains five fixtures, generated by generate.py from named coordinates and colours, so every expected value in the tests is a number the generator wrote deliberately rather than one copied out of the output.

Merge criteria

Enumerated from the plan bullet first, per ADR 0021.

Criterion	State
Free-form (type 4) parses a real stream	✅ mesh_free_form.pdf, vertices and strip flag asserted
Lattice-form (type 5) parses a real stream	✅ mesh_lattice.pdf; the no-flag bug is the test
Coons (type 6) parses a real stream	✅ mesh_coons.pdf, four corner colours asserted
Tensor (type 7) parses a real stream	✅ mesh_tensor.pdf, 16 control points asserted
A mesh with /Function decodes	✅ mesh_function.pdf, one parametric value per vertex
A mesh produces colour at a point	✅ MeshTriangle::color_at, Gouraud-interpolated
Outside a triangle is None, not black	✅ asserted in a_free_form_mesh_interpolates_across_a_triangle
Truncated streams stop rather than inventing vertices	✅ asserted; BitReader::read returns None
Curved patch edges rendered exactly	❌ deferred — flattened to two triangles, reported by is_approximate
Mutation-checked	✅ 5 mutations, all killed (type-5 flag, tensor stride, per-point colour, inside test, strip flag)