perf(graphics): read RLGR bits through a 64-bit window

[Willie Abrams (willie)]

RLGR decoding read bits through bitvec's BitSlice, which took about three quarters of a Progressive tile's decode time. The decoder now reads through a 64-bit MSB-first window over the tile's bytes and counts runs with leading_zeros/leading_ones. The encoder is unchanged.

A full-screen 1920x1080 Progressive update now takes 29.2 ms to decode instead of 35.8 ms (mean, release build, Apple M4 Pro).

Its output matched the old decoder's on 6 million random well-formed tiles in both RLGR modes. The existing RLGR tests pass, and new unit tests cover the bit reader.

[Copilot]

Copilot review overview

🟢 Approval recommended

The optimized reader preserves decoding semantics and handles boundary conditions correctly.

Review effort: Balanced
Findings: None

What changed in this PR

Optimizes RLGR decoding by replacing bit-slice reads with a 64-bit MSB-first window.

Changes:

• Adds efficient bit reading and run counting.
• Adds boundary and end-of-input tests.
• Simplifies bit-width calculation.

No material findings identified.

File	Description
crates/​ironrdp-graphics/​src/​rlgr.rs	Implements and tests the optimized RLGR bit reader.

[github-actions]

This pull request may overlap with #2036.

PR 2036 is a perf(graphics) change to progressive decoding whose description explicitly measures decode speed with the new RLGR reader, which is the BitReader introduced here. Both PRs share scope around RLGR/progressive decode performance in ironrdp-graphics; exact file overlap needs human confirmation.

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]

Perf-only refactor of the RemoteFX RLGR decoder in crates/ironrdp-graphics/src/rlgr.rs, replacing bitvec BitSlice reads with a private 64-bit-window BitReader and rewriting compute_n_index with leading_zeros. Independent review confirms wire-faithful MSB-first semantics: read() refuses short reads without consuming, skip_run clamps runs at the tile end so peek64's zero padding is never consumed, all read widths are provably within 32 bits (k, kr <= 10; n_index <= 32), and compute_n_index is exactly equivalent. Encoder (BitStream) is untouched. No correctness, protocol, or API defects found. Two valid low-severity candidates are published: the perf and differential-equivalence claims are not reproducible in-repo (no RLGR benchmark or fuzz target), and BitReader.len restates data.len() * 8.

1. [skeptical] Perf motivation is not reproducible: no RLGR benchmark or fuzz target exists in-repo — low 🟡 — crates/ironrdp-graphics/src/rlgr.rs
   The PR's stated benefit (29.2 ms vs 35.8 ms full-screen Progressive decode; output matched on 6M random tiles) was measured off-tree. benches/ contains only perfenc.rs and fuzz/ has no rlgr or progressive-decode target, so neither the claimed speedup nor equivalence on adversarial tiles is checkable by reviewers or CI, leaving this hot path without regression protection. Correctness itself is adequately supported by the new unit tests and the existing RLGR tests, so this does not block acceptance; a committed decode benchmark or fuzz target exercising decode with attacker-controlled tiles would make the change's benefit durable and verifiable.
2. [code-compressor] BitReader.len duplicates data.len() * 8 — low 🟡 — crates/ironrdp-graphics/src/rlgr.rs
   len is pure derived state: set once in new() to data.len() * 8 and never mutated, while data is immutable for the reader's lifetime. remaining() and skip_run()'s loop bound could use self.data.len() * 8 - self.pos (or a small private total_bits() helper) instead, dropping the field and the invariant that len stay consistent with data. The multiply is negligible next to the window work these methods already do, so the cache buys no measurable performance; behavior is identical since the value is provably constant. Tradeoff: skip_run's loop condition becomes slightly longer.