Skip to content

feat(graphics,egfx): add progressive RFX decode and EGFX integration#1197

Open
Greg Lamberson (glamberson) wants to merge 2 commits intoDevolutions:masterfrom
lamco-admin:feat/progressive-rfx-decode
Open

feat(graphics,egfx): add progressive RFX decode and EGFX integration#1197
Greg Lamberson (glamberson) wants to merge 2 commits intoDevolutions:masterfrom
lamco-admin:feat/progressive-rfx-decode

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

Depends on #1196.
Part of the multi-codec EGFX implementation described in #1158 (Section 4).

Summary

Add progressive RemoteFX decode pipeline and wire it into the EGFX client and server.

ironrdp-graphics: Progressive decode algorithms (first-pass and upgrade-pass coefficient reconstruction via SRL + inverse DWT), tile state tracking for multi-pass refinement, surface-level tile management with automatic quality progression.

ironrdp-egfx: WireToSurface2 dispatch for progressive codec in GraphicsPipelineClient. Progressive surface state stored on the server for multi-pass frame scheduling.

Changes

3 files, 1,393 lines.

Test plan

  • All 5 xtask gates pass
  • Progressive decode has inline unit tests

@GlassOnTin
Copy link
Copy Markdown
Contributor

The dwt_extrapolate.rs band layout matches the asymmetric subband sizes (33+31 / 17+16 / 9+8) that Windows Server 2025 emits on every Progressive region we observed during our downstream EGFX work — captures all had region.flags & RFX_DWT_REDUCE_EXTRAPOLATE = 0x01 set, so this path is the common case for modern Windows. Without extrapolate-mode handling the desktop is blank in cmd / PowerShell windows, so this is the right shape.

ProgressiveDecoder::decode_bitmap API is cleaner than the one we shipped in Haven (we keep per-context state at the caller; you keep it in a BTreeMap per codec_context_id and expose delete_context for DeleteEncodingContext — better).

One minor observation: WBT_TILE_FIRST tiles with flags & RFX_TILE_DIFFERENCE = 0x01 (coefficient deltas against a previously-decoded same tile) — does decode_first_pass handle that? Our captures didn't include this case so we only handle the non-difference path; flagging in case it bit you too.

Tested against a captured Server 2025 session: the three Progressive PDUs we have on disk decoded clean. Happy to share the captures + a smoke-test bin if it'd help.

@glamberson
Copy link
Copy Markdown
Contributor Author

Thanks for the careful review and especially for confirming the dwt_extrapolate.rs band layout against your Server 2025 captures, that asymmetric (33+31 / 17+16 / 9+8) layout was the part I was least confident about given the spec only documents the symmetric case. Good to hear the captures all had RFX_DWT_REDUCE_EXTRAPOLATE set — that matches what I saw on the synthetic test fixtures and confirms this is the modern-Windows common case.

On the BTreeMap / delete_context framing: the per-context state moved to the decoder when I realized DeleteEncodingContext can arrive interleaved with frames from other contexts, so caller-side state would force every consumer to re-implement the same lookup logic. Glad it lines up with where you would have ended up.

On RFX_TILE_DIFFERENCE: confirmed gap. The PDU layer parses the flags byte on TileSimple and TileFirst, but the graphics decoder unconditionally sets is_difference = false in decode_first and never reads the flag for coefficient-delta accumulation. Per MS-RDPRFX §3.1.8.1.7.1 a TILE_FIRST with flags & 0x01 = 1 should add the new RLGR-decoded coefficients to the previous decoded coefficients at the same (quantIdx, x, y) slot rather than replace them, and we do not do that. This will be a follow-up PR rather than expanding scope here, since it needs the per-tile previous-coefficient retention plus its own test fixtures (and ideally a captured difference-encoded frame, which is exactly the case your captures didn't include). Yes please on the captures + smoke-test bin offer if it's not too much trouble — would help close that follow-up faster.

Add progressive RemoteFX decode pipeline and wire it into the EGFX
client and server:

ironrdp-graphics: Progressive decode algorithms (first-pass and
upgrade-pass coefficient reconstruction via SRL + inverse DWT),
tile state tracking for multi-pass refinement, surface-level tile
management with automatic quality progression.

ironrdp-egfx: WireToSurface2 dispatch for progressive codec in
GraphicsPipelineClient. Progressive surface state stored on the
server for multi-pass frame scheduling.
The merged progressive primitives PR (Devolutions#1196) added validation that
a ProgressiveRegion must contain at least one rectangle. The test
used an empty rects vec which now fails decoding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants