mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-13 09:20:07 -06:00
Jeremy Hooke
bb414d220a
Fixes #3460 Related: #2936 (slow GPU kernel compiles on Windows/DX12). This PR builds the GPU kernels without wgpu's workgroup-memory zeroing, which took a DX12 release start from ~87 s to 1.4 s on a GTX 1070 with the new median kernels; it may shorten the compiles there too, but does not close it. ## Problem Raising Shadows on a photo with dark lettering on a bright panel makes bright blocks grow beside the letters. On a Canon CR3 of a bus (6960×4640) with a black fleet number on a yellow panel, Shadows +75 puts bright yellow rectangles in the gaps where the digits pinch together and inside the 4's counter. At Shadows +5 there is none of this; at +75 it shows at any zoom. Narrow gaps between dark objects generally can do it: the pinned corpus has the same thing in the gap between two posts under a machine (Hasselblad, `coverage-6568.fff`). ## Cause From process V2 the Highlights/Shadows base (`local::tone_base`) is the edge-aware filter of a separable median of log luminance, at a radius of 0.15 % of the long edge (10 px here), so that thin detail (twigs, lines, #632) takes its surroundings' shift. Text strokes and gaps are about that wide, so each row and column window is close to half letter and half panel, and the median picks one tone or the other in rectangles: it fills the gaps between the digits and the 4's counter with the letters' tone. The guided filter keeps those blocks, so panel pixels whose base reads as deep shadow get Shadows' full lift (`2^(1.7·s·w^1.5)`, up to ~1.3 EV at +75). ## Examples Top: before, bottom: after. The fleet number on the bus photo, Whites +54, Blacks +31, Shadows +75, at 300 %: <img width="1020" height="976" alt="example_photo_pair" src="https://github.com/user-attachments/assets/cc5b164f-612d-4d7d-bda3-db6afd8b177f" /> The gap between two posts in the corpus' Hasselblad `coverage-6568.fff` (CC0, raw.pixls.us), Shadows +100, at 300 %: <!-- image: example_corpus_pair.png --> <img width="900" height="1216" alt="example_corpus_pair" src="https://github.com/user-attachments/assets/feb4b240-194e-40b1-b5b9-c4419f840df6" /> Synthetic lettering at the scale of a bus number in a 7000 px photo, Shadows +75, at 200 %: <!-- image: example_pair.png --> <img width="880" height="820" alt="example_pair" src="https://github.com/user-attachments/assets/bd02e600-dda0-4648-85e9-f89ce046eb08" /> ## Change `local::detail_free` (CPU) and its GPU twin: each value of log luminance is clamped between its separable median at r (as before) and the median of a 3× wider neighbourhood (the median at r of a box-downscaled copy, enlarged bilinearly, like `guided_fast`'s coefficients). The base is the guided filter of that plane. - A twig or speck in an even surround has both medians on the surround's side and takes the nearer, as under the median alone (#632 unchanged). - The gap between two strokes sits in a wider neighbourhood that is mostly panel: the medians disagree and the gap keeps its own tone. The strokes themselves count as fine dark detail on the panel (both medians agree) and keep their contrast, as twigs do. - Steps between regions stay exact (the fine median keeps them). - GPU: the coarse median reuses the box/bilinear resize, plus one `clamp_between_k` map kernel. CPU and GPU match (max 1 level). - GPU median kernels rewritten (`median_h` / `median_v`): each workgroup loads its taps into workgroup memory and selects by rank, 4 neighbouring outputs per thread. The old partial selection sort over a private 49-tap array sat in scratch memory. Same selection, so bit for bit the CPU's (`median_matches_cpu_bit_for_bit`, every r 1–24); renders are byte-identical to the old kernels. On 6960×4640 at r = 10 (M2 Max) the median takes 191 → 47 ms. - GPU kernels are built without wgpu's zeroing of workgroup memory (as `nn.rs` and `faces.rs` already are): on DX12 it made FXC take ~87 s over the new median kernels at every start of a release build (GTX 1070, 1.4 s without it). Every element of `med_tile` the kernels read is loaded before the barrier; renders are byte-identical. - Cost on the 6960×4640 bus CR3 with Whites +54, Blacks +31, Shadows +75 (M2 Max): GPU planes 231 ms on `main` → 113 ms here (257 ms with this PR's first kernels); CPU base ~388 → ~453 ms. The CPU path enlarges the coarse median while it clamps, into the fine median in place (`detail_free_matches_its_reference`, bit for bit). - New process V8 (`Process::LATEST`). Existing edits keep their process and render as before, so `RENDER_CACHE_VERSION` is unchanged (it is 76 on `main`). - Docs: `process-versions.md` (V8 row, a paragraph on the lettering fix, revision history), `parity-checklist.md` (LR-EDIT-LIGHT-HIGHLIGHTS / -SHADOWS notes). An earlier revision of this PR clamped each value to the quartiles of its window instead. It fixed the lettering but stopped treating isolated lines between r/2 and r wide as detail (twigs of 3–6 px against a dark wall were lifted less than the wall, as under V1), which the corpus run below caught. A fixed window can't tell an isolated line from a gap between strokes; a wider neighbourhood can. ## Process version The new base applies from a new process, **V8** (V7 plus `local::detail_free`; `local::base_keeps_gaps` is true from V8, CPU and GPU). v0.19.0 shipped V3 to V6, and crystal grain (#3630) took V7 on `main`, so V1 to V7 render exactly as before: `tests_legacy` still locks V1 and V2, and `only_process_v8_on_takes_the_detail_free_base` checks that every earlier process keeps the plain median. New photos get V8; photos already edited keep the blocks until Update to Current Process. The faster median kernels and the workgroup-memory change don't change pixels, so they apply to every process. They stay in this PR: the zeroing only costs time on the new median kernels (on `main` the kernels compile in 0.8 s on the GTX 1070), so on its own it would speed up nothing. ## How it was checked - [x] New test `gaps_between_letters_keep_their_own_tone_in_the_base`: a row of synthetic "8"s sized like the bus number. With the median, 360 panel pixels sit more than 1 EV above their base (worst 2.25 EV); now none do (worst 0.8 EV). It fails with the median alone. - [x] New test `isolated_lines_up_to_the_detail_radius_stay_out_of_the_base`: light lines 1–9 px wide on a dark ground (r = 10) stay out of the base, as under the median. It fails with the quartile clamp (a 6 px line 0.31 of the way into the base). - [x] New test `detail_free_keeps_steps_and_regions`: steps and flat fields pass through unchanged; tiny, empty and NaN planes don't panic. - [x] The #632 tests (twig/stalk contrast, local contrast, no halo at a strong edge, V1 unchanged) and the median reference tests pass unchanged. - [x] `lightcraft-gpu` integration tests on a real adapter, including `highlights_shadows_base_matches_on_every_process` (max difference 1 level). - [x] Corpus A/B: all 451 pinned raws (`cargo xtask corpus --download`) rendered at full size before and after, with Shadows +100 and with Highlights −100 (902 pairs, GPU). No failures. RMSE median 0.18 % (Shadows) / 0.10 % (Highlights), max 0.86 %; pixels changed by more than 3 % at most 0.07 % of a photo; worst 8×8-block change median 10 / 7 levels, 99th percentile 44 / 41, max 61. I looked at the 22 largest local changes (old over new, at the worst block): no new artefacts. Most are a slightly different tone in small gaps among foliage or branches; one is the Hasselblad gap above. - [x] User-visible change: rendered the bus photo at full size and at 2400 px with Whites +54, Blacks +31 and Shadows +5 / +75, and looked at the number, the plate, foliage against the sky and a brick lattice fence. - [x] New tests `median_matches_cpu_bit_for_bit` (GPU, M2 Max) and `detail_free_matches_its_reference` (CPU). - [x] After merging `main` and moving the base to V7 (Linux, Ryzen 5 7545U, RADV): `cargo test --release` of `lightcraft-develop`, `-pipeline` (including `tests_legacy`), `-catalog`, `-engine`, and `-gpu` on the real adapter (skipping `running_out_of_device_memory_falls_back_without_stopping_the_gpu`, which fills RAM on an APU; see above); `cargo fmt`, `cargo clippy --workspace --all-targets -D warnings`, `cargo xtask layers`, `roots`, `assets`, `parity` and `wasm`. Not run: the full `cargo xtask ci`, and the macOS-only `tests_light::v1_and_v2_render_as_before_v3` (V1 and V2 don't change, so it shouldn't need re-capturing). - [x] After merging `main` again (with crystal grain as V7) and moving the base to V8 (Linux, Ryzen 5 7545U, RADV): `cargo test --release` of `lightcraft-develop`, `-pipeline` (including `tests_legacy` and the grain tests), `-catalog`, `-engine`, and `-gpu` on the real adapter (skipping `running_out_of_device_memory_falls_back_without_stopping_the_gpu`); `cargo fmt`, `cargo clippy --workspace --all-targets -D warnings`, `cargo xtask layers`, `roots`, `assets` and `parity`. Not run: `wasm` and the full `cargo xtask ci`. - [x] `cargo xtask ci --quick` (with `CRAFT_FONTS_DIR`): all steps pass. After the median change: all steps pass again (without `CRAFT_FONTS_DIR`), and the CR3 renders byte-identical to the first kernels on GPU and CPU. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Highlights and Shadows adjustments better preserve clean gaps in fine details such as lettering while maintaining the existing treatment of broader image features. * Tone adjustments are less likely to include isolated, narrow lines in the base used for processing. * The updated detail handling helps retain fine structure while reducing unwanted influence from closely packed details. * **Documentation** * Updated process notes to explain how fine-detail handling affects Highlights and Shadows adjustments. <!-- end of auto-generated comment: release notes by coderabbit.ai -->