fix(graphics): read the DWT variant from REGION flags

[Ki Hyun Park (kihyun1998)]

The Progressive decoder chose the DWT variant from bit 0 of the RFX_PROGRESSIVE_CONTEXT flags. MS-RDPEGFX 2.2.4.2.1.4 defines that bit as RFX_SUBBAND_DIFFING; the variant is bit 0 of each RFX_PROGRESSIVE_REGION's flags (RFX_DWT_REDUCE_EXTRAPOLATE, 2.2.4.2.1.5). Tiles decoded correctly only while a server set both bits to the same value.

Each REGION now selects its own variant. The decoder kept the CONTEXT value only for this choice, so its per-surface fallback and the MissingBlock("CONTEXT") error are removed; CONTEXT is optional per 2.2.4.2.1.4. A stream without CONTEXT for an unseen codec context now decodes instead of failing.

ProgressiveContextPdu::uses_reduce_extrapolate is deprecated, and CONTEXT_FLAG_SUBBAND_DIFFING names the bit it reads.

Fixes #2041

[Ki Hyun Park (kihyun1998)]

Validation:

• progressive_dwt_variant_comes_from_region_flags is the reproduction from Progressive: tiles can decode garbled because the wavelet variant is read from the wrong flag #2041, run on the Windows capture wts2_progressive_tile_first_mixed_25tiles.bin. It fails on master: the 16 base tiles decode differently when only the CONTEXT bit changes. It also asserts that clearing the REGION flag does change the pixels, so a decoder that ignored both flags would fail too.
• The egfx tests that observed decoder state through MissingBlock("CONTEXT") now observe the surface's sub-band reference with a difference tile: it must survive ResetGraphics and DeleteEncodingContext, and fail after DeleteSurface.
• Each new or changed assertion was seen failing under a targeted mutation: reading the CONTEXT bit, hard-coding either variant, restoring the CONTEXT gate, keeping references on DeleteSurface, dropping them on ResetGraphics, and dropping them on DeleteEncodingContext.
• cargo xtask ci passes locally.

FreeRDP reads the two flags the same way (progressive.c: sub = context->flags & RFX_SUBBAND_DIFFING; extrapolate = region->flags & RFX_DWT_REDUCE_EXTRAPOLATE;) and does not require a CONTEXT before a REGION.

Note

Human-tuned, LLM-assisted content.

[Benoît Cortier (CBenoit)]

PR automation is failing because of a picky-krb 0.12.5 incompatibility, fixed on master by #2074. Please rebase on master to fix it.

Update: no rebase needed anymore. picky-krb 0.12.5 was yanked from crates.io (re-released as 0.13.0), so the API check builds again without changes to this branch. PR automation has been re-run here and passes.

[github-actions]

This pull request may overlap with #2010.

Both PRs modify the RFX Progressive decoder in crates/ironrdp-graphics/src/progressive.rs and the progressive PDU handling in ironrdp-pdu, including the decode_bitmap flow and its error behavior, so their edits plausibly touch the same code region.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

[github-actions]

PR 2085 is a correct protocol conformance fix: the Progressive decoder conflated bit 0 of the optional RFX_PROGRESSIVE_CONTEXT flags (RFX_SUBBAND_DIFFING, MS-RDPEGFX 2.2.4.2.1.4) with the DWT variant, which is actually signaled per REGION via RFX_DWT_REDUCE_EXTRAPOLATE (2.2.4.2.1.5). The head now reads the variant from each REGION before decoding that region's tiles, drops the now-dead CONTEXT fallback map and MissingBlock("CONTEXT") gate (CONTEXT is optional), renames the CONTEXT flag constant, and deprecates the misleading accessor. Verified against head code: the per-REGION write to the surface-level flag is unobservable in decode behavior (fresh tiles are re-seeded from FirstPassOptions at line 949 for first-pass tiles, and TILE_UPGRADE on a pass==0 tile exits at line 1699 before reading it), the deprecated CONTEXT accessor has no remaining in-tree callers with changed semantics, and minor test-local duplication exists in the egfx tests. The Windows-capture regression test discrim…

1. [skeptical] Deprecated uses_reduce_extrapolate keeps a misleading name while its body changes meaning — low 🟡 — crates/ironrdp-pdu/src/codecs/rfx/progressive.rs
   No in-tree caller of ProgressiveContextPdu::uses_reduce_extrapolate remains after this PR, and the retained body now tests CONTEXT_FLAG_SUBBAND_DIFFING, so a downstream caller upgrading gets a different boolean from the same input under a method name that still asserts 'reduce extrapolate' - the exact conflation this PR fixes, now preserved behind #[deprecated]. Because the old body was spec-incorrect there is no compatible body to keep; removing the method outright (permitted by a 0.x minor bump) or renaming it to a sub-band-diffing accessor would avoid extending the confusion for one release merely to defer the break.
2. [code-compressor] Three progressive lifecycle tests repeat the same setup/assert scaffolding — low 🟡 — crates/ironrdp-egfx/src/client.rs
   assert_progressive_context_is_deleted (2291-2299), progressive_context_survives_graphics_reset (2301-2326), and progressive_context_is_deleted_with_encoding_context (2328-2344) all follow: create client, wire progressive_tile_stream(0, 0, 64, 64, 0), apply a lifecycle step, then wire progressive_tile_stream(0, 0, 64, 64, TILE_FLAG_DIFFERENCE) and assert ok/err. A single helper taking an impl FnOnce(&mut GraphicsPipelineClient) lifecycle step and returning the wire_progressive result (matching the existing closure style, which already accommodates the ResetGraphics test's extra CreateSurface call) would reduce the three bodies to one-line assertions, removing roughly 10 duplicated lines while preserving behavior.

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

[github-actions]

[skeptical] Per-REGION write to surface-level use_reduce_extrapolate is unobservable dead state — low 🟡 — The only non-test reader of SurfaceTiles::use_reduce_extrapolate is get_or_create's seeding of a new TileState (line 1111). Every production caller is in decode_tile_block: TILE_SIMPLE/TILE_FIRST immediately overwrite the tile flag from FirstPassOptions (line 949) before reconstruct_to_rgba, and a TILE_UPGRADE on a freshly created tile exits at pass == 0 without reading it. The added per-REGION assignment (and the now constant-false SurfaceTiles::new arguments at lines 1352 and 1363) therefore carries no decode behavior; the field, its constructor parameter, and this write could be removed or the retention explicitly justified, since the redefined doc ('last REGION decoded into this grid') suggests influence the field does not have.

[github-actions]

[code-compressor] TILE_FLAG_DIFFERENCE imported locally three times in the tests module — low 🟡 — The same `use ironrdp_pdu::codecs::rfx::progressive::TILE_FLAG_DIFFERENCE;` line is added inside assert_progressive_context_is_deleted (2292), progressive_context_survives_graphics_reset (2303), and progressive_context_is_deleted_with_encoding_context (2330). Hoisting one import to the top of the mod tests block, which already carries module-level imports, removes two duplicated lines and per-function import noise with identical behavior, since the item is unambiguous within the module.