diff --git a/e2e/playwright/fixtures/sceneFixture.ts b/e2e/playwright/fixtures/sceneFixture.ts index 06be6fdfc..75f6b059d 100644 --- a/e2e/playwright/fixtures/sceneFixture.ts +++ b/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') +} diff --git a/e2e/playwright/sketch-tests.spec.ts b/e2e/playwright/sketch-tests.spec.ts index 4179b88a5..311d73d5f 100644 --- a/e2e/playwright/sketch-tests.spec.ts +++ b/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 page.mouse.move(handle1Location.x, handle1Location.y) - await page.mouse.down() - await page.mouse.move(handle1Location.x + 50, handle1Location.y + 50, { - steps: 5, - }) - await page.mouse.up() - await editor.expectEditor.toContain('length = 231.59, angle = -34') + await expect + .poll(async () => { + await editor.expectEditor.toContain('length = 156.54, angle = -28') + await page.mouse.move(handle1Location.x, handle1Location.y) + await page.mouse.down() + await page.mouse.move( + handle1Location.x + 50, + 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 page.mouse.down() - await page.mouse.move(handle2Location.x, handle2Location.y - 50, { - steps: 5, - }) - await page.mouse.up() - await editor.expectEditor.toContain('length = 167.36, angle = -14') + await expect + .poll(async () => { + await page.mouse.move(handle2Location.x, handle2Location.y) + await page.mouse.down() + await page.mouse.move(handle2Location.x, handle2Location.y - 50, { + steps: 5, + }) + 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.mouse.move(handle3Location.x, handle3Location.y) - await page.mouse.down() - await page.mouse.move(handle3Location.x, handle3Location.y + 110, { - steps: 5, - }) - await page.mouse.up() - await editor.expectEditor.toContain('length = 219.2, angle = -56') + await page.waitForTimeout(500) // Wait for deferred execution + await expect + .poll(async () => { + await editor.expectEditor.toContain('length = 167.36, angle = -14') + await page.mouse.move(handle3Location.x, handle3Location.y) + await page.mouse.down() + await page.mouse.move(handle3Location.x, handle3Location.y + 110, { + 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) - profile004 = circle(sketch003, center = [143.91, 136.89], radius = 71.63) - - sketch001 = startSketchOn(XZ) - profile001 = startProfileAt([106.68, 89.77], sketch001) - |> line(end = [132.34, 157.8]) - |> line(end = [67.65, -460.55], tag = $seg01) - |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) - |> close() - extrude001 = extrude(profile001, length = 500) - 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 = 219.2, angle = -56) - |> angledLine( - angle = segAng(rectangleSegmentA001), - length = -segLen(rectangleSegmentA001), - angle = -151, - length = 116.27, - ) - |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) - |> close() - profile003 = startProfileAt([-201.08, 254.17], sketch002) - |> line(end = [103.55, 33.32]) - |> line(end = [48.8, -153.54]) - `, +sketch003 = startSketchOn(XY) +profile004 = circle(sketch003, center = [143.91, 136.89], radius = 71.63) + +sketch001 = startSketchOn(XZ) +profile001 = startProfileAt([106.68, 89.77], sketch001) + |> line(end = [132.34, 157.8]) + |> line(end = [67.65, -460.55], tag = $seg01) + |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) + |> close() +extrude001 = extrude(profile001, length = 500) +sketch002 = startSketchOn(extrude001, face = seg01) +profile002 = startProfileAt([83.39, 329.15], sketch002) + |> angledLine(angle = 0, length = 119.61, tag = $rectangleSegmentA001) + |> angledLine(length = 219.2, angle = -56) + |> angledLine(angle = -151, length = 116.27) + |> 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 + ) + }) } ) }) diff --git a/src/clientSideScene/sceneEntities.ts b/src/clientSideScene/sceneEntities.ts index e2c8f709c..cba376523 100644 --- a/src/clientSideScene/sceneEntities.ts +++ b/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) => { diff --git a/src/clientSideScene/sceneInfra.ts b/src/clientSideScene/sceneInfra.ts index fbb7e5468..aa83423d2 100644 --- a/src/clientSideScene/sceneInfra.ts +++ b/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) { diff --git a/src/components/ModelingMachineProvider.tsx b/src/components/ModelingMachineProvider.tsx index 2151bb6c4..5698d1f7a 100644 --- a/src/components/ModelingMachineProvider.tsx +++ b/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 || [], diff --git a/src/machines/modelingMachine.ts b/src/machines/modelingMachine.ts index 557d33509..a1113ac02 100644 --- a/src/machines/modelingMachine.ts +++ b/src/machines/modelingMachine.ts @@ -3792,11 +3792,7 @@ export const modelingMachine = setup({ }, initial: 'splitting sketch pipe', - entry: [ - 'assign tool in context', - 'reset selections', - 'tear down client sketch', - ], + entry: ['assign tool in context', 'reset selections'], }, 'Circle three point tool': { states: {