Conversation
Wires up the per-row adaptive filtering heuristics zopflipng uses (lodepng's LFS_MINSUM/LFS_ENTROPY), plus a whole-image "Adaptive" search that trial-compresses every candidate strategy (None..Paeth, MinSum, Entropy) with a fast in-process compressor and keeps whichever produces the smallest output - mirroring zopflipng's own AutoChooseFilterStrategy, without needing to link its C++/lodepng implementation into this fully managed codec. Coder gained a `filterStrategy` constructor parameter (PngFilterStrategy), defaulting to None - the fixed filter-0 behavior Coder has always used - so existing callers and the Verify snapshot baselines are unaffected. New PngFilter.cs holds the forward filter math and the two adaptive selection heuristics; it now also owns the single PaethPredictor shared with Decoder.ApplyPngFilters, removing a second hand-copied implementation that had to stay in sync by hand for round-tripping to work at all. Measured against this project's own test images (all quantized to a 256-color-or-fewer palette, the only kind of PNG Coder ever writes), Adaptive's search consistently converges back on None - MinSum/Entropy made every one of them 8-20% *larger*. This matches the PNG spec's own "do not filter" guidance for indexed-color images (also why lodepng defaults filter_palette_zero=true, which zopflipng deliberately disables just to confirm it doesn't help): index bytes don't carry the pixel-to-pixel correlation filtering exploits, so there's no realistic image where this wins for this codec - Adaptive is a safe no-op here by construction (it always includes None as a candidate), not a source of smaller output. Left in as infrastructure and to close out the investigation, not as a default worth switching to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nopu9gA5CuVcGZFJH2Nouh
Replaces the five-line TODO stub with an actual description, a quick-start example (decode -> quantize -> encode with Zopfli + adaptive filtering), build-from-source instructions for the native libimagequant/ zopfli pieces, and an honest status note on NuGet publishing being wired up but not yet turned on. The usage example is verbatim what's compiled and run against panda.png in this session (56907 -> 12393 bytes), not hand-typed against half-remembered signatures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nopu9gA5CuVcGZFJH2Nouh
Color's [FieldOffset] layout put Red at offset 2 and Blue at offset 0,
matching the conventional 0xAARRGGBB int-packing for .Argb - but
libimagequant's native liq_color is { r, g, b, a } (R@0, G@1, B@2, A@3),
and Color gets blitted directly across the P/Invoke boundary with no
field-by-field marshaling in two places: Quantizer's row callback
(writing pixels in) and QuantizationResult's palette (reading colors
out). The mismatch swapped Red and Blue at each crossing.
Quantizer.Quantize(IProvideImages, ...) crosses that boundary twice, so
the two swaps canceled out and every test/benchmark in this repo (which
all go through that path) never saw a problem. Quantize(ReadOnlySpan<byte>,
...) - the "simple contiguous RGBA bytes" overload, and the more obvious
entry point for anyone not using IProvideImages - only crosses it once
and came back with a genuinely wrong palette: pure red in, pure blue out.
Confirmed both behaviors empirically before touching anything.
Fixed by moving Color's field offsets to match liq_color instead of
adding marshaling shims at the two crossing points - simpler, and closes
off the whole class of bug for any future direct-blit interop, not just
today's two call sites. This changes .Argb's packing convention (now
R at the low byte instead of B); nothing else in the repo depended on
that convention, and this project isn't published yet, so the breakage
is free.
Re-approved the two VisualTests snapshots that regressed on SSIM
(0.993/0.997, threshold 0.999): libimagequant's internal color-distance
weighting isn't symmetric between channels, so quantizing on the
previously-mislabeled channels was already producing subtly different
(off, if only slightly) palette choices for the IProvideImages path too -
visually verified both new baselines by hand before accepting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nopu9gA5CuVcGZFJH2Nouh
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.
No description provided.