fix(graphics): keep ZGFX compression linear once the history is full

[Greg Lamberson (glamberson)]

The ZGFX compressor keeps its history in a Vec and, once the history reaches its 2.5 MB limit, add_to_history drains the front of the Vec and walks every entry of the match table to rebase and prune positions. add_to_history runs once per literal byte and once per match, so after the history fills, every emitted token costs a copy of the whole window plus a pass over the table. With ClearCodec output from a 1280x800 desktop fed through one Compressor, the first three frames took 10 to 51 ms each, and the next three took 0.38, 4.9 and 6.8 seconds. #1344 bounded the hash table, which is a different limit, and this shows up on any long session that uses CompressionMode::Auto or Always. The compressor came from my own #1097.

This keeps the history as a fixed ring of HISTORY_SIZE bytes addressed by absolute stream position, the same layout the decompressor already uses with FixedCircularBuffer. Appending is a copy into the ring, the match table stores absolute positions, and a candidate is skipped when it has fallen out of the window or is further back than MAX_MATCH_DISTANCE, so nothing has to be rebased. The encoded output format is unchanged, and so is the rule in MS-RDPEGFX 3.1.9.1.2 that every output byte, including those of segments sent uncompressed, is recorded in the history.

With the same input the output is the same size on every frame, and the frames after the history fills now take 9 to 15 ms each.

A new test compresses more than twice HISTORY_SIZE through one Compressor, in segments that repeat earlier content, and decompresses every segment with one Decompressor, which covers matches that reach across the wrap point. A second test checks the ring directly, including a push larger than the ring. The xtask fmt, lints, tests, typos and locks checks pass.

#2003 builds on this: it sends H.264 surface commands uncompressed and records their bytes in the ring.

[github-actions]

The PR replaces the ZGFX compressor's drained Vec history and per-append position rebasing with a fixed 2.5 MB ring addressed by absolute stream positions, fixing an evidenced O(window)-per-token slowdown once the history fills. Independent verification of the head confirms the ring arithmetic (push, byte_back, prefix_at, holds_prefix_at) is correct including oversized pushes and wrap-around; the live candidate set under the newest-16 selection plus the distance filter is equivalent to the old rebase-and-prune scheme; emitted distances stay <= MAX_MATCH_DISTANCE (2,097,152) < HISTORY_SIZE (2,500,000), so the decompressor contract and the MS-RDPEGFX rule that every output byte is recorded in history are preserved, and only match selection may differ, which the spec permits. New tests cover round trips across the wrap point and direct ring behavior. No correctness, protocol, or performance defects were found beyond four valid low-severity candidates from the specialists, all verified ac…

[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 #2003.

Both touch the ZGFX compressor's history maintenance in crates/ironrdp-graphics/src/zgfx. PR 2003 appends uncompressed H.264 bytes to the compressor history without match searching, while this PR reimplements that history as a fixed ring with absolute positions, so they modify overlapping code paths that would need reconciliation.

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]

The PR replaces the ZGFX compressor's Vec history (drain plus full match-table rebase per token after fill) with a fixed HISTORY_SIZE ring addressed by absolute u64 stream positions, skipped at lookup time. Independently verified: ring arithmetic in push (including oversized inputs and wrap), byte_back/prefix_at indexing and their debug_assert invariant established at both call sites, and match extension preserving the old guard (match_len < distance equals the previous hist_pos + match_len < history.len() bound). Wire format and window semantics are unchanged. The two valid code-compressor candidates are both correct, optional, behavior-preserving maintainability improvements (deduplicating the evict-and-push position logic in add_to_history, and delegating History::push to the crate-internal FixedCircularBuffer); both accepted at low severity. No correctness, protocol, or safety defects found.

1. [code-compressor] Evict-and-push position logic duplicated between the two indexing loops in add_to_history — low 🟡 — crates/ironrdp-graphics/src/zgfx/compressor.rs
   The main sampling loop (lines 85-94) and the boundary loop (lines 102-116) both do entry lookup, cap eviction via entry.remove(0), then push. Extracting a record_position(prefix, pos) helper collapses both call sites and one duplicate HashMap lookup; behavior is preserved because sampled positions strictly increase, making the dedup check a no-op in the main loop while the boundary loop keeps its dedup semantics. Pure maintainability, no correctness impact.

[Greg Lamberson (glamberson)]

The review's other item, the duplicated evict-and-push in add_to_history, is done, with one difference from the suggestion. A new record_position(prefix, pos) does the lookup, the eviction at 32 positions and the push, and both loops call it. I dropped the entry.last() != Some(&pos) guard and didn't move it into the helper, because it can't fire. A position is recorded once, when its third byte arrives, and the main loop and the boundary loop cover different positions. I checked that by counting the guard's hits over the ZGFX tests and 240,000 add_to_history calls with chunks of 0 to 3000 bytes and alphabets of 2 to 256 symbols, and it never fired. A debug_assert! in the helper now states the invariant. Two new tests cover the helper, one for the cap of 32 and one for the positions recorded inside a chunk and across the append point.