Repository navigation
fix: round color quants, add measurement example and update deps - #37
Merged
Merged
Conversation
Every quantizer in the conversion shader cast with `uint(clamp(...))`, which truncates. That biased every sample about half a code value low, in all four quantizers and in both their limited-range and full-range branches, so it applied to 8-bit and 10-bit output alike. The bias does not stay small on the way back out. The inverse BT.2020 matrix scales U by 1.8814 for blue, so one LSB of chroma truncation reaches the decoded image as nearly two code values. Measured on an Arc A310 (ANV), BT.709 luma over 16384 samples of a synthetic non-grey frame, against the same math computed on the CPU: before: mean signed error -0.4997, mean |error| 0.4997 after: mean signed error -0.0006, mean |error| 0.2501 which is a biased error becoming an unbiased one, and the average magnitude halving to what rounding's noise floor should be. Both ranges behave the same. `round()` rather than `+ 0.5` inside the clamp: with the addition inside, the clamp's upper bound has to stay at 255.0 (or 1023.0) for the top value to still clamp down, which is one more invariant for a reader to verify. Every input here is non-negative, so round-half-away-from-zero has no surprises. `color_convert.spv` is checked in, so it is regenerated here in the same commit via `shader/compile.sh`; source and binary would otherwise disagree and nothing would change at runtime.
The rounding fix in the previous commit was found by accident, while calibrating an unrelated HDR check, and nothing in the tree would have caught it going the other way. Truncation is not a visible failure, it is a uniform half-code darkening, so it needs measuring rather than looking at. `convert_quantization` runs a synthetic frame through `ColorConverter`, reads the luma plane back off the GPU, and scores every sample against the same BT.709 math on the CPU under two hypotheses: that the shader rounds, and that it truncates. It covers NV12 and P010, limited and full range, which is all four quantizer branches that have a per-pixel plane to check. Luma only, deliberately: NV12 and P010 chroma is a 2x2 average, which would mix averaging error into the result. The frame is deliberately not grey, since grey converts to exact code values under BT.709 and cannot distinguish the two hypotheses at all. It exits non-zero when a case truncates, so it works as a manual regression check and not only as a readout. Against the shader as it was before the fix it reports all four cases as TRUNCATES with a -0.50 bias; against the fixed one, all four round with a bias under 0.005. 10-bit needs a Main10 encoder for the conversion target, which not every driver exposes, so those two cases report as skipped rather than failing.
ash moves to rev f4c2ca3 (0.38.0+1.4.352, from +1.4.329). No source changes were needed. Everything else in the lockfile moves to its latest compatible version; `cargo update` has nothing left to do afterwards. The new `defmt`, `defmt-macros`, `defmt-parser` and `jiff-core` entries are target-gated transitive deps of `jiff`, reached through tracing-subscriber's dev-dependency. Nothing builds them on Linux. CI's full set passes on stable 1.98.1 with the bump in place.
This was referenced Sep 10, 2026
Owner
|
Thanks! |
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
2 participants
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.
Description
Per issue #36 made the color quants rounded so they don't get truncated with colors shifted ever so slightly.
Also bump the
ashdependency to recent upstream commit and update rest of packages (and lockfile) withcargo update.The measurement example is added to help detect color inaccuracies and regressions in future 🙂