fix(kvm): exclude the BGRX pad byte from tile change-detection #126

Merged

David merged 1 commit from fix/VA-127-tile-hash-skip-bgrx-pad-byte into main

2026-07-08 11:08:24 +02:00

David commented

2026-07-08 04:53:38 +02:00

Owner

Summary

Fixes the badly-flickering remote desktop on hosts whose X11 driver returns a nondeterministic BGRX pad byte (observed on ds-08: an Intel/glamor Xorg LightDM greeter). The tile change-detector hashed the unused 4th byte of every BGRX pixel, which the JPEG encoder never ships, so a static screen re-shipped all 510 tiles every frame and the browser repainted the whole canvas ~30x/sec.

Tracked in VA-127, which is required for VAPP-77.

Root cause (proven on ds-08 by elimination)

20 back-to-back import -window root (plain XGetImage) grabs showed exactly two distinct images differing by 73 pixels (the greeter clock indicator blinking). The visible screen is static except that one ~1-tile element.
The agent shipped all 510 tiles every frame, but the JPEG byte total was pinned to exactly two values.
The JPEG uses only B,G,R; the hash used all four BGRX bytes. The only per-frame variation the hash saw that the JPEG did not is the pad byte, so on this driver the pad byte is nondeterministic per capture and hashing it flagged every tile as changed on every frame.

Hosts whose driver returns a constant pad byte never flickered, which is why this reproduced only on some hosts. The tile-diff core was otherwise correct (its existing tests use Rgb, which has no pad byte).

Fix

hash_tile now takes the PixelLayout and hashes only the color bytes per pixel (the first 3), skipping the BGRX pad byte via a new PixelLayout::color_bytes(). This matches exactly what encode_tile_to_jpeg ships, so "changed" tracks what the viewer sees. For Rgb (Wayland/grim, 3 bpp) the hashed bytes are identical in value and order to before, so that path is byte-for-byte unchanged.

Tests

New bgrx_pad_byte_noise_ships_zero_tiles: a BGRX frame with identical R,G,B but every pad byte flipped between two frames ships zero tiles (fails before this change, passes after).
hash_is_stable_for_identical_bytes extended to assert a pad-byte flip does not change the tile hash while a color-byte flip does.
hash_distinguishes_rgb_and_bgrx_layouts_consistently updated off the old bpp signature and made to actually hash both layouts.
just pre-commit (fmt + clippy -D warnings + full workspace tests) green.

## Summary Fixes the badly-flickering remote desktop on hosts whose X11 driver returns a nondeterministic BGRX pad byte (observed on ds-08: an Intel/glamor Xorg LightDM greeter). The tile change-detector hashed the unused 4th byte of every BGRX pixel, which the JPEG encoder never ships, so a static screen re-shipped all 510 tiles every frame and the browser repainted the whole canvas ~30x/sec. Tracked in VA-127, which is required for VAPP-77. ## Root cause (proven on ds-08 by elimination) - 20 back-to-back `import -window root` (plain `XGetImage`) grabs showed exactly two distinct images differing by 73 pixels (the greeter clock indicator blinking). The visible screen is static except that one ~1-tile element. - The agent shipped all 510 tiles every frame, but the JPEG byte total was pinned to exactly two values. - The JPEG uses only B,G,R; the hash used all four BGRX bytes. The only per-frame variation the hash saw that the JPEG did not is the pad byte, so on this driver the pad byte is nondeterministic per capture and hashing it flagged every tile as changed on every frame. Hosts whose driver returns a constant pad byte never flickered, which is why this reproduced only on some hosts. The tile-diff core was otherwise correct (its existing tests use `Rgb`, which has no pad byte). ## Fix `hash_tile` now takes the `PixelLayout` and hashes only the color bytes per pixel (the first 3), skipping the BGRX pad byte via a new `PixelLayout::color_bytes()`. This matches exactly what `encode_tile_to_jpeg` ships, so "changed" tracks what the viewer sees. For `Rgb` (Wayland/grim, 3 bpp) the hashed bytes are identical in value and order to before, so that path is byte-for-byte unchanged. ## Tests - New `bgrx_pad_byte_noise_ships_zero_tiles`: a BGRX frame with identical R,G,B but every pad byte flipped between two frames ships zero tiles (fails before this change, passes after). - `hash_is_stable_for_identical_bytes` extended to assert a pad-byte flip does not change the tile hash while a color-byte flip does. - `hash_distinguishes_rgb_and_bgrx_layouts_consistently` updated off the old `bpp` signature and made to actually hash both layouts. - `just pre-commit` (fmt + clippy `-D warnings` + full workspace tests) green.

David added 1 commit

2026-07-08 04:53:38 +02:00

fix(kvm): exclude the BGRX pad byte from tile change-detection

Check / fmt + clippy + tests (pull_request) Successful in 6m37s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

fbd8add35d

The desktop tile stream hashes each 64x64 tile to decide whether it changed since the previous frame. For X11 captures the frame is BGRX (4 bytes/pixel) and hash_tile hashed all four bytes including the unused 4th pad byte, while encode_tile_to_jpeg ships only B,G,R. On X11 drivers that return a nondeterministic pad byte per capture (observed on ds-08: Intel/glamor Xorg LightDM greeter) every tile hashed as changed on every frame, so the agent re-encoded and re-shipped the whole screen ~30x/sec even though the visible pixels were static. The browser repainted the full canvas continuously and the desktop flickered, while the journal spammed the per-frame tiles-shipped line.

Hash only the color bytes the viewer actually receives: hash_tile now takes the PixelLayout and feeds the first 3 bytes of each pixel to the FNV accumulator, skipping the BGRX pad byte via PixelLayout::color_bytes(). For Rgb (Wayland/grim, 3 bpp) this hashes the same bytes in the same order as before, so that path is byte-for-byte unchanged.

Proven on ds-08 by elimination: 20 back-to-back XGetImage grabs showed exactly two states differing by 73 pixels (a blinking clock indicator), yet the agent shipped all 510 tiles every frame with the JPEG byte total pinned to two values, so the only per-frame variation was in the bytes the hash saw but the JPEG did not, which is the pad byte.

Add a diff_changed_tiles regression test (a BGRX frame with unchanged RGB but flipped pad bytes ships zero tiles), extend the hash_tile test to assert a pad-byte flip does not change the hash while a color-byte flip does, and update the RGB/BGRX layout test off the old bpp signature.

#VA-127