Repository navigation
perf: the transforms read their rows straight from the bands - #147
Merged
Merged
Conversation
The inverse transform is 70 % of a 4K decode's GPU time. Its workgroups loaded the tile's 40x40 neighbourhood into workgroup memory before the horizontal pass read it from there: a phase of loads and a barrier that the horizontal pass can do without, since mirroring keeps a coordinate's parity and a row's even and odd samples each come from one band. On an Apple M3 Max, frame after frame, the inverse transform takes 0.30 instead of 0.38 ms at 4K and 0.067 instead of 0.105 ms at 1080p; the encoder's forward transform, changed the same way, 0.32 instead of 0.38 ms at 4K. Decodes and encoded frames are the same to the byte. Less workgroup memory alone, taller tiles and rows read without mirroring were measured and gained nothing more (D68). Claude-Session: https://claude.ai/code/session_019fJGoCTvP5e7n599q3ZQ42
The review found no defect in the shaders: their whole outputs matched the old shaders' at 5760 level sizes from 8 to 140, with the workgroup arrays poisoned as well. It found three things to say better: - The inverse transform's horizontal pass takes a row's even and odd samples from two bands because mirroring keeps a coordinate's parity. It does only up to the level's width less two past an edge, where the second reflection is clamped; the stored samples read four past it at most, so levels must be at least six wide. The minimum image size of 128 makes them eight. The shader and D68 say so, and a test holds every level of the smallest frames to eight. - The 8-bit output section still named the window array. - The old workgroup memory was 11.25 KiB, not 11.5. Claude-Session: https://claude.ai/code/session_019fJGoCTvP5e7n599q3ZQ42
This was referenced Oct 30, 2026
Merged
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.
Purpose
The inverse transform is 70 % of a 4K decode's GPU time on an Apple M3 Max (0.38 of 0.54 ms, frame after frame), and #86's goal is decoding at a stream's pace level with upstream's. Its workgroups spent a whole phase loading the tile's 40x40 neighbourhood into workgroup memory, and a barrier, before the horizontal pass read it from there.
Changes
idwt.wgsl: the horizontal pass reads its 16 samples straight from the bands. Whole-sample symmetric mirroring keeps a coordinate's parity, so a row's even samples come from one band (LL or LH by the row's parity) and its odd ones from the other (HL or HH). The load phase, its 6400 bytes of workgroup memory and a barrier go. The last level's 8-bit samples go to an array of their own,levels(4 KiB), whichidwt_packed.wgslandidwt_planes.wgslread.fdwt.wgsl: the GPU encoder's forward transform, the same way, reading its rows throughimage_at.tests/shaders.rs: the workgroup variables checked for writes before reads (D25), now different for the two transforms.ARCHITECTURE.md: the "Inverse transform" section, the forward transform's row in the encoder's stage table, a note on D13, and decision D68 with the measurements, the variants that gained nothing (less workgroup memory alone, taller tiles, rows read without mirroring), and why.CHANGELOG.md: under [Unreleased], Changed.Testing
emberwave-cli decode, and GPU-encoded frames of the 1080p, 1440p and 4K scenes and of the 4K game frame throughemberwave-cli encode --gpu, before and after, on Metal.cargo test --release --workspaceon the M3 Max (Metal), including the plane-texture stores;cargo fmt --check;cargo clippy --workspace --all-targets -D warnings.Benchmark impact
On the M3 Max, frame after frame, GPU timestamps, median of 400 (decoder) or 300 (encoder) frames, three rounds in turn with the build before, on a busy machine (load 4 to 6; disturbed rounds left out):
BENCHMARKS.mdkeeps its tables until the next full run on a quiet machine.Review
Our review agent checked correctness for odd and tiny level sizes, mirroring and parity, writes before reads (D25), barrier placement for WARP, and the docs against the code. It found no defect in the shaders:
horizontalandlevelspoisoned with NaN, a poisoning that does catch a removed write.Three low findings, fixed in fb2e5ea:
Related issue
Part of #86 (decoding at a stream's pace).