Skip to content

Commit d4a337f

Browse files
Satyam RajSatyam Raj
authored andcommitted
fix: reserve camera preview before capture exclusions
1 parent f22c9b4 commit d4a337f

4 files changed

Lines changed: 54 additions & 4 deletions

File tree

‎electron/cameraPreview.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { EventEmitter } from "node:events";
22
import type { BrowserWindow, WebContents } from "electron";
33
import { describe, expect, it, vi } from "vitest";
4+
import { collectMacCaptureExcludedWindowIds } from "../src/lib/nativeMacRecording";
45
import { CameraPreviewController } from "./cameraPreview";
56

67
function setup() {
78
const owner = Object.assign(new EventEmitter(), { send: vi.fn() });
89
const overlay = Object.assign(new EventEmitter(), {
10+
getMediaSourceId: () => "window:123:0",
911
webContents: { send: vi.fn() },
1012
showInactive: vi.fn(),
1113
hide: vi.fn(),
@@ -24,6 +26,32 @@ function setup() {
2426
const jpeg = "data:image/jpeg;base64,/9j/";
2527

2628
describe("floating camera preview lifecycle", () => {
29+
it("reserves the same exclusion ID before the renderer enables the preview", () => {
30+
const { preview, sender, overlay, create } = setup();
31+
const reserved = preview.ensure(sender);
32+
const excludedIds = collectMacCaptureExcludedWindowIds([reserved!.getMediaSourceId()]);
33+
expect(excludedIds).toContain(123);
34+
overlay.emit("ready-to-show");
35+
expect(overlay.showInactive).not.toHaveBeenCalled();
36+
preview.configure(sender, true, true);
37+
expect(preview.ensure(sender)).toBe(reserved);
38+
expect(create).toHaveBeenCalledOnce();
39+
expect(overlay.showInactive).toHaveBeenCalledOnce();
40+
expect(excludedIds).toContain(123);
41+
});
42+
43+
it("ensuring an existing preview preserves visibility and refuses a different owner", () => {
44+
const { preview, sender, overlay, create } = setup();
45+
preview.configure(sender, true, true);
46+
overlay.emit("ready-to-show");
47+
expect(preview.ensure(sender)).toBe(overlay);
48+
expect(preview.ensure({} as WebContents)).toBeNull();
49+
preview.frame(sender, jpeg);
50+
expect(overlay.webContents.send).toHaveBeenCalledWith("camera-preview:frame", jpeg);
51+
expect(overlay.hide).not.toHaveBeenCalled();
52+
expect(create).toHaveBeenCalledOnce();
53+
});
54+
2755
it("waits for the window to paint, then shows without taking focus", () => {
2856
const { preview, sender, overlay } = setup();
2957
preview.configure(sender, true, true);

‎electron/cameraPreview.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ export class CameraPreviewController {
2424
) => void,
2525
) {}
2626

27-
configure(owner: WebContents, enabled: boolean, visible: boolean): void {
28-
if (this.owner && this.owner !== owner) return;
27+
/** Reserve the capture-exclusion window before renderer configuration arrives. */
28+
ensure(owner: WebContents): BrowserWindow | null {
29+
if (this.owner && this.owner !== owner) return null;
2930
if (!this.owner) {
3031
this.owner = owner;
3132
this.onOwnerDestroyed = () => {
@@ -49,9 +50,15 @@ export class CameraPreviewController {
4950
if (this.visible) this.window?.showInactive();
5051
});
5152
}
53+
return this.window;
54+
}
55+
56+
configure(owner: WebContents, enabled: boolean, visible: boolean): void {
57+
const previewWindow = this.ensure(owner);
58+
if (!previewWindow) return;
5259
this.visible = enabled && visible;
53-
if (this.visible && this.ready) this.window.showInactive();
54-
else if (!this.visible) this.window.hide();
60+
if (this.visible && this.ready) previewWindow.showInactive();
61+
else if (!this.visible) previewWindow.hide();
5562
}
5663

5764
hide(sender: WebContents): void {

‎electron/ipc/handlers.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,6 +2147,7 @@ export function registerIpcHandlers(
21472147
* pick would record them unless the picker is told to leave them out.
21482148
*/
21492149
function macPickerExcludedWindowIds() {
2150+
ensureMacCameraPreviewWindow();
21502151
const appWindowSourceIds = [getMainWindow(), getNotesWindow(), getCameraPreviewWindow()]
21512152
.filter((window): window is BrowserWindow => !!window && !window.isDestroyed())
21522153
.map((window) => window.getMediaSourceId());
@@ -2480,6 +2481,17 @@ export function registerIpcHandlers(
24802481
createCameraPreviewWindow,
24812482
resizeCameraPreviewWindow,
24822483
);
2484+
function ensureMacCameraPreviewWindow() {
2485+
const hud = getMainWindow();
2486+
if (
2487+
process.platform === "darwin" &&
2488+
hud &&
2489+
!hud.isDestroyed() &&
2490+
new URL(hud.webContents.getURL()).searchParams.get("windowType") === "hud-overlay"
2491+
) {
2492+
cameraPreview.ensure(hud.webContents);
2493+
}
2494+
}
24832495
ipcMain.handle("camera-preview:configure", (event, enabled: unknown, visible: unknown) => {
24842496
if (
24852497
process.platform !== "darwin" ||
@@ -3175,6 +3187,7 @@ export function registerIpcHandlers(
31753187
normalizeCursorCaptureMode(request.cursor?.mode) ?? "editable-overlay";
31763188
// A source from Apple's picker records through the session that holds the pick,
31773189
// and needs no Screen Recording grant -- so nothing here may go near one.
3190+
ensureMacCameraPreviewWindow();
31783191
const pickerSession = isMacPickerSourceId(request.source.sourceId) ? macPickerSession : null;
31793192
// Normally caught by `get-selected-source` first; this is the last word (#965).
31803193
pickerSession?.forgetSelectionUnlessExcluding(macPickerExcludedWindowIds());

‎technical-documentation/testing/manual-e2e-checklist.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,3 +999,5 @@ These checks are the human slice: an agent cannot click Apple's picker (see abov
999999
| 2026-10-19 | dev `codex/macos-floating-camera-preview`, rebased on `main` @ `3620cf8`, plus #1288 review changes; Electron 41.2.1; native helper rebuilt directly with `swiftc` + macOS 26.5 SDK | macOS 26.7, Apple M5 Pro, MacBook Pro Camera | **Partial — preview review slice passed** | Computer-use verified the complete rounded rectangular camera frame, aspect-preserving resize, camera-off hiding the HUD preview control, and show/hide during a finalized camera take. In the same take, after 5 s settling per condition, 30 s process-tree CPU deltas were **17.5% hidden / 22.3% shown** (100%=one core; includes Electron children and capture helper). Native `recording-stopped` reported **0 droppedVideoFrames over the entire take**, covering both intervals; this counts screen-writer backpressure/append failures, not upstream capture or webcam drops. Screen 1 at 3024×1964, saved webcam verified 1920×1080 (30 fps requested); mic/system audio off, system cursor. One sequential pair, no statistical claim. **Skipped:** Apple’s picker, fullscreen Spaces, other cameras/platforms, restart/cancel, export, exhaustive exclusion and remaining checklist. |
10001000

10011001
| 2026-10-19 | dev #1288 on `8c2608d9` + preview lifetime/quit fixes; Electron 41.2.1 | macOS 26.7, Apple Silicon, MacBook Pro Camera | **Partial — lifecycle slice passed** | Computer-use camera off/on removed/restored the HUD preview control. Normal Cmd+Q exited the OpenScreen Electron process with the preview enabled. Unit tests verify the same preview window survives initial camera-off and repeated toggles, with one owner-destruction listener and cleanup on HUD destruction. **Skipped:** recording, export, capture-exclusion measurements, other platforms and remaining checklist; no native capture changes in this slice. |
1002+
1003+
| 2026-10-21 | dev #1288 on `20975d30` + synchronous preview reservation before capture exclusions; Electron 41.2.1; existing helper rebuilt from this branch on Oct 19 | macOS 26.7, Apple Silicon, MacBook Pro Camera | **Partial — capture-exclusion ordering slice passed** | Computer-use started native display recording with the preview hidden, showed it during the take and finalized into the editor. Helper diagnostics resolved both requested app window IDs and reported zero screen-writer drops; a decoded late screen frame contained no floating preview. Unit regression covers reserving the exclusion window before any renderer configuration and reusing it when enabled later without altering an existing preview’s visibility. **Skipped:** Apple’s picker (ordering path checked in code), forced renderer-effect race on desktop (covered by unit regression), other platforms, export, exhaustive frame-by-frame exclusion and remaining checklist. |

0 commit comments

Comments
 (0)