fix(playback): run embedded MPV position poll outside the Angular zone

With provideZoneChangeDetection a zone-registered interval runs app-wide
change detection every 500 ms for the entire stream. Register the drift
poll via NgZone.runOutsideAngular and never re-enter the zone — the drift
path is rAF → setEmbeddedMpvBounds IPC and touches no Angular state.
Frame-copy sessions skip the measurement entirely: the canvas is laid out
by the DOM and moves with the layout, so only the native-view child
window can go stale on a position-only shift.

Addresses the Codex P2 review finding on #1476.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-08-24 10:31:11 +02:00
1 parent 6c440fc437
commit 52e6471a0a
3 files changed
+109 -9

No files matched your search

+7 -1
View File
@@ -675,7 +675,13 @@ stale coordinates and rendered offset from the DOM stage. The session
controller therefore polls the host bounds every 500 ms while a session is
active, compares them against the last synced bounds with a half-pixel
tolerance, and schedules a re-sync only on drift — idle cost is one
`getBoundingClientRect` per tick with no IPC.
`getBoundingClientRect` per tick with no IPC. The interval is registered
outside Angular's zone (a zone timer would run app-wide change detection
every tick for the whole stream) and never re-enters it, because the drift
path is rAF → `setEmbeddedMpvBounds` IPC and touches no Angular state.
Frame-copy skips the measurement entirely: its canvas is laid out by the
DOM and moves with the layout, so only the native-view child window can go
stale on a position-only shift.
### Controls ownership by engine
@@ -1,3 +1,4 @@
import { NgZone } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import {
EmbeddedMpvSession,
@@ -24,6 +25,8 @@ describe('EmbeddedMpvSessionController position drift poll', () => {
disposeEmbeddedMpvSession: jest.Mock;
setEmbeddedMpvBounds: jest.Mock;
onEmbeddedMpvSessionUpdate: jest.Mock;
attachEmbeddedMpvFrameView?: jest.Mock;
detachEmbeddedMpvFrameView?: jest.Mock;
};
beforeEach(() => {
@@ -126,8 +129,82 @@ describe('EmbeddedMpvSessionController position drift poll', () => {
await jest.advanceTimersByTimeAsync(2000);
expect(electron.setEmbeddedMpvBounds).not.toHaveBeenCalled();
});
it('registers the poll outside the Angular zone', async () => {
// With zone change detection, a zone-registered interval would run
// app-wide change detection every 500 ms for the whole stream.
const zone = TestBed.inject(NgZone);
const runOutsideAngular = jest.spyOn(zone, 'runOutsideAngular');
const controller = TestBed.inject(EmbeddedMpvSessionController);
const teardown = controller.startSession(
createHost(),
createPlayback(),
0.5
);
await waitFor(
() => controller.sessionId() === 'mpv-1',
'session to start'
);
expect(runOutsideAngular).toHaveBeenCalled();
teardown();
});
it('skips drift measurement for the frame-copy engine', async () => {
// Frame-copy paints into a DOM canvas that moves with the layout, so
// a position-only shift needs no re-sync (size changes still arrive
// through ResizeObserver).
electron.getEmbeddedMpvSupport.mockResolvedValue({
supported: true,
platform: 'linux',
engine: 'frame-copy',
});
electron.attachEmbeddedMpvFrameView = jest
.fn()
.mockResolvedValue(true);
electron.detachEmbeddedMpvFrameView = jest.fn();
const rect = { left: 10, top: 20, width: 640, height: 360 };
const host = {
getBoundingClientRect: () => ({ ...rect }),
} as HTMLElement;
const controller = TestBed.inject(EmbeddedMpvSessionController);
await waitFor(
() => controller.support()?.engine === 'frame-copy',
'support to load'
);
const teardown = controller.startSession(host, createPlayback(), 0.5);
await waitFor(
() => controller.sessionId() === 'mpv-1',
'session to start'
);
await waitFor(
() => electron.setEmbeddedMpvBounds.mock.calls.length > 0,
'initial bounds sync'
);
electron.setEmbeddedMpvBounds.mockClear();
rect.left = 29;
await jest.advanceTimersByTimeAsync(2000);
expect(electron.setEmbeddedMpvBounds).not.toHaveBeenCalled();
teardown();
});
});
function createHost(): HTMLElement {
return {
getBoundingClientRect: () => ({
left: 10,
top: 20,
width: 640,
height: 360,
}),
} as HTMLElement;
}
function createPlayback(): ResolvedPortalPlayback {
return {
streamUrl: 'https://example.com/movie.mp4',
@@ -1,6 +1,7 @@
import {
DestroyRef,
Injectable,
NgZone,
computed,
effect,
inject,
@@ -75,6 +76,7 @@ export class EmbeddedMpvSessionController {
);
private readonly destroyRef = inject(DestroyRef);
private readonly zone = inject(NgZone);
private readonly unsubscribeSessionUpdate?: () => void;
private boundsProvider: EmbeddedMpvBoundsProvider = (host) =>
@@ -182,14 +184,29 @@ export class EmbeddedMpvSessionController {
// the measured bounds against the last synced ones and re-syncs only
// on drift, so the idle cost is one getBoundingClientRect per tick
// with no IPC.
const positionPoll = window.setInterval(() => {
if (!activeSessionId || !lastSyncedBounds) {
return;
}
if (boundsDiffer(this.boundsProvider(host), lastSyncedBounds)) {
scheduleBoundsSync();
}
}, POSITION_POLL_INTERVAL_MS);
//
// The interval runs outside Angular's zone: with zone change
// detection a zone-registered timer would run app-wide change
// detection every tick for the whole stream. It never re-enters the
// zone — the drift path is rAF → setEmbeddedMpvBounds IPC and
// touches no Angular state. Frame-copy paints into a DOM canvas
// that moves with the layout, so only native-view can go stale on a
// position-only shift; the poll skips the measurement there.
const positionPoll = this.zone.runOutsideAngular(() =>
window.setInterval(() => {
if (!activeSessionId || !lastSyncedBounds) {
return;
}
if (untracked(() => this.isFrameCopyEngine())) {
return;
}
if (
boundsDiffer(this.boundsProvider(host), lastSyncedBounds)
) {
scheduleBoundsSync();
}
}, POSITION_POLL_INTERVAL_MS)
);
// Page zoom and monitor DPI rescale the CSS→native-pixel mapping the
// backend applies to these bounds. Moving the window to a display