Merge branch 'main' into pierremtb/adhoc/pnpm

.github/workflows/e2e-tests.yml

@@ -143,7 +143,7 @@ jobs:
       # TODO: break this in its own job, for now it's not slowing down the overall execution as ubuntu is the quickest,
       # but we could do better. This forces a large 1/1 shard of all 20 snapshot tests that runs in about 3 minutes.
       run: |
-        PLATFORM=web pnpm playwright test --config=playwright.config.ts --retries="3" --update-snapshots --grep=@snapshot  --shard=1/1
+        PLATFORM=web pnpm playwright test --config=playwright.config.ts --retries="3" --update-snapshots --grep=@snapshot --trace=on --shard=1/1
       env:
         CI: true
         NODE_ENV: development
@@ -154,7 +154,7 @@ jobs:
     - uses: actions/upload-artifact@v4
       if: ${{ !cancelled() && (success() || failure()) }}
       with:
-        name: playwright-report-${{ matrix.os }}-snapshot-${{ matrix.shardIndex }}-${{ github.sha }}
+        name: playwright-report-snapshots-${{ matrix.os }}-snapshot-${{ matrix.shardIndex }}-${{ github.sha }}
         path: playwright-report/
         include-hidden-files: true
         retention-days: 30

e2e/playwright/fixtures/toolbarFixture.ts

@@ -82,6 +82,16 @@ export class ToolbarFixture {
   startSketchPlaneSelection = async () =>
     doAndWaitForImageDiff(this.page, () => this.startSketchBtn.click(), 500)
 
+  exitSketch = async () => {
+    await this.exitSketchBtn.click()
+    await expect(
+      this.page.getByRole('button', { name: 'Start Sketch' })
+    ).toBeVisible()
+    await expect(
+      this.page.getByRole('button', { name: 'Start Sketch' })
+    ).not.toBeDisabled()
+  }
+
   editSketch = async () => {
     await this.editSketchBtn.first().click()
     // One of the rare times we want to allow a arbitrary wait

e2e/playwright/point-click.spec.ts

@@ -170,8 +170,7 @@ test.describe('Point-and-click tests', { tag: ['@skipWin'] }, () => {
         })
 
         await test.step('Clean up so that `_sketchOnAChamfer` util can be called again', async () => {
-          await toolbar.exitSketchBtn.click()
-          await scene.waitForExecutionDone()
+          await toolbar.exitSketch()
         })
         await test.step('Check there is no errors after code created in previous steps executes', async () => {
           await editor.expectState({
@@ -202,7 +201,9 @@ test.describe('Point-and-click tests', { tag: ['@skipWin'] }, () => {
       }, file)
       await page.setBodyDimensions({ width: 1000, height: 500 })
       await homePage.goToModelingScene()
-      await scene.waitForExecutionDone()
+      await expect(
+        page.getByTestId('model-state-indicator-receive-reliable')
+      ).toBeVisible()
 
       const sketchOnAChamfer = _sketchOnAChamfer(page, editor, toolbar, scene)
 
@@ -390,6 +391,7 @@ profile001 = startProfileAt([205.96, 254.59], sketch002)
       }, file)
       await page.setBodyDimensions({ width: 1000, height: 500 })
       await homePage.goToModelingScene()
+
       await scene.waitForExecutionDone()
 
       const sketchOnAChamfer = _sketchOnAChamfer(page, editor, toolbar, scene)

e2e/playwright/testing-selections.spec.ts

@@ -775,7 +775,7 @@ profile003 = startProfileAt([40.16, -120.48], sketch006)
    )
   `)
     await expect(
-      page.getByTestId('model-state-indicator-execution-done')
+      page.getByTestId('model-state-indicator-receive-reliable')
     ).toBeVisible()
 
     await u.openAndClearDebugPanel()

src/clientSideScene/CameraControls.ts

@@ -22,7 +22,7 @@ import {
   UnreliableSubscription,
 } from 'lang/std/engineConnection'
 import { EngineCommand } from 'lang/std/artifactGraph'
-import { toSync, uuidv4 } from 'lib/utils'
+import { toSync, uuidv4, getNormalisedCoordinates } from 'lib/utils'
 import { deg2Rad } from 'lib/utils2d'
 import { isReducedMotion, roundOff, throttle } from 'lib/utils'
 import * as TWEEN from '@tweenjs/tween.js'
@@ -109,6 +109,7 @@ export class CameraControls {
   interactionGuards: MouseGuard = cameraMouseDragGuards.Zoo
   isFovAnimationInProgress = false
   perspectiveFovBeforeOrtho = 45
+
   // NOTE: Duplicated state across Provider and singleton. Mapped from settingsMachine
   _setting_allowOrbitInSketchMode = false
   get isPerspective() {
@@ -456,11 +457,19 @@ export class CameraControls {
       if (this.syncDirection === 'engineToClient') {
         const newCmdId = uuidv4()
 
+        // Nonsense to do anything until the video stream is established.
+        if (!this.engineCommandManager.elVideo) return
+
+        const { x, y } = getNormalisedCoordinates(
+          event,
+          this.engineCommandManager.elVideo,
+          this.engineCommandManager.streamDimensions
+        )
         this.throttledEngCmd({
           type: 'modeling_cmd_req',
           cmd: {
             type: 'highlight_set_entity',
-            selected_at_window: { x: event.clientX, y: event.clientY },
+            selected_at_window: { x, y },
           },
           cmd_id: newCmdId,
         })

src/components/Stream.tsx

@@ -47,6 +47,8 @@ export const Stream = () => {
     overallState === NetworkHealthState.Ok ||
     overallState === NetworkHealthState.Weak
 
+  engineCommandManager.elVideo = videoRef.current
+
   /**
    * Execute code and show a "building scene message"
    * in Stream.tsx in the meantime.
@@ -272,7 +274,7 @@ export const Stream = () => {
 
     if (btnName(e.nativeEvent).left) {
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      sendSelectEventToEngine(e, videoRef.current)
+      sendSelectEventToEngine(e)
     }
   }
 
@@ -294,7 +296,7 @@ export const Stream = () => {
       return
     }
 
-    sendSelectEventToEngine(e, videoRef.current)
+    sendSelectEventToEngine(e)
       .then(({ entity_id }) => {
         if (!entity_id) {
           // No entity selected. This is benign

src/hooks/useSetupEngineManager.ts

@@ -101,10 +101,7 @@ export function useSetupEngineManager(
         streamRef?.current?.offsetWidth ?? 0,
         streamRef?.current?.offsetHeight ?? 0
       )
-      engineCommandManager.handleResize({
-        streamWidth: width,
-        streamHeight: height,
-      })
+      engineCommandManager.handleResize(engineCommandManager.streamDimensions)
     }, 500)
 
     const onOnline = () => {

src/lang/std/engineConnection.ts

@@ -1447,11 +1447,17 @@ export class EngineCommandManager extends EventTarget {
     commandId: string
   }
   settings: SettingsViaQueryString
-  width: number = 1337
-  height: number = 1337
+
+  streamDimensions = {
+    // Random defaults that are overwritten pretty much immediately
+    width: 1337,
+    height: 1337,
+  }
+
+  elVideo: HTMLVideoElement | null = null
 
   /**
-   * Export intent traxcks the intent of the export. If it is null there is no
+   * Export intent tracks the intent of the export. If it is null there is no
    * export in progress. Otherwise it is an enum value of the intent.
    * Another export cannot be started if one is already in progress.
    */
@@ -1554,15 +1560,14 @@ export class EngineCommandManager extends EventTarget {
       return
     }
 
-    this.width = width
-    this.height = height
+    this.streamDimensions = {
+      width,
+      height,
+    }
 
     // If we already have an engine connection, just need to resize the stream.
     if (this.engineConnection) {
-      this.handleResize({
-        streamWidth: width,
-        streamHeight: height,
-      })
+      this.handleResize(this.streamDimensions)
       return
     }
 
@@ -1858,27 +1863,22 @@ export class EngineCommandManager extends EventTarget {
     return
   }
 
-  handleResize({
-    streamWidth,
-    streamHeight,
-  }: {
-    streamWidth: number
-    streamHeight: number
-  }) {
+  handleResize({ width, height }: { width: number; height: number }) {
     if (!this.engineConnection?.isReady()) {
       return
     }
 
-    this.width = streamWidth
-    this.height = streamHeight
+    this.streamDimensions = {
+      width,
+      height,
+    }
 
     const resizeCmd: EngineCommand = {
       type: 'modeling_cmd_req',
       cmd_id: uuidv4(),
       cmd: {
         type: 'reconfigure_stream',
-        width: streamWidth,
-        height: streamHeight,
+        ...this.streamDimensions,
         fps: 60,
       },
     }

src/lib/selections.ts

@@ -646,16 +646,17 @@ export function codeToIdSelections(
 }
 
 export async function sendSelectEventToEngine(
-  e: MouseEvent | React.MouseEvent<HTMLDivElement, MouseEvent>,
-  el: HTMLVideoElement
+  e: React.MouseEvent<HTMLDivElement, MouseEvent>
 ) {
-  const { x, y } = getNormalisedCoordinates({
-    clientX: e.clientX,
-    clientY: e.clientY,
-    el,
-    streamWidth: engineCommandManager.width,
-    streamHeight: engineCommandManager.height,
-  })
+  // No video stream to normalise against, return immediately
+  if (!engineCommandManager.elVideo)
+    return Promise.reject('video element not ready')
+
+  const { x, y } = getNormalisedCoordinates(
+    e,
+    engineCommandManager.elVideo,
+    engineCommandManager.streamDimensions
+  )
   const res = await engineCommandManager.sendSceneCommand({
     type: 'modeling_cmd_req',
     cmd: {

src/lib/utils.ts

@@ -161,25 +161,20 @@ export function toSync<F extends AsyncFn<F>>(
   }
 }
 
-export function getNormalisedCoordinates({
-  clientX,
-  clientY,
-  streamWidth,
-  streamHeight,
-  el,
-}: {
-  clientX: number
-  clientY: number
-  streamWidth: number
-  streamHeight: number
-  el: HTMLElement
-}) {
-  const { left, top, width, height } = el?.getBoundingClientRect()
-  const browserX = clientX - left
-  const browserY = clientY - top
+export function getNormalisedCoordinates(
+  e: PointerEvent | React.MouseEvent<HTMLDivElement, MouseEvent>,
+  elVideo: HTMLVideoElement,
+  streamDimensions: {
+    width: number
+    height: number
+  }
+) {
+  const { left, top, width, height } = elVideo?.getBoundingClientRect()
+  const browserX = e.clientX - left
+  const browserY = e.clientY - top
   return {
-    x: Math.round((browserX / width) * streamWidth),
-    y: Math.round((browserY / height) * streamHeight),
+    x: Math.round((browserX / width) * streamDimensions.width),
+    y: Math.round((browserY / height) * streamDimensions.height),
   }
 }
 

src/wasm-lib/Cargo.lock

@@ -730,7 +730,7 @@ dependencies = [
 
 [[package]]
 name = "derive-docs"
-version = "0.1.38"
+version = "0.1.39"
 dependencies = [
  "Inflector",
  "anyhow",
@@ -1724,7 +1724,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-lib"
-version = "0.2.38"
+version = "0.2.39"
 dependencies = [
  "anyhow",
  "approx 0.5.1",
@@ -1791,7 +1791,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-test-server"
-version = "0.1.38"
+version = "0.1.39"
 dependencies = [
  "anyhow",
  "hyper 0.14.32",

src/wasm-lib/derive-docs/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "derive-docs"
 description = "A tool for generating documentation from Rust derive macros"
-version = "0.1.38"
+version = "0.1.39"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

src/wasm-lib/kcl-test-server/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-test-server"
 description = "A test server for KCL"
-version = "0.1.38"
+version = "0.1.39"
 edition = "2021"
 license = "MIT"
 

src/wasm-lib/kcl/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-lib"
 description = "KittyCAD Language implementation and tools"
-version = "0.2.38"
+version = "0.2.39"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"