perf(graphics): portable SIMD inverse DWT (wide + SWAR)#1383
perf(graphics): portable SIMD inverse DWT (wide + SWAR)#1383irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 1 commit into
Conversation
The RFX inverse DWT is ~half of the web client's frame-decode time (decode is ~93% of the frame on a 1080p RemoteFX replay). Replace the scalar inverse DWT with a portable SIMD implementation using the `wide` crate (`i16x8`), which lowers to wasm simd128, x86 SSE/AVX, and ARM NEON from one source. Overflow-free SWAR average identities keep the kernel entirely in `i16` lanes (no widen/narrow): `ceil_avg(a,b) = (a|b) - ((a^b)>>1)` and `floor_avg(a,b) = (a&b) + ((a^b)>>1)`, with `(2x+1)>>1 == x` / `(x+x)>>1 == x` simplifying the edge rows. Every other op is wrapping `i16` arithmetic, so the output is bit-exact with the previous scalar code — no `unsafe`, no cfg split. The encode path is unchanged. Bit-exactness verified by the replay-bench framebuffer CRC, the existing native DWT tests, and an exhaustive check of the SWAR identities over all i16 pairs. Bench (1080p RemoteFX replay, wasm release +simd128): inverse DWT ~2x faster, ~39% off the decode stage (~1598 ms -> ~985 ms).
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
This is looking good to me, do you have any concern to address before removing the draft status?
| num-derive.workspace = true # TODO: remove | ||
| num-traits.workspace = true # TODO: remove | ||
| yuv = { version = "0.8", features = ["rdp"] } | ||
| wide = "0.7" # portable SIMD for the inverse DWT (wasm/x86/ARM); `std::simd` is still nightly-only |
There was a problem hiding this comment.
praise: Thank you for documenting the rationale. At some point we may migrate to std::simd, when it’s stabilized.
There was a problem hiding this comment.
Pull request overview
This PR accelerates the RemoteFX (RFX) decode hot path in ironrdp-graphics by replacing the scalar inverse DWT kernel with a portable-SIMD implementation using the wide crate, targeting wasm simd128 as well as native SIMD backends.
Changes:
- Add portable SIMD helpers (
i16x8) plus SWAR ceil/floor average primitives to keep the inverse DWT bit-exact without widening. - Rewrite inverse DWT horizontal and vertical passes to operate on 8-lane vectors and 8-column tiles.
- Add
wideas a direct dependency ofironrdp-graphics(and updateCargo.lockaccordingly).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/ironrdp-graphics/src/dwt.rs | Replaces scalar inverse DWT with portable SIMD (wide) horizontal/vertical passes and SWAR average helpers. |
| crates/ironrdp-graphics/Cargo.toml | Adds wide = "0.7" dependency for portable SIMD. |
| Cargo.lock | Locks the new wide dependency in the workspace lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn vld(s: &[i16], off: usize) -> i16x8 { | ||
| i16x8::from(<[i16; 8]>::try_from(&s[off..off + 8]).expect("off + 8 within bounds")) | ||
| } |
| debug_assert!( | ||
| sw % 8 == 0 && sw <= MAX_SUBBAND_WIDTH, | ||
| "sw must be a multiple of 8 and <= MAX_SUBBAND_WIDTH" | ||
| ); |
| debug_assert!( | ||
| sw % 8 == 0 && sw <= MAX_SUBBAND_WIDTH, | ||
| "sw must be a multiple of 8 and <= MAX_SUBBAND_WIDTH" | ||
| ); |
| /// Loads 8 contiguous `i16` from `s[off..]` into a vector. The caller guarantees `off + 8 <= s.len()`. | ||
| #[inline] | ||
| fn vld(s: &[i16], off: usize) -> i16x8 { | ||
| i16x8::from(<[i16; 8]>::try_from(&s[off..off + 8]).expect("off + 8 within bounds")) |
There was a problem hiding this comment.
wide has an API for creating a SIMD from a slice. Use i16x8::from_slice_unaligned
There was a problem hiding this comment.
Also, you can get subslice without repeating off: s[off..][..8]. IMHO, it's cleaner.
You can make use of this pattern in many places in this file.
| fn vst(s: &mut [i16], off: usize, v: i16x8) { | ||
| s[off..off + 8].copy_from_slice(&v.to_array()); | ||
| } |
There was a problem hiding this comment.
| fn vst(s: &mut [i16], off: usize, v: i16x8) { | |
| s[off..off + 8].copy_from_slice(&v.to_array()); | |
| } | |
| fn vst(s: &mut [i16], off: usize, v: i16x8) { | |
| s[off..off + 8].copy_from_slice(v.as_array()); | |
| } |
No need to make an array. You can get a reference to inner data directly.
| /// Ceil average `(a + b + 1) >> 1`, overflow-free (SWAR; arithmetic shift). | ||
| #[inline] | ||
| fn ceil_avg(a: i16x8, b: i16x8) -> i16x8 { | ||
| (a | b) - ((a ^ b) >> 1) |
There was a problem hiding this comment.
It would be beneficial to contribute to wide to add this operation directly there. There is already a specialized SIMD for that - _mm_avg_epu16, but wide doesn't have this API. So we need to use other SIMD operations to achieve that. If the API was present is wide, we could do it at the cost of only a single SIMD operation, which is faster.
I used to contribute to wide when optimizing IronVNC with SIMDs. The author of the library was open to contributions.
Summary
On the WASM web client, frame decode dominates (~93% of frame time on a 1080p RemoteFX replay), and within decode the RFX inverse DWT was ~48% (the YCbCr→RGBA convert is already SIMD via
yuv; the entropy/RLE stages are inherently sequential). This vectorizes the inverse DWT with the portablewidecrate (i16x8), so the same code lowers to wasmsimd128, x86 SSE/AVX, and ARM NEON — desktop and browser both benefit.The encode path is unchanged.
How it stays bit-exact (no
unsafe, nocfgsplit)The lifting steps need i32 intermediates only for the averages. Overflow-free SWAR identities let the whole kernel stay in
i16lanes (no widen/narrow):ceil_avg(a,b) = (a|b) - ((a^b)>>1)≡(a + b + 1) >> 1floor_avg(a,b) = (a&b) + ((a^b)>>1)≡(a + b) >> 1and
(2x+1)>>1 == x/(x+x)>>1 == xsimplify the first/last rows. Every other op is wrappingi16arithmetic, identical to the oldi32-intermediate-then-as i16truncation.Performance
1080p RemoteFX replay, headless Chromium, wasm release
+simd128, 8-pass median:wideSIMD→ inverse DWT ~2×, ~39% off the decode stage. (Absolute ms carry ~±15% machine-load noise; the ratio is stable. Per-frame this is a throughput win — decode was already within real-time budget.)
Correctness
Verified bit-exact three ways:
i16 × i16pairs (0 mismatches).Notes
wideis a single-user dep inironrdp-graphics; chosen overstd::simd(still nightly-only) and over per-arch intrinsics (one portable kernel vs three).bench/draw-*(renderer) and the DWT measurements were taken on the replay-bench harness branch (the capture corpus is gitignored).