Skip to content

fix: dithering math correctness + core performance#45

Merged
g4bri3lDev merged 2 commits into
OpenDisplay:mainfrom
davelee98:fix/dithering-review-findings
Jul 5, 2026
Merged

fix: dithering math correctness + core performance#45
g4bri3lDev merged 2 commits into
OpenDisplay:mainfrom
davelee98:fix/dithering-review-findings

Conversation

@davelee98

Copy link
Copy Markdown
Contributor

Addresses confirmed math errors and hot-path optimizations from a dithering code review. Output-changing correctness fixes plus behavior-preserving performance work; regression fixtures regenerated once for the intentional output changes.

Correctness

  • Bayer DC bias: the ordered-dither matrix used v/16 − 0.5 (mean −1/32), darkening every ordered-dithered image by ~8/255. Switched to the standard zero-mean ((v+0.5)/16 − 0.5). Regenerated ordered fixtures came out ~3% brighter (white +3%, black −2.5%), exactly the predicted correction.
  • Ordered-dither threshold scaling: a full ±0.5 threshold is 14× the GRAYSCALE_16 quantization step, swamping detail with noise. Now scaled to 1/(levels−1) for grayscale palettes; mono/color palettes keep ±0.5 (tuned, tested behavior).
  • Auto tone-mapping: clamp the percentile remap to [0,1] so outliers saturate at display black/white instead of extrapolating (dark outliers were crushed to pure black); honor strength and preserve chroma for near-black pixels; clamp skewness before powf (a negative base produced NaN that poisoned every output pixel on high-key images — surfaced by a new test).
  • Shadows/highlights: pivot the S-curve at perceptual mid-gray (gamma space) instead of linear 0.5 (≈ sRGB 188, so "shadows" covered ~74% of the perceptual range); clamp inputs to [0,1].

Performance

  • Combined OKLab matrix: folded sRGB→XYZ→LMS into Ottosson's single matrix (+ matching inverse), halving per-pixel matrix work and removing composition drift.
  • Ordered-dither gamma LUT: removes 3 powf/pixel → ~13% faster, byte-identical output.
  • Gamut edge hoist: O(n²) edge geometry moved out of the per-pixel loop.
  • Dropped a planned HashMap memoization — benchmarks showed it a net pessimization (near-zero hit rate on gradients / error-accumulated buffers made error diffusion 16-21% slower).

Tests

New: Bayer zero-mean, midpoint duty-cycle, GS16 ramp fidelity, auto-tone outlier/chroma, OKLab round-trip identity (caught a transcribed inverse-matrix coefficient during development). All 51 core unit + 3 regression tests pass; regression fixtures byte-identical after the perf commit, confirming the optimizations preserve behavior.

🤖 Generated with Claude Code

davelee98 and others added 2 commits July 4, 2026 20:21
…tone-map edge cases

Phase 1 correctness fixes from the code review:

- Bayer matrix: use the standard zero-mean normalization ((v+0.5)/16 − 0.5).
  The old (v/16 − 0.5) matrix had mean −1/32, darkening every ordered-dithered
  image by ~8/255. Regenerated ordered fixtures are ~3% brighter as expected.
- Ordered dither: scale the threshold amplitude to the palette's quantization
  step for grayscale palettes (1/(levels−1)). A full ±0.5 threshold was 14× the
  GS16 step, swamping fine detail with noise; color/mono palettes keep ±0.5.
- auto_compress_dynamic_range: clamp the percentile remap to [0,1] so dark/bright
  outliers saturate at the display black/white points instead of extrapolating
  past them (bottom 2% previously crushed to pure black).
- auto_compress dark-pixel branch: honor strength and preserve chroma via a shared
  scale_dark_pixel helper, instead of flattening near-black pixels to neutral gray.
- auto_compress strength: clamp skewness to [0,1] before powf — high-key images
  could produce a negative base and NaN-poison every output pixel.
- apply_shadows_highlights: pivot the S-curve at perceptual mid-gray (gamma space)
  rather than linear 0.5 (≈ sRGB 188), and clamp inputs to [0,1].
- OKLab: fold sRGB→XYZ→LMS into Ottosson's single combined matrix (and matching
  inverse), halving per-pixel matrix work and removing composition drift.

New tests: Bayer zero-mean, midpoint duty cycle, GS16 ramp fidelity, auto-tone
outlier/chroma, and an OKLab round-trip identity guard. Regression fixtures
regenerated (ordered + burkes + FS-mono all shifted by the above).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… edges

Phase 2 optimizations from the code review, kept only where benchmarks showed a win:

- Ordered dither: precompute a 16×256 per-threshold gamma LUT so the three
  srgb_fraction_to_linear (powf) calls per pixel become table lookups. ~13% faster
  on 400×300; byte-identical output (same inputs to the same function).
- gamut_compress: hoist the O(n²) palette-edge geometry (OKLab + linear endpoints)
  out of the per-pixel loop instead of rebuilding it for every pixel. Neutral on the
  6-color palette, cleaner, and scales with palette size.

Dropped (measured net pessimization): the HashMap memoization of nearest-palette
lookups for both error diffusion and ordered dither. On the benchmark workloads the
cache hit rate is near zero — gradients have ~unique triples and error diffusion's
accumulated fractional error defeats exact u8-triple reuse — so the hashing overhead
made error diffusion 16-21% slower. The real hot-path win for error diffusion is the
combined OKLab matrix landed in Phase 1.

Regression fixtures remain byte-identical, confirming these changes are behavior-preserving.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@g4bri3lDev g4bri3lDev merged commit f7a3fc8 into OpenDisplay:main Jul 5, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants