display: bulk-copy the frame path, keep a back buffer, cache the scaling - #1
Open
ya-luotao wants to merge 2 commits into
Open
display: bulk-copy the frame path, keep a back buffer, cache the scaling#1ya-luotao wants to merge 2 commits into
ya-luotao wants to merge 2 commits into
Conversation
`App::redraw()` re-converted every pixel of the scanout on every frame with `u32::from_le_bytes`, and rescaled per pixel (a division per axis) whenever the window size differed from the scanout. At 1920x1080 that is 2M conversions per frame, and the guest emits a frame for every pointer move. The viewer now keeps a persistent `0RGB` back buffer of scanout size. A `Frame`'s damage rectangle is clipped to the scanout and only that rectangle is copied out of the mmap slot, one `memcpy` per row: softbuffer's `0RGB` `u32` is `[b, g, r, x]` in memory on macOS (`NoneSkipFirst` + `Order32Little`, softbuffer-0.4.8 `backends/cg.rs:326`), which is exactly the guest's BGRX layout, so no per-pixel conversion is needed at all. Malformed or absent rectangles fall back to the whole frame, and frames that arrive between two redraws have their rectangles coalesced into a bounding box. The present path still blits the whole back buffer every redraw: softbuffer's macOS `buffer_mut()` hands out a fresh zeroed `Vec` and `present()` moves it into a `CGDataProvider` (`cg.rs:261`, `cg.rs:298`), so the surface buffer does not persist between frames, and `present_with_damage` ignores its damage (`cg.rs:364`). That blit is one `copy_from_slice`. Scaling, used when the window size differs from the scanout (resize, fullscreen, a 1x external display), keeps nearest-neighbour but drops the per-pixel division: integer factors duplicate pixels and rows with slice fills and copies, other sizes use an x-index table built once per (source, destination) width, and destination rows sampling the same source row are copied rather than recomputed. The window's size policy is unchanged; the scale factor is now logged once at window creation. `MSB_DISPLAY_STATS=1` prints a one-line summary per second: frames, redraws, bytes copied, ms per redraw. Viewer CPU (`top -l 6 -s 1`, samples 2-6 averaged; sandbox `omarchy`, 1920x1080 BGRX scanout, window 1:1 to device pixels on a 2x display): scenario before after idle desktop 0.0 % 0.0 % pointer motion 89.4 % 24.6 % full-screen churn 40.9 % 10.6 % Pointer motion is Hyprland's cursor driven in a loop from the host; churn is `foot` printing 200 MB at a time. After: ~46 frames/s during motion, ~20/s during churn, 5.4 ms per redraw. Note for follow-up work: Hyprland's FLUSH rectangle covers the whole scanout in both scenarios (8.3 MB per frame at 1920x1080, per the stats line), so the win above comes from the bulk copies, not yet from the damage rectangle. The rectangle path is exercised by unit tests and pays off as soon as the guest sends real damage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012LJqoJBYbk5RiNCmiR4zFC
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3db67cbe9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+71
to
+73
| /// A rectangle of the scanout, in scanout pixels. | ||
| #[derive(Clone, Copy)] | ||
| struct Rect { |
There was a problem hiding this comment.
This insertion begins a large block of new types, inherent implementations, and free functions without the mandated Types, Methods, and Functions section delimiters; only the later test module is sectioned. Reorganize these additions under the repository's exact Rust section headers.
AGENTS.md reference: AGENTS.md:L159-L161
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The damage rectangle can never be smaller than the whole scanout with a Wayland compositor: Linux's virtio-gpu driver sets `ignore_damage_clips` whenever the plane's framebuffer object changes, because uploads are done per buffer (v6.12 `drivers/gpu/drm/virtio/virtgpu_plane.c:91-97`), and a compositor page-flips between buffers on every frame. Measurements on the previous commit confirmed it: 8.3 MB copied per frame at 1920x1080, in every scenario. With the rectangle always covering everything, the viewer-side back buffer only added work. The runtime already writes a complete frame into the slot (`read_2d_resource`), and softbuffer's macOS surface buffer never persists — `buffer_mut` returns a fresh zeroed `Vec` and `present` moves it into a `CGDataProvider` (softbuffer-0.4.8 `backends/cg.rs:261`, `:298`), while `present_with_damage` ignores its damage (`:364`). So a redraw was doing slot -> back (8.3 MB) + back -> surface (8.3 MB) where one copy will do. The viewer now blits the slot's bytes straight into the surface buffer, which is a single `memcpy` because softbuffer's `0RGB` `u32` is `[b, g, r, x]` in memory on macOS — the guest's BGRX layout, top byte ignored. The scaler used when the window is not the scanout size now reads BGRX directly, keeping its integer-factor and index-table fast paths and expanding each source row once. `Rect`, `Damage`, the rectangle clipping/union and the back buffer are gone; `rect` stays in the protocol (it costs nothing and a guest that does send real damage would be worth supporting) but the viewer ignores it, with the kernel reference in a comment. Viewer CPU and cost per redraw, same method as the previous commit (`top -l 6 -s 1`, samples 2-6 averaged), the two builds measured back to back against the same sandbox and the same driver load: scenario b3db67c this commit pointer motion 33.5 % 5.35 ms 31.9 % 5.17 ms (66 frames/s) full-screen churn 11.9 % 5.46 ms 11.1 % 5.20 ms (~22 frames/s) The frame rate is set by the guest, so the honest signal is the time per redraw: 3-5 % less, consistently. It is a small win because a redraw is dominated by `present()` (CGImage plus the CATransaction), not by the copy — removing 8.3 MB of memcpy saves about 0.2 ms of a 5.3 ms redraw. The real gain is 157 fewer lines of machinery that could never pay off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012LJqoJBYbk5RiNCmiR4zFC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Viewer-side frame path for
msb display(stacks ongpu-m0, part of the superradcompany#1482 series).App::redraw()re-converted every pixel of the scanout on every frame withu32::from_le_bytes, and rescaled per pixel whenever the window size differed from the scanout. The guest emits a frame for every pointer move, so at 1920x1080 that was 2M conversions per mouse event.Now:
0RGBback buffer of scanout size; aFrame's damage rectangle is clipped and only that rectangle is copied out of the mmap slot, onememcpyper row (softbuffer's0RGBon macOS is[b,g,r,x]in memory — the guest's BGRX layout, so no conversion at all);copy_from_slice(softbuffer 0.4.8's macOSbuffer_mut()hands out a freshVecevery time andpresent_with_damageignores damage, so the whole back buffer must be blitted each redraw);MSB_DISPLAY_STATS=1prints frames/s, redraws/s, MB/s copied and ms per redraw.Viewer CPU (
top -l 6 -s 1, samples 2-6 averaged; sandboxomarchy, 1920x1080 BGRX, window 1:1 to device pixels on a 2x display):Six unit tests cover rect clipping, damage coalescing,
copy_rect, integer and non-integer scaling, and the too-small-buffer guard.Follow-up (not in this PR): Hyprland's FLUSH rectangle currently covers the whole scanout on every frame, so the rectangle path is not yet exercised in practice, and
read_2d_resourceinthird_party/msb_krun_devicesstill copies the whole scanout per flush on the gpu worker thread. Both are being looked at separately.🤖 Generated with Claude Code
https://claude.ai/code/session_01PN7mepn7ryjXupoHbQjFmR
Update: second commit 323595d drops the back buffer and the damage machinery again — Linux v6.12
drivers/gpu/drm/virtio/virtgpu_plane.c:91-97setsignore_damage_clipswhenever the plane's fb changes (page flip), so a double-buffered compositor always flushes the whole scanout and the rect path could never fire. The viewer now blits the slot straight into the surface (one memcpy). Back-to-back numbers against b3db67c on the same sandbox are in that commit message (5.35 → 5.17 ms/redraw; the table above is from the first round and not directly comparable across sandbox instances).