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

crates/ironrdp-egfx/src/client.rs

@@ -712,9 +712,8 @@ impl GraphicsPipelineClient {
         }
         // The Progressive decoder is deliberately NOT reset here either. Its context lifetime
         // is driven by DeleteEncodingContext and DeleteSurface; MS-RDPEGFX 3.3.5.14 only
-        // resizes the Graphics Output Buffer. Windows establishes a codec context once and
-        // never re-sends SYNC + CONTEXT afterwards, so dropping it here makes every later
-        // payload fail with MissingBlock("CONTEXT").
+        // resizes the Graphics Output Buffer. Dropping it here would discard the tile state
+        // that later upgrade passes refine.
         // The ClearCodec decoder is deliberately NOT reset here. MS-RDPEGFX 3.3.5.14 only
         // resizes the Graphics Output Buffer; cache lifetime is driven by the stream instead,
         // through CLEARCODEC_FLAG_CACHE_RESET (2.2.4.1), which ClearCodecDecoder::decode
@@ -1984,45 +1983,7 @@ mod tests {
         }))
     }
 
-    fn progressive_context_stream(with_context: bool) -> Vec<u8> {
-        use ironrdp_pdu::codecs::rfx::RfxRectangle;
-        use ironrdp_pdu::codecs::rfx::progressive::{
-            ProgressiveBlock, ProgressiveContextPdu, ProgressiveFrameBeginPdu, ProgressiveFrameEndPdu,
-            ProgressiveRegion, ProgressiveSyncPdu, encode_progressive_stream,
-        };
-
-        let mut blocks = Vec::new();
-        if with_context {
-            blocks.push(ProgressiveBlock::Sync(ProgressiveSyncPdu));
-            blocks.push(ProgressiveBlock::Context(ProgressiveContextPdu {
-                context_id: 0,
-                tile_size: 0x0040,
-                flags: 0,
-            }));
-        }
-        blocks.push(ProgressiveBlock::FrameBegin(ProgressiveFrameBeginPdu {
-            frame_index: 0,
-            region_count: 1,
-        }));
-        blocks.push(ProgressiveBlock::Region(ProgressiveRegion {
-            tile_size: 0x40,
-            rects: vec![RfxRectangle {
-                x: 0,
-                y: 0,
-                width: 64,
-                height: 64,
-            }],
-            quant_vals: vec![],
-            quant_prog_vals: vec![],
-            flags: 0,
-            tiles: vec![],
-        }));
-        blocks.push(ProgressiveBlock::FrameEnd(ProgressiveFrameEndPdu));
-
-        encode_progressive_stream(&blocks).unwrap()
-    }
-
-    fn progressive_tile_stream(tile_x: u16, tile_y: u16, rect_width: u16, rect_height: u16) -> Vec<u8> {
+    fn progressive_tile_stream(tile_x: u16, tile_y: u16, rect_width: u16, rect_height: u16, tile_flags: u8) -> Vec<u8> {
         use ironrdp_graphics::progressive::{COEFFICIENTS_PER_COMPONENT, encode_first_pass};
         use ironrdp_pdu::codecs::rfx::RfxRectangle;
         use ironrdp_pdu::codecs::rfx::progressive::{
@@ -2072,7 +2033,7 @@ mod tests {
                     quant_idx_cr: 0,
                     x_idx: tile_x,
                     y_idx: tile_y,
-                    flags: 0,
+                    flags: tile_flags,
                     y_data: component_data,
                     cb_data: component_data,
                     cr_data: component_data,
@@ -2143,7 +2104,7 @@ mod tests {
                 frame_id: 2,
             }))
             .unwrap();
-        wire_progressive(&mut client, progressive_tile_stream(1, 1, 36, 36)).unwrap();
+        wire_progressive(&mut client, progressive_tile_stream(1, 1, 36, 36, 0)).unwrap();
         client
             .handle_pdu(GfxPdu::EndFrame(crate::pdu::EndFramePdu { frame_id: 2 }))
             .unwrap();
@@ -2328,16 +2289,21 @@ mod tests {
     }
 
     fn assert_progressive_context_is_deleted(clear: impl FnOnce(&mut GraphicsPipelineClient)) {
+        use ironrdp_pdu::codecs::rfx::progressive::TILE_FLAG_DIFFERENCE;
+
         let mut client = progressive_client();
-        wire_progressive(&mut client, progressive_context_stream(true)).unwrap();
+        wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, 0)).unwrap();
         clear(&mut client);
-        assert!(wire_progressive(&mut client, progressive_context_stream(false)).is_err());
+        // A difference tile adds to the reference its surface retained, so it fails once that is gone.
+        assert!(wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, TILE_FLAG_DIFFERENCE)).is_err());
     }
 
     #[test]
     fn progressive_context_survives_graphics_reset() {
+        use ironrdp_pdu::codecs::rfx::progressive::TILE_FLAG_DIFFERENCE;
+
         let mut client = progressive_client();
-        wire_progressive(&mut client, progressive_context_stream(true)).unwrap();
+        wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, 0)).unwrap();
 
         client
             .handle_pdu(GfxPdu::ResetGraphics(crate::pdu::ResetGraphicsPdu {
@@ -2355,26 +2321,26 @@ mod tests {
             }))
             .unwrap();
 
-        // Windows never re-sends SYNC + CONTEXT after a reset, so a CONTEXT-less
-        // continuation has to keep decoding.
-        assert!(wire_progressive(&mut client, progressive_context_stream(false)).is_ok());
+        // A difference tile still finds the reference retained before the reset.
+        assert!(wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, TILE_FLAG_DIFFERENCE)).is_ok());
     }
 
     #[test]
     fn progressive_context_is_deleted_with_encoding_context() {
+        use ironrdp_pdu::codecs::rfx::progressive::TILE_FLAG_DIFFERENCE;
+
         let mut client = progressive_client();
-        wire_progressive(&mut client, progressive_context_stream(true)).unwrap();
+        wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, 0)).unwrap();
         client
             .handle_pdu(GfxPdu::DeleteEncodingContext(DeleteEncodingContextPdu {
                 surface_id: 1,
                 codec_context_id: 7,
             }))
             .unwrap();
 
-        // The context's tiles are gone, but the surface survives and keeps the band layout it
-        // was given, so a payload reusing the id decodes from scratch. Windows deletes a codec
-        // context as it opens the next one and never repeats SYNC + CONTEXT.
-        assert!(wire_progressive(&mut client, progressive_context_stream(false)).is_ok());
+        // The context's tiles are gone, but its surface keeps the sub-band reference. Windows
+        // deletes a codec context as it opens the next one.
+        assert!(wire_progressive(&mut client, progressive_tile_stream(0, 0, 64, 64, TILE_FLAG_DIFFERENCE)).is_ok());
     }
 
     #[test]

crates/ironrdp-graphics/src/progressive.rs

@@ -1063,7 +1063,7 @@ pub struct SurfaceTiles {
     pub tiles_wide: u16,
     /// Height of the surface in tiles.
     pub tiles_high: u16,
-    /// Whether the associated context uses reduce-extrapolate DWT.
+    /// Whether the last REGION decoded into this grid uses reduce-extrapolate DWT.
     pub use_reduce_extrapolate: bool,
     /// Tile storage, indexed by `y_idx * tiles_wide + x_idx`.
     /// `None` entries haven't received any progressive data yet.
@@ -1289,7 +1289,6 @@ pub struct ProgressiveDecoder {
     references: BTreeMap<SubBandDiffingTileKey, DecDwtQ>,
     frame_tiles: BTreeMap<(u16, u32), BTreeSet<(u16, u16)>>,
     frame_active: bool,
-    surface_context_flags: BTreeMap<u16, bool>,
 }
 
 impl ProgressiveDecoder {
@@ -1300,7 +1299,6 @@ impl ProgressiveDecoder {
             references: BTreeMap::new(),
             frame_tiles: BTreeMap::new(),
             frame_active: false,
-            surface_context_flags: BTreeMap::new(),
         }
     }
 
@@ -1339,38 +1337,6 @@ impl ProgressiveDecoder {
 
         let blocks = decode_progressive_stream(bitmap_data)?;
 
-        // Extract the band-layout flag from the CONTEXT block when present.
-        // Per MS-RDPEGFX 2.2.4.2 the SYNC + CONTEXT blocks establish a codec
-        // context once (keyed by `(surface_id, codec_context_id)`) and are not
-        // required to be
-        // repeated on subsequent frames that reference the same context.
-        // Real-world servers (xrdp, GNOME Remote Desktop) omit the CONTEXT
-        // block on every frame after the first one that established the
-        // context. The strict requirement rejected each of those frames with
-        // `MissingBlock("CONTEXT")`, freezing the image on the coarse first
-        // pass.
-        //
-        // Fall back to the value stored when the context was first created, then to the last
-        // one this surface described: Windows opens a new codec context id mid-session,
-        // deletes the previous one, and never repeats SYNC + CONTEXT, so a per-context lookup
-        // alone rejects the new context. The retained value is scoped to its surface and
-        // released with it. Only error when no source is available at all.
-        let signalled = blocks.iter().find_map(|block| match block {
-            ProgressiveBlock::Context(ctx) => Some(ctx.uses_reduce_extrapolate()),
-            _ => None,
-        });
-        if let Some(flag) = signalled {
-            self.surface_context_flags.insert(surface_id, flag);
-        }
-        let use_reduce_extrapolate = signalled
-            .or_else(|| {
-                self.contexts
-                    .get(&(surface_id, codec_context_id))
-                    .map(|c| c.surface.use_reduce_extrapolate)
-            })
-            .or_else(|| self.surface_context_flags.get(&surface_id).copied())
-            .ok_or(ProgressiveDecodeError::MissingBlock("CONTEXT"))?;
-
         // Direct users of the decoder get one self-contained frame per call.
         // The EGFX client brackets multiple payloads with begin_frame/end_frame.
         if !self.frame_active {
@@ -1383,7 +1349,7 @@ impl ProgressiveDecoder {
         let context = match contexts.entry((surface_id, codec_context_id)) {
             Entry::Occupied(e) => e.into_mut(),
             Entry::Vacant(e) => {
-                let surface = SurfaceTiles::new(surface_width, surface_height, use_reduce_extrapolate)?;
+                let surface = SurfaceTiles::new(surface_width, surface_height, false)?;
                 e.insert(ProgressiveContext { surface })
             }
         };
@@ -1394,9 +1360,8 @@ impl ProgressiveDecoder {
         let surface_resized =
             context.surface.tiles_wide != expected_wide || context.surface.tiles_high != expected_high;
         if surface_resized {
-            context.surface = SurfaceTiles::new(surface_width, surface_height, use_reduce_extrapolate)?;
+            context.surface = SurfaceTiles::new(surface_width, surface_height, false)?;
         }
-        context.surface.use_reduce_extrapolate = use_reduce_extrapolate;
 
         let frame_tiles = all_frame_tiles.entry((surface_id, codec_context_id)).or_default();
         if surface_resized {
@@ -1426,6 +1391,10 @@ impl ProgressiveDecoder {
                 _ => continue,
             };
 
+            // Each REGION names its own DWT variant (MS-RDPEGFX 2.2.4.2.1.5).
+            let use_reduce_extrapolate = region.uses_reduce_extrapolate();
+            context.surface.use_reduce_extrapolate = use_reduce_extrapolate;
+
             let mut region_tiles = BTreeMap::new();
             for tile_block in &region.tiles {
                 let tiles = decode_tile_block(
@@ -1544,7 +1513,6 @@ impl ProgressiveDecoder {
             .retain(|(reference_surface_id, _, _), _| *reference_surface_id != surface_id);
         self.frame_tiles
             .retain(|(context_surface_id, _), _| *context_surface_id != surface_id);
-        self.surface_context_flags.remove(&surface_id);
     }
 
     /// Reset codec-context state while retaining surface sub-band references.
@@ -2249,19 +2217,15 @@ mod tests {
     }
 
     #[test]
-    fn decoder_context_fallback_is_scoped_by_surface() {
+    fn decoder_does_not_require_a_context_block() {
+        // MS-RDPEGFX 2.2.4.2.1.4 makes RFX_PROGRESSIVE_CONTEXT optional.
         let mut decoder = ProgressiveDecoder::new();
-        let stream_with_context = minimal_progressive_stream(true);
-        let stream_without_context = minimal_progressive_stream(false);
-
-        assert!(decoder.decode_bitmap(1, 0, 640, 480, &stream_with_context).is_ok());
-        assert!(matches!(
-            decoder.decode_bitmap(2, 0, 640, 480, &stream_without_context),
-            Err(ProgressiveDecodeError::MissingBlock("CONTEXT"))
-        ));
 
-        assert!(decoder.decode_bitmap(2, 0, 640, 480, &stream_with_context).is_ok());
-        assert!(decoder.decode_bitmap(2, 0, 640, 480, &stream_without_context).is_ok());
+        assert!(
+            decoder
+                .decode_bitmap(1, 0, 640, 480, &minimal_progressive_stream(false))
+                .is_ok()
+        );
     }
 
     fn rect(x: u16, y: u16, width: u16, height: u16) -> ironrdp_pdu::codecs::rfx::RfxRectangle {

crates/ironrdp-pdu/src/codecs/rfx/progressive.rs

@@ -394,9 +394,20 @@ pub struct ProgressiveContextPdu {
     pub flags: u8,
 }
 
-/// Bit 0 of context flags: use reduce-extrapolate DWT.
+/// RFX_DWT_REDUCE_EXTRAPOLATE in [2.2.4.2.1.5] RFX_PROGRESSIVE_REGION flags.
+///
+/// Indicates that the discrete wavelet transform (DWT) uses the "Reduce-Extrapolate" method.
+///
+/// [2.2.4.2.1.5]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/ffa9dcfc-610c-4cc5-86ba-d9435cfb37aa
 pub const FLAG_DWT_REDUCE_EXTRAPOLATE: u8 = 0x01;
 
+/// RFX_SUBBAND_DIFFING in [2.2.4.2.1.4] RFX_PROGRESSIVE_CONTEXT flags.
+///
+/// Indicates that sub-band diffing is enabled.
+///
+/// [2.2.4.2.1.4]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpegfx/89c2eaef-6bd1-4cb1-a80e-bfc161ccdbbf
+pub const CONTEXT_FLAG_SUBBAND_DIFFING: u8 = 0x01;
+
 /// RFX_TILE_DIFFERENCE in TILE_SIMPLE and TILE_FIRST flags.
 ///
 /// The tile payload contains DWT coefficient deltas from the retained reference
@@ -407,9 +418,12 @@ impl ProgressiveContextPdu {
     const NAME: &'static str = "ProgressiveContext";
     const FIXED_PART_SIZE: usize = 1 /* ctxId */ + 2 /* tileSize */ + 1 /* flags */;
 
-    /// Whether the reduce-extrapolate DWT variant is selected.
+    /// Whether bit 0 of the CONTEXT flags is set.
+    ///
+    /// That bit is RFX_SUBBAND_DIFFING, not the DWT variant.
+    #[deprecated(note = "the DWT variant is `ProgressiveRegion::uses_reduce_extrapolate`")]
     pub fn uses_reduce_extrapolate(&self) -> bool {
-        self.flags & FLAG_DWT_REDUCE_EXTRAPOLATE != 0
+        self.flags & CONTEXT_FLAG_SUBBAND_DIFFING != 0
     }
 }
 
@@ -1151,9 +1165,8 @@ mod tests {
         let original = ProgressiveContextPdu {
             context_id: 0,
             tile_size: 0x0040,
-            flags: FLAG_DWT_REDUCE_EXTRAPOLATE,
+            flags: CONTEXT_FLAG_SUBBAND_DIFFING,
         };
-        assert!(original.uses_reduce_extrapolate());
 
         let mut buf = [0u8; ProgressiveContextPdu::FIXED_PART_SIZE];
         original.encode(&mut WriteCursor::new(&mut buf)).unwrap();
@@ -1290,7 +1303,7 @@ mod tests {
             ProgressiveBlock::Context(ProgressiveContextPdu {
                 context_id: 0,
                 tile_size: 0x0040,
-                flags: FLAG_DWT_REDUCE_EXTRAPOLATE,
+                flags: CONTEXT_FLAG_SUBBAND_DIFFING,
             }),
             ProgressiveBlock::FrameBegin(ProgressiveFrameBeginPdu {
                 frame_index: 0,

crates/ironrdp-testsuite-core/tests/egfx/wire_to_surface_real_world.rs

@@ -175,6 +175,74 @@ fn wts2_progressive_tile_first_mixed_25tiles() {
     assert_eq!(diff_count, 9, "expected exactly 9 difference-encoded tiles");
 }
 
+/// The DWT variant comes from each REGION's `RFX_DWT_REDUCE_EXTRAPOLATE` flag (MS-RDPEGFX
+/// 2.2.4.2.1.5). Bit 0 of the CONTEXT flags is `RFX_SUBBAND_DIFFING` (MS-RDPEGFX 2.2.4.2.1.4)
+/// and must not change how a non-difference tile decodes.
+#[test]
+fn progressive_dwt_variant_comes_from_region_flags() {
+    use ironrdp_graphics::progressive::ProgressiveDecoder;
+    use ironrdp_pdu::codecs::rfx::progressive::{
+        ProgressiveContextPdu, ProgressiveFrameBeginPdu, ProgressiveFrameEndPdu, ProgressiveRegion, ProgressiveSyncPdu,
+        encode_progressive_stream,
+    };
+
+    let bytes = include_bytes!("../../test_data/egfx/haven/wts2_progressive_tile_first_mixed_25tiles.bin");
+    let GfxPdu::WireToSurface2(pdu) = decode(bytes) else {
+        panic!("expected WireToSurface2");
+    };
+    let region = decode_progressive_stream(&pdu.bitmap_data)
+        .expect("decode progressive stream")
+        .into_iter()
+        .find_map(|block| match block {
+            ProgressiveBlock::Region(region) => Some(region),
+            _ => None,
+        })
+        .expect("expected Region block");
+    assert!(region.uses_reduce_extrapolate());
+
+    // Difference tiles need a reference that is not part of the capture.
+    let base_tiles: Vec<_> = region
+        .tiles
+        .iter()
+        .filter(|tile| matches!(tile, ProgressiveTile::First(first) if first.flags & TILE_FLAG_DIFFERENCE == 0))
+        .cloned()
+        .collect();
+
+    let decode_with = |context_flags: u8, region_flags: u8| {
+        let stream = encode_progressive_stream(&[
+            ProgressiveBlock::Sync(ProgressiveSyncPdu),
+            ProgressiveBlock::Context(ProgressiveContextPdu {
+                context_id: 0,
+                tile_size: 0x40,
+                flags: context_flags,
+            }),
+            ProgressiveBlock::FrameBegin(ProgressiveFrameBeginPdu {
+                frame_index: 0,
+                region_count: 1,
+            }),
+            ProgressiveBlock::Region(ProgressiveRegion {
+                flags: region_flags,
+                tiles: base_tiles.clone(),
+                ..region.clone()
+            }),
+            ProgressiveBlock::FrameEnd(ProgressiveFrameEndPdu),
+        ])
+        .expect("encode progressive stream");
+        let mut decoder = ProgressiveDecoder::new();
+        decoder.begin_frame();
+        let tiles = decoder
+            .decode_bitmap(pdu.surface_id, pdu.codec_context_id, 1280, 800, &stream)
+            .expect("decode base tiles");
+        decoder.end_frame();
+        tiles.into_iter().map(|tile| tile.pixels).collect::<Vec<_>>()
+    };
+
+    let captured = decode_with(0x01, region.flags);
+    assert_eq!(captured.len(), 16);
+    assert!(captured == decode_with(0x00, region.flags));
+    assert!(captured != decode_with(0x01, 0x00));
+}
+
 /// Verify that decoding a difference fixture without a prior retained tile reference
 /// returns `MissingTileReference` as required by MS-RDPRFX 3.1.8.1.7.1 and #1698.
 #[test]