Repository navigation
Capture a surface at the revision its caller listed, in one round trip - #90
Conversation
Since yas-run#54 a command wait that runs out before any command it could report on has started answers TIMEOUT (NOT_FOUND stays for an exited terminal or an evicted index). The client_host test still expected NOT_FOUND, so it failed on main.
Since yas-run#54 the server answers TIMEOUT when a command wait runs out before any command it could report on has started; wait_terminal_command's documentation still said NOT_FOUND, which stays for an evicted index or an exited terminal.
capture_surface looks the window's revision up (a WATCH snapshot) before CAPTURE: two round trips, three for a caller that listed the windows first to choose one, as Ultimator's screenshots do. capture_surface_at(id, revision, format) sends CAPTURE with the revision the caller has; when the window changed since, the server answers STALE and it looks the window up again and captures that, as capture_surface does.
There was a problem hiding this comment.
Ultimator's review of e6036db
Adds Client::capture_surface_at(id, revision, format), which sends CAPTURE with a revision the caller already listed. It takes one round trip, and on STALE it falls back to capture_surface, which looks the window up again. capture_surface now shares the private capture helper. I checked it against the server's CAPTURE handler, which answers NOT_FOUND for a missing record and STALE on a revision mismatch before any transfer starts, and against how the client maps errors: call_ok turns a non-OK status into Error::Status, so status() is Some(Stale). Surface handles are allocated once and never reused, so a listed revision can't point at another window, and the fallback covers every stale case. The docs and test updates from #83's terminal-wait commits match what wait_terminal_command does. I found nothing to flag.
No issues found.
Session · @ultimator-agent review reviews again · @ultimator-agent fix fixes the open issues
What
This PR adds
Client::capture_surface_at(id, revision, format)to yas-client. It sendsCAPTUREwith the revision the caller already has (SurfaceInfo::revision, fromsurfaces()orsurface()), so a capture takes one round trip instead of two.If the window changed since the caller saw it, the server answers
STALE. The client then looks the window up again and captures it, ascapture_surfacedoes, so the call never fails because of a stale revision.capture_surfaceis unchanged: it looks the revision up (a WATCH snapshot), then sends CAPTURE. The CAPTURE half is now a privatecapture(id, revision, format)that both methods share.Why
Ultimator's desktop screenshot first lists the windows to find the one asked for, then captures it. With
capture_surfacethat costs three round trips (the list, the lookup insidecapture_surface, CAPTURE) before the PNG comes back. The lookup repeats what the list just said.Measured
Ultimator's netem harness,
desktop screenshotof an alacritty window, p50 in ms, n=15. The server is the same #85–#89 build in both runs. Before iscapture_surface, after iscapture_surface_atwith the listed revision:With a remote machine, a screenshot now takes 2 RT plus the image instead of 3. At RTT 0 it also saves about 66 ms. That cost belongs to the second surface WATCH snapshot when it comes right after the first: the extra lookup waited that long even locally.
Wire and server
No change.
Capture.revisionand theSTALEanswer already exist; this only lets callers supply the revision.Tests
surfaces_capture_take_input_and_close_with_the_paste_probe(withYAS_CLIENT_TEST_PASTE_PROBE):capture_surface_atat the revision just listed returns a PNG;CAPTUREat the old revision is answeredSTALE, andcapture_surface_atat that old revision still returns a PNG.Status { status: Stale }.surfaces_are_none_without_the_compositor:capture_surface_atof a missing window isNOT_FOUND.Checks
Run locally at e6036db:
cargo fmt --checkpasses;-p yas-client -p yas-cli --all-targetsgives no warnings;Based on #83's branch, so fork CI runs past the stale journal test. It is independent of #85–#89 and merges into them cleanly. Fork CI results will be posted here.