Skip to content

Commit ffe603a

Browse files
committed
fix(desktop): preserve resource rendering continuity
1 parent abc7a36 commit ffe603a

3 files changed

Lines changed: 91 additions & 13 deletions

File tree

apps/desktop/src/main/browser-agent/panel.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,26 @@ export function detachAttachedView(): void {
225225
}
226226
}
227227

228+
/**
229+
* Stops the attached view painting without giving up its compositor surface,
230+
* so showing it again is immediate. A hidden view takes no input either, which
231+
* is what lets renderer UI sit where it used to be.
232+
*/
233+
function hideAttachedView(): void {
234+
const view = attachedView
235+
if (!view || lastAppliedVisibility === false) return
236+
lastAppliedVisibility = false
237+
// Nothing to re-lay-out while hidden; the showing path rebinds.
238+
unbindHostResize()
239+
try {
240+
if (!view.webContents.isDestroyed()) view.setVisible(false)
241+
} catch (error) {
242+
logger.warn('Could not hide embedded browser view', {
243+
error: getErrorMessage(error, 'unknown'),
244+
})
245+
}
246+
}
247+
228248
/**
229249
* Detaches only when this exact view is the attached one. Closing a background
230250
* tab must not pull the visible tab out of the window.
@@ -297,12 +317,22 @@ export function layout(): void {
297317
const showing = active !== null && panelBounds !== null && win !== null
298318
const activeViewChanged = showing && attachedView !== active?.view
299319

300-
if (!showing || hostedWindow !== win || attachedView !== active?.view) {
301-
if (attachedView) {
302-
detachAttachedView()
303-
}
320+
// Detach only when the attached view cannot stay where it is: no tab is
321+
// active, a different tab took over, or the hosting window changed.
322+
//
323+
// A panel that is merely hidden keeps its view attached and invisible, for
324+
// the same reason occlusion does (see setPanelOccluded): removing the view
325+
// gives up its compositor surface, and rebuilding that on the way back is a
326+
// blank repaint that reads as the page having reloaded. Every switch to
327+
// another resource and back hides the panel, so that was every switch.
328+
if (
329+
attachedView !== null &&
330+
(active === null || win === null || hostedWindow !== win || attachedView !== active.view)
331+
) {
332+
detachAttachedView()
304333
}
305334
if (!showing || !active || !win || panelBounds === null) {
335+
hideAttachedView()
306336
return
307337
}
308338

apps/desktop/src/main/browser-agent/session.test.ts

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,12 +434,45 @@ describe('browser-agent session', () => {
434434
expect(content.addChildView).toHaveBeenCalledWith(tab.view)
435435
expect(view.setBounds).toHaveBeenCalledWith({ x: 100, y: 50, width: 800, height: 600 })
436436

437-
// Panel hidden: the view detaches.
437+
// Panel hidden: the view stops painting but stays attached. Detaching
438+
// would give up its compositor surface, and rebuilding that on the way
439+
// back is the blank repaint that reads as the page having reloaded —
440+
// which is every switch to another resource and back.
438441
const removeChildView = (
439442
win as unknown as { contentView: { removeChildView: ReturnType<typeof vi.fn> } }
440443
).contentView.removeChildView
444+
view.setVisible.mockClear()
441445
panel.setPanelBounds(null)
442-
expect(removeChildView).toHaveBeenCalledWith(tab.view)
446+
expect(view.setVisible).toHaveBeenCalledWith(false)
447+
expect(removeChildView).not.toHaveBeenCalled()
448+
449+
// Showing it again reuses the attached view rather than re-adding it.
450+
content.addChildView.mockClear()
451+
panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 })
452+
expect(view.setVisible).toHaveBeenLastCalledWith(true)
453+
expect(content.addChildView).not.toHaveBeenCalled()
454+
})
455+
456+
it('detaches the previous view when another tab becomes active', () => {
457+
const first = session.ensureTab()
458+
panel.setPanelBounds({ x: 0, y: 0, width: 800, height: 600 })
459+
const content = (
460+
win as unknown as {
461+
contentView: {
462+
addChildView: ReturnType<typeof vi.fn>
463+
removeChildView: ReturnType<typeof vi.fn>
464+
}
465+
}
466+
).contentView
467+
content.addChildView.mockClear()
468+
content.removeChildView.mockClear()
469+
470+
const second = session.addTab()
471+
472+
// Hiding keeps a view attached, but a tab switch still has to detach:
473+
// two native views stacked in the window would composite over each other.
474+
expect(content.removeChildView).toHaveBeenCalledWith(first.view)
475+
expect(content.addChildView).toHaveBeenCalledWith(second.view)
443476
})
444477

445478
// The measured report is the sole writer of bounds. A main-process
@@ -560,7 +593,7 @@ describe('browser-agent session', () => {
560593
expect(view.setBounds).toHaveBeenLastCalledWith({ x: 500, y: 40, width: 500, height: 760 })
561594
})
562595

563-
it('drops the resize listener when the view detaches', () => {
596+
it('drops the resize listener while the panel is hidden', () => {
564597
session.ensureTab()
565598
panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 })
566599
const onResize = hostResizeHandler(win)
@@ -835,12 +868,14 @@ describe('browser-agent session', () => {
835868
win as unknown as { contentView: { removeChildView: ReturnType<typeof vi.fn> } }
836869
).contentView
837870
contentView.removeChildView.mockClear()
871+
const view = session.requireTab().view as unknown as MockView
872+
view.setVisible.mockClear()
838873

839874
// The renderer goes silent — crashed, unmounted, or wedged. Without the
840875
// lease the native view keeps floating over whatever replaced the panel.
841876
await vi.advanceTimersByTimeAsync(6_000)
842877

843-
expect(contentView.removeChildView).toHaveBeenCalled()
878+
expect(view.setVisible).toHaveBeenCalledWith(false)
844879
} finally {
845880
vi.useRealTimers()
846881
}

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { cn, TabStrip, type TabStripItem } from '@sim/emcn'
55
import { Loader, TerminalWindow } from '@sim/emcn/icons'
6+
import { createLogger } from '@sim/logger'
67
import { FitAddon } from '@xterm/addon-fit'
78
import { Unicode11Addon } from '@xterm/addon-unicode11'
89
import { WebLinksAddon } from '@xterm/addon-web-links'
@@ -25,6 +26,8 @@ import {
2526
import { useMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context'
2627
import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store'
2728

29+
const logger = createLogger('TerminalSession')
30+
2831
/**
2932
* How long the panel must stop changing size before the PTY is told about it.
3033
* Long enough to cover a divider drag, short enough that a deliberate resize
@@ -89,16 +92,26 @@ const DARK_THEME = {
8992
* what makes rows freeze or tear mid-scroll. Disposing falls back to xterm's
9093
* DOM renderer: slower, but it cannot go stale.
9194
*
92-
* There is no canvas tier because `@xterm/addon-canvas` has no release for
93-
* xterm 6 — every published version, latest beta included, peers on xterm 5.
95+
* There is deliberately no canvas tier in between. `@xterm/addon-canvas` has
96+
* had no release since 2024 and none at all for xterm 6, while core xterm and
97+
* this addon ship in lockstep; adopting it would mean moving the core library
98+
* back a major version onto a renderer that is no longer published.
9499
*/
95100
function attachWebglRenderer(terminal: Terminal): void {
96101
try {
97102
const webgl = new WebglAddon()
98-
webgl.onContextLoss(() => webgl.dispose())
103+
webgl.onContextLoss(() => {
104+
logger.warn('Terminal WebGL context lost; falling back to the DOM renderer')
105+
webgl.dispose()
106+
})
99107
terminal.loadAddon(webgl)
100-
} catch {
101-
// No usable WebGL on this machine; the DOM renderer stays in place.
108+
} catch (error) {
109+
// The DOM renderer stays in place. Logged rather than swallowed: it is a
110+
// large, silent performance cliff, and "the terminal feels slow" is
111+
// otherwise indistinguishable from every other cause of slowness.
112+
logger.warn('Terminal WebGL unavailable; using the slower DOM renderer', {
113+
error: (error as Error).message,
114+
})
102115
}
103116
}
104117

0 commit comments

Comments
 (0)