Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 43 additions & 10 deletions server/cmd/api/api/display.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,32 @@ func (s *ApiService) patchDisplayLocked(ctx context.Context, req oapi.PatchDispl
}
width, height = realizedW, realizedH
}
// Re-assert the maximized window state via CDP so the window manager
// reflows the window onto the new root; applyResolutionAndConverge already
// verified the root size, which is the authoritative post-condition. Fatal on
// failure — returning 200 with the window stuck in its old state would
// leave the caller with no signal of the mismatch. The previous approach
// of restarting chromium so it could re-apply --start-maximized had the
// same effective contract but cost ~9s per resize and wiped browser-side
// state (Emulation.* overrides, devtools sessions); restart_chromium is
// still accepted for API compatibility but no longer triggers a restart
// on this path.
var restartForRate bool
if err == nil {
var optedOut bool
restartForRate, optedOut = chromiumRestartForDisplayChange(currentWidth, currentHeight, currentRefreshRate, width, height, refreshRate, req.Body.RestartChromium)
if optedOut {
log.Warn("refresh rate changed at the same resolution but restart_chromium=false; chromium keeps rendering at its previous rate until it restarts",
"from", currentRefreshRate, "to", refreshRate)
}
}
if restartForRate {
// Chromium relaunches maximized on the new mode, so the CDP
// maximize re-assert below isn't needed.
if restartErr := s.restartChromiumAndWait(ctx, "refresh rate change"); restartErr != nil {
log.Error("failed to restart chromium after refresh rate change", "error", restartErr)
err = fmt.Errorf("restart chromium after refresh rate change: %w", restartErr)
}
} else if err == nil {
// Re-assert the maximized window state via CDP so the window manager
// reflows the window onto the new root; applyResolutionAndConverge already
// verified the root size, which is the authoritative post-condition. Fatal on
// failure — returning 200 with the window stuck in its old state would
// leave the caller with no signal of the mismatch. Size changes don't
// restart chromium: Chromium picks them up live, and a restart costs
// about a second and wipes browser-side state (Emulation.* overrides, devtools
// sessions). Only refresh-rate-only changes restart; see
// chromiumRestartForDisplayChange.
if cdpErr := s.setWindowMaximizedViaCDP(ctx); cdpErr != nil {
log.Error("CDP maximize re-assert failed after Xorg resolution change", "error", cdpErr)
err = fmt.Errorf("CDP maximize re-assert failed: %w", cdpErr)
Expand Down Expand Up @@ -199,6 +214,24 @@ func (s *ApiService) patchDisplayLocked(ctx context.Context, req oapi.PatchDispl
}, nil
}

// chromiumRestartForDisplayChange reports whether a headful display change
// needs a Chromium restart. Chromium on Linux re-reads the screen's refresh
// rate when the X screen size changes, but ignores a mode switch that only
// changes the refresh rate, so it keeps its old frame interval until it is
// relaunched. Size-only and size-plus-rate changes are picked up live. The
// sizes compared are the realized X root sizes, since libxcvt rounding can map
// a different requested size onto the current one. restartChromium=false opts
// out of the restart; optedOut reports that case so the caller can log it.
func chromiumRestartForDisplayChange(fromW, fromH, fromRate, toW, toH, toRate int, restartChromium *bool) (restart, optedOut bool) {
if toRate == fromRate || toW != fromW || toH != fromH {
return false, false
}
if restartChromium != nil && !*restartChromium {
return false, true
}
return true, false
}

// resolveDisplayParams merges the request body with the current display
// state, returning the final width, height, and refresh rate plus whether
// any field would actually change. Callers use the changed flag to skip
Expand Down
29 changes: 29 additions & 0 deletions server/cmd/api/api/display_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,35 @@ func TestResolveDisplayParams(t *testing.T) {
}
}

func TestChromiumRestartForDisplayChange(t *testing.T) {
boolPtr := func(v bool) *bool { return &v }

cases := []struct {
name string
fromW, fromH, fromRR int
toW, toH, toRR int
restartChromium *bool
wantRestart, wantSkip bool
}{
{name: "rate up at same size", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1920, toH: 1080, toRR: 60, wantRestart: true},
{name: "rate down at same size", fromW: 1920, fromH: 1080, fromRR: 60, toW: 1920, toH: 1080, toRR: 25, wantRestart: true},
{name: "rate change with restart_chromium=true", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1920, toH: 1080, toRR: 60, restartChromium: boolPtr(true), wantRestart: true},
{name: "rate change with restart_chromium=false", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1920, toH: 1080, toRR: 60, restartChromium: boolPtr(false), wantSkip: true},
{name: "size only", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1280, toH: 800, toRR: 25},
{name: "size only with restart_chromium=true", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1280, toH: 800, toRR: 25, restartChromium: boolPtr(true)},
{name: "size and rate", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1280, toH: 800, toRR: 60},
{name: "no change", fromW: 1920, fromH: 1080, fromRR: 25, toW: 1920, toH: 1080, toRR: 25},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
restart, skipped := chromiumRestartForDisplayChange(tc.fromW, tc.fromH, tc.fromRR, tc.toW, tc.toH, tc.toRR, tc.restartChromium)
assert.Equal(t, tc.wantRestart, restart)
assert.Equal(t, tc.wantSkip, skipped)
})
}
}

func TestHeadlessRefreshRateSticky(t *testing.T) {
t.Run("setViewportOverride records sticky rate", func(t *testing.T) {
svc := &ApiService{}
Expand Down
Loading