Make quantization independent of the architecture (fixed histogram order, consistent NEON distance) - #131
Open
dustinkirkland wants to merge 1 commit into
Conversation
The same image quantized to a different palette on x86_64 and aarch64 (ImageOptim#130). Two causes: - The histogram was drained from a HashMap in iteration order, and the cluster bucketing, the f64 weight sum, median cut tie-breaking and k-means chunking are all order-sensitive. The hasher is deterministic, but hashbrown probes 16-byte groups with SSE2 on x86_64 and 8-byte groups with NEON on aarch64, so the map iterates in a different order per architecture. Sort the entries by colour before the cluster pass, and re-insert the posterized entries in key order (last-wins on collapsing keys otherwise depended on the same order). - The NEON f_pixel::diff summed the channel terms as r + (g + b) while the x86_64, scalar and WASM paths sum (r + g) + b; float addition is not associative, and the refinement loop amplifies the difference in the metric. Add in the same order. With both, x86_64 and aarch64 produce identical pixels and palettes for every image tested at --quality 85-95 and 0-100, including one that goes through the posterize rehash. Output on one architecture changes for images where either effect mattered: same quality target and algorithm, a fixed tie order and a consistent metric. Fixes ImageOptim#130.
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
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.
Fixes #130.
The same image quantizes to a different palette on x86_64 and aarch64 (details and measurements in #130). Two causes, three small changes:
Histogram::finalize_builderdrained the colourHashMapin iteration order, and everything after that is order-sensitive (cluster bucketing by position, the f64 weight sum, median cut's tie-breaking, k-means' 256-item chunks). The hasher is deterministic, but hashbrown's probe-group width is 16 bytes with SSE2 on x86_64 and 8 bytes with NEON on aarch64, so colliding keys land in different buckets and the map iterates in a different order per architecture. Sorttempby colour before the cluster pass. One sort of at mostmax_histogram_entriesitems.init_posterize_bitsre-inserts the old map with coarser keys; when several keys collapse, the last one inserted wins, which again depended on iteration order. Collect, sort by key, then extend. Last-wins semantics are kept (summing the boosts of collapsed entries would arguably be better, but that is a behaviour change and left for you to decide).f_pixel::diffadds in the same order as the other paths. The x86_64, scalar and WASM paths compute(r + g) + b; the NEON path usedvpaddq_f32and computedr + (g + b). Float addition is not associative, and the refinement loop amplifies the ulp differences in the metric into a different palette for some images (visible with--quality 0-100). Use the same association; it is also one instruction fewer.Verification (pngquant 3.0.3 rebuilt with these changes, x86_64 native and aarch64 under qemu-user, comparing decoded pixels and palette sizes): before, 4 of 5 noto-emoji sample bitmaps differed between the architectures at
--quality 85-95and one more at--quality 0-100; after, all 11 cases (5 images × 2 quality settings, plus a 262,144-colour image at--speed 10that goes through the posterize rehash) are identical on both. Each change was isolated: sorting the histogram alone fixed the 85-95 cases (and made the x86_64 output of the UN flag byte-for-pixel the previous aarch64 output); the NEON association alone explained the remaining 0-100 case (an x86_64 build with NEON's order reproduced the aarch64 output exactly).cargo testpasses on both.Output on a given architecture changes for images where either effect mattered: same quality target and algorithm, a fixed tie order and a consistent metric.