feat(png): a composed effort dial for the encoder - #625
Open
justin13888 wants to merge 107 commits into
Open
justin13888 wants to merge 107 commits into
justin13888 wants to merge 107 commits into
Conversation
`gamut_png::deconstruct` classifies every byte of a PNG into a typed `Segment` and reports the figures an encoder-efficiency comparison is built from: bits per pixel, what the DEFLATE stage achieved in isolation, how many bytes went to chunk framing, and which scanline filter each row chose. It works on any PNG, whichever encoder wrote it, which is the point: the same numbers can be read off libpng's, oxipng's or zopflipng's output and compared directly. Issue #224 asks for BPP efficiency and parity, and neither is answerable from a total byte count alone -- a size difference has to be attributable to a stage before it can be acted on. Shape follows `gamut_tiff::deconstruct` / `gamut_dng::deconstruct` for the entry point and verdict method, and `gamut_isobmff::segments` for the `Segment { range, kind }` tiling. gamut-png does not and must not depend on gamut-isobmff, and that walk is box-structured anyway, so PNG needs its own -- but the names are deliberately identical. Owned rather than borrowed, unlike the ISOBMFF one. Its segments borrow because they are the only route to an unknown box's bytes; PNG already has `metadata()` for payloads, so the report carries only counts and ranges and can be `Clone + PartialEq + Eq` and stored across a bench corpus without pinning every input buffer alive. Deliberately more tolerant than `metadata()`, which rejects an unknown critical chunk: a measurement tool that refuses to measure is useless. Unknown chunks of either criticality, CRC mismatches, a missing IEND, trailing bytes and a truncated tail are reported, not errored -- `gamut_dng::deconstruct`'s contract verbatim. Only a file with no header to report on fails. The filter histogram is the one part that costs work and can fail, so it is `Option`. The inflation bound needs no policy: PNG's filtered length is *exactly* determined by IHDR, so `max_out` is that length and a zlib bomb cannot exceed it by a byte; a hostile IHDR is handled by declining to inflate past the decoder's existing 64 MiB image budget. Everything else in the report comes from framing and IHDR, so it survives a corrupt, truncated or oversized stream. `RawChunk` gains its own `range`, taken from the offset `ChunkReader` already advances, so byte accounting cannot drift from framing arithmetic; the reader gains an `offset()` so a caller can bound a malformed tail. `PngHeader` gains `PartialEq, Eq` -- additive, and a plain `Copy` header should be comparable. Tests are the byte-accounting law, the family `docs/testing.md` names after `gamut-avif`/`gamut-heic`'s `tests/accounting.rs`. `assert_covers` re-derives the tiling rather than trusting `is_fully_classified`, which is the thing under test. Fixtures come from libpng wherever the claim is about reading a foreign file: interlaced streams, forced filters and sub-byte depths are all things `PngEncoder` cannot write, and a histogram checked against gamut's own filter choice would be self-consistent rather than correct. Two findings from writing them, both recorded in the code: * A trailer counts against `is_intact` even though §13.2 lets a decoder ignore trailing bytes. `bits_per_pixel` divides the whole file by the pixel count, so bytes outside the datastream inflate the headline figure and a size comparison has to know they are there. * The CRC fixture corrupts a stored CRC, not a payload. Corrupting IHDR's payload makes the header unparsable, which is a hard error and a different claim entirely. Refs #224
A `benches/` target compiles as a separate crate, so it can only reach `pub` items -- and every encoder stage is crate-private. Timing them one at a time needs a seam. `src/stages.rs` is that seam, and it is re-exports and nothing else. No wrapper bodies: a wrapper would be an executable line no gate ever runs, since bench targets carry `test = false` and neither `cargo test`, `cargo llvm-cov` nor `cargo mutants` reach them. It would drag the coverage floor and generate mutants no test could kill. `.cargo/mutants.toml` already states the rule this follows, in its `crates/gamut/**` entry: "pure feature-gated re-exports (no function bodies), so it carries no logic of its own to mutate." So this needs no new exclusion. The stage items become `pub` inside their still-private modules, which changes no effective visibility -- a `pub` item in a private module is unreachable. With the feature off the crate's public API is byte-identical to before. `test-support` follows the convention gamut-core, gamut-ifd and gamut-tonemap use for their `invariants` modules: additive, `doc(hidden)`, no SemVer guarantee, and never enabled by the `gamut` umbrella, so the shipped surface and `mise run check-ffi-features` are unaffected (both verified). `Crc32::new` gains an `expect(clippy::new_without_default)` rather than a `Default` impl. Nothing in the crate would call such an impl, so it would be an uncovered region and an unkillable mutant -- dead delegation added only to satisfy a lint. Refs #224
gamut-png was one of the few codec crates with no `benches/` directory, and
both `README.md` and `STATUS.md` claimed "output size is benchmarked against
libpng at maximum compression" -- a claim no code backed. This is that
benchmark.
Two tables print before the divan run, following gamut-deflate's and
gamut-dng's shape: output size and bits-per-pixel against libpng at zlib
level 9, then where the bytes went stage by stage. Every column of both comes
from `gamut_png::deconstruct` reading the encoded file back, so the libpng
column is a like-for-like measurement rather than two encoders' self-reports,
and a size difference can be attributed to filtering, to the colour-type
choice, or to DEFLATE.
libpng gets the *same source layout* gamut gets, with no `palette` option even
for palettisable rows -- handing it a palette would hand it gamut's own
reduction and the comparison would stop measuring anything. Its default
adaptive filtering is left alone: that is the honest baseline.
The measured baseline, recorded here so the next change has something to be
judged against (one machine; read the ratios, not the times):
input raw default best libpng-9 best/lp9
gradient_rgb8 196608 2831 2272 2393 -5.1%
photo_rgb8 196608 29885 20293 27467 -26.1%
noise_rgb8 196608 196983 196983 197280 -0.2%
grey_as_rgb8 196608 721 370 566 -34.6%
palette64_rgba8 262144 1274 715 1102 -35.1%
sprite_rgba8 262144 4181 3729 3889 -4.1%
flat_rgba8 262144 821 103 664 -84.5%
tiny_rgb8 768 136 135 138 -2.2%
gamut is smaller than libpng-9 on every row. The stage table shows why, and
where it is not: `sprite_rgba8` -- binary alpha over invisible colour noise --
stays TruecolorAlpha where the reduce cascade should reach it, which is
exactly the tRNS-colour-key and dirty-alpha gaps this issue is about.
Corpus notes, both of which cost a fixture rewrite to get right:
* 256x256 is the floor that means anything. RGB at that size is 192 KiB,
roughly six times the 32 KiB DEFLATE window, so LZ77 match behaviour is
real; a 64x64 image fits *inside* the window and would flatter both
encoders equally.
* The "incompressible" row is a full avalanche mix, not the plain
`i * 2654435761 >> 24` gamut-deflate's bench uses. Over a dense index that
top byte changes only once every few hundred `i`, so the first version of
this row compressed 97x and measured nothing at all. It now expands
slightly, as any lossless codec must on random data.
Per-stage rows sit behind `test-support` and are skipped without it, so plain
`cargo bench -p gamut-png` and `mise run bench` still work. No
`required-features` on the target: `mise run bench` passes no features, and
the whole bench would silently never run.
Refs #224, #149
`README.md` and `STATUS.md` have long claimed "output size is benchmarked
against libpng at maximum compression". The previous commit prints that
comparison, but a bench asserts nothing and does not run in the per-PR gate.
This makes the claim enforceable: a regression in the crate's reason to exist
fails the build, the same mechanism gamut-deflate's ratio contract and
gamut-webp/tests/effort.rs use.
Every budget carries its own written justification naming the stage that
spends the bytes, in the shape of gamut-cmm's precision-budget table, and
records what the row measured when the budget was set so drift shows up in
review rather than as a surprise red build. Measured at 128x128 -- half the
bench's side, so this stays fast enough for the coverage and mutation lanes.
row gamut libpng-9 ratio budget
gradient_rgb8 703 749 0.939 0.98
photo_rgb8 5843 7768 0.752 0.85
noise_rgb8 49348 49435 0.998 1.01
grey_as_rgb8 146 251 0.582 0.70
flat_rgba8 96 299 0.321 0.45
sprite_rgba8 1669 1733 0.963 1.00
palette64_rgba8 451 405 1.114 1.15
The last row is the finding, and the budget records it rather than hiding it.
gamut auto-palettises where libpng writes RGBA: at 256x256 that wins by 35%,
at 128x128 it loses by 11%. Measured with `deconstruct` across four sizes:
side gamut IDAT PLTE+tRNS libpng-9
128 451 121 273 405
160 511 181 273 572
192 564 234 273 707
256 715 385 273 1102
The cause is not that `reduce::analyze8` ignores the palette chunks -- it
counts them, estimating 280 bytes against an actual 273. It is that the model
compares *raw* sizes, and raw size does not predict compressed size when one
candidate's bytes are incompressible and the other's are not. Those 273 bytes
survive DEFLATE intact while the RGBA alternative compresses roughly 160x, so
the estimate sees 16 664 against 65 536 and picks palette by a 4x margin that
does not survive compression. The crossover sits near 160x160. Filed
separately; a cost model that weighs incompressible overhead against
compressible pixels is what tightens that budget.
Four tests, each failing for one reason: the budget table, a strictly-smaller
assertion for the rows that claim a structural win, an attribution test, and
determinism. The winning set is listed explicitly rather than derived from
`max_ratio < 1.0` -- a budget loosened past 1.0 during a regression would
otherwise drop out of that test silently, which is exactly when it should
fail. Not hypothetical: palette64 was in the derived set before it was
measured.
The attribution test is why `deconstruct` is a dependency here. Where both
encoders land on the same colour type and depth the filtered stream is
identical by construction, so comparing the *compressed* streams isolates
DEFLATE from filtering and from the colour-type choice.
The corpus moves to `tests/common/corpus.rs` and the bench includes it by
path. Budgets are only meaningful measured on the same pixels the table
reports, and two copies would drift invisibly -- a budget that no longer
describes the row it names.
libpng gets the same source layout with no palette hint and its own default
adaptive filtering. Handing it a palette would hand it gamut's reduction.
Refs #224
At `alpha == 0` the colour channels are invisible by definition, but the
source's bytes are still stored and still cost. `with_transparent_cleanup`
zeroes them. Off by default, and deliberately separate from
`with_auto_reduce`: every other reduction in this crate is exactly reversible,
and this one is only reversible in what you can see.
It pays three compounding ways -- transparent pixels become identical so a run
filters to zeros; `analyze8` keys its palette on the whole RGBA quad, so
invisible pixels that differ only in unseen colour stop costing an entry each;
and it is the precondition for a tRNS colour key, which needs one colour to
stand for "transparent".
One constant, not the neighbouring pixel's colour, and that was measured
rather than assumed. Inheriting the predecessor flattens a run just as well,
but leaves every invisible pixel a distinct RGBA quad, so the palette and tRNS
benefits both vanish: on a fixture alternating visible and invisible pixels it
collapsed nothing and saved exactly zero bytes (378 vs 378). Zeroing collapses
them to one entry.
Two halves to the claim, so two techniques. That nothing visible changes is
differential: libpng decodes both files and every pixel with non-zero alpha
must be byte-identical, with alpha itself identical everywhere. That it pays
is a size assertion against the same image encoded without it.
Measured, and the interaction is worth stating plainly -- on the 256x256
sprite this makes the file *larger*:
side clean total colour type IDAT
64 false 859 TruecolorAlpha 802
64 true 817 Indexed/8 549
128 false 1669 TruecolorAlpha 1612
128 true 1925 Indexed/8 1477
256 false 3729 TruecolorAlpha 3672
256 true 4589 Indexed/8 3781
The cleanup is not what regresses: its IDAT is smaller at every size. What
happens is that collapsing the invisible colours drops the image under the
256-colour cliff, so `analyze8` now offers a palette -- and the raw-size cost
model then picks it, exactly as it wrongly picks it for `palette64_rgba8` in
the previous commit. Same defect, second independent witness, and cleaning
makes it reachable on more images. The next commit fixes the model; this one
would have been a regression shipped alone.
Refs #224
`reduce::analyze8` chooses by comparing **raw** sizes, and raw size does not
predict compressed size when one candidate's bytes are incompressible and the
other's are not. A palette carries PLTE (and often tRNS) that DEFLATE cannot
touch, while the pixels it replaces may compress by two orders of magnitude.
Two independent measurements from the previous commits:
* `palette64_rgba8` at 128x128: PLTE + tRNS is a flat 273 bytes, the indexed
pixel data compresses to 121, and the RGBA alternative compresses to 405
in total. The estimate sees 16 664 against 65 536 and picks the palette by
4x. Finished files: 451 against libpng-9's 405 -- the only corpus row
where gamut lost.
* The sprite, once transparent-colour cleanup collapses its invisible pixels
under the 256-colour cliff, becomes palettisable and is then chosen at
every size: 817 vs 859 at 64x64, but 1925 vs 1669 at 128 and 4589 vs 3729
at 256.
Same defect, and cleaning made it reachable on more images.
Rather than guess a correction factor, `write_reduced_or_native` encodes both
candidates and keeps the smaller. That is exactly what
`FilterStrategy::BruteForce` already does for filters, it needs no tuned
constant, and it cannot be worse than either candidate alone. A tie keeps the
palette, which decodes with less work.
Only palette reductions pay for the second encode. Greyscale, alpha-drop and
16->8 demotion add no chunks, so for them the raw comparison is already sound
and the function returns immediately.
Measured after:
row before after
palette64_rgba8 @128 451 390 (libpng-9: 405, now a win)
sprite_rgba8 +clean @256 4589 2619 (uncleaned best: 3729)
The sprite is the striking one: cleanup was a 23% regression and is now a 30%
improvement, because the race stops the analysis's mistake from landing.
Two oracle tests changed, and the reason is worth stating rather than burying.
Both pinned a *colour type* as a proxy for "a reduction happened", and the
race decouples those: the analysis still offers a palette, the encoder now
declines it when it would cost bytes. On 32x32 fixtures with a handful of
repeating colours the unreduced stream genuinely wins, so the old expectations
were asserting the defect. They now assert the contract that matters -- the
pixels survive, and the smaller file is kept -- and a new
`a_palette_is_chosen_when_it_actually_wins` covers the other side of the race
at 192x192, where the fixed cost is amortised. Without it the palette encoding
path would only ever be exercised where it loses. The analysis contract itself
stays pinned by `reduce`'s own unit tests, which is where it belongs.
Refs #224
Both hot loops the new benchmark exposed, neither needing any `unsafe` in
gamut. Output is byte-identical: every row of the size table is unchanged, and
the oracle, determinism and size-contract suites all still pass. This buys
time, not bytes.
before after
crc32 420.8 MB/s 8.996 GB/s 21x
filter_image None 497.9 MB/s 16.26 GB/s 33x
filter_image Paeth 277.1 MB/s 1.202 GB/s 4.3x
filter_image MSA 46.7 MB/s 265.8 MB/s 5.7x
choose_min_sum_abs 68.0 MB/s 308.4 MB/s 4.5x
CRC-32 moves to `crc32fast`, which dispatches to PCLMULQDQ/AVX-512 on x86-64
and the `crc32` instructions on aarch64, with a table fallback elsewhere
including wasm32. Its `unsafe` stays inside that crate; gamut-png remains 100%
safe Rust, which is why this needed no policy change. The two existing unit
tests stay exactly as they were, now as a drift guard: they pin the polynomial
this module's doc claims, so a backend computing a different CRC-32 variant
fails here rather than silently producing files no decoder accepts.
The filter loops needed no dependency at all. Three structural pessimisations
were blocking the vectoriser, and removing them is most of the win:
* The `i >= bpp` test choosing between a real left-neighbour and an implicit
zero is loop-invariant. The row now splits into a `bpp`-long prologue
where `a` and `c` are zero and a body where they are not. That collapses
Sub to a copy in the prologue and, less obviously, Paeth to Up, because
`paeth(0, b, 0) == b` for every `b` -- at `b == 0` all three distances tie
and the spec's order picks `a`, which is also zero.
* The body reads five equal-length subslices, so the bounds checks fold away
instead of being re-proved per index.
* The filter is matched once outside the loop instead of once per byte, and
`out` is sized once instead of a capacity check per `push`.
Separately, `MinSumAbs` was filtering each scanline **six** times, not five:
`choose_min_sum_abs` computed all five candidates, returned only which one
won, and `filter_image` then recomputed exactly those bytes. It now hands back
the winning buffer, trading a `memcpy` per improvement for a full filter pass
per row.
`unfilter_row` is deliberately untouched. Forward filtering has no serial
dependency, so all five kernels vectorise; reconstruction reads
`row[i - bpp]` after writing it, so only `Up` would benefit and this is an
encoder-first crate.
Refs #224
…onvention
`gamut-png`'s STATUS gains an Efficiency section: the size table against
libpng-9, the throughput before/after, a per-axis scorecard of the nine things
a PNG encoder competes on, and the measured explanation of why the palette
choice is now a race rather than an estimate. Every number is reproduced by
`cargo bench -p gamut-png` and gated by `tests/size_contract.rs`.
Its README and STATUS both claimed "output size is benchmarked against libpng
at maximum compression" while no code did either. They now say what is true:
measured by the bench, enforced by the contract.
`docs/benchmarking.md` is new, and takes an owner for something that had none.
`docs/testing.md` disclaimed benchmarks by name, and `docs/README.md` makes
anything unlisted there "descriptive, not binding" -- so the conventions every
bench in the workspace already follows were binding on nobody. It is normative
for where a benchmark lives, what a size or ratio table must record, and where
a measured number is kept, and it hands the enforcement question back to
`testing.md` explicitly. The rule it turns on:
A benchmark reports. A test asserts. Only the test can fail a build.
It also records what CI actually does now, which changed under this branch:
`mise run lint`'s `--all-targets` compiles every bench on every PR, and the
Extended lane's `mise run bench-test` runs each once. Neither gates a number,
and the document says why that is still open rather than implying benches are
ungated.
Both normative documents change here because `docs/README.md` requires it: a
`docs/` file that contradicts another is a change to both.
Seven follow-ups filed with their measured evidence rather than left as prose:
#478 gamut-deflate: 8-byte-at-a-time longest_match -- the dominant cost of
every encode in the workspace, safe Rust, byte-identical output
#479 gamut-deflate: relax each length at its own nearest distance
#480 gamut-png: entropy and bigram heuristics, pruned two-tier trials
#481 gamut-png: tRNS colour key for grey and truecolour
#482 gamut-png: palette ordering and caller-supplied palette cleanup
#483 gamut-png: metadata policy, and the CLI's silent drop
#484 gamut-png: parallel filter trials, and a composed effort dial
Refs #224
CI's diff-scoped mutation run surfaced ten survivors across the four shards.
None was noise: each one names a claim the new code makes that nothing
actually checked.
Three needed only a fixture that could tell the difference:
* `is_fully_classified`'s `||` and its whole body. `deconstruct` cannot
produce a malformed tiling -- it is correct by construction -- so every
negative case has to be built by hand. Inline tests now assemble reports
with a gap, an empty segment, an overlap, a late start and an early end,
each isolating one half of the predicate.
* `ChunkStats`'s `count += 1` and `payload_bytes += len`. Every fixture
carried at most one chunk of each type, so the accumulate arm never ran
and `count` sat at the 1 it is inserted with. Two tests now cover it: a
hand-built file with two `tEXt` chunks, and a real multi-IDAT encode that
also ties the chunk table back to `idat_compressed`.
* `filter_histogram`'s `at += 1 + row_bytes`. Mutated to `*=` the cursor
stays at 0 and every row's filter byte is read from the same offset --
indistinguishable while every histogram test forced a *single* filter for
the whole image, because both report `height` of it. A fixture whose rows
genuinely choose differently now pins that at least two buckets are
non-empty.
Three were untestable where they stood, and moved rather than being papered
over:
* The inflation budget (`filtered_len == 0 || filtered_len > MAX`). Reaching
the boundary through `deconstruct` would need a real 64 MiB stream either
side of the cap, and a hostile IHDR cannot separate `>` from `>=` or `==`
because an over-budget file is rejected a second time when the inflated
length fails to match. Now `within_inflation_budget`, tested at 0, 1, the
cap and one past it.
* The palette-vs-native tie-break. Engineering two encodings of one image to
land on exactly equal lengths is not something a fixture can do reliably,
so `prefers_native` carries the comparison and a unit test pins the
documented rule: a tie keeps the palette.
* `clean_transparent`'s "is there anything to do" check. Mutated to `!=` it
returns `Some(unchanged copy)` for a fully opaque image instead of `None`,
which the encoder cannot see -- the bytes are identical either way. The
distinction is that the encoder must be able to tell "no work" from "work
that changed nothing", or it allocates a whole image for nothing, so the
test is on the function.
And one was an equivalent mutant, removed rather than tested: the
`start < png.len()` guard before pushing a `Truncated` segment can never be
false, because `next_chunk` returns `Ok(None)` when nothing is left and only
errors with bytes remaining. It was dead code wearing a safety net's clothes;
a `debug_assert` records why.
Refs #224
`gamut inspect` already answered "did every byte get accounted for?" for TIFF
and DNG. For PNG the same walk answers a second question -- where did the
bytes go? -- which is what makes an encoder comparison possible from the
command line, on files this crate did not write.
PNG prints on its own path rather than being flattened into `Summary`. It has
no IFD tree and no tag vocabulary, but it carries compression figures the
others have no equivalent for, and forcing both through one shape would lose
the half that matters.
Verified end to end on libpng's own `pngtest.png` -- Adam7 interlaced, 18
chunk types including five this crate does not recognise (`sTER`, `vpAg`,
`oFFs`, `pCAL`, `sCAL`):
image: 91x69 TruecolorAlpha depth 8, Adam7 interlaced
size: 8759 bytes (11.160 bits/pixel)
IDAT: 8119 bytes compressed from 25247 filtered (32.2%)
overhead: 640 bytes, of which 216 is chunk framing
filters: None 21 / Sub 15 / Up 52 / Average 10 / Paeth 33 (131 scanlines)
classified: yes
intact: yes
Every byte of a foreign file classified, and the filter distribution counted
across seven Adam7 passes. Truncating it to 4000 bytes reports
`truncated from offset 342 (3658 bytes)`, keeps every framing- and
IHDR-derived figure, drops only the histogram, and exits non-zero.
`Crc32::new`'s lint suppression changes from `expect` to `allow`, and the
reason is worth recording: `clippy::new_without_default` only fires when
`test-support` re-exports the type through `crate::stages`, so an `expect` is
*unfulfilled* in a default-feature build and fails there instead. That is
`expect` working correctly -- it caught its own obsolescence in one of two
configurations -- but a feature-dependent lint wants `allow`.
Refs #224
The one lawful PNG representation this encoder could not write. The crate said
so itself, at `decoder.rs:1327`: "the encoder cannot write interlaced files or
greyscale/truecolour tRNS colour keys". The decoder has always read them, so
only the encoder half was missing.
Three conditions, all necessary, because §11.3.2.1 gives a decoder exactly one
transparent colour and not a mask: every alpha is 0 or 255; at least one pixel
is transparent; and every transparent pixel shares one colour that no opaque
pixel uses. That last one is why `with_transparent_cleanup` pairs with this --
it collapses every invisible pixel to one colour, which is precisely what a
key needs.
Two passes, not one: the candidate is unknown until the first transparent
pixel is seen, so proving no *earlier* opaque pixel used it needs a second
look. The second only runs once the first has found a candidate.
The measurement changed the design twice, and both are recorded in the code
because neither is guessable:
* **It is worth ~7-9%, not the 25% the raw-byte arithmetic suggests.**
Dropping a channel removes 25% of the samples, but the alpha plane is
usually the most compressible plane in the image, so most of that is
already free. On a 128x128 sprite: 863 bytes keyed against 926 plain.
* **Only on a contiguous transparent region.** With the transparency
scattered by a hash instead, the invisible colour interleaves with the
visible gradient and wrecks the RGB channels' compressibility: `RGB+tRNS`
came out at 14 886 bytes against plain RGBA's 14 319, and the race
correctly declined the key. The first version of the fixture here was
scattered, and the tests failed until the shape matched what real sprites
and icons actually look like.
So keyed encodings join `Indexed` in `write_reduced_or_native`'s race rather
than being taken on the estimate. A `tRNS` chunk is incompressible in exactly
the way a `PLTE` is, and the same raw-size blind spot applies: at 32x32 and
64x64 the analysis offers a key and the race is right to refuse it.
Tests go through libpng in every case rather than round-tripping gamut against
itself: gamut writes the key and libpng interprets it, so a round trip could
agree on a wrong convention and prove nothing. That includes pinning the
payload bytes, since §11.3.2.1 wants three *16-bit big-endian* samples and a
decoder reading them as three bytes would key on the wrong colour.
Refs #224. Closes #481.
Axis 3 moves to done, with the measured figure rather than the raw-byte one: ~7-9% on a contiguous transparent region, because the alpha plane a key removes is usually the most compressible plane in the image. Refs #224
Palette index order is not free. It decides the `tRNS` chunk's length, and it
decides what the row filters see, because a filtered index stream is the
*difference* between neighbouring indices. Discovery order -- raster scan --
optimises neither.
Two rules. Transparent entries first, so the trailing-opaque `tRNS` trim cuts
as much as §11.3.2.1 allows; one late transparent entry used to pin the whole
chunk to full length. Then by Rec. 601 luma, so neighbouring indices are
neighbouring brightnesses and a smoothly shaded image produces small index
deltas rather than the arbitrary jumps discovery order gives.
Measured by disabling the ordering alone, so the figure is not confounded with
the colour key landing in the same branch:
row unordered ordered
sprite_rgba8 +clean 2619 2235 -14.7%
palette64_rgba8 715 726 +1.5%
A real trade, and worth stating rather than rounding to "it helps". The
sprite's gain is 35x the palette64 loss, and palette64's colours are synthetic
ramps whose discovery order already correlates with index adjacency -- the
case luma sorting is least able to improve and most able to disturb. The full
modified-Zeng ordering oxipng uses remains #482.
The rest of this commit closes the mutation gaps CI found in the previous
commit's colour key. All seven were in the cost estimate -- the guard deciding
whether to look for a key, the match on `all_gray`, and the arithmetic in both
arms -- and they share one cause worth recording, because it will recur:
**`write_reduced_or_native` makes the estimate much less observable.** A
mutated cost still produces a keyed candidate, which still races the unreduced
encoding, and the smaller still wins. So perturbing the estimate usually
changes which candidate is *offered* without changing the bytes that finally
win. That is the race doing its job -- it is exactly why the estimate stopped
being load-bearing -- but it means an estimate can no longer be tested through
the encoder.
So the arithmetic moves into `may_have_colour_key` and `keyed_size`, tested
directly, with the chunk costs as named constants derived from the spec
(2 + 12 for greyscale, 6 + 12 for truecolour) rather than as literals. Same
treatment the inflation budget and the palette tie-break already got.
Refs #224. Closes #482.
The sprite row's cleaned figure moves 2619 -> 2235 and palette64's 715 -> 726, which is the trade the ordering commit measured. Axis 4 moves to partial: ordering landed, modified-Zeng and the caller-supplied palette path remain. Refs #224
Sum-of-absolutes asks "are these bytes small?". DEFLATE asks "are these bytes
repetitive?". Those are different questions, and a row alternating 0 and 200
answers the first badly and the second beautifully -- which is why oxipng
dropped libpng's MinSum from every preset except its cheapest and its most
expensive.
That is a preset table, not published byte counts, so gamut measured it on its
own corpus. IDAT bytes at `Level::Best`, each heuristic alone:
input MinSumAbs Entropy Bigrams winner
gradient_rgb8 2215 2215 1505 Bigrams
photo_rgb8 25364 22427 19513 Bigrams
noise_rgb8 196890 196890 196890 tie
grey_as_rgb8 475 506 506 MinSumAbs
palette64_rgba8 990 899 770 Bigrams
sprite_rgba8 3672 3857 4062 MinSumAbs
flat_rgba8 573 573 605 MinSumAbs
tiny_rgb8 79 79 62 Bigrams
Bigrams wins four rows by 22-32%; MinSumAbs wins three by 5-6%. Neither
dominates and the margins run the wrong way to drop either, so both are in the
brute-force set -- which is also the shape of oxipng's own presets.
**Entropy is never the unique winner, and that is recorded as a negative
result rather than quietly merged.** It beats MinSumAbs on the photographic
and palette rows but loses to Bigrams on both, and ties MinSumAbs elsewhere.
The brute-force set resolves by taking the smallest, so a candidate dominated
everywhere costs a full filter pass and a full DEFLATE for nothing. It is not
in that set. It stays selectable, because eight images is a corpus and not a
proof, and `docs/benchmarking.md` asks for the negative result to be written
down so nobody re-derives it.
End to end, with Bigrams in the brute-force set:
row before after
gradient_rgb8 2272 1562 -31.2% (vs libpng-9: -5.1% -> -34.7%)
tiny_rgb8 135 119 -11.9% (vs libpng-9: -2.2% -> -13.8%)
photo_rgb8 20293 19570 -3.6% (vs libpng-9: -26.1% -> -28.8%)
The scorers share one `Scratch` allocated per image, not per scanline: the
bigram set is 8 KiB of bitset and rebuilding it per row would dominate the
very measurement it exists to make cheap. A test pins that the scratch does
not leak state between rows, because a stale one would silently score every
row after the first against the previous row's data.
`tests/backends.rs`'s `rgb8_best_bruteforce` golden is re-captured: Bigrams
wins on that fixture and takes its IDAT from 36 bytes to 21. That pin exists
to prove the *codec-abi seam* is inert, not to freeze the encoder, so the
comment there now records the re-capture and why -- an encoder change making
output *larger* would look identical at that assertion and would be a
regression.
Refs #224, #480.
`choose_by` seeded `best_score` with `u64::MAX` and improved on a strict `<`, so a row whose five candidates all scored `u64::MAX` left `best_bytes` untouched. `filter_image` hoists that buffer out of the row loop, so such a row was emitted under a filter byte of 0 carrying the *previous* row's residuals -- or, on the first row, nothing at all. `Score::Entropy` reached that sentinel whenever no byte value repeated in the filtered row, which is ordinary for narrow images. A 2x1 Gray8 `[1, 3]` encoded to a PNG whose IDAT is shorter than its image; a 2x2 `[0, 0, 0, 1]` encoded to a structurally valid PNG decoding to `[0, 0, 0, 0]` -- silent corruption, no error anywhere. Two independent fixes, because one is a class and the other an instance. `best_score` becomes `Option<u64>`, so "nothing chosen yet" is unrepresentable as a score and the first candidate is taken whatever any scorer returns; a future scorer cannot reintroduce this. And the entropy score is restated as `sum c*log2(n/c)`, the quantity its doc already claimed, which is non-negative and bounded by `8n*256` -- so it can no longer collide with a sentinel at all. The tie-break is unchanged: the only comparison is still a strict `<` over candidates 2..5, and candidate 1 is `FilterType::None`, first in the documented None/Sub/Up/Average/Paeth order. No pinned bytes move, because `sum_abs` and `Bigrams` are bounded far below `u64::MAX` and so always wrote on their first candidate already -- the two paths are bit-identical for every strategy in `BRUTE_FORCE_STRATEGIES`, and `MinEntropy` is not in that set. `tests/oracle.rs` gains the end-to-end sweep whose absence hid this: `MinEntropy` was scored by unit tests but never encoded with.
`(a << 8) | b` over two `u8`s is spelling out `u16::from_be_bytes`, and it costs two operators that carry no meaning of their own. One of them has no behavioural variant at all: the low byte of `a << 8` is zero, so `|` and `^` compute the same index, and no test can ever tell them apart. `.cargo/mutants.toml` would accept a line-scoped exclusion with that argument written out. Restructuring is better and the file already prefers it -- `deconstruct.rs` twice shapes code so an equivalent mutant is never generated rather than excluding one after the fact. Reading the pair as the big-endian `u16` it is leaves no operator to mutate. The bigram vectors gain the case none of them covered: (1,3), (3,2), (2,3) is three distinct pairs over two distinct second bytes, so an index that dropped the high byte would report two. Every existing vector happens to have as many pairs as second bytes.
`analyze8` reached its colour-key branch through `key.expect(...)` -- the only `expect` outside `#[cfg(test)]` in the crate's `src/`, which the house rule forbids in library code paths. Fold the option into the guard with a let-chain, as the palette scan at the top of the function already does. Behaviour is identical: when no key was found `keyed_size` is `usize::MAX`, and `best` has already been proven smaller than `input_size`, so `best == keyed_size` could never hold. `colour_key` carried the same shape one level down. Its `any_transparent` flag was assigned in exactly the arm that assigns `candidate`, so `!any_transparent` was a spelling of `candidate.is_none()` that the following `candidate?` discharges again -- an unkillable mutant in a file `.cargo/mutants.toml` does not exclude. Drop the flag and record in the doc why condition 2 needs no check of its own, including the caller gate (`may_have_colour_key` requires `!all_opaque`) that makes the `?` itself unreachable in practice.
`ordered_palette` was untested as a function: every palette fixture in the crate happens to have discovery order equal to sorted order, so none of them could tell it from the identity. The three Rec. 601 weights survived mutation to additions for exactly that reason. Pin the luma order on a five-entry fixture chosen so collapsing any one weight to an addition returns a different sequence, and tabulate the four columns in the doc comment so the choice of entries is auditable. Pin rule 1 separately, through `build_indexed`, on a palette whose transparent entry is discovered last -- the case first-appearance order gets wrong. In discovery order the `tRNS` alphas are `[255, 255, 0]` and the trailing-opaque trim cannot shorten them at all; sorted transparent-first they are `[0, 255, 255]` and the trim cuts two of three.
A PNG chunk type is four unvalidated bytes and the deconstruct walk never drops a chunk, so a hostile file chooses how many *distinct* types it carries: one per 12-byte chunk. Accumulating the per-type totals with a linear scan over the types seen so far was therefore quadratic in the file length, reachable from `gamut inspect` on an untrusted file — 4.8 MB of empty chunks took 40.9 s. A private `ChunkTally` keeps a `HashMap<[u8; 4], usize>` beside the stats vector, so each chunk costs O(1) and the public `Vec<ChunkStats>` keeps the first-appearance order it documents. The map is dropped at the end of the walk and never surfaced; `ChunkStats` stays `Copy` and `#[non_exhaustive]`. Hashing attacker-chosen keys is safe only because the default hasher is SipHash-1-3 with a per-process seed, so that is recorded on the type: a faster unseeded hasher would reopen the blow-up by a different route. `PngReport::chunk` stays a linear scan — O(distinct types) per call, not quadratic — and now documents that cost, and that summarising every type means iterating `chunks` once rather than calling it per type. The regression test asserts a self-calibrating ratio rather than a wall-clock ceiling, which would be flaky under `llvm-cov` and parallel test binaries: two files of equal byte length and equal chunk count, one distinct type per chunk against one repeated type, deconstructed back to back in one process. Measured 3–5x with the index and 1488x without it (18.0 s against 12.1 ms), so the 20x bound has ~4x of headroom above the fix and ~75x below the defect.
`with_transparent_cleanup` documented "no effect on an image with no fully transparent pixel, or on a layout with no alpha channel", but `cleaned_samples` was only reached from `EncodeImage<Rgba8>` and `EncodeImage<GrayAlpha8>`. `Rgba16` and `GrayAlpha16` carry an alpha channel and can carry fully transparent pixels, so a caller enabling the knob on a 16-bit sprite got the documented behaviour's opposite: silently nothing. `reduce::clean_transparent` cannot serve those layouts — it reads one-byte samples on a one-byte stride, whereas a 16-bit pixel is invisible only when its whole alpha sample is zero, and clearing a colour sample must clear all sixteen bits. Add `clean_transparent16`, its `u16` twin, beside the encoder. Working on the samples rather than on the big-endian bytes `encode_16bit` serialises keeps the ordering identical to the 8-bit paths: cleanup runs first, so `reduce::analyze16` sees the collapsed invisible pixels. `encode_16bit` therefore takes dimensions plus samples instead of the `ImageRef`, so the alpha layouts can hand it a cleaned buffer. The inline tests pin the two things the byte-wise reading would get wrong: an alpha sample of `0x0001` is visible (its high byte is zero), and every cleared colour sample is cleared in both bytes. `tests/transparent_cleanup.rs` adds the end-to-end halves for both layouts against libpng — `decode` rather than `decode_rgba8`, which would scale 16-bit samples down to 8 and hide exactly that low byte — plus the size claim and the byte-identical no-op on an opaque image. Correct the doc to describe what is now true.
Four corrections that this branch's new bench, size contract and golden re-capture made due. `benchmarking.md`'s counter table said "per-pixel or per-sample kernel -> ItemsCount", which reads as a rule `gamut-png`'s stage benches break: they count `BytesCount` over `crc32`, `pack_scanlines`, `filter_image` and `analyze8/16`. They do not break it. Those are byte-oriented stages of a codec pipeline whose natural item *is* a byte, and counting items would put their figures in a different unit from the crate's own encode benchmark and its size table, which are the figures a stage row exists to be read against. The workspace's actual `ItemsCount` users are all kernels whose item is not a byte -- `gamut-dsp` counts transform coefficients, `gamut-tonemap` `f32` samples, `gamut-color` `f64` samples and pixels, `gamut-bitstream` coded symbols, `gamut-cmm` transformed pixels -- and bytes per second would say nothing about any of them. So amend the rule rather than the bench: add the byte-oriented-stage row and sharpen the existing one to name the distinction it was always making. `testing.md`'s per-crate authority row for `gamut-png` named only "differential + conformance", omitting the size contract this branch adds, while `gamut-webp` names its own. Mirror it, and cite `crates/gamut-png/tests/size_contract.rs` from the technique table beside `gamut-webp/tests/effort.rs`. `mise.toml`'s `bench-test` comment says why `--benches` is passed and counts the workspace's benches to make the point; `gamut-png`'s is the sixteenth. (The "all 15 crates" at the top of the file is about `tooling/` and is a separate claim.) `gamut-png/tests/backends.rs`'s header says the goldens were captured before the seam existed, which the per-row note directly below it already contradicts for `rgb8_best_bruteforce`. State the exception in the header instead of leaving the two to disagree; no golden byte moves.
The report walk capped the *filtered* stream at 64 MiB while documenting that cap as matching the decoder's image budget. The decoder budgets the *decoded* buffer instead, and the two differ by exactly one filter byte per scanline: a 4096x4096 RGBA8 image is 67 108 864 native bytes, which decodes on the default budget, and 67 112 960 filtered, which the walk declined — so `deconstruct` reported an undamaged file as damaged and `gamut inspect` exited non-zero on it. Two constants asserted to agree had drifted, so make the agreement structural. `ihdr::native_bytes` is now the single definition of the quantity; `PngDecoder::check_limits` reads it (byte-identical behaviour, pinned by `byte_budget_is_exact`), and `MAX_FILTERED_BYTES` / `within_inflation_budget` give way to `fits_decode_budget(header, max_image_bytes)`. The budget is a parameter, so the inclusive boundary is reachable from a unit test without a 64 MiB fixture. Inflation stays bounded: a file that passes inflates to at most the native bytes plus one per scanline. Kept, against the plan: `idat_ratio`'s `filtered_len == 0` guard. It was to be deleted as unreachable, but it is reachable in thirteen header bytes. §11.2.1 admits 2^31-1 square, which at RGBA16 implies 2^65 filtered bytes; `adam7::expected_stream_len` refuses to wrap and `deconstruct` reports such a file rather than erroring, leaving `filtered_len` zero. `gamut inspect` prints the ratio for every file it reads, so replacing the guard with a `debug_assert!` would have put a panic on a hostile-input path. The branch is pinned by a new accounting test instead, which is what makes it killable rather than equivalent.
`Reduced::GrayKeyed` is reachable and correct, but nothing in the suite produced one, so neither `analyze8`'s `all_gray` split inside the keyed arm nor the encoder's arm for it had a test that could see them. Two tests, at the two scopes the placement rule forces. `Reduced` is private, so the analysis side is pinned inline: grey with binary alpha, 64 opaque levels, and a 65-entry palette that keeps the palette estimate (540 bytes) out of a race the key wins at 270. The encoder side needs libpng, and is pinned in `colour_key.rs` as the greyscale twin of the existing truecolour differential: colour type grey at depth 8, a two-byte `tRNS`, and an exact round trip. The key is grey 7 rather than 0 in both, so the `tRNS` sample's byte order is observable -- written little-endian it would read `[7, 0]`, which a key of 0 could not distinguish from the correct `[0, 7]`. The greyscale win is thinner than truecolour's, since dropping the alpha plane saves one byte per pixel rather than three against the same flat 14-byte chunk. Measured, it wins anyway at every square from 32 to 256: 499 bytes against 626 at 128, about 20%, so the fixture needs no size threshold.
`write_reduced_or_native` races a chunk-carrying reduction against the unreduced encoding, and its `carries_chunks` set decides which reductions enter that race. The palette member had both sides covered; the keyed members had only the winning one. The three existing negative tests here all stay RGBA because no key was ever *offered* -- partial alpha, two invisible colours, a collision with a visible pixel -- not because a valid key lost on size, so dropping `Rgb8Keyed` from the set would have gone unnoticed. Add the losing side at 32x32 on the existing fixture, reconstructing the candidate that lost: the encoder's `Rgb8Keyed` arm is the RGB stream through the same configuration plus one 18-byte `tRNS`, so the test can assert the declined encoding really was the larger one (279 bytes against RGBA's 274) rather than merely that RGBA survived. Parameterise the fixture by side to do it, and correct the module doc while it is in hand: the crossover was measured at 32, not below 128 as the `SIDE` comment claimed -- at 48 the key already wins, 347 against 353.
Two halves of one gap. The off-grid grey case had been weakened from an exact colour-type assertion to `COLOR_GRAY || COLOR_PALETTE`; that fixture produces grey at depth 8, so the palette arm was a branch no input could take. Assert the colour type exactly again and say in the comment where the palette case is covered instead. It is covered here. `a_palette_is_chosen_when_it_actually_wins` needs 64 colours before the race takes the palette at all, and 64 entries is depth 8, so the encoder's `depth < 8` path into `pack::pack_scanlines` and `index_bit_depth`'s `3..=4 => 2` arm were only ever reached by inputs whose palette was then declined. Four colours at 192x192, arranged by a finalizer-quality hash of the pixel index rather than in blocks: blocked, the RGBA stream compresses away and the race keeps it, which is why the 64-colour fixture needed 64 colours. Scattered, both streams sit near their entropy and the 2-bit packing is the whole difference -- 9500 bytes indexed (9216 of payload) against 19 135 as RGBA. A cheaper mix was tried first and rejected: one multiply and a shift is periodic in x, DEFLATE finds the period, and the same fixture came out at 272 bytes.
`PngReport::filters` was `Option<FilterHistogram>`, so "no histogram" conflated a file this reader declined to inflate with one whose compressed data is broken — and `is_intact` treated both as damage. Now that the walk budgets what the decoder budgets, that conflation is the last thing standing between a large sound PNG and an intact verdict. `FilterScan` is `Counted(FilterHistogram)` or `Skipped(SkippedFilterScan)`, the reason being `#[repr(u8)]` plain data with explicit, permanent, append-only discriminants: `OverBudget`, `CorruptStream`, `LengthMismatch`, `UndefinedFilterCode`. `SkippedFilterScan::is_damage` is the single source of truth for the grading question — only `OverBudget` is not damage, since it describes the reader's budget rather than the file — and `is_intact` narrows its conjunct to `!filters.is_damage()` rather than dropping it, because a corrupt zlib payload under a valid CRC is damage nothing else in the report can see. `PngReport::native_bytes` exposes the budgeted quantity, so a caller can tell what an `OverBudget` verdict was measured against. `gamut inspect` prints the reason through a `filter_skip_label` with a wildcard arm, and pushes a damage-bearing skip into the findings list before printing it — the exit message used to read "0 finding(s)" while exiting non-zero on a file whose only defect was its IDAT stream.
The module doc said the command exits non-zero when the file "is not fully accounted for" without saying what that is, and the three formats name it differently: TIFF and DNG gate on `is_fully_accounted()`, PNG on `is_intact()`. They are the same strength, which is worth writing down — PNG's `is_fully_classified()` is printed but is not the gate, being true by construction for every file `deconstruct` accepts, so gating on it would exit 0 on a truncated PNG. Also records that an over-budget filter scan is not a finding, and moves the stray `/// The display name of a format.` off `inspect_png` and back onto `format_name`.
It is dead in the shipped crate — the encoder calls `choose_by` directly, and the wrapper carried `allow(dead_code)` off the `test-support` feature to say so. What it added on top of `choose_by` was a fresh 9 KiB `Scratch` per call, which `Score::SumAbs` never reads: the bench row it existed to serve was therefore measuring a per-scanline allocation the encoder never performs, and its question — what the sum-of-absolute-residuals heuristic costs per row — is already answered by the `filter_image / MinSumAbs` row. It was also a wrapper body in a seam whose own module doc forbids them: `stages` is "re-exports and nothing else", because bench targets are reached by no gate, so a body there drags the coverage floor and generates mutants nothing can kill. Its one test moves to `choose_by(Score::SumAbs, ...)`, the call the encoder actually makes, and keeps its teeth: inverting `choose_by`'s comparison still fails it.
The table's ratios were chosen by hand, so nothing said what a budget meant or when it should move. Each `max_ratio` is now `measured` times a stated headroom, rounded up to two decimals, and `Budget::max_ratio` carries the procedure for refreshing the whole table after an encoder change. The refresh also adds the three rows the bench reported and nothing gated: both `+clean` columns and `tiny_rgb8`. `Budget` grows `fixture`, `side` and `cleanup` so a cleaned row shares its twin's pixels instead of duplicating them. Two rows take less than the default 5%. `sprite_rgba8` measures 0.963, where 5% rounds past 1.00 and would surrender the claim the row exists to make, so it takes 2%. `palette64_rgba8 +clean` takes 2% because there is nothing to protect: cleaning *costs* bytes there, 403 against the uncleaned 364. That last row's justification had it backwards -- it predicted shorter PLTE and tRNS and therefore a smaller file. Both halves of that are true and the file still grows, because collapsing the transparent entries rewrites pixels that were compressing well and at 128x128 the second effect wins. `with_transparent_cleanup` is a canonicalisation, not an optimisation. The row now says so, which is the drift this refresh exists to catch. The gradient and photo rows move on their own: 0.939 to 0.772 and 0.752 to 0.731, from this branch's encoder work. Refs #224
… pairs the spec forbids `PngEncoder::with_metadata` / `with_metadata_from` carry a decoded file's eXIf, iCCP, XMP, text and colour chunks into the encoder that rewrites its pixels, so a re-encode no longer drops every one of them. Two defects the spec settles are fixed on the way: * sRGB beside iCCP. PNG 3rd ed. §5.6 Table 5 states the constraint on both rows, and §11.3.2.5 repeats it: the two should not appear together. Both were written whenever both were set. The encode is now refused with `InvalidInput`, and `with_metadata` resolves the pair by §4.3 Table 1's colour-chunk priority (iCCP 2 outranks sRGB 3) so a file carrying both is still re-encodable. * tEXt/zTXt carried UTF-8. §11.3.3.2 interprets a tEXt text string as Latin-1 and §11.3.3.3 says an inflated zTXt is identical to it, while §11.3.3.1 restricts every keyword to Latin-1. Pushing a Rust `String`'s bytes stored mojibake for every code point above U+007F. Text is now converted once, at the setter, and a non-Latin-1 text is promoted to iTXt as §11.3.3.2 directs; a keyword no chunk can carry refuses the encode. Adds `with_cicp` (§11.3.2.6), without which preservation would silently drop the highest-precedence colour chunk a file carries.
…ncode carries Inline in `ancillary.rs` where the assertion reads a non-pub item (`text_entry`, `validate`, `write_text`), and in `tests/preservation.rs` for the public `with_metadata` pair. Each names the function whose mutation it kills. Also wires `gamut convert` to carry the input's metadata on the PNG path, with `--strip-metadata` as the opt-out, pinned by a binary-driving test because `gamut-cli` is outside the mutation globs and the coverage regex.
…our pair The keyword rule shipped as "code point under 256", which is neither of the clauses PNG states. §11.3.3.1 binds a keyword to code points 0x20-0x7E and 0xA1-0xFF, 1 to 79 bytes, with no leading, trailing or consecutive space and expressly not U+00A0; §11.3.3.1's closing paragraph restricts a tEXt/zTXt text string to that repertoire plus U+000A. Both are now implemented as written, so an empty keyword, a 200-byte one, U+00A0, 0x7F and 0x9F no longer pass, and a control character promotes to iTXt with everything else outside the repertoire rather than being written with no defined meaning. A null was accepted anywhere. It is the field separator, so `Auth\0or` does not merely offend the grammar — the chunk re-parses as a *different* annotation. §11.3.3.2 forbids it in a tEXt keyword and text string and §11.3.3.4 in an iTXt's text and translated keyword; all four are refused, as is a language tag outside BCP 47's subtag characters and an XMP packet that is not UTF-8. The refusal names the annotation's index and its keyword through the owned-context error channel, so a caller can act on it. The sRGB-beside-iCCP refusal goes. §5.6 Table 5 and §11.3.2.5 say only "should not" and "it is recommended", and §15 gives the BCP 14 keywords force "when, and only when, they appear in all capitals"; §4.3 Table 1 presupposes the pair and defines the outcome by ranking the chunks. libpng reads a file carrying both and returns the same pixels, which `tests/oracle.rs` now pins — so the four in-repo fixtures that had to be rewritten around the refusal are restored. BREAKING CHANGE: a text annotation whose keyword or text breaks §11.3.3 now fails the encode with `Error::InvalidInput` instead of being written. Keywords that were accepted before and are not now: empty, longer than 79 bytes, containing a null, a control character or U+00A0, and any with a leading, trailing or consecutive space.
`gamut convert` carried a PNG input's metadata and said nothing about the payloads it could not: a C2PA manifest store, signed over the bytes of the file it was made for, and a cICP whose matrix coefficients PNG does not allow. Silent loss is the defect class this path exists to remove, so both are now warned about on stderr, which the default verbosity shows. Also corrects the claim about the second read's cost: the metadata walk is cheap — it skips IDAT by length and never inflates a pixel — but reading the file from disk again is not, and that is what taking a path rather than the already-loaded bytes costs.
The M1 row sat behind a blank line, so it rendered as a table of its own rather than a row of the phase table. Attach it, and rewrite the section to state the repertoire of each field as its own clause gives it, what a carry drops and names, why both colour chunks are written, and which oracle gaps stop the claim being differential today.
… called Two mutants the diff gate reached and no test killed. `end_carry` could be replaced with nothing: the idempotence test set its own annotation *before* the carries, where the flag's state makes no difference, so it now sets one after a carry too — the case where mistaking a direct setter for part of the carry eats it on the next one. `DroppedMetadata::reason` and its `Display` could return an empty string. The lines they produce are the whole of what a user learns about metadata that did not survive, and the test that reads them drives the `gamut` binary from `gamut-cli`, which the mutation gate cannot see. Pin the words in gamut-png's own suite.
…y advises Four defects in the preservation path, all of them the same mistake in two directions: the writer was stricter than its own reader about clauses the specification does not bind, and looser than the file about the one field that carries a packet's identity. **A compressed XMP packet was rewritten uncompressed.** The packet leaves the read side through its own field rather than as a `TextChunk`, so `parse_itxt` bound §11.3.3.4's compression flag, language tag and translated keyword and then discarded all three for that one keyword; the writer, having no chunk-kind to consult, always emitted flag 0 with both strings empty. Measured on the fixture this commit adds: a 354-byte `iTXt` came back out as 3 734 bytes, a factor of 10.6, with the tag and translated keyword gone. `XmpFraming` now travels beside the packet on both read surfaces, and `with_xmp` — which has no source file to take framing from — takes the one §11.3.3.1 Table 21 recommends. **Setting the packet and then carrying one wrote two chunks.** A PNG carries one XMP packet under one reserved keyword, so `add_xmp` replaces rather than appends, like every other single-value payload. Appending left this crate's own first-wins reader discarding the carried packet: a silent loss inside the feature built to end silent loss. **Five keyword shapes this crate reads perfectly were refused on re-encode.** §15 gives the BCP 14 keywords force "when, and only when, they appear in all capitals", and every statement §11.3.3.1 makes about a keyword's shape is lowercase — the same argument that lets `sRGB` and `iCCP` be carried together. A leading space, a trailing space, consecutive spaces, a C0/C1 control and U+00A0 all round-trip through this crate's reader unchanged, so refusing to write them back failed a conversion over a file whose pixels are fine, and the only escape discarded the file's ICC profile too. They are now written verbatim and reported. A keyword no chunk can hold — outside Latin-1, or outside the 1–79 bytes all three chunks fix — is dropped and reported, as are a language tag outside §11.3.3.4's ASCII shape and an XMP packet that is not UTF-8. **Only a null byte still refuses**, because it is the field separator and the chunk would re-parse as a different annotation. `DroppedMetadata` becomes `MetadataNotice` and `dropped_metadata` becomes `metadata_notices`, because the channel now reports payloads that reached the output as well as payloads that did not; `MetadataNotice::carried` separates them, and `gamut convert` words the two cases differently. **§11.3.3.1 and §11.3.3.2 contradict each other about a `tEXt` text string.** §11.3.3.1's closing paragraph restricts `tEXt`/`zTXt` content to "the printable Latin-1 character set plus U+000A LINE FEED (LF)"; §11.3.3.2, which defines `tEXt`, says one sentence later that "The text string may contain any Latin-1 character". The more specific and more permissive clause is taken, so a conforming annotation is no longer silently promoted to a different chunk type. The keyword rule stays as written, being specific to keywords. BREAKING CHANGE: `DroppedMetadata` is renamed `MetadataNotice` and gains six variants; `PngEncoder::dropped_metadata() -> &[DroppedMetadata]` becomes `metadata_notices() -> Vec<MetadataNotice>`. `PngMetadata` and `DecodedPng` gain an `xmp_framing` field. An encode that carried a keyword outside §11.3.3.1's repertoire, length or spacing rules, a non-ASCII `iTXt` language tag, or an XMP packet that is not UTF-8 no longer fails; read `metadata_notices()` instead. Refs #483. Refs #600.
The metadata-preservation section claimed identity was preserved under a heading about identity, while the XMP packet — the largest payload the path carries — lost its compression flag, language tag and translated keyword. It also listed §11.3.3.1's keyword rules as enforced, when enforcing them refused five keyword shapes this crate's own reader accepts. Records instead: what the XMP packet's framing costs when it is lost (a 354-byte `iTXt` rewritten as 3 734, measured on the fixture); the three-way split between what refuses the encode, what is dropped and reported, and what is written verbatim and reported, with the §15 argument for the line; and the §11.3.3.1/§11.3.3.2 contradiction about a `tEXt` text string, quoting both halves from the vendored text rather than picking one silently. The "not done" list gains the seams #600 would close — the packet's parallel fields and its position among the annotations — and the efficiency table's metadata-hygiene axis no longer says `gamut convert` drops metadata on the PNG path, which this work made untrue. Refs #483. Refs #600.
`parse_itxt` binds §11.3.3.4's compression flag, language tag and translated
keyword and now hands all three to `XmpFraming`. The integration suite pins the
framed case; nothing pinned the unframed one, so a parser that reported every
packet compressed, or that kept an empty tag as `Some("")`, would have rewritten
a chunk conforming to §11.3.3.1 Table 21's recommended framing as something else
with no test failing.
Both directions are asserted here, inline, because `collect` is not public.
`with_metadata` carries `cICP`, `iCCP` and `sRGB` together and justifies it with §4.3 Table 1's Color Chunk Priority — but Table 1 ranks the chunks for a *reader*, and which one to honour depends on whether that reader has a colour-management module. gamut-png's own reader surfaces all of them and ranks none, so the justification is a claim about other readers, not about this crate. Resolving a profile against a rendering intent is `gamut-cmm`'s work (epic #323), and this encoder deliberately does not pre-empt it. Refs #483.
Three chunk citations on the read surface named the wrong clause, checked against `references/png/png-3.html`: cICP is §11.3.2.6 (§11.3.2.5 is sRGB), sRGB is §11.3.2.5 (§11.3.2.4 is sBIT), and eXIf is §11.3.4.5 (§11.3.4.4 is sPLT). A reader following one of these lands on a different chunk's clause, which is worse than no citation at all in a crate whose rule is that the specification is the source of truth. The crate also cites tRNS as §11.3.2.1 in six files, where the vendored text numbers it §11.3.1.1 and gives §11.3.2.1 to cHRM. That is outside this change's surface and is filed separately. Refs #483.
The rule that a tRNS chunk may omit its trailing opaque entries (§11.3.2.1) lived inside reduce.rs's build_indexed, where only the encoder-derived palette could reach it. It is a fact about tRNS, not about that path, and a second palette path is about to need it. Move it to palette.rs as trim_trailing_opaque, beside the OPAQUE constant that names the value the rule is about, and have build_indexed call it. Behaviour is unchanged; the loop is the same loop.
`encode_indexed8` wrote the caller's palette verbatim. Unlike the palette `reduce.rs` builds, which cannot hold either by construction, a caller's may hold entries nothing in the file names and entries naming a colour an earlier entry already names. Both go into an incompressible `PLTE`, and the count of them picks the index bit depth -- a 256-entry palette holding four colours cost 768 `PLTE` bytes and pinned every pixel to 8 bits where 2 would do. `PngPalette::cleaned` drops an entry no pixel and no in-range `bKGD` index marks, merges a later entry holding the same RGB *and* the same alpha as an earlier one, trims the trailing opaque `tRNS` bytes §11.3.2.1 lets a chunk omit, and returns the old-index -> new-index map. `encode_indexed8` derives the depth from what survives, remaps the image's indices, and moves a `bKGD` palette index with the entry it names, so the background still resolves to the colour the caller chose. An entry named only by that background survives with it; an index already out of range stays out of range rather than being renumbered back in. Alpha is part of an entry's identity: two entries sharing an RGB triple but not an alpha are different colours and both survive, and an entry `tRNS` omits compares as opaque rather than as absent. Surviving entries keep the caller's relative order -- reordering is a heuristic question, filed as #612. Nothing is reported, because nothing is observable: every surviving entry keeps its bytes and the map sends each old index to the entry holding the colour it named. libpng resolving a wholly redundant palette to the caller's exact RGBA is the test of that, rather than a round trip through our own decoder, which would resolve the file through the very palette the encoder wrote. Refs #482
Axis 4 said caller-supplied palette cleanup remained; it no longer does. Give it its own section beside the cost model: what is dropped, merged, trimmed and renumbered, that the `bKGD` index moves with its entry, and that the whole pass is silent because it is lossless. The measurement is a 64x64 four-colour picture handed a full 256-entry palette: 1194 bytes before, 162 after, against 164/162 for the tight palette holding the same four colours. Both "before" figures are measured on this branch's base. The two "after" figures are equal because after cleaning the two palettes *are* the same palette, which the encoder suite pins as a byte-for-byte file equality rather than as a size; the tight palette's own 2 bytes are the `tRNS` trim this path did not previously apply. The remainder axis 4 still names is ordering -- modified-Zeng for the derived palette, and any ordering of a caller's -- which is a heuristic chosen by measurement rather than a rule the specification states. Point it at #612, which holds that question, instead of at the umbrella issue. Refs #482
`trim_trailing_opaque` popped while `alphas.last() == Some(&OPAQUE)`. Invert that comparison and the loop never ends: an emptied vector answers `None`, and `None != Some(&OPAQUE)` holds forever, so it pops an empty vector for as long as anything is willing to wait. The diff mutation gate found it — the mutant did not survive, it timed out, which is a different fact and needs the opposite repair. An exclusion would have recorded the hang instead of removing it. Compute the length to keep instead: find the last entry that is not opaque, keep everything up to and including it, truncate. Same result, no loop, and every mutant of the new form changes the length a test already asserts. Refs #482
`gamut-deflate` has carried `with_optimal_parse_limit` since the optimal parse landed, but `PngEncoder` never passed it on, so PNG callers were pinned to the 1 MiB default span however large the image. Filtered scanline data is the one stream in a PNG whose length grows with the picture, so it is exactly the stream a caller can need to re-span. The knob governs the IDAT stream only. A compressed ancillary payload (`iCCP`, `zTXt`) is whatever the caller handed over rather than something the image size decides, so it keeps the default; the doc says so beside `with_effort`, which does govern every stream. Refs #484
`with_compression`, `with_effort`, `with_filter`, `with_optimal_parse_limit` and `with_auto_reduce` are five independent knobs, and nothing mapped one choice onto a sensible combination of them: a caller wanting the smallest file had to know that it means `Level::Best` and `BruteForce` and auto-reduce. `Preset` is that knowledge, named, over four rungs. The dial is additive. `Preset::Balanced` spells its knob values out rather than reading them back from `PngEncoder::new`, so that the two agree is a claim `the_balanced_rung_is_a_default_encoder` tests rather than a tautology the code arranges, and no existing default moves. `with_transparent_cleanup` is deliberately in no rung: it is this crate's one lossy knob, and a dial named for effort must not be what silently changes stored samples. The `Fast` rung fixes the Paeth predictor rather than skipping filtering. `FilterStrategy::None` is the obvious guess and is measurably wrong -- it hands DEFLATE a stream so much larger that the compressor loses more time than the filter pass saves, coming out both the largest result over the corpus (46 267 bytes against 17 530) and slower than fixed Paeth. Two things the ladder cannot promise, both recorded where they were found. Ordering holds over the corpus, not per row: a fixed predictor beats a per-row heuristic on a picture that suits it, and `demotable_rgb16` is smaller at `Fast` than at `Balanced`. And the oracle check covers the 8-bit rows only, because libpng's simplified API treats a 16-bit file as linear and converts it to sRGB, so it is not a depth-neutral resolver. Refs #484
`gamut convert` hardcoded best compression plus auto-reduce and exposed only `--png-effort`, so the whole-image filter search -- the crate's largest remaining size lever -- was unreachable from the command line, and so was any setting faster than the slowest one. `--png-preset` selects a rung. Its default, `small`, is byte-identical to what this command produced before: measured over all nine rows of the efficiency corpus, `Preset::Small` and the previous `with_compression(Best).with_auto_reduce(true)` pair agree exactly. This is a measurement rather than a gate -- `crates/gamut-cli/**` is excluded from both the mutation and coverage gates, so a test here would pin nothing that CI reads. `--png-effort` is applied after the preset and so still overrides it; its help no longer claims the best compression level is always in use, which a preset can now change. Refs #484
Axis 8 said "three independent knobs, no composed dial"; there are now five knobs and a dial over them, so the axis and the corpus tables behind it need to say what the dial actually does. The measurement is the point, and it is not the flattering one. The ladder is steep in time and shallow in size: `Small` costs roughly 190x `Balanced` to save 2.7%, `Smallest` roughly 1150x to save 6.2%. Published with its method -- warmed up, minimum of three interleaved passes, test profile rather than a bench -- and with the ratios a first unwarmed pass gave instead, so a reader can see how much of the third digit to trust. Two negative results recorded rather than smoothed over: the ladder is ordered over the corpus and not per row, with `demotable_rgb16` named as the counterexample, and the `Fast` rung's filter was settled by measuring four candidates rather than by argument -- `FilterStrategy::None` is dominated on both axes at once. The remaining axis-8 work is parallelism, now filed as #624. Refs #484
`Preset`'s own doc claimed "no rung produces a larger file than the rung above it on this crate's corpus", which is the per-row form that `demotable_rgb16` refutes -- the rung docs said one thing and the gate, the test module doc and STATUS said another. State the aggregate form the gate asserts, and say plainly that the per-row form is false and why. Also: `Fast`'s variant doc said "no filter search", which reads as no filtering at all; it fixes the Paeth predictor. Refs #484
The section attributed that step's 6.2x to "very nearly the seven-candidate search" without having isolated it, which is an assertion wearing a measurement's clothes. Measured by adding one knob at a time instead, and the attribution is stronger than the guess: the filter search is 6.26x on its own, while effort 15 and the unbounded parse limit each cost nothing. Refinement stops early at a fixed point, and a 64x64 row filters to far less than the 32 KiB window the span floor already covers -- so the parse limit is unobservable at corpus size by construction, which is worth saying rather than leaving as a suspiciously round 1.01x. This also makes the section the direct case for #624: the filter search is the one factor in the step that parallelism could reclaim. Refs #484
`Preset::Smallest` set the optimal-parse limit to `usize::MAX`, and the span is the only thing bounding the shortest-path parse's working set: `gamut-deflate` allocates three span-length vectors per refinement pass, 12 bytes for every byte of span. So one encode's peak memory grew without bound in the image -- measured 177.9 MiB at the 1 MiB default against 701.8 MiB unbounded for a 4096x4096 RGB photograph -- behind a rung named for size, in a crate whose encoders are asked to be allocation-conscious. Take a finite 8 MiB instead, chosen by measurement rather than by argument. At 8 MiB the span is the whole filtered stream, so byte-identical to no bound, for any image up to about 1670x1670 RGB or 1448x1448 RGBA, which is where every size win measured for this knob lives; past that it keeps -0.100% of the -0.106% no bound reaches on a 2048x2048 gradient and -0.028% of -0.044% on a 4096x4096 photograph, for 219.0 MiB rather than 701.8. Doubling to 16 MiB buys a further 0.005% there and costs another 94 MiB. Nothing the crate's tests measure moves: the corpus filters to less than the 32 KiB span floor, so every rung parses each row as one span whatever the limit says. Correct three claims while there. `with_optimal_parse_limit` named time as the cost, and time is not what scales -- the twelve runs behind the memory figures moved by under 2% and not monotonically. It called a wider span "a small ratio win on homogeneous material", and the sign is a property of the data: measured at 1024x1024, no bound against the default, -0.75% on a greyscale ramp and -0.18% on a gradient, but +0.10% on a photograph. And `Smallest` claimed "every knob at its size-optimal setting" for a knob no measurement in this crate could observe, since the whole corpus is 64x64. Refs #484
`PngPreset::to_codec` hand-mapped four variants onto four rungs, so a fifth rung added to `gamut_png::Preset` upstream would compile here in silence and be unreachable from the command line. Its two siblings in this file do not: `--webp-effort` and `--jxl-effort` both go through their codec's `from_level`. Give the CLI enum the ladder levels as explicit discriminants and resolve them through `Preset::from_level`, which this ladder added and documented for exactly this and then did not use -- `level()` had no consumer outside its own round-trip test. Keep the named value enum rather than the siblings' bare integer, because clap prints a rung's doc comment in --help and a number cannot. `the_png_preset_flag_offers_exactly_the_codec_ladder` compares the two lists, so a rung added upstream fails here instead of disappearing. Falsified by hand: dropping the `Smallest` variant makes it report `[0, 1, 2]` against `[0, 1, 2, 3]`. The `smallest` help text also claimed "every knob at its size-optimal setting"; it now says what the rung measures at, including that it buys nothing on three of the corpus's nine rows. Refs #484
`STATUS.md` names `cargo test -p gamut-png --test effort -- --nocapture` as the reproduction command for the ladder's tables, and that command printed nothing: there was no print anywhere in the file. A documented table nobody can regenerate goes stale in silence. The size-ordering test already encodes every corpus row at every rung to make its assertion, so keep the per-row matrix instead of summing it away and print it. The numbers were right; now they come out of the command that claims them, and refreshing the table is a paste. Assertion unchanged: still the strict aggregate ordering, still one thing failing for one reason. Refs #484
…buys nothing Three corrections to the effort ladder's section, all of them things the tables already implied and the prose did not say. The optimal-parse span is the one rung knob no number on that page observes: every measurement there is 64x64, where the whole corpus fits inside the 32 KiB span floor. So add the sweep that observes it -- the 1024x1024 sign table, where five rows win under a percent and a photograph loses a tenth of one; the 2048x2048 saturation curve, where 8 MiB takes -0.100% of the -0.106% no bound reaches; and the peak resident set at three sizes, where no bound costs 701.8 MiB against 177.9 at the default. That sweep is why `Preset::Smallest` now takes a finite span, and it says plainly that the direction is a property of the data rather than of the knob. The reproduction command claimed both the byte and the millisecond columns. It produces the byte columns as of the commit before this one; nothing here produces the millisecond columns in one step, and this now says so, and why they are not in `benches/encode.rs`. The per-row table shows the top rung tying with the one below it on three of nine rows and saving three bytes on a fourth. The prose said every rung buys something, which is true of the total and not of the rows. Say where 6.26x the time buys nothing. Refs #484
The 1024x1024 sweep isolates the span behind one filter heuristic. At the rung's own settings the filter search also gets a say, so measure there too: one encode of the same photograph at the whole `Preset::Smallest` rung emits 284 991 bytes at the 1 MiB default and 285 022 at 8 MiB, 16 MiB and no bound alike -- the three agree because that image's filtered stream is 3.1 MB and fits inside any of them. The rung takes the wider span anyway, on the strength of the five rows it saves more on. Write the losing row down beside the winning ones rather than leaving the section's conclusion to speak for a picture it does not describe. Refs #484
`mise run mutants-diff` reported one survivor in this branch's own diff: `replace << with >>` in `SMALLEST_OPTIMAL_PARSE_LIMIT`. Under it the constant is 0, the span floor raises that to the 32 KiB LZ77 window, and no test could tell -- this crate's corpus filters to less than 32 KiB, so every rung parses each row as one span whatever the limit says and every byte of every fixture is unchanged. (`cargo mutants --list` names that mutant by its operator and not by the item it is in, which is how grepping the list for the constant's name missed it.) Assert the two things the span owes instead, on the value, since an encode at corpus size cannot observe it: wider than the default, or the rung is not asking for anything, and finite, or one encode's parse state grows without bound in the image at 12 bytes per byte of span. Both ends are the reason the constant is what it is, and the mutation breaks the first. Falsified with the exact surviving expression: `8 >> 20` makes it report "the top rung's parse span is 0, not between the 1048576 default and no bound at all". Refs #484
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #484 asked for two things. This delivers item 2 — a composed effort dial over the knobs
gamut-pngalready has — and declines item 1, parallel filter trials, which needs a work-stealingdependency no admitted issue sanctions; that half is filed as #624 and linked below.
Stacked on #614 (
feat/482-png-caller-palette-cleaning) at commit7b1cc00, which is itselfstacked on #550. #614 has since taken further commits, so this branch is based on an earlier head
of it than its tip: read this pull request's diff against
7b1cc00, not against #614's currenthead, and merge it after #614.
git rev-list --count 7b1cc00..HEADis the number of commits thislane wrote — all of them, none of #614’s — and it is 13 at the head this description was last
edited for.
Four things change:
PngEncoder::with_optimal_parse_limit.gamut-deflatehas carried this knob since theoptimal parse landed, but PNG callers could never reach it and were pinned to the 1 MiB default
span however large the image. It governs the
IDATstream only — see decision 6 — and whatraising it costs is memory, measured and documented at the setter (decisions 8 and 9).
Preset, a four-rung ladder (Fast,Balanced,Small,Smallest) composing all fivesize/time knobs, with
level()/from_level()for a numeric CLI or FFI knob, followinggamut_webp::Effortrung for rung.gamut convert --png-preset. The command hardcoded best compression plus auto-reduce, so thewhole-image filter search — the crate's largest remaining size lever — was unreachable from the
command line. Its default,
small, is byte-identical to what the command produced before, byconstruction:
Preset::Smallsets exactly the knobs the command used to set by hand.STATUS.mdaxis 8, a measured ladder section and a measured parse-span section, plus theREADME.
What the measurement says, including where it is unflattering
The ladder is steep in time and shallow in size. Over the nine-row efficiency corpus at 64x64:
BalancedFastBalancedSmallSmallestSmallcosts roughly 190xBalancedto save 2.7%. That is the tradeLevel::Besthas alwayscarried; the dial does not change it, it makes it selectable and says what it costs. The ratios are
published as approximate on purpose: a first pass measured without warm-up gave 150x and 968x for
the same two rows, so the order of magnitude is the finding and the third digit is not. Method:
test profile rather than
cargo bench, one machine, every arm warmed up, minimum of threeinterleaved passes. Every arm is the same pure-Rust binary and no reference codec is linked into
this measurement, so nothing here depends on which native library the loader resolved. The byte
columns are reproduced by
cargo test -p gamut-png --test effort -- --nocapture, which now printsthe matrix the size-ordering test already computes; the millisecond columns have no one-step
reproduction in this repository and
STATUS.mdsays so and why.Decomposing the
SmalltoSmalleststep by adding one knob at a time settles what it buys timefrom, and it is not spread across the three knobs it changes: the brute-force filter search is
6.26x on its own, while raising the refinement budget from 6 to 15 costs 1.00x and the
parse span 1.01x. Refinement stops early at a fixed point, and the whole corpus filters to less
than the 32 KiB window the span floor already covers, so at 64x64 the parse span is inert by
construction. That makes the same table the direct case for #624: the filter search is the one
factor here that parallelism could reclaim.
The top rung is a bet on the material, not a guarantee about it. Per row,
Smallestisbyte-identical to
Smallongrey_as_rgb8andflat_rgba8, ties with every rung onnoise_rgb8,and saves three bytes on
demotable_rgb16: four of nine rows where 6.26x the time buys three bytesor fewer, three where it buys none. Its 3.6% over
Smallis earned on the five rows that are left.The parse span, measured where it is live, and bounded
Every number above is 64x64, where the parse span does nothing — so the first version of this
branch set the top rung's span to
usize::MAXand called it "size-optimal" on the strength of nomeasurement at all. Both halves of that were wrong, and both are now measured.
Memory is what scales, and unbounded meant unbounded.
gamut-deflate's shortest-path parseallocates three span-length vectors per refinement pass — 12 bytes per byte of span — and the span
is the only thing bounding them. Peak resident set for one encode per process (
/usr/bin/time -v,square RGB photograph,
Level::Best, refinement budget 1):Time did not scale: the same twelve runs moved by under 2% and not monotonically.
And a wider span does not always save bytes. Measured at 1024x1024, no bound against the 1 MiB
default: −0.75% on a greyscale ramp, −0.18% on a gradient, −0.12% on a sprite sheet, −0.08% on a
palette image, −0.07% on an opaque RGBA one, nothing on noise or on a picture already inside one
span — and +0.10% on a photograph, which at the full
Smallestrung with the filter search is+0.011% on the same picture. A wider span is a different cost model, not a better one.
So
Preset::Smallesttakes a finite 8 MiB span. At 8 MiB the span is the whole stream —byte-identical to no bound — for any image up to about 1670x1670 RGB or 1448x1448 RGBA, which is
where every win above lives; past that it keeps −0.100% of the −0.106% no bound reaches on a
2048x2048 gradient and −0.028% of −0.044% on a 4096x4096 photograph, for 219.0 MiB instead of
701.8. Doubling to 16 MiB buys a further 0.005% there and costs another 94 MiB.
The
Fastrung's filter was settled by measuring four candidates rather than by argument, and theobvious guess lost — see decision 4.
Validation
Every command below was run in the lane worktree, verbatim.
lint,testandmutants-diffwereeach re-run at the final head
e6d68bcafter the last documentation commit, so every row describesone tree rather than several.
cargo test -p gamut-png --all-featurescargo test -p gamut-png --doccargo clippy -p gamut-png --all-targets --all-features -- -D warningscargo clippy -p gamut-cli --all-targets --all-features -- -D warnings__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmtthenmise run fmt-checkfmt-tooling-checkloops over everytooling/*/Cargo.tomland one has no[workspace]table)mise run check-testsconvco check 7b1cc00..HEADmaster)mise run lint(whole workspace, memory-capped scope)mise run test(whole workspace, memory-capped scope)GAMUT_MUTANTS_BASE=7b1cc007ee74ca9404dfd8328af67c5ae0de58ab mise run mutants-diffmise run check-release-deps/check-ffi-features/check-ffi-headerwere not run and arenot applicable: no
Cargo.tomlchanged and no public C-surface type changed.mise run coveragewas not run: no new module with thin test reach —
tests/effort.rscovers the new code directly.The mutation base.
origin/feat/482-png-caller-palette-cleaningis pinned to the SHA7b1cc007ee74ca9404dfd8328af67c5ae0de58abrather than passed by name, because that branch hasadvanced since this one forked; the run reports
selection diff base:7b1cc007…. With the defaultbase the selection would fold in every mutant belonging to #614 and #550 underneath, and a count
taken that way is not evidence about this diff. Note that CI's own mutation shards diff against
master, which is that wider selection — a failure there on a file this pull request does nottouch belongs to the stack, not to this change.
Two things these gates cannot see. First, timings cannot fail a build without making it flaky,
so the ladder's time column is reported, not gated (#437 is the standing reason); what is gated
is the aggregate size ordering,
Preset::Balanced's byte-identity with a defaultPngEncoder, andthat no rung changes the pixels a file resolves to. Second, a local mutation run cannot see a
hang on this box — the runner caps address space, so a mutant that loops forever allocating
aborts and scores
caught, where CI has no such cap and reports a TIMEOUT. This run's guards wereulimit -v 8GiBper process underMemoryMax=32G, so its 12/14 is a count taken under a cap andis not evidence that CI's mutation gate is green. What argues against a hang here is the code
rather than the count: this diff adds no hand-written
Iterator::nextand no loop whose progresslives in a callee's return value — the two shapes that produce it — and its largest fixture is a
40 001-byte row, so no mutant can turn a bounded encode into an unbounded one.
The
Preset::Smallcontinuity claim behind--png-preset's default is a measurement, not agate:
Preset::Smalland the previouswith_compression(Best).with_auto_reduce(true)pair werecompared over all nine corpus rows and agreed byte for byte on every one. It is not pinned by a
test because
crates/gamut-cli/**is excluded from both the mutation and the coverage gate, so atest there would pin nothing CI reads, and the claim's subject is the CLI's old configuration
rather than anything
gamut-pngpromises.Findings raised against this diff before committing, and what became of them:
Preset::FastusedFilterStrategy::Noneand was 42x larger on the gradient row. Measuredfour candidates;
Noneturned out dominated on both axes. Fixed before the commit landed(decision 4).
demotable_rgb16. Diagnosed as real and correctbehaviour, not a defect; the contract was corrected to the aggregate one it can actually keep,
and the counterexample is recorded (decision 5).
decode_rgba8drives libpng's simplified API, which treats a 16-bit file as linear and converts to sRGB — a
stored 40 returns as 110 from a 16-bit file and as 40 from an 8-bit one (decision 7).
committing, per the crate's rule that a test whose mutant cannot be named is at the wrong scope.
Preset's own type doc still stated the per-row ordering claim thatdemotable_rgb16refutes — the rung docs said one thing while the gate, the test module doc and
STATUS.mdsaidanother. Found re-reading the diff after the first push; fixed in
7461c26b, along withFast's variant doc reading as "no filtering at all".STATUS.mdattributed theSmalltoSmalleststep to "very nearly the seven-candidatesearch" without having isolated it — an assertion wearing a measurement's clothes. Measured
instead (
e6d68bc); the attribution turned out stronger than the guess.with_optimal_parse_limit's wiring was falsified by hand (removing the call makesthe_optimal_parse_limit_reaches_the_idat_streamfail), and so was a collapsedSmallestrung(makes
every_rung_is_strictly_smaller_over_the_corpus_than_the_one_above_itfail).Second round, at head
f39fed1The table above is the first round's record and describes the tree at
e6d68bc. It is leftas written: it was true of that tree, and a correction that rewrites what it corrects cannot be
checked. Every row below was re-run at
f39fed1, after the last commit of this round, so itdescribes one tree rather than several.
cargo test -p gamut-png --all-featurescargo test -p gamut-png --doccargo test -p gamut-clithe_png_preset_flag_offers_exactly_the_codec_ladder__CARGO_TEST_ROOT=$(git rev-parse --show-toplevel) mise run fmtthenmise run fmt-checkfmt-tooling-checkloops over everytooling/*/Cargo.tomland one has no[workspace]table)mise run check-testsconvco check 7b1cc007ee74ca9404dfd8328af67c5ae0de58ab..HEADmastermise run lint(whole workspace, memory-capped scope)mise run test(whole workspace, memory-capped scope)ok, 0 failedGAMUT_MUTANTS_BASE=7b1cc007ee74ca9404dfd8328af67c5ae0de58ab mise run mutants-diffCorrecting one row of the first round's table. It reads
"
convco check 7b1cc00..HEAD| pass — no errors in 5 commits", and the Summary said "Only thelast five commits here are this lane's work". Both undercounted: there were seven then and there
are thirteen now, and every one of them is this lane's —
git rev-list --count 7b1cc007ee74ca9404dfd8328af67c5ae0de58ab..HEADis the derivation, and the Summary now names thecommand instead of a number.
mise run check-release-deps/check-ffi-features/check-ffi-headerwere not run and arenot applicable: no
Cargo.tomlchanged and no public C-surface type changed.mise run coveragewas not run: no new module with thin test reach.
The cap this round's mutation count was taken under. The runner's own guards were
ulimit -v 8GiBper process underMemoryMax=32G, and the run reported them; the 13/15 is a count taken under that cap. A local mutation run still cannotsee a hang, because the runner caps address space and a mutant that loops forever allocating aborts
and is scored
caughtwhere CI would report a TIMEOUT. The argument against a hang is unchangedand is about the code rather than the count: this round adds no hand-written
Iterator::nextandno loop whose progress lives in a callee's return value, and it makes the top rung's parse span
smaller, so no mutant here can turn a bounded encode into an unbounded one. The mutation base is
still the SHA and not the branch name, for the reason the first round's note gives.
Measurements published this round, and how they were taken. Every byte figure is exact and
deterministic. Every memory figure is
Maximum resident set sizefrom/usr/bin/time -varound aprocess that performs exactly one encode and exits, built at
opt-level = 3. Every arm is the samepure-Rust binary; no reference codec is linked into any of it, so none of it depends on which
native library the loader resolved. The wall-clock figures beside the memory table are from the
same twelve runs and are quoted only to show that time did not scale with the span; they were
taken while other work shared the machine, which is why no timing claim rests on them beyond that.
Findings raised against this round's diff before committing, and what became of them:
16 MiB and no bound, at three image sizes, before 8 MiB was written down. The 16 MiB arm is why
the constant's doc can say what doubling would buy (0.005%) and cost (94 MiB) rather than
asserting that 8 is enough.
8 << 20DID leave a mutation survivor, and the gate found it.>>gives 0, the span floorraises that to 32 KiB, and no test in this crate could tell: the corpus filters to less than
32 KiB, so every rung parses every row as one span whatever the limit says and not one byte
moves. This lane had checked for exactly this and concluded wrongly —
cargo mutants -p gamut-png --listwas grepped for the constant's name, and the tool names that mutant by itsoperator (
replace << with >>) and not by the item it sits in, so the grep came back empty andthe risk was written off. Repaired at the code rather than excluded:
the_smallest_rungs_parse_span_sits_between_the_default_and_no_bound_at_allasserts the twothings the span owes — wider than the default, or the rung asks for nothing, and finite, or one
encode's parse state grows without bound — on the value, because an encode at corpus size
cannot observe it. Falsified with the exact surviving expression.
Smallestvariant makes it report
[0, 1, 2]against the codec ladder[0, 1, 2, 3]. Changing adiscriminant instead does not reach the test at all —
rustcrejects the duplicate first, whichis a stronger guard than the test and worth knowing.
−0.85% on the full rung for raising the span; the largest win this round measured anywhere is
−0.75%, on
grey_as_rgb8at 1024x1024, and the fullPreset::Smallestrung onphoto_rgb8at 1024x1024 went the other way, +0.011%. The disagreement is almost certainlythe fixture and the size, which the review did not name and this round therefore could not
match; it does not touch the finding, which was that the sign is data-dependent, and this round
reproduces that independently in both directions. Recorded rather than reconciled.
as the rung's span, which is a different value — legitimate only because at 64x64 both exceed
the filtered stream and are the same parse. Said in the text rather than left for a reader to
work out.
Risks and rollout
Additive throughout; every existing knob works unchanged and no existing default moves.
Preset::Balancedis byte-identical to a defaultPngEncoderon every corpus row, gated bythe_balanced_rung_is_a_default_encoder, andgamut convert's default output is unchanged — theCLI's
smalldefault sets exactly the knobs the command set by hand before, so that identity isby construction and not only by measurement.
Three risks worth naming, and the first one is smaller than it was.
Preset::Smallestsets theoptimal-parse span to a finite 8 MiB, so its peak working set is bounded by roughly 100 MiB of
parse state whatever the image — measured 219.0 MiB total for a 4096x4096 RGB photograph against
177.9 at the 1 MiB default. That is still eight times the default's parse state, and a caller
encoding many images concurrently should count it; a caller who wants the default back sets
with_optimal_parse_limit(DeflateEncoder::DEFAULT_OPTIMAL_PARSE_LIMIT)after the rung, and acaller who has measured that their material wants more can still pass
usize::MAX, which thesetter now documents as unbounded in the image.
Presetis#[non_exhaustive], so a downstreamexhaustive
matchneeds a wildcard arm — new type, so nothing existing breaks. And the rungs'knob values are explicitly not the contract and may be re-tuned, which means
gamut convert'sdefault output may move with
Preset::Smallin a future revision; the rungs themselves, theordering and
Balanced's identity are what is promised.One risk this pull request removed rather than added: the CLI's
--png-presetno longer decidesfor itself which codec rung each of its values means, so a rung added to
gamut_png::Presetupstream fails
the_png_preset_flag_offers_exactly_the_codec_ladderinstead of silently beingunreachable from the command line.
Rollback is per commit: the dial, its CLI flag, the passthrough, the span's bound and the CLI's
routing are five independent commits.
Issue
Refs #484— notCloses, because item 1 of the issue is declined rather than delivered.Item 1 (parallel filter trials) is filed as #624, "gamut-png: parallelise the BruteForce filter
trials", with the measured cost that motivates it and the dependency decision it needs.
Two more findings from the second round are filed rather than taken, each on its own subject:
gamut convert's--png-effortnames one knob of five while every sibling--*-effortnames that codec's whole ladder. Repairing it renames a user-visible flag, which isbeyond what gamut-png: parallel filter trials, and a composed effort dial #484 asked for (decision 12).
gamut-deflate's ownwith_optimal_parse_limitdocuments its cost as time, where thecost that scales is the memory this pull request measured. The allocation is in
crates/gamut-deflate/**, outside this lane's manifest (decision 13).No existing issue was edited, closed, labelled or commented on.
Decisions taken
No human approved this plan. This is an unattended run under the following instruction, quoted from
the user:
and, after the plan was written: "Plan is approved. Continue automating solely based on agents."
The record below is what a human reads afterwards.
Appended during delivery, in the same shape.
Appended in the second round, in the same shape. Decision 8 corrects a claim published in the
first round; the sentence it corrects is quoted rather than rewritten, per this run's rule that a
correction may not rewrite its own subject.
Unresolved review notes
Three, and none of them is a defect in this change.
−8.17% on one fixture and −0.85% on the full rung; the largest win measured anywhere this round
is −0.75%, and the full
Preset::Smallestrung on a 1024x1024 photograph went the other way at+0.011%. The fixture and size behind those two figures were not named, so this round could not
match them. It does not touch the finding — the sign is data-dependent — which this round
reproduces independently in both directions and now publishes in full.
--png-effortand--png-presetstill name different kinds of thing, where every sibling--*-effortnames its codec's whole ladder. Filed as gamut-cli: --png-effort names one knob where every sibling --*-effort names the whole ladder #630; repairing it renames auser-visible flag, which is beyond what gamut-png: parallel filter trials, and a composed effort dial #484 asked for (decision 12).
gamut-deflate's ownwith_optimal_parse_limitstill documents its cost as time, where theallocation lives. Filed as gamut-deflate: with_optimal_parse_limit's real cost is memory, unbounded in the input, and the doc names only time #631 with this round's measurements;
crates/gamut-deflate/**is outside this lane's manifest (decision 13).