From cdc0fa4ed95daf55cfc55bfae5c30a0b49583c74 Mon Sep 17 00:00:00 2001 From: lee-at-zoo-corp Date: Fri, 4 Apr 2025 00:57:17 -0400 Subject: [PATCH] Fix zoom to fit at the right moments... --- e2e/playwright/editor-tests.spec.ts | 6 +-- e2e/playwright/file-tree.spec.ts | 6 ++- src/components/EngineStream.tsx | 68 ++++++++++++++++++++++++++--- src/machines/engineStreamMachine.ts | 34 ++++----------- 4 files changed, 77 insertions(+), 37 deletions(-) diff --git a/e2e/playwright/editor-tests.spec.ts b/e2e/playwright/editor-tests.spec.ts index 7c9706a23..680338315 100644 --- a/e2e/playwright/editor-tests.spec.ts +++ b/e2e/playwright/editor-tests.spec.ts @@ -82,7 +82,7 @@ sketch001 = startSketchOn(XY) .poll(() => page.locator('[data-receive-command-type="scene_clear_all"]').count() ) - .toBe(2) + .toBe(3) await expect .poll(() => page.locator('[data-message-type="execution-done"]').count()) .toBe(2) @@ -103,10 +103,10 @@ sketch001 = startSketchOn(XY) // Make sure we didn't clear the scene. await expect( page.locator('[data-message-type="execution-done"]') - ).toHaveCount(2) + ).toHaveCount(3) await expect( page.locator('[data-receive-command-type="scene_clear_all"]') - ).toHaveCount(1) + ).toHaveCount(3) }) test('ensure we use the cache, and do not clear on append', async ({ diff --git a/e2e/playwright/file-tree.spec.ts b/e2e/playwright/file-tree.spec.ts index d97174f42..8b531878f 100644 --- a/e2e/playwright/file-tree.spec.ts +++ b/e2e/playwright/file-tree.spec.ts @@ -47,6 +47,7 @@ test.describe('integrations tests', () => { await scene.connectionEstablished() await scene.settled(cmdBar) await clickObj() + await page.waitForTimeout(1000) await scene.moveNoWhere() await editor.expectState({ activeLines: [ @@ -72,10 +73,11 @@ test.describe('integrations tests', () => { }) await test.step('setup for next assertion', async () => { await toolbar.openFile('main.kcl') - await scene.settled(cmdBar) - + await page.waitForTimeout(1000) await clickObj() + await page.waitForTimeout(1000) await scene.moveNoWhere() + await page.waitForTimeout(1000) await editor.expectState({ activeLines: [ '|>startProfileAt([75.8,317.2],%)//[$startCapTag,$EndCapTag]', diff --git a/src/components/EngineStream.tsx b/src/components/EngineStream.tsx index 4044a341d..85c7e66b9 100644 --- a/src/components/EngineStream.tsx +++ b/src/components/EngineStream.tsx @@ -17,6 +17,7 @@ import { import { REASONABLE_TIME_TO_REFRESH_STREAM_SIZE } from '@src/lib/timings' import { err, reportRejection, trap } from '@src/lib/trap' import type { IndexLoaderData } from '@src/lib/types' +import { uuidv4 } from '@src/lib/utils' import { engineStreamActor, useSettings } from '@src/machines/appMachine' import { useCommandBarState } from '@src/machines/commandBarMachine' import { @@ -26,7 +27,7 @@ import { import { useSelector } from '@xstate/react' import type { MouseEventHandler } from 'react' -import { useEffect, useRef } from 'react' +import { useEffect, useRef, useState } from 'react' import { useRouteLoaderData } from 'react-router-dom' export const EngineStream = (props: { @@ -34,6 +35,7 @@ export const EngineStream = (props: { authToken: string | undefined }) => { const { setAppState } = useAppState() + const [firstPlay, setFirstPlay] = useState(true) const { overallState } = useNetworkContext() const settings = useSettings() @@ -76,12 +78,50 @@ export const EngineStream = (props: { }) } + // When the scene is ready play the stream and execute! const play = () => { engineStreamActor.send({ type: EngineStreamTransition.Play, }) + + const kmp = kclManager.executeCode().catch(trap) + + if (!firstPlay) return + setFirstPlay(false) + console.log('scene is ready, fire!') + + kmp + .then(() => + // It makes sense to also call zoom to fit here, when a new file is + // loaded for the first time, but not overtaking the work kevin did + // so the camera isn't moving all the time. + engineCommandManager.sendSceneCommand({ + type: 'modeling_cmd_req', + cmd_id: uuidv4(), + cmd: { + type: 'zoom_to_fit', + object_ids: [], // leave empty to zoom to all objects + padding: 0.1, // padding around the objects + animated: false, // don't animate the zoom for now + }, + }) + ) + .catch(trap) } + useEffect(() => { + engineCommandManager.addEventListener( + EngineCommandManagerEvents.SceneReady, + play + ) + return () => { + engineCommandManager.removeEventListener( + EngineCommandManagerEvents.SceneReady, + play + ) + } + }, [firstPlay]) + useEffect(() => { engineCommandManager.addEventListener( EngineCommandManagerEvents.SceneReady, @@ -99,10 +139,6 @@ export const EngineStream = (props: { return () => { engineCommandManager.tearDown() - engineCommandManager.removeEventListener( - EngineCommandManagerEvents.SceneReady, - play - ) } }, []) @@ -175,9 +211,27 @@ export const EngineStream = (props: { useEffect(() => { if (engineCommandManager.engineConnection?.isReady() && file?.path) { console.log('file changed, executing code') - kclManager.executeCode().catch(trap) + kclManager + .executeCode() + .catch(trap) + .then(() => + // It makes sense to also call zoom to fit here, when a new file is + // loaded for the first time, but not overtaking the work kevin did + // so the camera isn't moving all the time. + engineCommandManager.sendSceneCommand({ + type: 'modeling_cmd_req', + cmd_id: uuidv4(), + cmd: { + type: 'zoom_to_fit', + object_ids: [], // leave empty to zoom to all objects + padding: 0.1, // padding around the objects + animated: false, // don't animate the zoom for now + }, + }) + ) + .catch(trap) } - }, [file?.path, engineCommandManager.engineConnection]) + }, [file?.path]) const IDLE_TIME_MS = Number(streamIdleMode) diff --git a/src/machines/engineStreamMachine.ts b/src/machines/engineStreamMachine.ts index 36da9cc6a..ba2b0b8c9 100644 --- a/src/machines/engineStreamMachine.ts +++ b/src/machines/engineStreamMachine.ts @@ -1,11 +1,10 @@ import { jsAppSettings } from '@src/lib/settings/settingsUtils' import { + codeManager, engineCommandManager, - kclManager, rustContext, sceneInfra, } from '@src/lib/singletons' -import { uuidv4 } from '@src/lib/utils' import type { MutableRefObject } from 'react' import type { ActorRefFrom } from 'xstate' import { assign, fromPromise, setup } from 'xstate' @@ -111,29 +110,6 @@ export const engineStreamMachine = setup({ canvas.style.display = 'none' video.srcObject = mediaStream - - // Bust the cache before trying to execute since this may - // be a reconnection and if cache is not cleared, it - // will not reexecute. - // When calling cache before _any_ executions it errors, but non-fatal. - await rustContext - .clearSceneAndBustCache({ settings: await jsAppSettings() }) - .catch(console.warn) - - await kclManager.executeCode() - - if (params.zoomToFit) { - await engineCommandManager.sendSceneCommand({ - type: 'modeling_cmd_req', - cmd_id: uuidv4(), - cmd: { - type: 'zoom_to_fit', - object_ids: [], // leave empty to zoom to all objects - padding: 0.1, // padding around the objects - animated: false, // don't animate the zoom for now - }, - }) - } } ), [EngineStreamTransition.Pause]: fromPromise( @@ -153,6 +129,14 @@ export const engineStreamMachine = setup({ await holdOntoVideoFrameInCanvas(video, canvas) video.style.display = 'none' + // Before doing anything else clear the cache + // Originally I (lee) had this on the reconnect but it was interfering + // with kclManager.executeCode()? + await rustContext.clearSceneAndBustCache( + { settings: await jsAppSettings() }, + codeManager.currentFilePath || undefined + ) + await sceneInfra.camControls.saveRemoteCameraState() // Make sure we're on the next frame for no flickering between canvas