Phase 1 — always fit window to guest surface¶
Parent: PLAN-display-window-sizing.md.
Goal¶
Make the ryll window auto-resize to match the guest surface
on every SurfaceCreated event (and every auto-create
fallback in ImageReady), not just the first one. This is
the bug fix for the uncalibrated-sextant report — the user
sees the window settle at a stable size matching whatever
resolution the guest has actually chosen, regardless of how
many intermediate modes the guest probed during boot.
The hamburger toggle and the --no-obey-guest-size CLI
flag land in phase 2. Tests of the full state machine
land in phase 3. Doc updates land in phase 4. This
phase is the minimum diff needed to fix the bug, plus a
small extracted helper that phase 3 can unit-test.
Scope¶
In scope:
- Replace
initial_resize_done: boolwithlast_auto_resize: Option<(u32, u32)>onRyllApp. The new field stores the (8-aligned) dimensions we last asked the viewport for, and is the single source of truth for "does the window already match the surface?". - Move the resize decision into a pure helper
compute_auto_resizeso phase 3 can drive it from a unit test without touching egui. - Honour
pending_resizeon everyupdate()call, gated only on: - The window is not maximised/fullscreen.
- The (8-aligned) target differs from
last_auto_resize. - Re-seed
last_sent_resizewith the (8-aligned) target every time we auto-resize, somaybe_send_monitors_resizedoesn't echo our own resize back to the guest as a freshVDAgentMonitorsConfig. - Constrain
pending_resizetriggers to the primary surface —(display_channel_id, surface_id) == (0, 0). Today both theSurfaceCreatedarm (ryll/src/app.rs:799) and the auto-create arm inImageReady(ryll/src/app.rs:835) setpending_resizefor any surface, which on multi-monitor setups would let a secondary surface event resize the primary window. - Reset
last_auto_resize(and clearpending_resize, which is already done) inRyllApp::reconnectatryll/src/app.rs:646-700, so a reconnect to a guest with a different resolution re-fits the window. - One focused unit test of
compute_auto_resizecovering the four interesting cases (no-pending, dedup match, fresh size, maximised). Broader state-machine tests belong to phase 3.
Out of scope (later phases):
- Hamburger toggle and CLI flag (phase 2).
- Tests that drive
RyllApp::updateend-to-end with synthesised channel events (phase 3). - ARCHITECTURE.md and README.md updates (phase 4).
- Any letterboxing/scaling when the window cannot match the surface (future work, master plan).
Grounding — what's there today¶
RyllApp carries the resize machinery on three fields:
pending_resize: Option<(f32, f32)>(app.rs:242) — set when a surface event arrives, consumed inupdate().initial_resize_done: bool(app.rs:248) — the one-shot gate this phase removes.last_sent_resize: Option<(u32, u32)>(app.rs:203) — last(width, height)we sent on the resize channel viamaybe_send_monitors_resize. Used to dedup outgoingVDAgentMonitorsConfigmessages.
pending_resize is set in two places — both unconditional
on the surface key today:
ChannelEvent::SurfaceCreatedarm (app.rs:785-800) sets it from(width, height)of the new surface.ChannelEvent::ImageReadyauto-create branch (app.rs:823-836) sets it when ryll has to fabricate a surface because the server drew before sendingSURFACE_CREATE.
The resize is consumed in RyllApp::update
(app.rs:1670-1688):
if let Some((w, h)) = self.pending_resize.take() {
if !self.initial_resize_done {
let total_h = h + STATS_BAR_HEIGHT;
ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(
egui::vec2(w, total_h),
));
// …seed last_sent_resize…
self.initial_resize_done = true;
info!("app: initial window resize to {}x{}", w as u32, h as u32);
} else {
debug!("app: surface resize {}x{} (window already sized)", w, h);
}
}
maybe_send_monitors_resize
(app.rs:1539-1570)
runs every frame, samples the live viewport interior,
subtracts STATS_BAR_HEIGHT (zero when maximised or
fullscreen), 8-aligns, and sends to the guest if the
result differs from last_sent_resize.
reconnect
(app.rs:646-700) clears
pending_resize and last_sent_resize but not
initial_resize_done — meaning a reconnect today does not
re-fit the window even on the new initial surface. With
this phase's change the gate goes away, so reconnect
behaviour quietly improves alongside the main fix.
Design¶
State¶
Replace:
// Pending viewport resize from a new surface
pending_resize: Option<(f32, f32)>,
// Whether the window has been auto-sized to the remote
// surface yet. …
initial_resize_done: bool,
with:
// Pending viewport resize from a new primary-surface event.
// Only set for (display_channel_id, surface_id) == (0, 0).
pending_resize: Option<(f32, f32)>,
// (8-aligned width, height) of the last auto-resize we
// issued. None until the first resize. Used to dedup so we
// don't re-issue ViewportCommand::InnerSize every frame
// while the window already matches the surface, and so
// reconnect can re-fit by clearing it.
last_auto_resize: Option<(u32, u32)>,
Initialise to None in RyllApp::new (replace the
initial_resize_done: false line). Reset to None in
RyllApp::reconnect alongside the existing
pending_resize = None reset.
Trigger sites — primary-surface only¶
In process_events change both pending_resize writes to
fire only when the affected key is (0, 0):
ChannelEvent::SurfaceCreated {
display_channel_id,
surface_id,
width,
height,
} => {
info!(/* unchanged */);
self.surfaces.insert(
(display_channel_id, surface_id),
DisplaySurface::new(surface_id, width, height),
);
if display_channel_id == 0 && surface_id == 0 {
self.pending_resize = Some((width as f32, height as f32));
}
}
// In the auto-create branch of ImageReady:
if let std::collections::hash_map::Entry::Vacant(e) =
self.surfaces.entry((display_channel_id, surface_id))
{
let surf_w = left + width;
let surf_h = top + height;
info!(/* unchanged */);
e.insert(DisplaySurface::new(surface_id, surf_w, surf_h));
if display_channel_id == 0 && surface_id == 0 {
self.pending_resize = Some((surf_w as f32, surf_h as f32));
}
}
The (0, 0) check is intentionally literal rather than
"the renderer's current primary key" — at the moment a new
surface arrives we want to react to it as the primary, not
to whatever the renderer happened to pick last frame. The
renderer's primary lookup at
app.rs:2553-2559
already prefers surface_id == 0, so the two definitions
agree.
Decision helper¶
Add a free function near maybe_send_monitors_resize:
/// Decide whether to issue a `ViewportCommand::InnerSize`
/// to fit the remote surface, and what size to ask for.
///
/// `pending` is the surface size pulled from
/// `pending_resize` (logical pixels, f32). `last_auto`
/// is the (8-aligned) size of the last auto-resize we
/// issued, or None if we have not auto-resized yet. `is_max`
/// is true when the viewport is maximised or fullscreen
/// and we should not change the inner size.
///
/// Returns Some((width, height, aligned_w, aligned_h))
/// where `(width, height)` are the values to pass to
/// `ViewportCommand::InnerSize` (with `STATS_BAR_HEIGHT`
/// added to the height — see the call site) and
/// `(aligned_w, aligned_h)` are the values to store in
/// `last_auto_resize` and seed into `last_sent_resize`.
/// Returns None when no resize should fire.
fn compute_auto_resize(
pending: Option<(f32, f32)>,
last_auto: Option<(u32, u32)>,
is_max: bool,
) -> Option<(f32, f32, u32, u32)> {
let (w, h) = pending?;
if is_max {
return None;
}
let aligned_w = ((w as u32).max(8) / 8) * 8;
let aligned_h = ((h as u32).max(8) / 8) * 8;
if last_auto == Some((aligned_w, aligned_h)) {
return None;
}
Some((w, h, aligned_w, aligned_h))
}
Notes on the alignment: maybe_send_monitors_resize uses
x -= x % 8; x = x.max(8) which yields the same result
as ((x.max(8)) / 8) * 8 for x >= 8. For x < 8 the
two differ: the existing code returns 8 (subtract-mod
underflows? no — 0 - 0 = 0, then .max(8) = 8; for
x = 7, 7 - 7 = 0, .max(8) = 8); the new helper
returns 8 too because .max(8) / 8 * 8 = 8. They agree.
Use whichever is cleaner; I lean to the new form because
it's a single expression. Either is fine — pick one and
match it inside maybe_send_monitors_resize only if
trivial; do not refactor that function as part of this
phase if the change is awkward.
Call site¶
In RyllApp::update, replace the existing block at
app.rs:1667-1688:
// Resize viewport to match the remote surface (plus stats
// bar) whenever a new primary surface differs from the
// size we last fitted to. Maximised/fullscreen windows
// are left alone — we cannot meaningfully change their
// inner size, and the surface will render at native size
// inside the available area.
let pending = self.pending_resize.take();
let is_max = ctx.input(|i| {
i.viewport().maximized.unwrap_or(false)
|| i.viewport().fullscreen.unwrap_or(false)
});
if let Some((w, h, aw, ah)) =
compute_auto_resize(pending, self.last_auto_resize, is_max)
{
let total_h = h + STATS_BAR_HEIGHT;
ctx.send_viewport_cmd(egui::ViewportCommand::InnerSize(
egui::vec2(w, total_h),
));
// Seed last_sent_resize so maybe_send_monitors_resize
// doesn't echo our own resize back to the guest as a
// VDAgentMonitorsConfig change.
self.last_sent_resize = Some((aw, ah));
self.last_auto_resize = Some((aw, ah));
info!(
"app: window resize to {}x{} (surface)",
w as u32, h as u32
);
}
Two subtle behaviours flowing from this:
- When the window is maximised and a surface event
arrives, we drop the resize on the floor (consumed by
take()and discarded). The surface still renders at native size inside the maximised window, which means it may overflow or leave empty space until the user un-maximises. This matches the existing maximised/fullscreen story inmaybe_send_monitors_resize. - The previous "initial" log line goes away. Replace with
a single log line for every successful auto-resize (per
the snippet above). The
elsebranch'sdebug!("app: surface resize … (window already sized)")also goes away; the dedup is silent. If we want to keep visibility in--verbose, add adebug!insidecompute_auto_resize's caller for theNonebranch. Skip the debug log for now to keep the diff minimal — phase 4's docs phase can revisit if needed.
Reconnect¶
In reconnect() at
app.rs:646-700, the
existing self.pending_resize = None; stays. Add
self.last_auto_resize = None; next to it. Remove the
initial_resize_done field entirely so there is no
self.initial_resize_done = … to write at all.
Constructor¶
In RyllApp::new at
app.rs:551-643, remove
initial_resize_done: false, and add
last_auto_resize: None, in the same place.
Tests¶
Add one focused unit test inside the existing
#[cfg(test)] mod tests block at
app.rs:3611:
#[test]
fn compute_auto_resize_decisions() {
// No pending event => no resize.
assert_eq!(compute_auto_resize(None, None, false), None);
// Pending size, never resized before, not maximised =>
// resize to the aligned target.
assert_eq!(
compute_auto_resize(Some((1024.0, 768.0)), None, false),
Some((1024.0, 768.0, 1024, 768)),
);
// Same target as last_auto => skip (dedup).
assert_eq!(
compute_auto_resize(
Some((1024.0, 768.0)),
Some((1024, 768)),
false,
),
None,
);
// Maximised => skip even when target differs.
assert_eq!(
compute_auto_resize(
Some((1024.0, 768.0)),
Some((640, 480)),
true,
),
None,
);
// Non-aligned size gets aligned to 8 px boundary.
assert_eq!(
compute_auto_resize(Some((1366.0, 770.0)), None, false),
Some((1366.0, 770.0, 1360, 768)),
);
// Differs after alignment from last_auto => resize.
assert_eq!(
compute_auto_resize(
Some((1280.0, 800.0)),
Some((1024, 768)),
false,
),
Some((1280.0, 800.0, 1280, 800)),
);
}
This is a pure-function test — no egui context, no async,
no channels. Phase 3 covers the round-trip with
maybe_send_monitors_resize and the channel-event path.
Step-level guidance¶
Single step, single commit.
| Step | Effort | Model | Isolation | Brief for sub-agent |
|---|---|---|---|---|
| 1a | medium | sonnet | none | Implement everything in this phase as described in Design and Tests. Files: ryll/src/app.rs only. Concretely: (1) drop the initial_resize_done: bool field declaration, the false initialiser in RyllApp::new, and the self.initial_resize_done = … write in update; (2) add last_auto_resize: Option<(u32, u32)> next to pending_resize on the struct, initialise to None in RyllApp::new, reset to None in RyllApp::reconnect; (3) add the compute_auto_resize free function as specified — place it directly above or below maybe_send_monitors_resize to keep the resize logic colocated; (4) replace the resize block in update with the compute_auto_resize-driven version; (5) gate both pending_resize writes (in SurfaceCreated and the auto-create branch of ImageReady) on display_channel_id == 0 && surface_id == 0; (6) add the compute_auto_resize_decisions unit test inside the existing tests module. Do not refactor maybe_send_monitors_resize — alignment differences between its inline form and the helper's expression form are equivalent for the values we care about. Run pre-commit run --all-files and make test before handing back. Single commit titled Always fit window to guest surface size.. Do not commit until the management session has reviewed. |
Medium effort and sonnet because the surface area is one
file, the design specifies every change, and the brief
front-loads the call sites by line number. Opus would not
add value. No worktree isolation: a plain git revert is
trivial if something goes wrong.
Success criteria¶
pre-commit run --all-filespasses.make testpasses, including the newcompute_auto_resize_decisionstest.- The
initial_resize_donefield and all references to it are gone (grep -rn initial_resize_donereturns nothing). pending_resizeis only written from primary-surface events.- Booting against uncalibrated-sextant (or any guest that cycles modes during boot) leaves the window matching the final guest surface size, regardless of intermediate probes.
Review checklist¶
-
initial_resize_doneis removed entirely (struct field,RyllApp::newinitialiser, and the write inupdate). -
last_auto_resizeis initialised inRyllApp::newand reset inreconnect. -
compute_auto_resizeis a free function (not a method) and is unit-tested. - Both
pending_resizewrites check(0, 0). -
last_sent_resizeis re-seeded with the new aligned dimensions on every auto-resize, not only the first. - No new
info!/debug!lines beyond the single "window resize to WxH (surface)" entry. - No changes outside
ryll/src/app.rs. - Commit message follows the project's conventions (Co-Authored-By with model + context window + effort level + any active settings, Signed-off-by, Prompt: paragraph).
Follow-up¶
Phase 2 adds the obey_guest_size field, the
--no-obey-guest-size CLI flag, and the hamburger
checkbox. The check inside compute_auto_resize (or the
caller) becomes if !obey_guest_size || is_max { return
None; } — phase 2's brief will spell out the exact
threading.