diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts index f35593b5e1..06fc7938d8 100644 --- a/e2e/playwright/point-click.spec.ts +++ b/e2e/playwright/point-click.spec.ts @@ -756,6 +756,17 @@ test(`Offset plane point-and-click`, async ({ }) await scene.expectPixelColor([74, 74, 74], testPoint, 15) }) + + await test.step('Delete offset plane via feature tree selection', async () => { + await editor.closePane() + const operationButton = await toolbar.getFeatureTreeOperation( + 'Offset Plane', + 0 + ) + await operationButton.click({ button: 'left' }) + await page.keyboard.press('Backspace') + await scene.expectPixelColor([50, 51, 96], testPoint, 15) + }) }) const loftPointAndClickCases = [ @@ -851,6 +862,75 @@ loftPointAndClickCases.forEach(({ shouldPreselect }) => { }) await scene.expectPixelColor([89, 89, 89], testPoint, 15) }) + + await test.step('Delete loft via feature tree selection', async () => { + await editor.closePane() + const operationButton = await toolbar.getFeatureTreeOperation('Loft', 0) + await operationButton.click({ button: 'left' }) + await page.keyboard.press('Backspace') + await scene.expectPixelColor([254, 254, 254], testPoint, 15) + }) + }) +}) + +// TODO: merge with above test. Right now we're not able to delete a loft +// right after creation via selection for some reason, so we go with a new instance +test('Loft and offset plane deletion via selection', async ({ + context, + page, + homePage, + scene, +}) => { + const initialCode = `sketch001 = startSketchOn('XZ') + |> circle({ center = [0, 0], radius = 30 }, %) + plane001 = offsetPlane('XZ', 50) + sketch002 = startSketchOn(plane001) + |> circle({ center = [0, 0], radius = 20 }, %) +loft001 = loft([sketch001, sketch002]) +` + await context.addInitScript((initialCode) => { + localStorage.setItem('persistCode', initialCode) + }, initialCode) + await page.setBodyDimensions({ width: 1000, height: 500 }) + await homePage.goToModelingScene() + + // One dumb hardcoded screen pixel value + const testPoint = { x: 575, y: 200 } + const [clickOnSketch1] = scene.makeMouseHelpers(testPoint.x, testPoint.y) + const [clickOnSketch2] = scene.makeMouseHelpers(testPoint.x, testPoint.y + 80) + + await test.step(`Delete loft`, async () => { + // Check for loft + await scene.expectPixelColor([89, 89, 89], testPoint, 15) + await clickOnSketch1() + await expect(page.locator('.cm-activeLine')).toHaveText(` + |> circle({ center = [0, 0], radius = 30 }, %) + `) + await page.keyboard.press('Backspace') + // Check for sketch 1 + await scene.expectPixelColor([254, 254, 254], testPoint, 15) + }) + + await test.step('Delete sketch002', async () => { + await page.waitForTimeout(1000) + await clickOnSketch2() + await expect(page.locator('.cm-activeLine')).toHaveText(` + |> circle({ center = [0, 0], radius = 20 }, %) + `) + await page.keyboard.press('Backspace') + // Check for plane001 + await scene.expectPixelColor([228, 228, 228], testPoint, 15) + }) + + await test.step('Delete plane001', async () => { + await page.waitForTimeout(1000) + await clickOnSketch2() + await expect(page.locator('.cm-activeLine')).toHaveText(` + plane001 = offsetPlane('XZ', 50) + `) + await page.keyboard.press('Backspace') + // Check for sketch 1 + await scene.expectPixelColor([254, 254, 254], testPoint, 15) }) }) @@ -1030,4 +1110,12 @@ extrude001 = extrude(40, sketch001) }) await scene.expectPixelColor([49, 49, 49], testPoint, 15) }) + + await test.step('Delete shell via feature tree selection', async () => { + await editor.closePane() + const operationButton = await toolbar.getFeatureTreeOperation('Shell', 0) + await operationButton.click({ button: 'left' }) + await page.keyboard.press('Backspace') + await scene.expectPixelColor([99, 99, 99], testPoint, 15) + }) }) diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index 79106a5ed9..dc346c61ab 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -1149,11 +1149,17 @@ export async function deleteFromSelection( ((selection?.artifact?.type === 'wall' || selection?.artifact?.type === 'cap') && varDec.node.init.type === 'PipeExpression') || - selection.artifact?.type === 'sweep' + selection.artifact?.type === 'sweep' || + selection.artifact?.type === 'plane' || + !selection.artifact // aka expected to be a shell at this point ) { let extrudeNameToDelete = '' let pathToNode: PathToNode | null = null - if (selection.artifact?.type !== 'sweep') { + if ( + selection.artifact && + selection.artifact.type !== 'sweep' && + selection.artifact.type !== 'plane' + ) { const varDecName = varDec.node.id.name traverse(astClone, { enter: (node, path) => { @@ -1169,6 +1175,17 @@ export async function deleteFromSelection( pathToNode = path extrudeNameToDelete = dec.id.name } + if ( + dec.init.type === 'CallExpression' && + dec.init.callee.name === 'loft' && + dec.init.arguments?.[0].type === 'ArrayExpression' && + dec.init.arguments?.[0].elements.some( + (a) => a.type === 'Identifier' && a.name === varDecName + ) + ) { + pathToNode = path + extrudeNameToDelete = dec.id.name + } } }, }) diff --git a/src/lang/std/artifactGraph.ts b/src/lang/std/artifactGraph.ts index f038d94288..9ce7e3763f 100644 --- a/src/lang/std/artifactGraph.ts +++ b/src/lang/std/artifactGraph.ts @@ -77,7 +77,7 @@ interface SegmentArtifactRich extends BaseArtifact { /** A Sweep is a more generic term for extrude, revolve, loft and sweep*/ interface SweepArtifact extends BaseArtifact { type: 'sweep' - subType: 'extrusion' | 'revolve' + subType: 'extrusion' | 'revolve' | 'loft' pathId: string surfaceIds: Array edgeIds: Array @@ -85,7 +85,7 @@ interface SweepArtifact extends BaseArtifact { } interface SweepArtifactRich extends BaseArtifact { type: 'sweep' - subType: 'extrusion' | 'revolve' + subType: 'extrusion' | 'revolve' | 'loft' path: PathArtifact surfaces: Array edges: Array @@ -398,6 +398,33 @@ export function getArtifactsToUpdate({ artifact: { ...path, sweepId: id }, }) return returnArr + } else if ( + cmd.type === 'loft' && + response.type === 'modeling' && + response.data.modeling_response.type === 'loft' + ) { + returnArr.push({ + id, + artifact: { + type: 'sweep', + subType: 'loft', + id, + // TODO: make sure to revisit this choice, don't think it matters for now + pathId: cmd.section_ids[0], + surfaceIds: [], + edgeIds: [], + codeRef: { range, pathToNode }, + }, + }) + for (const sectionId of cmd.section_ids) { + const path = getArtifact(sectionId) + if (path?.type === 'path') + returnArr.push({ + id: sectionId, + artifact: { ...path, sweepId: id }, + }) + } + return returnArr } else if ( cmd.type === 'solid3d_get_extrusion_face_info' && response?.type === 'modeling' && diff --git a/src/wasm-lib/kcl/src/std/loft.rs b/src/wasm-lib/kcl/src/std/loft.rs index a43bbba0b4..bd3e03721a 100644 --- a/src/wasm-lib/kcl/src/std/loft.rs +++ b/src/wasm-lib/kcl/src/std/loft.rs @@ -156,5 +156,8 @@ async fn inner_loft( .await?; // Using the first sketch as the base curve, idk we might want to change this later. - do_post_extrude(sketches[0].clone(), 0.0, exec_state, args).await + let mut sketch = sketches[0].clone(); + // Override its id with the loft id so we can get its faces later + sketch.id = id; + do_post_extrude(sketch, 0.0, exec_state, args).await }