Add basic axis case for Helix point-and-click (#5240)

* Move Helix button to a section with offset plane (3d 'construction' elements) Fixes #5234 * Add generix axis case for Helix point-and-click Fixes #5072 #5236 #5073 * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Trigger CI * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Trigger CI * Clean up * Temp remove point and click test * Add back point and click test * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Clean up * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores) * Fix helix arg after change to kwargs * More fixes wrt helix arg after change to kwargs * Fixed thanks to @adamchalmers --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
KittyCAD · Feb 7, 2025 · b82eec8 · b82eec8
1 parent 5dc4213
commit b82eec8
Show file tree

Hide file tree

Showing 8 changed files with 275 additions and 2 deletions.
diff --git a/e2e/playwright/fixtures/toolbarFixture.ts b/e2e/playwright/fixtures/toolbarFixture.ts
@@ -20,6 +20,7 @@ export class ToolbarFixture {
   shellButton!: Locator
   revolveButton!: Locator
   offsetPlaneButton!: Locator
+  helixButton!: Locator
   startSketchBtn!: Locator
   lineBtn!: Locator
   rectangleBtn!: Locator
@@ -49,6 +50,7 @@ export class ToolbarFixture {
     this.shellButton = page.getByTestId('shell')
     this.revolveButton = page.getByTestId('revolve')
     this.offsetPlaneButton = page.getByTestId('plane-offset')
+    this.helixButton = page.getByTestId('helix')
     this.startSketchBtn = page.getByTestId('sketch')
     this.lineBtn = page.getByTestId('line')
     this.rectangleBtn = page.getByTestId('corner-rectangle')

diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts
@@ -775,6 +775,71 @@ openSketch = startSketchOn('XY')
     })
   })
 
+  test('Helix point-and-click', async ({
+    context,
+    page,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    // One dumb hardcoded screen pixel value
+    const testPoint = { x: 620, y: 257 }
+    const expectedOutput = `helix001 = helix(revolutions = 1, angleStart = 360, counterClockWise = false, radius = 5, axis = 'X', length = 5)`
+
+    await homePage.goToModelingScene()
+
+    await test.step(`Look for the red of the default plane`, async () => {
+      await scene.expectPixelColor([96, 52, 52], testPoint, 15)
+    })
+    await test.step(`Go through the command bar flow`, async () => {
+      await toolbar.helixButton.click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'revolutions',
+        currentArgValue: '1',
+        headerArguments: {
+          AngleStart: '',
+          Axis: '',
+          CounterClockWise: '',
+          Length: '',
+          Radius: '',
+          Revolutions: '',
+        },
+        highlightedHeaderArg: 'revolutions',
+        commandName: 'Helix',
+      })
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+    })
+
+    await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
+      await editor.expectEditor.toContain(expectedOutput)
+      await editor.expectState({
+        diagnostics: [],
+        activeLines: [expectedOutput],
+        highlightedCode: '',
+      })
+      // Red plane is now gone, white helix is there
+      await scene.expectPixelColor([250, 250, 250], testPoint, 15)
+    })
+
+    await test.step('Delete offset plane via feature tree selection', async () => {
+      await editor.closePane()
+      const operationButton = await toolbar.getFeatureTreeOperation('Helix', 0)
+      await operationButton.click({ button: 'left' })
+      await page.keyboard.press('Backspace')
+      // Red plane is back
+      await scene.expectPixelColor([96, 52, 52], testPoint, 15)
+    })
+  })
+
   const loftPointAndClickCases = [
     { shouldPreselect: true },
     { shouldPreselect: false },

diff --git a/...apshots/extrude-on-default-planes-should-be-stable-YZ-1-Google-Chrome-linux.png b/...apshots/extrude-on-default-planes-should-be-stable-YZ-1-Google-Chrome-linux.png
diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts
@@ -683,6 +683,63 @@ export function addOffsetPlane({
   }
 }
 
+/**
+ * Append a helix to the AST
+ */
+export function addHelix({
+  node,
+  revolutions,
+  angleStart,
+  counterClockWise,
+  radius,
+  axis,
+  length,
+}: {
+  node: Node<Program>
+  revolutions: Expr
+  angleStart: Expr
+  counterClockWise: boolean
+  radius: Expr
+  axis: string
+  length: Expr
+}): { modifiedAst: Node<Program>; pathToNode: PathToNode } {
+  const modifiedAst = structuredClone(node)
+  const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.HELIX)
+  const variable = createVariableDeclaration(
+    name,
+    createCallExpressionStdLibKw(
+      'helix',
+      null, // Not in a pipeline
+      [
+        createLabeledArg('revolutions', revolutions),
+        createLabeledArg('angleStart', angleStart),
+        createLabeledArg('counterClockWise', createLiteral(counterClockWise)),
+        createLabeledArg('radius', radius),
+        createLabeledArg('axis', createLiteral(axis)),
+        createLabeledArg('length', length),
+      ]
+    )
+  )
+
+  // TODO: figure out smart insertion than just appending at the end
+  const argIndex = 0
+  modifiedAst.body.push(variable)
+  const pathToNode: PathToNode = [
+    ['body', ''],
+    [modifiedAst.body.length - 1, 'index'],
+    ['declaration', 'VariableDeclaration'],
+    ['init', 'VariableDeclarator'],
+    ['arguments', 'CallExpressionKw'],
+    [argIndex, ARG_INDEX_FIELD],
+    ['arg', LABELED_ARG_FIELD],
+  ]
+
+  return {
+    modifiedAst,
+    pathToNode,
+  }
+}
+
 /**
  * Return a modified clone of an AST with a named constant inserted into the body
  */

diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts
@@ -76,6 +76,14 @@ export type ModelingCommandSchema = {
     plane: Selections
     distance: KclCommandValue
   }
+  Helix: {
+    revolutions: KclCommandValue
+    angleStart: KclCommandValue
+    counterClockWise: boolean
+    radius: KclCommandValue
+    axis: string
+    length: KclCommandValue
+  }
   'change tool': {
     tool: SketchTool
   }
@@ -447,6 +455,53 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
       },
     },
   },
+  Helix: {
+    description: 'Create a helix or spiral in 3D about an axis.',
+    icon: 'helix',
+    status: 'development',
+    needsReview: true,
+    args: {
+      revolutions: {
+        inputType: 'kcl',
+        defaultValue: '1',
+        required: true,
+        warningMessage:
+          'The helix workflow is new and under tested. Please break it and report issues.',
+      },
+      angleStart: {
+        inputType: 'kcl',
+        defaultValue: KCL_DEFAULT_DEGREE,
+        required: true,
+      },
+      counterClockWise: {
+        inputType: 'options',
+        required: true,
+        options: [
+          { name: 'True', isCurrent: false, value: true },
+          { name: 'False', isCurrent: true, value: false },
+        ],
+      },
+      radius: {
+        inputType: 'kcl',
+        defaultValue: KCL_DEFAULT_LENGTH,
+        required: true,
+      },
+      axis: {
+        inputType: 'options',
+        required: true,
+        options: [
+          { name: 'X Axis', isCurrent: true, value: 'X' },
+          { name: 'Y Axis', isCurrent: false, value: 'Y' },
+          { name: 'Z Axis', isCurrent: false, value: 'Z' },
+        ],
+      },
+      length: {
+        inputType: 'kcl',
+        defaultValue: KCL_DEFAULT_LENGTH,
+        required: true,
+      },
+    },
+  },
   Fillet: {
     description: 'Fillet edge',
     icon: 'fillet3d',

diff --git a/src/lib/constants.ts b/src/lib/constants.ts
@@ -58,6 +58,7 @@ export const KCL_DEFAULT_CONSTANT_PREFIXES = {
   SEGMENT: 'seg',
   REVOLVE: 'revolve',
   PLANE: 'plane',
+  HELIX: 'helix',
 } as const
 /** The default KCL length expression */
 export const KCL_DEFAULT_LENGTH = `5`

diff --git a/src/lib/toolbar.ts b/src/lib/toolbar.ts
@@ -290,9 +290,15 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
       ],
       {
         id: 'helix',
-        onClick: () => console.error('Helix not yet implemented'),
+        onClick: () => {
+          commandBarActor.send({
+            type: 'Find and select command',
+            data: { name: 'Helix', groupId: 'modeling' },
+          })
+        },
+        hotkey: 'H',
         icon: 'helix',
-        status: 'kcl-only',
+        status: DEV || IS_NIGHTLY_OR_DEBUG ? 'available' : 'kcl-only',
         title: 'Helix',
         description: 'Create a helix or spiral in 3D about an axis.',
         links: [{ label: 'KCL docs', url: 'https://zoo.dev/docs/kcl/helix' }],

diff --git a/src/machines/modelingMachine.ts b/src/machines/modelingMachine.ts
@@ -42,6 +42,7 @@ import {
 } from 'components/Toolbar/EqualLength'
 import { revolveSketch } from 'lang/modifyAst/addRevolve'
 import {
+  addHelix,
   addOffsetPlane,
   addSweep,
   deleteFromSelection,
@@ -276,6 +277,7 @@ export type ModelingMachineEvent =
   | { type: 'Fillet'; data?: ModelingCommandSchema['Fillet'] }
   | { type: 'Chamfer'; data?: ModelingCommandSchema['Chamfer'] }
   | { type: 'Offset plane'; data: ModelingCommandSchema['Offset plane'] }
+  | { type: 'Helix'; data: ModelingCommandSchema['Helix'] }
   | { type: 'Text-to-CAD'; data: ModelingCommandSchema['Text-to-CAD'] }
   | { type: 'Prompt-to-edit'; data: ModelingCommandSchema['Prompt-to-edit'] }
   | {
@@ -1615,6 +1617,73 @@ export const modelingMachine = setup({
         }
       }
     ),
+    helixAstMod: fromPromise(
+      async ({
+        input,
+      }: {
+        input: ModelingCommandSchema['Helix'] | undefined
+      }) => {
+        if (!input) return new Error('No input provided')
+        // Extract inputs
+        const ast = kclManager.ast
+        const {
+          revolutions,
+          angleStart,
+          counterClockWise,
+          radius,
+          axis,
+          length,
+        } = input
+
+        for (const variable of [revolutions, angleStart, radius, length]) {
+          // Insert the variable if it exists
+          if (
+            'variableName' in variable &&
+            variable.variableName &&
+            variable.insertIndex !== undefined
+          ) {
+            const newBody = [...ast.body]
+            newBody.splice(
+              variable.insertIndex,
+              0,
+              variable.variableDeclarationAst
+            )
+            ast.body = newBody
+          }
+        }
+
+        const valueOrVariable = (variable: KclCommandValue) =>
+          'variableName' in variable
+            ? variable.variableIdentifierAst
+            : variable.valueAst
+
+        const result = addHelix({
+          node: ast,
+          revolutions: valueOrVariable(revolutions),
+          angleStart: valueOrVariable(angleStart),
+          counterClockWise,
+          radius: valueOrVariable(radius),
+          axis,
+          length: valueOrVariable(length),
+        })
+
+        const updateAstResult = await kclManager.updateAst(
+          result.modifiedAst,
+          true,
+          {
+            focusPath: [result.pathToNode],
+          }
+        )
+
+        await codeManager.updateEditorWithAstAndWriteToFile(
+          updateAstResult.newAst
+        )
+
+        if (updateAstResult?.selections) {
+          editorManager.selectRange(updateAstResult?.selections)
+        }
+      }
+    ),
     sweepAstMod: fromPromise(
       async ({
         input,
@@ -1974,6 +2043,11 @@ export const modelingMachine = setup({
           reenter: true,
         },
 
+        Helix: {
+          target: 'Applying helix',
+          reenter: true,
+        },
+
         'Prompt-to-edit': 'Applying Prompt-to-edit',
       },
 
@@ -2734,6 +2808,19 @@ export const modelingMachine = setup({
       },
     },
 
+    'Applying helix': {
+      invoke: {
+        src: 'helixAstMod',
+        id: 'helixAstMod',
+        input: ({ event }) => {
+          if (event.type !== 'Helix') return undefined
+          return event.data
+        },
+        onDone: ['idle'],
+        onError: ['idle'],
+      },
+    },
+
     'Applying sweep': {
       invoke: {
         src: 'sweepAstMod',