diff --git a/e2e/playwright/feature-tree-pane.spec.ts b/e2e/playwright/feature-tree-pane.spec.ts index a2c1fba1e33..d4eb6a52da6 100644 --- a/e2e/playwright/feature-tree-pane.spec.ts +++ b/e2e/playwright/feature-tree-pane.spec.ts @@ -394,26 +394,23 @@ test.describe('Feature Tree pane', () => { await cmdBar.expectState({ commandName: 'Offset plane', stage: 'arguments', - currentArgKey: 'distance', + currentArgKey: 'offset', currentArgValue: initialInput, headerArguments: { - Plane: '1 plane', - Distance: initialInput, + Offset: initialInput, }, - highlightedHeaderArg: 'distance', + highlightedHeaderArg: 'offset', }) }) - await test.step('Edit the distance argument and submit', async () => { + await test.step('Edit the offset argument and submit', async () => { await expect(cmdBar.currentArgumentInput).toBeVisible() await cmdBar.currentArgumentInput.locator('.cm-content').fill(newInput) await cmdBar.progressCmdBar() await cmdBar.expectState({ stage: 'review', headerArguments: { - Plane: '1 plane', - // We show the calculated value in the argument summary - Distance: '15', + Offset: '15', }, commandName: 'Offset plane', }) diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts index bc1b1f33a1e..2f05252d68b 100644 --- a/e2e/playwright/point-click.spec.ts +++ b/e2e/playwright/point-click.spec.ts @@ -912,18 +912,10 @@ openSketch = startSketchOn(XY) toolbar, cmdBar, }) => { - // One dumb hardcoded screen pixel value - const testPoint = { x: 700, y: 200 } - // TODO: replace the testPoint selection with a feature tree click once that's supported #7544 - const [clickOnXzPlane] = scene.makeMouseHelpers(testPoint.x, testPoint.y) const expectedOutput = `plane001 = offsetPlane(XZ, offset = 5)` - await homePage.goToModelingScene() await scene.settled(cmdBar) - await test.step(`Look for the blue of the XZ plane`, async () => { - //await scene.expectPixelColor([50, 51, 96], testPoint, 15) // FIXME - }) await test.step(`Go through the command bar flow`, async () => { await toolbar.offsetPlaneButton.click() await expect @@ -933,30 +925,30 @@ openSketch = startSketchOn(XY) stage: 'arguments', currentArgKey: 'plane', currentArgValue: '', - headerArguments: { Plane: '', Distance: '' }, + headerArguments: { Plane: '', Offset: '' }, highlightedHeaderArg: 'plane', commandName: 'Offset plane', }) - await clickOnXzPlane() + const xz = await toolbar.getFeatureTreeOperation('Front plane', 0) + await xz.click() await cmdBar.expectState({ stage: 'arguments', - currentArgKey: 'distance', + currentArgKey: 'offset', currentArgValue: '5', - headerArguments: { Plane: '1 plane', Distance: '' }, - highlightedHeaderArg: 'distance', + headerArguments: { Plane: '1 plane', Offset: '' }, + highlightedHeaderArg: 'offset', commandName: 'Offset plane', }) await cmdBar.progressCmdBar() }) - await test.step(`Confirm code is added to the editor, scene has changed`, async () => { + await test.step(`Confirm code is added to the editor`, async () => { await editor.expectEditor.toContain(expectedOutput) await editor.expectState({ diagnostics: [], activeLines: [expectedOutput], highlightedCode: '', }) - await scene.expectPixelColor([74, 74, 74], testPoint, 15) }) await test.step('Delete offset plane via feature tree selection', async () => { @@ -967,7 +959,6 @@ openSketch = startSketchOn(XY) ) await operationButton.click({ button: 'left' }) await page.keyboard.press('Delete') - //await scene.expectPixelColor([50, 51, 96], testPoint, 15) // FIXME }) }) diff --git a/rust/kcl-lib/src/execution/artifact.rs b/rust/kcl-lib/src/execution/artifact.rs index e1501d36819..4606861a022 100644 --- a/rust/kcl-lib/src/execution/artifact.rs +++ b/rust/kcl-lib/src/execution/artifact.rs @@ -185,6 +185,15 @@ pub struct Solid2d { pub path_id: ArtifactId, } +#[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)] +#[ts(export_to = "Artifact.ts")] +#[serde(rename_all = "camelCase")] +pub struct PlaneOfFace { + pub id: ArtifactId, + pub face_id: ArtifactId, + pub code_ref: CodeRef, +} + #[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)] #[ts(export_to = "Artifact.ts")] #[serde(rename_all = "camelCase")] @@ -325,6 +334,7 @@ pub enum Artifact { Path(Path), Segment(Segment), Solid2d(Solid2d), + PlaneOfFace(PlaneOfFace), StartSketchOnFace(StartSketchOnFace), StartSketchOnPlane(StartSketchOnPlane), Sweep(Sweep), @@ -346,6 +356,7 @@ impl Artifact { Artifact::Solid2d(a) => a.id, Artifact::StartSketchOnFace(a) => a.id, Artifact::StartSketchOnPlane(a) => a.id, + Artifact::PlaneOfFace(a) => a.id, Artifact::Sweep(a) => a.id, Artifact::Wall(a) => a.id, Artifact::Cap(a) => a.id, @@ -367,6 +378,7 @@ impl Artifact { Artifact::Solid2d(_) => None, Artifact::StartSketchOnFace(a) => Some(&a.code_ref), Artifact::StartSketchOnPlane(a) => Some(&a.code_ref), + Artifact::PlaneOfFace(a) => Some(&a.code_ref), Artifact::Sweep(a) => Some(&a.code_ref), Artifact::Wall(_) => None, Artifact::Cap(_) => None, @@ -387,6 +399,7 @@ impl Artifact { | Artifact::Segment(_) | Artifact::Solid2d(_) | Artifact::StartSketchOnFace(_) + | Artifact::PlaneOfFace(_) | Artifact::StartSketchOnPlane(_) | Artifact::Sweep(_) => None, Artifact::Wall(a) => Some(&a.face_code_ref), @@ -406,6 +419,7 @@ impl Artifact { Artifact::Solid2d(_) => Some(new), Artifact::StartSketchOnFace { .. } => Some(new), Artifact::StartSketchOnPlane { .. } => Some(new), + Artifact::PlaneOfFace { .. } => Some(new), Artifact::Sweep(a) => a.merge(new), Artifact::Wall(a) => a.merge(new), Artifact::Cap(a) => a.merge(new), diff --git a/rust/kcl-lib/src/execution/artifact/mermaid_tests.rs b/rust/kcl-lib/src/execution/artifact/mermaid_tests.rs index 537d3238494..da66a57d0fe 100644 --- a/rust/kcl-lib/src/execution/artifact/mermaid_tests.rs +++ b/rust/kcl-lib/src/execution/artifact/mermaid_tests.rs @@ -87,6 +87,7 @@ impl Artifact { Artifact::Solid2d(a) => vec![a.path_id], Artifact::StartSketchOnFace(a) => vec![a.face_id], Artifact::StartSketchOnPlane(a) => vec![a.plane_id], + Artifact::PlaneOfFace(a) => vec![a.face_id], Artifact::Sweep(a) => vec![a.path_id], Artifact::Wall(a) => vec![a.seg_id, a.sweep_id], Artifact::Cap(a) => vec![a.sweep_id], @@ -151,6 +152,10 @@ impl Artifact { // Note: Don't include these since they're parents: plane_id. Vec::new() } + Artifact::PlaneOfFace { .. } => { + // Note: Don't include these since they're parents: face_id. + Vec::new() + } Artifact::Sweep(a) => { // Note: Don't include these since they're parents: path_id. let mut ids = Vec::new(); @@ -262,6 +267,7 @@ impl ArtifactGraph { } Artifact::StartSketchOnFace { .. } | Artifact::StartSketchOnPlane { .. } + | Artifact::PlaneOfFace { .. } | Artifact::Sweep(_) | Artifact::Wall(_) | Artifact::Cap(_) @@ -381,6 +387,14 @@ impl ArtifactGraph { )?; node_path_display(output, prefix, None, code_ref)?; } + Artifact::PlaneOfFace(PlaneOfFace { code_ref, .. }) => { + writeln!( + output, + "{prefix}{id}[\"PlaneOfFace
{:?}\"]", + code_ref_display(code_ref) + )?; + node_path_display(output, prefix, None, code_ref)?; + } Artifact::Sweep(sweep) => { writeln!( output, diff --git a/rust/kcl-lib/src/execution/mod.rs b/rust/kcl-lib/src/execution/mod.rs index 17e8d835e21..38568fed14f 100644 --- a/rust/kcl-lib/src/execution/mod.rs +++ b/rust/kcl-lib/src/execution/mod.rs @@ -4,7 +4,9 @@ use std::sync::Arc; use anyhow::Result; #[cfg(feature = "artifact-graph")] -pub use artifact::{Artifact, ArtifactCommand, ArtifactGraph, CodeRef, StartSketchOnFace, StartSketchOnPlane}; +pub use artifact::{ + Artifact, ArtifactCommand, ArtifactGraph, CodeRef, PlaneOfFace, StartSketchOnFace, StartSketchOnPlane, +}; use cache::GlobalState; pub use cache::{bust_cache, clear_mem_cache}; #[cfg(feature = "artifact-graph")] diff --git a/rust/kcl-lib/src/std/planes.rs b/rust/kcl-lib/src/std/planes.rs index 8bd78698d9f..e387f455c8f 100644 --- a/rust/kcl-lib/src/std/planes.rs +++ b/rust/kcl-lib/src/std/planes.rs @@ -124,6 +124,17 @@ pub(crate) async fn inner_plane_of( }; let plane_info = plane_info.make_right_handed(); + #[cfg(feature = "artifact-graph")] + { + use crate::execution::{Artifact, ArtifactId, CodeRef, PlaneOfFace}; + // Create artifact used only by the UI, not the engine. + exec_state.add_artifact(Artifact::PlaneOfFace(PlaneOfFace { + id: ArtifactId::from(plane_id), + face_id: face_id.into(), + code_ref: CodeRef::placeholder(args.source_range), + })); + } + Ok(Plane { artifact_id: plane_id.into(), id: plane_id, diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index 9965618476e..612804f7d44 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -1,5 +1,4 @@ import type { BodyItem } from '@rust/kcl-lib/bindings/BodyItem' -import type { Name } from '@rust/kcl-lib/bindings/Name' import type { Node } from '@rust/kcl-lib/bindings/Node' import type { NonCodeMeta } from '@rust/kcl-lib/bindings/NonCodeMeta' @@ -27,11 +26,7 @@ import { isNodeSafeToReplace, isNodeSafeToReplacePath, } from '@src/lang/queryAst' -import { - ARG_INDEX_FIELD, - LABELED_ARG_FIELD, - UNLABELED_ARG, -} from '@src/lang/queryAstConstants' +import { ARG_INDEX_FIELD, LABELED_ARG_FIELD } from '@src/lang/queryAstConstants' import { getNodePathFromSourceRange } from '@src/lang/queryAstNodePathUtils' import { addTagForSketchOnFace, @@ -422,56 +417,6 @@ export function sketchOnExtrudedFace( } } -/** - * Append an offset plane to the AST - */ -export function addOffsetPlane({ - node, - plane, - insertIndex, - offset, - planeName, -}: { - node: Node - plane: Node | Node // Can be DefaultPlaneStr or string for offsetPlanes - insertIndex?: number - offset: Expr - planeName?: string -}): { modifiedAst: Node; pathToNode: PathToNode } { - const modifiedAst = structuredClone(node) - const newPlaneName = - planeName ?? findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.PLANE) - - const newPlane = createVariableDeclaration( - newPlaneName, - createCallExpressionStdLibKw('offsetPlane', plane, [ - createLabeledArg('offset', offset), - ]) - ) - - const insertAt = - insertIndex !== undefined - ? insertIndex - : modifiedAst.body.length - ? modifiedAst.body.length - : 0 - - modifiedAst.body.length - ? modifiedAst.body.splice(insertAt, 0, newPlane) - : modifiedAst.body.push(newPlane) - const pathToNode: PathToNode = [ - ['body', ''], - [insertAt, 'index'], - ['declaration', 'VariableDeclaration'], - ['init', 'VariableDeclarator'], - ['unlabeled', UNLABELED_ARG], - ] - return { - modifiedAst, - pathToNode, - } -} - /** * Add an import call to load a part */ diff --git a/src/lang/modifyAst/faces.test.ts b/src/lang/modifyAst/faces.test.ts index 9adc0cd467b..716831d65c4 100644 --- a/src/lang/modifyAst/faces.test.ts +++ b/src/lang/modifyAst/faces.test.ts @@ -1,6 +1,7 @@ import { type Artifact, type ArtifactGraph, + type PlaneArtifact, assertParse, recast, } from '@src/lang/wasm' @@ -8,16 +9,24 @@ import type { Selection, Selections } from '@src/lib/selections' import { enginelessExecutor } from '@src/lib/testHelpers' import { err } from '@src/lib/trap' import { + addOffsetPlane, addShell, retrieveFaceSelectionsFromOpArgs, + retrieveNonDefaultPlaneSelectionFromOpArg, } from '@src/lang/modifyAst/faces' import { initPromise } from '@src/lang/wasmUtils' -import { engineCommandManager, kclManager } from '@src/lib/singletons' +import { + engineCommandManager, + kclManager, + rustContext, +} from '@src/lib/singletons' import { getCodeRefsByArtifactId } from '@src/lang/std/artifactGraph' import { createPathToNodeForLastVariable } from '@src/lang/modifyAst' import { stringToKclExpression } from '@src/lib/kclHelpers' import type { KclCommandValue } from '@src/lib/commandTypes' import env from '@src/env' +import type { DefaultPlaneStr } from '@src/lib/planes' +import type { StdLibCallOp } from '@src/lang/queryAst' // Unfortunately, we need the real engine here it seems to get sweep faces populated beforeAll(async () => { @@ -46,9 +55,10 @@ async function getAstAndArtifactGraph(code: string) { const { artifactGraph, execState: { operations }, + variables, } = kclManager await new Promise((resolve) => setTimeout(resolve, 100)) - return { ast, artifactGraph, operations } + return { ast, artifactGraph, operations, variables } } function createSelectionFromArtifacts( @@ -93,18 +103,52 @@ thing2 = startSketchOn(case, face = END) const multiSolidsShell = `${multiSolids} shell001 = shell([thing1, thing2], faces = [END, END], thickness = 5)` -describe('Testing addShell', () => { - const cylinder = `sketch001 = startSketchOn(XY) +const cylinder = `sketch001 = startSketchOn(XY) profile001 = circle(sketch001, center = [0, 0], radius = 10) extrude001 = extrude(profile001, length = 10)` - function getCapFromCylinder(artifactGraph: ArtifactGraph) { - const endFace = [...artifactGraph.values()].find( - (a) => a.type === 'cap' && a.subType === 'end' - ) - return createSelectionFromArtifacts([endFace!], artifactGraph) - } +const box = `sketch001 = startSketchOn(XY) +profile001 = startProfile(sketch001, at = [0, 0]) + |> xLine(length = 10) + |> yLine(length = 10) + |> xLine(length = -10) + |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) + |> close() +extrude001 = extrude(profile001, length = 10)` + +const boxWithOneTag = `sketch001 = startSketchOn(XY) +profile001 = startProfile(sketch001, at = [0, 0]) + |> xLine(length = 10, tag = $seg01) + |> yLine(length = 10) + |> xLine(length = -10) + |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) + |> close() +extrude001 = extrude(profile001, length = 10)` + +const boxWithTwoTags = `sketch001 = startSketchOn(XY) +profile001 = startProfile(sketch001, at = [0, 0]) + |> xLine(length = 10, tag = $seg01) + |> yLine(length = 10, tag = $seg02) + |> xLine(length = -10) + |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) + |> close() +extrude001 = extrude(profile001, length = 10)` + +function getCapFromCylinder(artifactGraph: ArtifactGraph) { + const endFace = [...artifactGraph.values()].find( + (a) => a.type === 'cap' && a.subType === 'end' + ) + return createSelectionFromArtifacts([endFace!], artifactGraph) +} +function getFacesFromBox(artifactGraph: ArtifactGraph, count: number) { + const twoWalls = [...artifactGraph.values()] + .filter((a) => a.type === 'wall') + .slice(0, count) + return createSelectionFromArtifacts(twoWalls, artifactGraph) +} + +describe('Testing addShell', () => { it('should add a basic shell call on cylinder end cap', async () => { const { artifactGraph, ast } = await getAstAndArtifactGraph(cylinder) const faces = getCapFromCylinder(artifactGraph) @@ -174,34 +218,9 @@ shell001 = shell(extrude001, faces = END, thickness = 1) await enginelessExecutor(ast) }) - const box = `sketch001 = startSketchOn(XY) -profile001 = startProfile(sketch001, at = [0, 0]) - |> xLine(length = 10) - |> yLine(length = 10) - |> xLine(length = -10) - |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) - |> close() -extrude001 = extrude(profile001, length = 10) -` - const boxWithTwoTags = `sketch001 = startSketchOn(XY) -profile001 = startProfile(sketch001, at = [0, 0]) - |> xLine(length = 10, tag = $seg01) - |> yLine(length = 10, tag = $seg02) - |> xLine(length = -10) - |> line(endAbsolute = [profileStartX(%), profileStartY(%)]) - |> close() -extrude001 = extrude(profile001, length = 10)` - - function getTwoFacesFromBox(artifactGraph: ArtifactGraph) { - const twoWalls = [...artifactGraph.values()] - .filter((a) => a.type === 'wall') - .slice(0, 2) - return createSelectionFromArtifacts(twoWalls, artifactGraph) - } - it('should add a shell call on box for 2 walls', async () => { const { artifactGraph, ast } = await getAstAndArtifactGraph(box) - const faces = getTwoFacesFromBox(artifactGraph) + const faces = getFacesFromBox(artifactGraph, 2) const thickness = (await stringToKclExpression('1')) as KclCommandValue const result = addShell({ ast, artifactGraph, faces, thickness }) if (err(result)) { @@ -218,7 +237,7 @@ shell001 = shell(extrude001, faces = [seg01, seg02], thickness = 1)`) const { artifactGraph, ast } = await getAstAndArtifactGraph(`${boxWithTwoTags} shell001 = shell(extrude001, faces = [seg01, seg02], thickness = 1)`) - const faces = getTwoFacesFromBox(artifactGraph) + const faces = getFacesFromBox(artifactGraph, 2) const thickness = (await stringToKclExpression('2')) as KclCommandValue const nodeToEdit = createPathToNodeForLastVariable(ast) const result = addShell({ @@ -339,3 +358,227 @@ shell001 = shell(extrude001, faces = END, thickness = 0.1) expect(selections.faces.graphSelections[1].artifact!.type).toEqual('cap') }) }) + +describe('Testing retrieveNonDefaultPlaneSelectionFromOpArg', () => { + it('should find an offset plane on an offset plane', async () => { + const code = `plane001 = offsetPlane(XY, offset = 1) +plane002 = offsetPlane(plane001, offset = 2)` + const { artifactGraph, operations } = await getAstAndArtifactGraph(code) + const op = operations.findLast( + (o) => o.type === 'StdLibCall' && o.name === 'offsetPlane' + ) as StdLibCallOp + const selections = retrieveNonDefaultPlaneSelectionFromOpArg( + op.unlabeledArg!, + artifactGraph + ) + if (err(selections)) throw selections + expect(selections.graphSelections).toHaveLength(1) + expect(selections.graphSelections[0].artifact!.type).toEqual('plane') + expect( + (selections.graphSelections[0].artifact as PlaneArtifact).codeRef + .pathToNode[1][0] + ).toEqual(0) + }) + + it('should find an offset plane on a sweep face', async () => { + const code = `${cylinder} +plane001 = offsetPlane(planeOf(extrude001, face = END), offset = 1)` + const { artifactGraph, operations } = await getAstAndArtifactGraph(code) + const op = operations.find( + (o) => o.type === 'StdLibCall' && o.name === 'offsetPlane' + ) as StdLibCallOp + const selections = retrieveNonDefaultPlaneSelectionFromOpArg( + op.unlabeledArg!, + artifactGraph + ) + if (err(selections)) throw selections + + expect(selections.graphSelections).toHaveLength(1) + expect(selections.graphSelections[0].artifact!.type).toEqual('cap') + const cap = [...artifactGraph.values()].find( + (a) => a.type === 'cap' && a.subType === 'end' + ) + expect(selections.graphSelections[0].artifact!.id).toEqual(cap!.id) + }) +}) + +describe('Testing addOffsetPlane', () => { + it.each(['XY', 'XZ', 'YZ'])( + 'should add a basic offset plane call on default plane %s and then edit it', + async (name) => { + const { artifactGraph, ast, variables } = await getAstAndArtifactGraph('') + const offset = (await stringToKclExpression('1')) as KclCommandValue + const id = rustContext.getDefaultPlaneId(name) + if (err(id)) { + throw id + } + const plane: Selections = { + graphSelections: [], + otherSelections: [{ name, id }], + } + const result = addOffsetPlane({ + ast, + artifactGraph, + variables, + plane, + offset, + }) + if (err(result)) { + throw result + } + + const newCode = recast(result.modifiedAst) + expect(newCode).toContain(`plane001 = offsetPlane(${name}, offset = 1)`) + await enginelessExecutor(ast) + + const newOffset = (await stringToKclExpression('2')) as KclCommandValue + const nodeToEdit = createPathToNodeForLastVariable(result.modifiedAst) + const result2 = addOffsetPlane({ + ast: result.modifiedAst, + artifactGraph, + variables, + plane, + offset: newOffset, + nodeToEdit, + }) + if (err(result2)) { + throw result2 + } + const newCode2 = recast(result2.modifiedAst) + expect(newCode2).not.toContain(`offset = 1`) + expect(newCode2).toContain(`plane001 = offsetPlane(${name}, offset = 2)`) + await enginelessExecutor(result2.modifiedAst) + } + ) + + it('should add an offset plane call on offset plane and then edit it', async () => { + const code = `plane001 = offsetPlane(XY, offset = 1)` + const { artifactGraph, ast, variables } = await getAstAndArtifactGraph(code) + const offset = (await stringToKclExpression('2')) as KclCommandValue + const artifact = [...artifactGraph.values()].find((a) => a.type === 'plane') + const plane: Selections = { + graphSelections: [ + { + artifact, + codeRef: artifact!.codeRef, + }, + ], + otherSelections: [], + } + const result = addOffsetPlane({ + ast, + artifactGraph, + variables, + plane, + offset, + }) + if (err(result)) { + throw result + } + + const newCode = recast(result.modifiedAst) + expect(newCode).toContain(`${code} +plane002 = offsetPlane(plane001, offset = 2)`) + await enginelessExecutor(ast) + + const newOffset = (await stringToKclExpression('3')) as KclCommandValue + const nodeToEdit = createPathToNodeForLastVariable(result.modifiedAst) + const result2 = addOffsetPlane({ + ast: result.modifiedAst, + artifactGraph, + variables, + plane, + offset: newOffset, + nodeToEdit, + }) + if (err(result2)) { + throw result2 + } + const newCode2 = recast(result2.modifiedAst) + expect(newCode2).not.toContain(`offset = 2`) + expect(newCode2).toContain(`${code} +plane002 = offsetPlane(plane001, offset = 3)`) + await enginelessExecutor(result2.modifiedAst) + }) + + it('should add an offset plane call on cylinder end cap and allow edits', async () => { + const { artifactGraph, ast, variables } = + await getAstAndArtifactGraph(cylinder) + const plane = getCapFromCylinder(artifactGraph) + const offset = (await stringToKclExpression('2')) as KclCommandValue + const result = addOffsetPlane({ + ast, + artifactGraph, + variables, + plane, + offset, + }) + if (err(result)) { + throw result + } + + const newCode = recast(result.modifiedAst) + expect(newCode).toContain(`${cylinder} +plane001 = offsetPlane(planeOf(extrude001, face = END), offset = 2)`) + await enginelessExecutor(ast) + + const newOffset = (await stringToKclExpression('3')) as KclCommandValue + const nodeToEdit = createPathToNodeForLastVariable(result.modifiedAst) + const result2 = addOffsetPlane({ + ast: result.modifiedAst, + artifactGraph, + variables, + plane, + offset: newOffset, + nodeToEdit, + }) + if (err(result2)) { + throw result2 + } + const newCode2 = recast(result2.modifiedAst) + expect(newCode2).not.toContain(`offset = 2`) + expect(newCode2).toContain(`${cylinder} +plane001 = offsetPlane(planeOf(extrude001, face = END), offset = 3)`) + await enginelessExecutor(result2.modifiedAst) + }) + + it('should add an offset plane call on box wall and allow edits', async () => { + const { artifactGraph, ast, variables } = await getAstAndArtifactGraph(box) + const plane = getFacesFromBox(artifactGraph, 1) + const offset = (await stringToKclExpression('10')) as KclCommandValue + const result = addOffsetPlane({ + ast, + artifactGraph, + variables, + plane, + offset, + }) + if (err(result)) { + throw result + } + + const newCode = recast(result.modifiedAst) + expect(newCode).toContain(`${boxWithOneTag} +plane001 = offsetPlane(planeOf(extrude001, face = seg01), offset = 10)`) + await enginelessExecutor(ast) + + const newOffset = (await stringToKclExpression('20')) as KclCommandValue + const nodeToEdit = createPathToNodeForLastVariable(result.modifiedAst) + const result2 = addOffsetPlane({ + ast: result.modifiedAst, + artifactGraph, + variables, + plane, + offset: newOffset, + nodeToEdit, + }) + if (err(result2)) { + throw result2 + } + const newCode2 = recast(result2.modifiedAst) + expect(newCode2).not.toContain(`offset = 10`) + expect(newCode2).toContain(`${boxWithOneTag} +plane001 = offsetPlane(planeOf(extrude001, face = seg01), offset = 20)`) + await enginelessExecutor(result2.modifiedAst) + }) +}) diff --git a/src/lang/modifyAst/faces.ts b/src/lang/modifyAst/faces.ts index f973287be11..a30eddadde3 100644 --- a/src/lang/modifyAst/faces.ts +++ b/src/lang/modifyAst/faces.ts @@ -12,6 +12,7 @@ import { setCallInAst, } from '@src/lang/modifyAst' import { + getSelectedPlaneAsNode, getVariableExprsFromSelection, retrieveSelectionsFromOpArg, valueOrVariable, @@ -19,8 +20,10 @@ import { import type { Artifact, ArtifactGraph, + Expr, PathToNode, Program, + VariableMap, } from '@src/lang/wasm' import type { KclCommandValue } from '@src/lib/commandTypes' import type { Selection, Selections } from '@src/lib/selections' @@ -29,6 +32,7 @@ import { mutateAstWithTagForSketchSegment } from '@src/lang/modifyAst/addEdgeTre import { getArtifactOfTypes, getCapCodeRef, + getFaceCodeRef, getSweepArtifactFromSelection, } from '@src/lang/std/artifactGraph' import type { OpArg, OpKclValue } from '@rust/kcl-lib/bindings/Operation' @@ -56,43 +60,18 @@ export function addShell({ const modifiedAst = structuredClone(ast) // 2. Prepare unlabeled and labeled arguments - // Inferring solids from the faces selection, maybe someday we can expose this but no need for now - const solids: Selections = { - graphSelections: faces.graphSelections.flatMap((f) => { - const sweep = getSweepArtifactFromSelection(f, artifactGraph) - if (err(sweep) || !sweep) return [] - return { - artifact: sweep as Artifact, - codeRef: sweep.codeRef, - } - }), - otherSelections: [], - } - // Map the sketches selection into a list of kcl expressions to be passed as unlabeled argument - const lastChildLookup = true - const vars = getVariableExprsFromSelection( - solids, - modifiedAst, - nodeToEdit, - lastChildLookup, - artifactGraph - ) - if (err(vars)) { - return vars - } - - const sketchesExpr = createVariableExpressionsArray(vars.exprs) - const facesExprs = getFacesExprsFromSelection( - modifiedAst, + const result = buildSolidsAndFacesExprs( faces, - artifactGraph + artifactGraph, + modifiedAst, + nodeToEdit ) - const facesExpr = createVariableExpressionsArray(facesExprs) - if (!facesExpr) { - return new Error('No faces found in the selection') + if (err(result)) { + return result } - const call = createCallExpressionStdLibKw('shell', sketchesExpr, [ + const { solidsExpr, facesExpr, pathIfPipe } = result + const call = createCallExpressionStdLibKw('shell', solidsExpr, [ createLabeledArg('faces', facesExpr), createLabeledArg('thickness', valueOrVariable(thickness)), ]) @@ -108,7 +87,7 @@ export function addShell({ ast: modifiedAst, call, pathToEdit: nodeToEdit, - pathIfNewPipe: vars.pathIfPipe, + pathIfNewPipe: pathIfPipe, variableIfNewDecl: KCL_DEFAULT_CONSTANT_PREFIXES.SHELL, }) if (err(pathToNode)) { @@ -121,6 +100,86 @@ export function addShell({ } } +export function addOffsetPlane({ + ast, + artifactGraph, + variables, + plane, + offset, + nodeToEdit, +}: { + ast: Node + artifactGraph: ArtifactGraph + variables: VariableMap + plane: Selections + offset: KclCommandValue + nodeToEdit?: PathToNode +}): + | { + modifiedAst: Node + pathToNode: PathToNode + } + | Error { + // 1. Clone the ast so we can edit it + const modifiedAst = structuredClone(ast) + + // 2. Prepare unlabeled and labeled arguments + let planeExpr: Expr | undefined + const hasFaceToOffset = plane.graphSelections.some( + (sel) => sel.artifact?.type === 'cap' || sel.artifact?.type === 'wall' + ) + if (hasFaceToOffset) { + const result = buildSolidsAndFacesExprs( + plane, + artifactGraph, + modifiedAst, + nodeToEdit + ) + if (err(result)) { + return result + } + + const { solidsExpr, facesExpr } = result + planeExpr = createCallExpressionStdLibKw('planeOf', solidsExpr, [ + createLabeledArg('face', facesExpr), + ]) + } else { + planeExpr = getSelectedPlaneAsNode(plane, variables) + if (!planeExpr) { + return new Error('No plane found in the selection') + } + } + + const call = createCallExpressionStdLibKw('offsetPlane', planeExpr, [ + createLabeledArg('offset', valueOrVariable(offset)), + ]) + + // Insert variables for labeled arguments if provided + if ('variableName' in offset && offset.variableName) { + insertVariableAndOffsetPathToNode(offset, modifiedAst, nodeToEdit) + } + + // 3. If edit, we assign the new function call declaration to the existing node, + // otherwise just push to the end + const pathToNode = setCallInAst({ + ast: modifiedAst, + call, + pathToEdit: nodeToEdit, + pathIfNewPipe: undefined, + variableIfNewDecl: KCL_DEFAULT_CONSTANT_PREFIXES.PLANE, + }) + if (err(pathToNode)) { + return pathToNode + } + + return { + modifiedAst, + pathToNode, + } +} + +// Utilities + function getFacesExprsFromSelection( ast: Node, faces: Selections, @@ -245,3 +304,107 @@ export function retrieveFaceSelectionsFromOpArgs( const faces = { graphSelections, otherSelections: [] } return { solids, faces } } + +export function retrieveNonDefaultPlaneSelectionFromOpArg( + planeArg: OpArg, + artifactGraph: ArtifactGraph +): Selections | Error { + if (planeArg.value.type !== 'Plane') { + return new Error( + 'Unsupported case for edit flows at the moment, check the KCL code' + ) + } + + const planeArtifact = getArtifactOfTypes( + { + key: planeArg.value.artifact_id, + types: ['plane', 'planeOfFace'], + }, + artifactGraph + ) + if (err(planeArtifact)) { + return new Error("Couldn't retrieve plane or planeOfFace artifact") + } + + if (planeArtifact.type === 'plane') { + return { + graphSelections: [ + { + artifact: planeArtifact, + codeRef: planeArtifact.codeRef, + }, + ], + otherSelections: [], + } + } else if (planeArtifact.type === 'planeOfFace') { + const faceArtifact = getArtifactOfTypes( + { key: planeArtifact.faceId, types: ['cap', 'wall'] }, + artifactGraph + ) + if (err(faceArtifact)) { + return new Error("Couldn't retrieve face artifact for planeOfFace") + } + + const codeRef = getFaceCodeRef(faceArtifact) + if (!codeRef) { + return new Error("Couldn't retrieve code reference for face artifact") + } + + return { + graphSelections: [ + { + artifact: faceArtifact, + codeRef, + }, + ], + otherSelections: [], + } + } + + return new Error('Unsupported plane artifact type') +} + +function buildSolidsAndFacesExprs( + faces: Selections, + artifactGraph: ArtifactGraph, + modifiedAst: Node, + nodeToEdit?: PathToNode +) { + const solids: Selections = { + graphSelections: faces.graphSelections.flatMap((f) => { + const sweep = getSweepArtifactFromSelection(f, artifactGraph) + if (err(sweep) || !sweep) return [] + return { + artifact: sweep as Artifact, + codeRef: sweep.codeRef, + } + }), + otherSelections: [], + } + // Map the sketches selection into a list of kcl expressions to be passed as unlabeled argument + const lastChildLookup = true + const vars = getVariableExprsFromSelection( + solids, + modifiedAst, + nodeToEdit, + lastChildLookup, + artifactGraph + ) + if (err(vars)) { + return vars + } + + const pathIfPipe = vars.pathIfPipe + const solidsExpr = createVariableExpressionsArray(vars.exprs) + const facesExprs = getFacesExprsFromSelection( + modifiedAst, + faces, + artifactGraph + ) + const facesExpr = createVariableExpressionsArray(facesExprs) + if (!facesExpr) { + return new Error('No faces found in the selection') + } + + return { solidsExpr, facesExpr, pathIfPipe } +} diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index c2b888328e6..e90afe2dfb9 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -145,7 +145,7 @@ export type ModelingCommandSchema = { // Enables editing workflow nodeToEdit?: PathToNode plane: Selections - distance: KclCommandValue + offset: KclCommandValue } Helix: { // Enables editing workflow @@ -686,12 +686,13 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< }, plane: { inputType: 'selection', - selectionTypes: ['plane'], + selectionTypes: ['plane', 'cap', 'wall'], multiple: false, required: true, skip: true, + hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit), }, - distance: { + offset: { inputType: 'kcl', defaultValue: KCL_DEFAULT_LENGTH, required: true, diff --git a/src/lib/operations.ts b/src/lib/operations.ts index e2bae61e67b..fab4521ea62 100644 --- a/src/lib/operations.ts +++ b/src/lib/operations.ts @@ -1,7 +1,10 @@ import type { Operation } from '@rust/kcl-lib/bindings/Operation' import type { CustomIconName } from '@src/components/CustomIcon' -import { retrieveFaceSelectionsFromOpArgs } from '@src/lang/modifyAst/faces' +import { + retrieveFaceSelectionsFromOpArgs, + retrieveNonDefaultPlaneSelectionFromOpArg, +} from '@src/lang/modifyAst/faces' import { retrieveAxisOrEdgeSelectionsFromOpArg } from '@src/lang/modifyAst/sweeps' import { getNodeFromPath, @@ -400,8 +403,8 @@ const prepareToEditShell: PrepareToEditCallback = async ({ operation }) => { // 2. Convert the thickness argument from a string to a KCL expression const thickness = await stringToKclExpression( codeManager.code.slice( - operation.labeledArgs?.['thickness']?.sourceRange[0], - operation.labeledArgs?.['thickness']?.sourceRange[1] + operation.labeledArgs?.thickness?.sourceRange[0], + operation.labeledArgs?.thickness?.sourceRange[1] ) ) if (err(thickness) || 'errors' in thickness) { @@ -429,58 +432,58 @@ const prepareToEditOffsetPlane: PrepareToEditCallback = async ({ name: 'Offset plane', groupId: 'modeling', } - if ( - operation.type !== 'StdLibCall' || - !operation.labeledArgs || - !operation.unlabeledArg || - !('offset' in operation.labeledArgs) || - !operation.labeledArgs.offset - ) { - return baseCommand + if (operation.type !== 'StdLibCall') { + return { reason: 'Wrong operation type' } } - // TODO: Implement conversion to arbitrary plane selection - // once the Offset Plane command supports it. - const stdPlane = operation.unlabeledArg - const planeName = getStringValue(codeManager.code, stdPlane.sourceRange) - if (!isDefaultPlaneStr(planeName)) { - // TODO: error handling - return baseCommand - } - const planeId = rustContext.getDefaultPlaneId(planeName) - if (err(planeId)) { - // TODO: error handling - return baseCommand + // 1. Map the plane and faces arguments to plane or face selections + if (!operation.unlabeledArg) { + return { reason: `Couldn't retrieve operation arguments` } } - const plane: Selections = { - graphSelections: [], - otherSelections: [ - { - name: planeName, - id: planeId, - }, - ], + let plane: Selections | undefined + const maybeDefaultPlaneName = getStringValue( + codeManager.code, + operation.unlabeledArg.sourceRange + ) + if (isDefaultPlaneStr(maybeDefaultPlaneName)) { + const id = rustContext.getDefaultPlaneId(maybeDefaultPlaneName) + if (err(id)) { + return { reason: "Couldn't retrieve default plane ID" } + } + + plane = { + graphSelections: [], + otherSelections: [{ id, name: maybeDefaultPlaneName }], + } + } else { + const result = retrieveNonDefaultPlaneSelectionFromOpArg( + operation.unlabeledArg, + kclManager.artifactGraph + ) + if (err(result)) { + return { reason: result.message } + } + plane = result } - // Convert the distance argument from a string to a KCL expression - const distanceResult = await stringToKclExpression( + // 2. Convert the offset argument from a string to a KCL expression + const offset = await stringToKclExpression( codeManager.code.slice( - operation.labeledArgs.offset.sourceRange[0], - operation.labeledArgs.offset.sourceRange[1] + operation.labeledArgs?.offset?.sourceRange[0], + operation.labeledArgs?.offset?.sourceRange[1] ) ) - - if (err(distanceResult) || 'errors' in distanceResult) { - return baseCommand + if (err(offset) || 'errors' in offset) { + return { reason: "Couldn't retrieve thickness argument" } } // Assemble the default argument values for the Offset Plane command, // with `nodeToEdit` set, which will let the Offset Plane actor know // to edit the node that corresponds to the StdLibCall. const argDefaultValues: ModelingCommandSchema['Offset plane'] = { - distance: distanceResult, plane, + offset, nodeToEdit: pathToNodeFromRustNodePath(operation.nodePath), } diff --git a/src/machines/modelingMachine.ts b/src/machines/modelingMachine.ts index 53ef5ec76b4..4089faa9eee 100644 --- a/src/machines/modelingMachine.ts +++ b/src/machines/modelingMachine.ts @@ -45,7 +45,6 @@ import { import { angleLengthInfo } from '@src/components/Toolbar/angleLengthInfo' import { updateModelingState } from '@src/lang/modelingWorkflows' import { - addOffsetPlane, insertNamedConstant, replaceValueAtNodePath, } from '@src/lang/modifyAst' @@ -87,7 +86,6 @@ import { updatePathToNodesAfterEdit, artifactIsPlaneWithPaths, isCursorInFunctionDefinition, - getSelectedPlaneAsNode, } from '@src/lang/queryAst' import { getFaceCodeRef, @@ -138,7 +136,7 @@ import type { Plane } from '@rust/kcl-lib/bindings/Plane' import type { Point3d } from '@rust/kcl-lib/bindings/ModelingCmd' import { getNodePathFromSourceRange } from '@src/lang/queryAstNodePathUtils' import { letEngineAnimateAndSyncCamAfter } from '@src/clientSideScene/CameraControls' -import { addShell } from '@src/lang/modifyAst/faces' +import { addShell, addOffsetPlane } from '@src/lang/modifyAst/faces' import { intersectInfo } from '@src/components/Toolbar/Intersect' import { addHelix } from '@src/lang/modifyAst/geometry' @@ -2556,72 +2554,20 @@ export const modelingMachine = setup({ return Promise.reject(new Error(NO_INPUT_PROVIDED_MESSAGE)) } - // Extract inputs - const ast = kclManager.ast - const { plane: selection, distance, nodeToEdit } = input - - let insertIndex: number | undefined = undefined - let planeName: string | undefined = undefined - - // If this is an edit flow, first we're going to remove the old plane - if (nodeToEdit && typeof nodeToEdit[1][0] === 'number') { - // Extract the plane name from the node to edit - const planeNameNode = getNodeFromPath( - ast, - nodeToEdit, - 'VariableDeclaration' - ) - if (err(planeNameNode)) { - console.error('Error extracting plane name') - } else { - planeName = planeNameNode.node.declaration.id.name - } - - const newBody = [...ast.body] - newBody.splice(nodeToEdit[1][0], 1) - ast.body = newBody - insertIndex = nodeToEdit[1][0] - } - - const selectedPlane = getSelectedPlaneAsNode( - selection, - kclManager.variables - ) - if (!selectedPlane) { - return trap('No plane selected') - } - - // Get the default plane name from the selection - const offsetPlaneResult = addOffsetPlane({ - node: ast, - plane: selectedPlane, - offset: - 'variableName' in distance - ? distance.variableIdentifierAst - : distance.valueAst, - insertIndex, - planeName, + const { ast, artifactGraph, variables } = kclManager + const astResult = addOffsetPlane({ + ...input, + ast, + artifactGraph, + variables, }) - - // Insert the distance variable if the user has provided a variable name - if ( - 'variableName' in distance && - distance.variableName && - typeof offsetPlaneResult.pathToNode[1][0] === 'number' - ) { - const insertIndex = Math.min( - offsetPlaneResult.pathToNode[1][0], - distance.insertIndex - ) - const newBody = [...offsetPlaneResult.modifiedAst.body] - newBody.splice(insertIndex, 0, distance.variableDeclarationAst) - offsetPlaneResult.modifiedAst.body = newBody - // Since we inserted a new variable, we need to update the path to the extrude argument - offsetPlaneResult.pathToNode[1][0]++ + if (err(astResult)) { + return Promise.reject(astResult) } + const { modifiedAst, pathToNode } = astResult await updateModelingState( - offsetPlaneResult.modifiedAst, + modifiedAst, EXECUTION_TYPE_REAL, { kclManager, @@ -2629,7 +2575,7 @@ export const modelingMachine = setup({ codeManager, }, { - focusPath: [offsetPlaneResult.pathToNode], + focusPath: [pathToNode], } ) }