fix bug and remove flash in sketch mode (#6346)

* fix bug and remove flash

* add test

* fix tests

* fix tests

e2e/playwright/fixtures/sceneFixture.ts

@@ -257,6 +257,14 @@ export class SceneFixture {
     await expectPixelColor(this.page, colour, coords, diff)
   }
 
+  expectPixelColorNotToBe = async (
+    colour: [number, number, number] | [number, number, number][],
+    coords: { x: number; y: number },
+    diff: number
+  ) => {
+    await expectPixelColorNotToBe(this.page, colour, coords, diff)
+  }
+
   get gizmo() {
     return this.page.locator('[aria-label*=gizmo]')
   }
@@ -278,37 +286,69 @@ function isColourArray(
   return isArray(colour[0])
 }
 
-export async function expectPixelColor(
+type PixelColorMatchMode = 'matches' | 'differs'
+
+export async function checkPixelColor(
   page: Page,
   colour: [number, number, number] | [number, number, number][],
   coords: { x: number; y: number },
-  diff: number
+  diff: number,
+  mode: PixelColorMatchMode
 ) {
   let finalValue = colour
+  const isMatchMode = mode === 'matches'
+  const actionText = isMatchMode ? 'expecting' : 'not expecting'
+  const functionName = isMatchMode
+    ? 'ExpectPixelColor'
+    : 'ExpectPixelColourNotToBe'
+
   await expect
     .poll(
       async () => {
         const pixel = (await getPixelRGBs(page)(coords, 1))[0]
         if (!pixel) return null
         finalValue = pixel
+
+        let matches
         if (!isColourArray(colour)) {
-          return pixel.every(
+          matches = pixel.every(
             (channel, index) => Math.abs(channel - colour[index]) < diff
           )
+        } else {
+          matches = colour.some((c) =>
+            c.every((channel, index) => Math.abs(pixel[index] - channel) < diff)
+          )
         }
-        return colour.some((c) =>
+
-          c.every((channel, index) => Math.abs(pixel[index] - channel) < diff)
+        return isMatchMode ? matches : !matches
-        )
       },
       { timeout: 10_000 }
     )
     .toBeTruthy()
     .catch((cause) => {
       throw new Error(
-        `ExpectPixelColor: point ${JSON.stringify(
+        `${functionName}: point ${JSON.stringify(
           coords
-        )} was expecting ${colour} but got ${finalValue}`,
+        )} was ${actionText} ${colour} but got ${finalValue}`,
         { cause }
       )
     })
 }
+
+export async function expectPixelColor(
+  page: Page,
+  colour: [number, number, number] | [number, number, number][],
+  coords: { x: number; y: number },
+  diff: number
+) {
+  await checkPixelColor(page, colour, coords, diff, 'matches')
+}
+
+export async function expectPixelColorNotToBe(
+  page: Page,
+  colour: [number, number, number] | [number, number, number][],
+  coords: { x: number; y: number },
+  diff: number
+) {
+  await checkPixelColor(page, colour, coords, diff, 'differs')
+}

e2e/playwright/sketch-tests.spec.ts

@@ -3069,10 +3069,8 @@ test.describe('manual edits during sketch mode', () => {
     sketch002 = startSketchOn(extrude001, face = seg01)
     profile002 = startProfileAt([83.39, 329.15], sketch002)
       |> angledLine(angle = 0, length = 119.61, tag = $rectangleSegmentA001)
-      |> angledLine(angle = segAng(rectangleSegmentA001) - 90, length = 156.54, angle = -28)
+      |> angledLine(length = 156.54, angle = -28)
       |> angledLine(
-          angle = segAng(rectangleSegmentA001),
-          length = -segLen(rectangleSegmentA001),
           angle = -151,
           length = 116.27,
         )
@@ -3112,14 +3110,23 @@ test.describe('manual edits during sketch mode', () => {
 
     await test.step('Edit sketch by dragging handle', async () => {
       await page.waitForTimeout(500)
-      await editor.expectEditor.toContain('length = 156.54, angle = -28')
+      await expect
-      await page.mouse.move(handle1Location.x, handle1Location.y)
+        .poll(async () => {
-      await page.mouse.down()
+          await editor.expectEditor.toContain('length = 156.54, angle = -28')
-      await page.mouse.move(handle1Location.x + 50, handle1Location.y + 50, {
+          await page.mouse.move(handle1Location.x, handle1Location.y)
-        steps: 5,
+          await page.mouse.down()
-      })
+          await page.mouse.move(
-      await page.mouse.up()
+            handle1Location.x + 50,
-      await editor.expectEditor.toContain('length = 231.59, angle = -34')
+            handle1Location.y + 50,
+            {
+              steps: 5,
+            }
+          )
+          await page.mouse.up()
+          await editor.expectEditor.toContain('length = 231.59, angle = -34')
+          return true
+        })
+        .toBeTruthy()
       // await page.waitForTimeout(1000) // Wait for update
     })
 
@@ -3136,13 +3143,18 @@ test.describe('manual edits during sketch mode', () => {
     await test.step('Edit sketch again', async () => {
       await editor.expectEditor.toContain('length = 231.59, angle = -34')
       await page.waitForTimeout(500)
-      await page.mouse.move(handle2Location.x, handle2Location.y)
+      await expect
-      await page.mouse.down()
+        .poll(async () => {
-      await page.mouse.move(handle2Location.x, handle2Location.y - 50, {
+          await page.mouse.move(handle2Location.x, handle2Location.y)
-        steps: 5,
+          await page.mouse.down()
-      })
+          await page.mouse.move(handle2Location.x, handle2Location.y - 50, {
-      await page.mouse.up()
+            steps: 5,
-      await editor.expectEditor.toContain('length = 167.36, angle = -14')
+          })
+          await page.mouse.up()
+          await editor.expectEditor.toContain('length = 167.36, angle = -14')
+          return true
+        })
+        .toBeTruthy()
     })
 
     await test.step('add whole other sketch before current sketch', async () => {
@@ -3159,14 +3171,20 @@ test.describe('manual edits during sketch mode', () => {
 
     const handle3Location = { x: 844, y: 212 }
     await test.step('edit sketch again', async () => {
-      await editor.expectEditor.toContain('length = 167.36, angle = -14')
+      await page.waitForTimeout(500) // Wait for deferred execution
-      await page.mouse.move(handle3Location.x, handle3Location.y)
+      await expect
-      await page.mouse.down()
+        .poll(async () => {
-      await page.mouse.move(handle3Location.x, handle3Location.y + 110, {
+          await editor.expectEditor.toContain('length = 167.36, angle = -14')
-        steps: 5,
+          await page.mouse.move(handle3Location.x, handle3Location.y)
-      })
+          await page.mouse.down()
-      await page.mouse.up()
+          await page.mouse.move(handle3Location.x, handle3Location.y + 110, {
-      await editor.expectEditor.toContain('length = 219.2, angle = -56')
+            steps: 5,
+          })
+          await page.mouse.up()
+          await editor.expectEditor.toContain('length = 219.2, angle = -56')
+          return true
+        })
+        .toBeTruthy()
     })
 
     // exit sketch and assert whole code
@@ -3174,32 +3192,27 @@ test.describe('manual edits during sketch mode', () => {
       await toolbar.exitSketch()
       await editor.expectEditor.toContain(
         `myVar1 = 5
-    sketch003 = startSketchOn(XY)
+sketch003 = startSketchOn(XY)
-    profile004 = circle(sketch003, center = [143.91, 136.89], radius = 71.63)
+profile004 = circle(sketch003, center = [143.91, 136.89], radius = 71.63)
-    
+
-    sketch001 = startSketchOn(XZ)
+sketch001 = startSketchOn(XZ)
-    profile001 = startProfileAt([106.68, 89.77], sketch001)
+profile001 = startProfileAt([106.68, 89.77], sketch001)
-      |> line(end = [132.34, 157.8])
+  |> line(end = [132.34, 157.8])
-      |> line(end = [67.65, -460.55], tag = $seg01)
+  |> line(end = [67.65, -460.55], tag = $seg01)
-      |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
-      |> close()
+  |> close()
-    extrude001 = extrude(profile001, length = 500)
+extrude001 = extrude(profile001, length = 500)
-    sketch002 = startSketchOn(extrude001, face = seg01)
+sketch002 = startSketchOn(extrude001, face = seg01)
-    profile002 = startProfileAt([83.39, 329.15], sketch002)
+profile002 = startProfileAt([83.39, 329.15], sketch002)
-      |> angledLine(angle = 0, length = 119.61, tag = $rectangleSegmentA001)
+  |> angledLine(angle = 0, length = 119.61, tag = $rectangleSegmentA001)
-      |> angledLine(angle = segAng(rectangleSegmentA001) - 90, length = 219.2, angle = -56)
+  |> angledLine(length = 219.2, angle = -56)
-      |> angledLine(
+  |> angledLine(angle = -151, length = 116.27)
-          angle = segAng(rectangleSegmentA001),
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
-          length = -segLen(rectangleSegmentA001),
+  |> close()
-          angle = -151,
+profile003 = startProfileAt([-201.08, 254.17], sketch002)
-          length = 116.27,
+  |> line(end = [103.55, 33.32])
-        )
+  |> line(end = [48.8, -153.54])
-      |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+`,
-      |> close()
-    profile003 = startProfileAt([-201.08, 254.17], sketch002)
-      |> line(end = [103.55, 33.32])
-      |> line(end = [48.8, -153.54])
-    `,
         { shouldNormalise: true }
       )
       await editor.expectState({
@@ -3231,10 +3244,8 @@ test.describe('manual edits during sketch mode', () => {
     sketch002 = startSketchOn(extrude001, face = seg01)
     profile002 = startProfileAt([83.39, 329.15], sketch002)
       |> angledLine(angle = 0, length = 119.61, tag = $rectangleSegmentA001)
-      |> angledLine(angle = segAng(rectangleSegmentA001) - 90, length = 156.54, angle = -28)
+      |> angledLine(length = 156.54, angle = -28)
       |> angledLine(
-           angle = segAng(rectangleSegmentA001),
-           length = -segLen(rectangleSegmentA001),
            angle = -151,
            length = 116.27,
          )
@@ -3350,7 +3361,21 @@ test.describe('manual edits during sketch mode', () => {
         // this checks sketch segments have been drawn
         await verifyArrowHeadColor(arrowHeadWhite)
       })
-      await page.waitForTimeout(100)
+
+      await test.step('make a change to the code and expect pixel color to change', async () => {
+        // defends against a regression where sketch would duplicate in the scene
+        // https://github.com/KittyCAD/modeling-app/issues/6345
+        await editor.replaceCode(
+          'startProfileAt([75.8, 317.2',
+          'startProfileAt([75.8, 217.2'
+        )
+        // expect not white anymore
+        await scene.expectPixelColorNotToBe(
+          TEST_COLORS.WHITE,
+          arrowHeadLocation,
+          15
+        )
+      })
     }
   )
 })

src/clientSideScene/sceneEntities.ts

@@ -669,7 +669,10 @@ export class SceneEntities {
     variableDeclarationName: string
   }> {
     const prepared = this.prepareTruncatedAst(sketchNodePaths, maybeModdedAst)
-    if (err(prepared)) return Promise.reject(prepared)
+    if (err(prepared)) {
+      this.tearDownSketch({ removeAxis: false })
+      return Promise.reject(prepared)
+    }
     const { truncatedAst, variableDeclarationName } = prepared
 
     const { execState } = await executeAstMock({
@@ -695,6 +698,8 @@ export class SceneEntities {
     const scale = this.sceneInfra.getClientSceneScaleFactor(dummy)
 
     const callbacks: (() => SegmentOverlayPayload | null)[] = []
+    this.sceneInfra.pauseRendering()
+    this.tearDownSketch({ removeAxis: false })
 
     for (const sketchInfo of sketchesInfo) {
       const { sketch } = sketchInfo
@@ -766,7 +771,11 @@ export class SceneEntities {
           segPathToNode,
           ['CallExpression', 'CallExpressionKw']
         )
-        if (err(_node1)) return
+        if (err(_node1)) {
+          this.tearDownSketch({ removeAxis: false })
+          this.sceneInfra.resumeRendering()
+          return
+        }
         const callExpName = _node1.node?.callee?.name.name
 
         const initSegment =
@@ -862,6 +871,9 @@ export class SceneEntities {
     )
     position && this.intersectionPlane.position.set(...position)
     this.sceneInfra.scene.add(group)
+
+    this.sceneInfra.resumeRendering()
+
     this.sceneInfra.camControls.enableRotate = false
     this.sceneInfra.overlayCallbacks(callbacks)
 
@@ -967,6 +979,8 @@ export class SceneEntities {
       position: origin,
       maybeModdedAst: modifiedAst,
       draftExpressionsIndices,
+    }).catch(() => {
+      return { truncatedAst: modifiedAst }
     })
     this.sceneInfra.setCallbacks({
       onClick: async (args) => {

src/clientSideScene/sceneInfra.ts

@@ -264,6 +264,8 @@ export class SceneInfra {
     hasBeenDragged: boolean
   } | null = null
   mouseDownVector: null | Vector2 = null
+  private isRenderingPaused = false
+  private lastFrameTime = 0
 
   constructor(engineCommandManager: EngineCommandManager) {
     // SCENE
@@ -348,8 +350,20 @@ export class SceneInfra {
     TWEEN.update() // This will update all tweens during the animation loop
     if (!this.isFovAnimationInProgress) {
       this.camControls.update()
-      this.renderer.render(this.scene, this.camControls.camera)
+
-      this.labelRenderer.render(this.scene, this.camControls.camera)
+      // If rendering is paused, only render if enough time has passed to maintain smooth animation
+      if (this.isRenderingPaused) {
+        const currentTime = performance.now()
+        if (currentTime - this.lastFrameTime > 1000 / 30) {
+          // Limit to 30fps while paused
+          this.renderer.render(this.scene, this.camControls.camera)
+          this.labelRenderer.render(this.scene, this.camControls.camera)
+          this.lastFrameTime = currentTime
+        }
+      } else {
+        this.renderer.render(this.scene, this.camControls.camera)
+        this.labelRenderer.render(this.scene, this.camControls.camera)
+      }
     }
   }
 
@@ -685,6 +699,15 @@ export class SceneInfra {
     const scale = this.getClientSceneScaleFactor(dummy)
     return getLength(a, b) / scale
   }
+  pauseRendering() {
+    this.isRenderingPaused = true
+    // Store the current time to prevent unnecessary updates
+    this.lastFrameTime = performance.now()
+  }
+
+  resumeRendering() {
+    this.isRenderingPaused = false
+  }
 }
 
 function baseUnitTomm(baseUnit: BaseUnit) {

src/components/ModelingMachineProvider.tsx

@@ -1612,9 +1612,6 @@ export const ModelingMachineProvider = ({
           async ({ input: { sketchDetails, selectionRanges } }) => {
             if (!sketchDetails) return
             if (!sketchDetails.sketchEntryNodePath?.length) return
-            if (Object.keys(sceneEntitiesManager.activeSegments).length > 0) {
-              sceneEntitiesManager.tearDownSketch({ removeAxis: false })
-            }
             sceneInfra.resetMouseListeners()
             await sceneEntitiesManager.setupSketch({
               sketchEntryNodePath: sketchDetails?.sketchEntryNodePath || [],

src/machines/modelingMachine.ts

@@ -3792,11 +3792,7 @@ export const modelingMachine = setup({
           },
 
           initial: 'splitting sketch pipe',
-          entry: [
+          entry: ['assign tool in context', 'reset selections'],
-            'assign tool in context',
-            'reset selections',
-            'tear down client sketch',
-          ],
         },
         'Circle three point tool': {
           states: {