Skip to content

Commit 89b7801

Browse files
Sayan-Sayan-
committed
display: converge X root on the configure stop/start resize path
The batched chromium-configure stop/start path applied the Neko screen reconfig with a bare setResolutionViaNeko and trusted the RPC return, unlike the live-resize path (PatchDisplay), which polls the X root and retries the reconfig when it does not converge. Neko applies screen configuration asynchronously and can silently drop a reconfig that lands while a previous one is still in flight, leaving the X root at the dummy DDX default (3840x2160). When that happens on the configure path, Chromium is then restarted in --kiosk and maximizes onto the stale root, so the live view renders the browser in a small corner of the streamed frame. It is intermittent because it only manifests when the reconfig is dropped, and the bare RPC returns success without confirming it took effect. Extract the apply-poll-retry loop into applyResolutionAndConverge and route both PatchDisplay and the configure stop/start path through it, so the two resize paths share one convergence guarantee instead of the guard living on only one of them (which is how this regression arose).
1 parent ae64afa commit 89b7801

2 files changed

Lines changed: 77 additions & 82 deletions

File tree

‎server/cmd/api/api/chromium_configure.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ func chromiumDisplayApplyWhileStopped(ctx context.Context, s *ApiService, plan *
690690
}
691691
var err error
692692
if s.isNekoEnabled() {
693-
err = s.setResolutionViaNeko(ctx, w, h, rr)
693+
_, _, err = s.applyResolutionAndConverge(ctx, w, h, rr, true)
694694
} else {
695695
err = s.setResolutionXorgViaXrandr(ctx, w, h, rr)
696696
}

‎server/cmd/api/api/display.go‎

Lines changed: 76 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -115,92 +115,26 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ
115115

116116
// Route to appropriate resolution change handler
117117
if displayMode == "xorg" {
118-
useNeko := s.isNekoEnabled()
119-
if useNeko {
120-
log.Info("using Neko API for Xorg resolution change")
121-
err = s.setResolutionViaNeko(ctx, width, height, refreshRate)
122-
} else {
123-
log.Info("using xrandr for Xorg resolution change (Neko disabled)")
124-
err = s.setResolutionXorgViaXrandr(ctx, width, height, refreshRate)
125-
}
126-
// Re-assert the maximized window state via CDP, then verify the
127-
// X root has reached the requested size. Mutter reflows a
128-
// maximized (or fullscreen) window onto the new root
129-
// automatically, so the CDP call's only job is to make sure the
130-
// window is in the state that triggers the reflow. The X root
131-
// poll is the authoritative post-condition: it's the value the
132-
// server actually set and stays panel-robust if mutter ever
133-
// gains a taskbar (a maximized window would then be smaller
134-
// than the root by the panel's reserved space).
135-
//
136-
// Both are fatal on failure — returning 200 with the X root
137-
// still at the old size, or the window stuck in normal state,
138-
// would leave the caller with no signal of the mismatch. The
139-
// previous approach of restarting chromium so it could re-apply
140-
// --start-maximized had the same effective contract (the
141-
// restart blocked the response) but cost ~9s per resize and
142-
// wiped browser-side state (Emulation.* overrides, devtools
143-
// sessions). The restart_chromium request field is still
144-
// accepted for API compatibility but no longer triggers a
145-
// restart on this path.
118+
var realizedW, realizedH int
119+
realizedW, realizedH, err = s.applyResolutionAndConverge(ctx, width, height, refreshRate, s.isNekoEnabled())
146120
if err == nil {
147-
// Wait for the X root to settle, then use it as the
148-
// realized size in the response. The wait returns early
149-
// when either (a) xrandr reports the requested size — the
150-
// common case — or (b) consecutive reads are stable for a
151-
// short window, capturing libxcvt's rounded size on
152-
// requests it can't honour exactly (CVT 8-pixel grid +
153-
// FWXGA bump for 1360×768 → 1366×768).
154-
//
155-
// The poll absorbs transient X root states — chromium
156-
// running in --kiosk briefly pushes the root to the dummy
157-
// DDX's max mode (3840×2160) while mutter settles on the
158-
// new screen, and a single immediate read would catch that
159-
// transient instead of the steady-state size.
160-
//
161-
// On the Neko path, retry the reconfig RPC when X never
162-
// converges. Neko applies screen configuration asynchronously
163-
// and can drop/clobber a PATCH that lands while a previous
164-
// reconfig (e.g. its boot 1920x1080) is still in flight —
165-
// the new request is silently lost and X stays at the dummy
166-
// DDX default (3840x2160). Polling X harder won't recover;
167-
// only re-issuing the reconfig will.
168-
const maxAttempts = 3
169-
var realizedW, realizedH int
170-
var converged bool
171-
for attempt := 0; attempt < maxAttempts; attempt++ {
172-
realizedW, realizedH, converged = s.waitForXRootRealized(ctx, width, height, 10*time.Second)
173-
if converged {
174-
break
175-
}
176-
if !useNeko || attempt == maxAttempts-1 {
177-
break
178-
}
179-
log.Warn("X root did not converge after resize, retrying neko reconfig",
180-
"attempt", attempt+1,
121+
if realizedW != width || realizedH != height {
122+
log.Info("X root differs from request after resize",
181123
"requested", fmt.Sprintf("%dx%d", width, height),
182124
"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
183-
if retryErr := s.setResolutionViaNeko(ctx, width, height, refreshRate); retryErr != nil {
184-
err = retryErr
185-
break
186-
}
187-
}
188-
if err == nil {
189-
if !converged {
190-
log.Error("display did not converge to requested mode after retries",
191-
"requested", fmt.Sprintf("%dx%d", width, height),
192-
"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
193-
err = fmt.Errorf("display did not converge to %dx%d (X root reports %dx%d)", width, height, realizedW, realizedH)
194-
} else {
195-
if realizedW != width || realizedH != height {
196-
log.Info("X root differs from request after resize",
197-
"requested", fmt.Sprintf("%dx%d", width, height),
198-
"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
199-
}
200-
width, height = realizedW, realizedH
201-
}
202125
}
126+
width, height = realizedW, realizedH
203127
}
128+
// Re-assert the maximized window state via CDP so mutter reflows the
129+
// window onto the new root; applyResolutionAndConverge already verified
130+
// the root size, which is the authoritative post-condition. Fatal on
131+
// failure — returning 200 with the window stuck in its old state would
132+
// leave the caller with no signal of the mismatch. The previous approach
133+
// of restarting chromium so it could re-apply --start-maximized had the
134+
// same effective contract but cost ~9s per resize and wiped browser-side
135+
// state (Emulation.* overrides, devtools sessions); restart_chromium is
136+
// still accepted for API compatibility but no longer triggers a restart
137+
// on this path.
204138
if err == nil {
205139
if cdpErr := s.setWindowMaximizedViaCDP(ctx); cdpErr != nil {
206140
log.Error("CDP maximize re-assert failed after Xorg resolution change", "error", cdpErr)
@@ -590,6 +524,67 @@ func abs(x int) int {
590524
return x
591525
}
592526

527+
// applyResolutionAndConverge sets the Xorg resolution — via Neko when enabled,
528+
// otherwise xrandr — and waits for the X root to actually reach the requested
529+
// size, returning the realized dimensions.
530+
//
531+
// The poll absorbs transient X root states: chromium running in --kiosk briefly
532+
// pushes the root to the dummy DDX's max mode (3840x2160) while mutter settles
533+
// on the new screen, and a single immediate read would catch that transient. It
534+
// also captures libxcvt's rounded size on requests the driver can't honour
535+
// exactly (CVT 8-pixel grid + FWXGA bump for 1360x768 -> 1366x768).
536+
//
537+
// On the Neko path it re-issues the reconfig when the root never converges.
538+
// Neko applies screen configuration asynchronously and can drop/clobber a
539+
// request that lands while a previous reconfig (e.g. its boot 1920x1080) is
540+
// still in flight — the request is silently lost and X stays at the dummy DDX
541+
// default (3840x2160). Polling X harder won't recover it, only re-issuing will.
542+
// setResolutionViaNeko returns as soon as the request is accepted, so every
543+
// caller that needs the resolution to have taken effect — the live-resize path
544+
// before re-asserting the window, and the configure stop/start path before
545+
// relaunching Chromium in --kiosk — must go through here rather than call
546+
// setResolutionViaNeko directly.
547+
func (s *ApiService) applyResolutionAndConverge(ctx context.Context, width, height, refreshRate int, useNeko bool) (int, int, error) {
548+
log := logger.FromContext(ctx)
549+
var err error
550+
if useNeko {
551+
log.Info("using Neko API for Xorg resolution change")
552+
err = s.setResolutionViaNeko(ctx, width, height, refreshRate)
553+
} else {
554+
log.Info("using xrandr for Xorg resolution change (Neko disabled)")
555+
err = s.setResolutionXorgViaXrandr(ctx, width, height, refreshRate)
556+
}
557+
if err != nil {
558+
return 0, 0, err
559+
}
560+
const maxAttempts = 3
561+
var realizedW, realizedH int
562+
var converged bool
563+
for attempt := 0; attempt < maxAttempts; attempt++ {
564+
realizedW, realizedH, converged = s.waitForXRootRealized(ctx, width, height, 10*time.Second)
565+
if converged {
566+
break
567+
}
568+
if !useNeko || attempt == maxAttempts-1 {
569+
break
570+
}
571+
log.Warn("X root did not converge after resize, retrying neko reconfig",
572+
"attempt", attempt+1,
573+
"requested", fmt.Sprintf("%dx%d", width, height),
574+
"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
575+
if retryErr := s.setResolutionViaNeko(ctx, width, height, refreshRate); retryErr != nil {
576+
return realizedW, realizedH, retryErr
577+
}
578+
}
579+
if !converged {
580+
log.Error("display did not converge to requested mode after retries",
581+
"requested", fmt.Sprintf("%dx%d", width, height),
582+
"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
583+
return realizedW, realizedH, fmt.Errorf("display did not converge to %dx%d (X root reports %dx%d)", width, height, realizedW, realizedH)
584+
}
585+
return realizedW, realizedH, nil
586+
}
587+
593588
// setViewportViaCDP resizes the browser viewport using the CDP
594589
// Emulation.setDeviceMetricsOverride command. This is near-instant and does
595590
// not require restarting Chromium or Xvfb.

0 commit comments

Comments
 (0)