Merge pull request #192 from shopware/feat/canvas-readiness

feat: canvas readiness

Author: feliopterix

AGENTS.md

@@ -56,3 +56,30 @@
 - State action migrations must use `AnimationSystem.fromTargets(...).play()` and `Toolbox.enableTool()`; lingering `animate()` or `useTool()` calls can still let `yarn build` exit 0 while `vite-plugin-dts` reports TS2339 API drift
 - When swapping canvases under WebGPU, `DIVEEnvironment.setRenderer()` must run before disposing the previous `WebGPURenderer`; disposing the old renderer first can crash `PMREMGenerator.dispose()` inside Three's `NodeManager.delete` with `usedTimes` access errors
 - `OrientationDisplay.tick()` should size its overlay viewport from `DIVERenderer.canvas.clientHeight` and restore the prior `webgpurenderer.autoClear` value; unit tests can fall back to the saved viewport height when the mock omits `canvas`
+- Neighboring `dive-demo` local verification can use a `node_modules/@shopware-ag/dive` symlink to this repo when `yalc` is absent, as long as this repo's `build/` artifacts are present
+- 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
+- `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
+- 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(...)`
+- `DIVECanvasLifecycleManager` in `src/engine/canvas/` is again the single owner of canvas readiness state: it keeps the waiter promises, resolves `waitForHealthyCanvas()`, and advances readiness via its own `tick()`
+- `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
+- `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
+- `DIVEView.tick()` should always call `DIVECanvasLifecycleManager.tick()` before honoring the paused/render path so canvas readiness can continue progressing even while rendering is paused
+- `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
+- `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
+- `DIVECanvasLifecycleManager.waitForHealthyCanvas()` 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
+- `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
+- `DIVERenderer` no longer owns DOM/canvas readiness logic; it only initializes WebGPU/environment state, swaps canvases, and handles render/resize calls
+- The old `DIVEResizeManager` compatibility layer has been removed entirely on v3; canvas ownership now lives directly between `DIVEView` and `DIVECanvasLifecycleManager`
+- `DIVEView.setCanvas()` must not force an immediate `onResize()` on the swapped canvas; the `DIVECanvasLifecycleManager` is the single source of truth for resize propagation
+- `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
+- In `DIVECanvasLifecycleManager`, keep raw measurement in `_getCanvasLayout()` and the valid-layout fast path inside `waitForHealthyCanvas()`/`tick()`; there is no longer a separate public readiness accessor
+- `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
+- `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
+- `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
+- `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
+- `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
+- In `View.test.ts`, the `waitForHealthyCanvas` mock should be explicitly typed as `Promise<DIVECanvasLayout | null>`; otherwise the stale `null` path triggers a TypeScript error on `mockResolvedValue(null)`
+- `DIVECanvasLifecycleManager` now keeps its shared waiter state under `_healthyCanvasPromise` and `_resolveHealthyCanvas` so the promise naming matches the canvas-health narrative
+- 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`
+- 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
+- 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

@@ -205,15 +205,31 @@ export class DIVE {
             return;
         }
 
-        if (!this.mainView.renderer.initialized) {
-            await this.mainView.renderer.init();
+        if (this.mainView.renderer.initialized) {
+            this._clock.start();
+        } else {
+            const initPromise = this.mainView.init();
+
+            queueMicrotask(() => {
+                if (!this._disposed) {
+                    this._clock.start();
+                }
+            });
+
+            try {
+                await initPromise;
+            } catch (error) {
+                this.dispose();
+                console.error(
+                    'DIVE.startAsync: Failed to initialize. Error:',
+                    error,
+                );
+            }
         }
 
         if (this._disposed) {
             return;
         }
-
-        this._clock.start();
     }
 
     public stop(): void {

src/engine/__test__/Dive.test.ts

@@ -24,7 +24,7 @@ vi.mock('../view/View.ts', async (importOriginal) => {
     const actual = await importOriginal<typeof import('../view/View.ts')>();
     return {
         ...actual,
-        DIVEView: vi.fn(function (this: any) {
+        DIVEView: vi.fn(function (this: any, _scene, _camera, _settings) {
             const renderer = {
                 initialized: false,
                 canvas: {
@@ -34,15 +34,15 @@ vi.mock('../view/View.ts', async (importOriginal) => {
                         height: 100,
                     }),
                 },
-                init: vi.fn(() => {
-                    renderer.initialized = true;
-                    return Promise.resolve();
-                }),
                 dispose: vi.fn(),
                 onResize: vi.fn(),
                 render: vi.fn(),
                 setCanvas: vi.fn(),
             };
+            this.init = vi.fn(() => {
+                renderer.initialized = true;
+                return Promise.resolve();
+            });
             this.dispose = vi.fn();
             this.onResize = vi.fn();
             this.tick = vi.fn();
@@ -54,6 +54,7 @@ vi.mock('../view/View.ts', async (importOriginal) => {
                 },
             };
             this.canvas = renderer.canvas;
+            this.dispose = vi.fn();
             return this;
         }),
     };
@@ -232,6 +233,13 @@ describe('DIVE', () => {
         expect(dive).toBeDefined();
     });
 
+    it('should register the main view with the clock', () => {
+        const dive = new DIVE();
+
+        expect(dive.clock.addTicker).toHaveBeenCalledTimes(1);
+        expect(dive.clock.addTicker).toHaveBeenCalledWith(dive.mainView);
+    });
+
     it('should instantiate in development DIVE_NODE_ENV', () => {
         process.env.DIVE_NODE_ENV = 'development';
         const dive = new DIVE();
@@ -298,6 +306,10 @@ describe('DIVE', () => {
         const dive = new DIVE(settings);
         await waitForAsync();
         expect(dive['_orientationDisplay']).toBeDefined();
+        expect(dive.clock.addTicker).toHaveBeenCalledTimes(2);
+        expect(dive.clock.addTicker).toHaveBeenCalledWith(
+            dive['_orientationDisplay'],
+        );
     });
 
     it('should not initialize axis camera when displayAxes is false', () => {
@@ -359,12 +371,12 @@ describe('DIVE', () => {
             autoStart: false,
         });
 
-        dive.mainView.renderer.init.mockRejectedValueOnce(error);
+        vi.mocked(dive.mainView.init).mockRejectedValueOnce(error);
         dive.start();
         await waitForAsync();
 
         expect(errorSpy).toHaveBeenCalledWith(
-            'DIVE.start: Failed to initialize the WebGPU renderer.',
+            'DIVE.startAsync: Failed to initialize. Error:',
             error,
         );
     });
@@ -376,7 +388,7 @@ describe('DIVE', () => {
 
         await dive.startAsync();
 
-        expect(dive.mainView.renderer.init).toHaveBeenCalled();
+        expect(dive.mainView.init).toHaveBeenCalled();
         expect(dive.clock.start).toHaveBeenCalled();
     });
 
@@ -386,26 +398,26 @@ describe('DIVE', () => {
         expect(dive.clock.stop).toHaveBeenCalled();
     });
 
-    it('should not start the clock after dispose when renderer init resolves late', async () => {
+    it('should queue the clock start while renderer init is pending', async () => {
         const dive = new DIVE({
             autoStart: false,
         });
         let resolveInit: (() => void) | undefined;
 
-        dive.mainView.renderer.init.mockImplementationOnce(
+        vi.mocked(dive.mainView.init).mockImplementationOnce(
             () =>
                 new Promise<void>((resolve) => {
                     resolveInit = resolve;
                 }),
         );
 
         const pendingStart = dive.startAsync();
-        await dive.dispose();
+        await Promise.resolve();
+
+        expect(dive.clock.start).toHaveBeenCalledTimes(1);
 
         resolveInit?.();
         await pendingStart;
-
-        expect(dive.clock.start).not.toHaveBeenCalled();
     });
 
     it('should get the canvas', () => {