Skip to content

Commit 61e8b8a

Browse files
committed
feat: integrate canvas health check into tick function of CLM.
1 parent 5d3c3f4 commit 61e8b8a

8 files changed

Lines changed: 295 additions & 243 deletions

File tree

AGENTS.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,25 @@
6060
- The `dive-demo` orientation display example now uses a single `QuickView` canvas with `displayAxes: true`; the previous side-by-side comparison against a manually wired `OrientationDisplay` plugin is no longer the expected snapshot shape
6161
- `dive-demo` views that gate UI interactivity on `QuickView` readiness, such as `DiveSwitchCanvas` and `DiveTargetAnimation`, need to wait for a non-zero canvas layout plus a small initial delay before constructing `QuickView`; on CI `Linux + xvfb + llvmpipe`, starting too early leaves control buttons permanently disabled
6262
- In `dive-demo`, replacing the fixed QuickView startup sleep with a shared layout-driven wait helper (`ResizeObserver` plus animation-frame verification) keeps the initial load stable, but canvas-switch flows still need to yield one DOM frame after committing the active-panel state before calling `mainView.setCanvas(...)`
63-
- `DIVECanvasLifecycleManager` in `src/engine/canvas/` now owns canvas parent observation, renderable-layout bootstrapping, and steady-state resize propagation through one shared `setInterval(..., 16)` bootstrap loop plus one long-lived `ResizeObserver`
64-
- `DIVECanvasLifecycleManager` now starts its shared 16ms bootstrap lazily directly inside `waitForRenderableCanvas()`; the constructor and `setCanvas()` only reset state and do not start polling on their own
65-
- `DIVECanvasLifecycleManager.waitForRenderableCanvas()` now owns both bootstrap creation and waiting inline, and falls back to a direct ready-layout read once bootstrap has finished
63+
- `DIVECanvasLifecycleManager` in `src/engine/canvas/` is again the single owner of canvas readiness state: it keeps the waiter promises, resolves `waitForRenderableCanvas()`, and advances readiness via its own `tick()`
64+
- `DIVECanvasLifecycleManager.tick()` must early-return while the current canvas remains valid; only invalid, detached, or freshly swapped canvases should re-enter the two-sample stabilization path
65+
- `DIVECanvasLifecycleManager.tick()` should stay as a shallow entrypoint that does the dispose guard and then delegates the actual lifecycle progression to the private `_checkCanvasHealth()` helper for readability
66+
- `DIVEView.tick()` should always call `DIVECanvasLifecycleManager.tick()` before honoring the paused/render path so canvas readiness can continue progressing even while rendering is paused
6667
- `DIVECanvasLifecycleManager` keeps its layout/readiness helpers as private member methods instead of top-level module helpers, so the canvas lifecycle logic stays co-located inside the class
6768
- `DIVEView.init()`, `DIVERenderer.init()`, and `DIVEEnvironment.init()` should stay `async` and explicitly `await` their cached `_initPromise` values; this repo prefers the consistent async method shape over collapsing those branches to direct promise returns
68-
- `DIVECanvasLifecycleManager.waitForRenderableCanvas()` can take an optional `AbortSignal`; aborting now only resolves that individual waiter with `null` and removes the waiter listener, while the shared bootstrap interval continues for other callers
69+
- `DIVECanvasLifecycleManager.waitForRenderableCanvas()` can take an optional `AbortSignal`; aborting resolves only that individual waiter with `null`, while the CLM's shared readiness state keeps progressing through later `tick()` calls
6970
- `DIVEView` now uses an internal `AbortController` to invalidate pending init work on `dispose()` and `setCanvas()`; even with abort support, `renderer !== this._renderer` remains as the stale-renderer guard after awaited renderer initialization
7071
- `DIVERenderer` no longer owns DOM/canvas readiness logic; it only initializes WebGPU/environment state, swaps canvases, and handles render/resize calls
7172
- The old `DIVEResizeManager` compatibility layer has been removed entirely on v3; canvas ownership now lives directly between `DIVEView` and `DIVECanvasLifecycleManager`
7273
- `DIVEView.setCanvas()` must not force an immediate `onResize()` on the swapped canvas; the `DIVECanvasLifecycleManager` is the single source of truth for resize propagation
7374
- `DIVECanvasLifecycleManager.setCanvas()` must reset its cached width/height so an equally sized replacement canvas still emits the initial resize sync for the new renderer/camera pair
74-
- In `DIVECanvasLifecycleManager`, keep raw measurement in `_getCanvasLayout()`; the readiness-gated direct-layout reuse now lives inline in `waitForRenderableCanvas()` because it only has one caller
75+
- In `DIVECanvasLifecycleManager`, keep raw measurement in `_getCanvasLayout()` and the valid-layout fast path inside `waitForRenderableCanvas()`/`tick()`; there is no longer a separate public readiness accessor
7576
- `DIVEView` should pass a named `_handleCanvasResize` callback into `DIVECanvasLifecycleManager` instead of an inline lambda, so the renderer/camera resize orchestration stays explicit while the CLM remains decoupled
7677
- `DIVEView` invalidation branches after async init are best covered by disposing the view while `renderer.init()` is still pending and by invoking the `DIVECanvasLifecycleManager` resize callback directly to assert the `onResize` + immediate render path
77-
- `DIVECanvasLifecycleManager` coverage is easiest to keep at 100% with fake timers around the shared 16ms bootstrap poll, plus a few explicit private-path tests for aborted waiter signals and direct-layout fallbacks after bootstrap
78+
- `DIVECanvasLifecycleManager` keeps a single steady-state `ResizeObserver` on the canvas itself; parent changes are handled in `tick()` as validity invalidations rather than as observed resize events
79+
- `DIVEView` does not inject the clock into `DIVECanvasLifecycleManager`; only `DIVEView` itself is a `DIVETicker`, and `DIVE.startAsync()` must start the `DIVEClock` before awaiting `mainView.init()` so the CLM's internal `tick()` can progress inside the view loop
80+
- `DIVECanvasLifecycleManager` coverage is easiest to keep at 100% with explicit `tick()` advancement in tests, observer invalidation cases, and signal-based waiter success/stale-resolution assertions
7881
- In `View.test.ts`, the `waitForRenderableCanvas` mock should be explicitly typed as `Promise<DIVECanvasLayout | null>`; otherwise the stale `null` path triggers a TypeScript error on `mockResolvedValue(null)`
82+
- In `Dive.test.ts`, `mainView.init` is typed as a plain async method, so tests should narrow it with `vi.mocked(...)` before calling mock-only helpers like `mockRejectedValueOnce` or `mockImplementationOnce`
7983
- Full focused coverage for `CanvasLifecycleManager.ts` now needs explicit tests for parentless bootstrap polling, renderable-to-zero resets during stabilization, same-size canvas swaps, waiter-only aborts, and the private direct-layout fallback after bootstrap completion
84+
- Focused single-file coverage in this repo should use `vitest --coverage.include=<path>`; running one suite with the default global `src/**/*` coverage scope still enforces repo-wide thresholds and will fail even when the targeted file itself is at 100%

src/engine/Dive.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,31 @@ export class DIVE {
205205
return;
206206
}
207207

208-
if (!this.mainView.renderer.initialized) {
209-
await this.mainView.init();
208+
if (this.mainView.renderer.initialized) {
209+
this._clock.start();
210+
} else {
211+
const initPromise = this.mainView.init();
212+
213+
queueMicrotask(() => {
214+
if (!this._disposed) {
215+
this._clock.start();
216+
}
217+
});
218+
219+
try {
220+
await initPromise;
221+
} catch (error) {
222+
this.dispose();
223+
console.error(
224+
'DIVE.startAsync: Failed to initialize. Error:',
225+
error,
226+
);
227+
}
210228
}
211229

212230
if (this._disposed) {
213231
return;
214232
}
215-
216-
this._clock.start();
217233
}
218234

219235
public stop(): void {

src/engine/__test__/Dive.test.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ vi.mock('../view/View.ts', async (importOriginal) => {
2424
const actual = await importOriginal<typeof import('../view/View.ts')>();
2525
return {
2626
...actual,
27-
DIVEView: vi.fn(function (this: any) {
27+
DIVEView: vi.fn(function (this: any, _scene, _camera, _settings) {
2828
const renderer = {
2929
initialized: false,
3030
canvas: {
@@ -54,6 +54,7 @@ vi.mock('../view/View.ts', async (importOriginal) => {
5454
},
5555
};
5656
this.canvas = renderer.canvas;
57+
this.dispose = vi.fn();
5758
return this;
5859
}),
5960
};
@@ -232,6 +233,13 @@ describe('DIVE', () => {
232233
expect(dive).toBeDefined();
233234
});
234235

236+
it('should register the main view with the clock', () => {
237+
const dive = new DIVE();
238+
239+
expect(dive.clock.addTicker).toHaveBeenCalledTimes(1);
240+
expect(dive.clock.addTicker).toHaveBeenCalledWith(dive.mainView);
241+
});
242+
235243
it('should instantiate in development DIVE_NODE_ENV', () => {
236244
process.env.DIVE_NODE_ENV = 'development';
237245
const dive = new DIVE();
@@ -298,6 +306,10 @@ describe('DIVE', () => {
298306
const dive = new DIVE(settings);
299307
await waitForAsync();
300308
expect(dive['_orientationDisplay']).toBeDefined();
309+
expect(dive.clock.addTicker).toHaveBeenCalledTimes(2);
310+
expect(dive.clock.addTicker).toHaveBeenCalledWith(
311+
dive['_orientationDisplay'],
312+
);
301313
});
302314

303315
it('should not initialize axis camera when displayAxes is false', () => {
@@ -359,12 +371,12 @@ describe('DIVE', () => {
359371
autoStart: false,
360372
});
361373

362-
dive.mainView.init.mockRejectedValueOnce(error);
374+
vi.mocked(dive.mainView.init).mockRejectedValueOnce(error);
363375
dive.start();
364376
await waitForAsync();
365377

366378
expect(errorSpy).toHaveBeenCalledWith(
367-
'DIVE.start: Failed to initialize the WebGPU renderer.',
379+
'DIVE.startAsync: Failed to initialize. Error:',
368380
error,
369381
);
370382
});
@@ -386,26 +398,26 @@ describe('DIVE', () => {
386398
expect(dive.clock.stop).toHaveBeenCalled();
387399
});
388400

389-
it('should not start the clock after dispose when renderer init resolves late', async () => {
401+
it('should queue the clock start while renderer init is pending', async () => {
390402
const dive = new DIVE({
391403
autoStart: false,
392404
});
393405
let resolveInit: (() => void) | undefined;
394406

395-
dive.mainView.init.mockImplementationOnce(
407+
vi.mocked(dive.mainView.init).mockImplementationOnce(
396408
() =>
397409
new Promise<void>((resolve) => {
398410
resolveInit = resolve;
399411
}),
400412
);
401413

402414
const pendingStart = dive.startAsync();
403-
await dive.dispose();
415+
await Promise.resolve();
416+
417+
expect(dive.clock.start).toHaveBeenCalledTimes(1);
404418

405419
resolveInit?.();
406420
await pendingStart;
407-
408-
expect(dive.clock.start).not.toHaveBeenCalled();
409421
});
410422

411423
it('should get the canvas', () => {

src/engine/canvas/CanvasLifecycleManager.ts

Lines changed: 82 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export class DIVECanvasLifecycleManager {
1313
private _height: number = 0;
1414
private _canvas: HTMLCanvasElement;
1515
private _disposed: boolean = false;
16-
private _bootstrapInterval: ReturnType<typeof setInterval> | null = null;
16+
private _isCanvasHealthy: boolean = false;
1717
private _observedParent: HTMLElement | null = null;
1818
private _pendingStableLayout: DIVECanvasLayout | null = null;
1919
private _bootstrapPromise: Promise<DIVECanvasLayout | null> | null = null;
@@ -30,18 +30,35 @@ export class DIVECanvasLifecycleManager {
3030
this._resizeObserver = new ResizeObserver((entries) => {
3131
const entry = entries[0];
3232
const { width, height } = entry.contentRect;
33+
34+
if (!this._hasRenderableSize(width, height)) {
35+
this._width = width;
36+
this._height = height;
37+
this._invalidateCanvasHealth();
38+
return;
39+
}
40+
3341
this._applyResize(width, height);
3442
});
3543
}
3644

45+
public tick(): void {
46+
if (this._disposed) {
47+
return;
48+
}
49+
50+
this._checkCanvasHealth();
51+
}
52+
3753
public setCanvas(canvas: HTMLCanvasElement): void {
3854
this._canvas = canvas;
3955
this._width = 0;
4056
this._height = 0;
4157
this._resizeObserver.disconnect();
58+
this._isCanvasHealthy = false;
4259
this._observedParent = null;
4360
this._pendingStableLayout = null;
44-
this._completeBootstrap(null);
61+
this._resolvePendingWaiters(null);
4562
}
4663

4764
public async waitForRenderableCanvas(
@@ -53,76 +70,29 @@ export class DIVECanvasLifecycleManager {
5370
}
5471

5572
if (
56-
!this._bootstrapPromise &&
57-
canvas === this._canvas &&
73+
this._isCanvasHealthy &&
5874
canvas.parentElement !== null &&
59-
this._observedParent === canvas.parentElement
75+
canvas.parentElement === this._observedParent &&
76+
this._hasRenderableSize(this._width, this._height)
6077
) {
61-
const layout = this._getCanvasLayout(canvas);
62-
63-
if (this._hasRenderableSize(layout.width, layout.height)) {
64-
return layout;
65-
}
78+
return {
79+
width: this._width,
80+
height: this._height,
81+
};
6682
}
6783

6884
if (!this._bootstrapPromise) {
6985
this._bootstrapPromise = new Promise((resolve) => {
7086
this._resolveBootstrap = resolve;
7187
});
72-
73-
this._bootstrapInterval = setInterval(() => {
74-
if (this._disposed) {
75-
this._completeBootstrap(null);
76-
return;
77-
}
78-
79-
const currentCanvas = this._canvas;
80-
const parent = currentCanvas.parentElement;
81-
82-
if (!parent) {
83-
this._observedParent = null;
84-
this._pendingStableLayout = null;
85-
return;
86-
}
87-
88-
if (parent !== this._observedParent) {
89-
this._observedParent = parent;
90-
this._pendingStableLayout = null;
91-
}
92-
93-
const layout = this._getCanvasLayout(currentCanvas);
94-
95-
if (!this._hasRenderableSize(layout.width, layout.height)) {
96-
this._pendingStableLayout = null;
97-
return;
98-
}
99-
100-
if (
101-
this._pendingStableLayout === null ||
102-
this._pendingStableLayout.width !== layout.width ||
103-
this._pendingStableLayout.height !== layout.height
104-
) {
105-
this._pendingStableLayout = layout;
106-
return;
107-
}
108-
109-
this._resizeObserver.observe(currentCanvas);
110-
this._resizeObserver.observe(parent);
111-
this._applyResize(layout.width, layout.height);
112-
this._completeBootstrap(layout);
113-
}, 16);
11488
}
11589

116-
const bootstrapPromise = this._bootstrapPromise!;
90+
const bootstrapPromise = this._bootstrapPromise;
11791

11892
if (!signal) {
11993
return await bootstrapPromise;
12094
}
12195

122-
if (signal.aborted) {
123-
return null;
124-
}
125-
12696
return await new Promise((resolve) => {
12797
const onAbort = (): void => {
12898
signal.removeEventListener('abort', onAbort);
@@ -150,22 +120,64 @@ export class DIVECanvasLifecycleManager {
150120

151121
public dispose(): void {
152122
this._disposed = true;
153-
this._completeBootstrap(null);
123+
this._isCanvasHealthy = false;
124+
this._resolvePendingWaiters(null);
154125
this._resizeObserver.disconnect();
155126
}
156127

157-
private _applyResize(width: number, height: number): void {
158-
if (width === this._width && height === this._height) {
128+
private _checkCanvasHealth(): void {
129+
const canvas = this._canvas;
130+
const parent = canvas.parentElement;
131+
132+
if (this._isCanvasHealthy) {
133+
if (
134+
parent === this._observedParent &&
135+
this._hasRenderableSize(this._width, this._height)
136+
) {
137+
return;
138+
}
139+
140+
this._invalidateCanvasHealth();
141+
}
142+
143+
if (!parent) {
159144
return;
160145
}
161146

162-
this._width = width;
163-
this._height = height;
147+
if (parent !== this._observedParent) {
148+
this._observedParent = parent;
149+
this._pendingStableLayout = null;
150+
}
151+
152+
const layout = this._getCanvasLayout(canvas);
164153

165-
if (!this._hasRenderableSize(width, height)) {
154+
if (!this._hasRenderableSize(layout.width, layout.height)) {
155+
this._pendingStableLayout = null;
166156
return;
167157
}
168158

159+
if (
160+
this._pendingStableLayout === null ||
161+
this._pendingStableLayout.width !== layout.width ||
162+
this._pendingStableLayout.height !== layout.height
163+
) {
164+
this._pendingStableLayout = layout;
165+
return;
166+
}
167+
168+
this._resizeObserver.observe(canvas);
169+
this._applyResize(layout.width, layout.height);
170+
this._isCanvasHealthy = true;
171+
this._resolvePendingWaiters(layout);
172+
}
173+
174+
private _applyResize(width: number, height: number): void {
175+
if (width === this._width && height === this._height) {
176+
return;
177+
}
178+
179+
this._width = width;
180+
this._height = height;
169181
this._onResize(width, height);
170182
}
171183

@@ -185,12 +197,14 @@ export class DIVECanvasLifecycleManager {
185197
};
186198
}
187199

188-
private _completeBootstrap(layout: DIVECanvasLayout | null): void {
189-
if (this._bootstrapInterval !== null) {
190-
clearInterval(this._bootstrapInterval);
191-
this._bootstrapInterval = null;
192-
}
200+
private _invalidateCanvasHealth(): void {
201+
this._isCanvasHealthy = false;
202+
this._observedParent = null;
203+
this._pendingStableLayout = null;
204+
this._resizeObserver.disconnect();
205+
}
193206

207+
private _resolvePendingWaiters(layout: DIVECanvasLayout | null): void {
194208
this._pendingStableLayout = null;
195209

196210
const resolveBootstrap = this._resolveBootstrap;

0 commit comments

Comments
 (0)