Revert "Correct hovering highlights on HiDPI screens + correct 2 flakey tests" (#5535)

* Revert "Correct hovering highlights on HiDPI screens + correct 2 flakey tests…"

This reverts commit 5743b9ced0.

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

e2e/playwright/fixtures/toolbarFixture.ts

@@ -82,16 +82,6 @@ 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,7 +170,8 @@ test.describe('Point-and-click tests', { tag: ['@skipWin'] }, () => {
         })
 
         await test.step('Clean up so that `_sketchOnAChamfer` util can be called again', async () => {
-          await toolbar.exitSketch()
+          await toolbar.exitSketchBtn.click()
+          await scene.waitForExecutionDone()
         })
         await test.step('Check there is no errors after code created in previous steps executes', async () => {
           await editor.expectState({
@@ -201,9 +202,7 @@ test.describe('Point-and-click tests', { tag: ['@skipWin'] }, () => {
       }, file)
       await page.setBodyDimensions({ width: 1000, height: 500 })
       await homePage.goToModelingScene()
-      await expect(
-        page.getByTestId('model-state-indicator-receive-reliable')
-      ).toBeVisible()
+      await scene.waitForExecutionDone()
 
       const sketchOnAChamfer = _sketchOnAChamfer(page, editor, toolbar, scene)
 
@@ -391,7 +390,6 @@ 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/snapshots/prompt-to-edit/prompt-to-edit-snapshot-tests-spec-ts--change-colour.snap.json

@@ -29,5 +29,5 @@
       }
     }
   ],
-  "kcl_version": "0.2.38"
+  "kcl_version": "0.2.39"
 }

e2e/playwright/testing-selections.spec.ts

@@ -775,7 +775,7 @@ profile003 = startProfileAt([40.16, -120.48], sketch006)
    )
   `)
     await expect(
-      page.getByTestId('model-state-indicator-receive-reliable')
+      page.getByTestId('model-state-indicator-execution-done')
     ).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, getNormalisedCoordinates } from 'lib/utils'
+import { toSync, uuidv4 } from 'lib/utils'
 import { deg2Rad } from 'lib/utils2d'
 import { isReducedMotion, roundOff, throttle } from 'lib/utils'
 import * as TWEEN from '@tweenjs/tween.js'
@@ -109,7 +109,6 @@ 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() {
@@ -457,19 +456,11 @@ 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, y },
+            selected_at_window: { x: event.clientX, y: event.clientY },
           },
           cmd_id: newCmdId,
         })

src/components/Stream.tsx

@@ -47,8 +47,6 @@ 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.
@@ -274,7 +272,7 @@ export const Stream = () => {
 
     if (btnName(e.nativeEvent).left) {
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      sendSelectEventToEngine(e)
+      sendSelectEventToEngine(e, videoRef.current)
     }
   }
 
@@ -296,7 +294,7 @@ export const Stream = () => {
       return
     }
 
-    sendSelectEventToEngine(e)
+    sendSelectEventToEngine(e, videoRef.current)
       .then(({ entity_id }) => {
         if (!entity_id) {
           // No entity selected. This is benign

src/hooks/useSetupEngineManager.ts

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

src/lang/std/engineConnection.ts

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

src/lib/selections.ts

@@ -646,17 +646,16 @@ export function codeToIdSelections(
 }
 
 export async function sendSelectEventToEngine(
-  e: React.MouseEvent<HTMLDivElement, MouseEvent>
+  e: MouseEvent | React.MouseEvent<HTMLDivElement, MouseEvent>,
+  el: HTMLVideoElement
 ) {
-  // 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 { x, y } = getNormalisedCoordinates({
+    clientX: e.clientX,
+    clientY: e.clientY,
+    el,
+    streamWidth: engineCommandManager.width,
+    streamHeight: engineCommandManager.height,
+  })
   const res = await engineCommandManager.sendSceneCommand({
     type: 'modeling_cmd_req',
     cmd: {

src/lib/utils.ts

@@ -161,20 +161,25 @@ export function toSync<F extends AsyncFn<F>>(
   }
 }
 
-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
+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
   return {
-    x: Math.round((browserX / width) * streamDimensions.width),
-    y: Math.round((browserY / height) * streamDimensions.height),
+    x: Math.round((browserX / width) * streamWidth),
+    y: Math.round((browserY / height) * streamHeight),
   }
 }