Skip to content

Commit

Permalink
Nadro/3716/mvp revolve (#3728)
Browse files Browse the repository at this point in the history
* chore: Implemented a executeAst interrupt to stop processing a KCL program

* fix: added a catch since this promise was not being caught

* fix: fmt formatting, need to fix some tsc errors next.

* fix: fixing tsc errors

* fix: cleaning up comment

* fix: only rejecting pending modeling commands

* fix: adding constant for rejection message, adding rejection in WASM send command

* fix: tsc, lint, fmt checks

* feat: first pass over revolve with basic hard coded X axis

* fix: updated revolve status for DEV only

* fix: adding some TODOs to warn others about the Revolve MVP

* fix: fmt, lint, tsc checks

* fix: codespell got me

* fix: xstate v5 upgrade

* fix: removing this fix for a different PR. Not needed for initial MVP

* fix: renaming extrude function to sweep since it fixes extrude and revolve now

* fix: updating selection logic to support revolve

* fix: renaming extrude to sweep since it adds revolve

* fix: swapping as for type in function parameters

* fix: updated from object destruct to structuredClone

* fix: addressing PR comments

* fix: one other typo for return value of revolve
  • Loading branch information
nadr0 committed Sep 17, 2024
1 parent 61c7d98 commit 8c5b146
Show file tree
Hide file tree
Showing 10 changed files with 242 additions and 22 deletions.
28 changes: 24 additions & 4 deletions src/components/ModelingMachineProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
import { applyConstraintAngleLength } from './Toolbar/setAngleLength'
import {
Selections,
canExtrudeSelection,
canSweepSelection,
handleSelectionBatch,
isSelectionLastLine,
isRangeInbetweenCharacters,
Expand All @@ -62,8 +62,8 @@ import {
} from 'lang/modifyAst'
import { Program, parse, recast } from 'lang/wasm'
import {
doesSceneHaveSweepableSketch,
getNodePathFromSourceRange,
hasExtrudableGeometry,
isSingleCursorInPipe,
} from 'lang/queryAst'
import { exportFromEngine } from 'lib/exportFromEngine'
Expand Down Expand Up @@ -528,12 +528,32 @@ export const ModelingMachineProvider = ({
// they have no selection, we should enable the button
// so they can select the face through the cmdbar
// BUT only if there's extrudable geometry
if (hasExtrudableGeometry(kclManager.ast)) return true
if (doesSceneHaveSweepableSketch(kclManager.ast)) return true
return false
}
if (!isPipe) return false

return canExtrudeSelection(selectionRanges)
return canSweepSelection(selectionRanges)
},
'has valid revolve selection': ({ context: { selectionRanges } }) => {
// A user can begin extruding if they either have 1+ faces selected or nothing selected
// TODO: I believe this guard only allows for extruding a single face at a time
const isPipe = isSketchPipe(selectionRanges)

if (
selectionRanges.codeBasedSelections.length === 0 ||
isRangeInbetweenCharacters(selectionRanges) ||
isSelectionLastLine(selectionRanges, codeManager.code)
) {
// they have no selection, we should enable the button
// so they can select the face through the cmdbar
// BUT only if there's extrudable geometry
if (doesSceneHaveSweepableSketch(kclManager.ast)) return true
return false
}
if (!isPipe) return false

return canSweepSelection(selectionRanges)
},
'has valid selection for deletion': ({
context: { selectionRanges },
Expand Down
100 changes: 98 additions & 2 deletions src/lang/modifyAst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,15 @@ export function extrudeSketch(
node: Program,
pathToNode: PathToNode,
shouldPipe = false,
distance = createLiteral(4) as Expr
distance: Expr = createLiteral(4)
):
| {
modifiedAst: Program
pathToNode: PathToNode
pathToExtrudeArg: PathToNode
}
| Error {
const _node = { ...node }
const _node = structuredClone(node)
const _node1 = getNodeFromPath(_node, pathToNode)
if (err(_node1)) return _node1
const { node: sketchExpression } = _node1
Expand Down Expand Up @@ -342,6 +342,102 @@ export function extrudeSketch(
}
}

export function revolveSketch(
node: Program,
pathToNode: PathToNode,
shouldPipe = false,
angle: Expr = createLiteral(4)
):
| {
modifiedAst: Program
pathToNode: PathToNode
pathToRevolveArg: PathToNode
}
| Error {
const _node = structuredClone(node)
const _node1 = getNodeFromPath(_node, pathToNode)
if (err(_node1)) return _node1
const { node: sketchExpression } = _node1

// determine if sketchExpression is in a pipeExpression or not
const _node2 = getNodeFromPath<PipeExpression>(
_node,
pathToNode,
'PipeExpression'
)
if (err(_node2)) return _node2
const { node: pipeExpression } = _node2

const isInPipeExpression = pipeExpression.type === 'PipeExpression'

const _node3 = getNodeFromPath<VariableDeclarator>(
_node,
pathToNode,
'VariableDeclarator'
)
if (err(_node3)) return _node3
const { node: variableDeclarator, shallowPath: pathToDecleration } = _node3

const revolveCall = createCallExpressionStdLib('revolve', [
createObjectExpression({
angle: angle,
// TODO: hard coded 'X' axis for revolve MVP, should be changed.
axis: createLiteral('X'),
}),
createIdentifier(variableDeclarator.id.name),
])

if (shouldPipe) {
const pipeChain = createPipeExpression(
isInPipeExpression
? [...pipeExpression.body, revolveCall]
: [sketchExpression as any, revolveCall]
)

variableDeclarator.init = pipeChain
const pathToRevolveArg: PathToNode = [
...pathToDecleration,
['init', 'VariableDeclarator'],
['body', ''],
[pipeChain.body.length - 1, 'index'],
['arguments', 'CallExpression'],
[0, 'index'],
]

return {
modifiedAst: _node,
pathToNode,
pathToRevolveArg,
}
}

// We're not creating a pipe expression,
// but rather a separate constant for the extrusion
const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.REVOLVE)
const VariableDeclaration = createVariableDeclaration(name, revolveCall)
const sketchIndexInPathToNode =
pathToDecleration.findIndex((a) => a[0] === 'body') + 1
const sketchIndexInBody = pathToDecleration[sketchIndexInPathToNode][0]
if (typeof sketchIndexInBody !== 'number')
return new Error('expected sketchIndexInBody to be a number')
_node.body.splice(sketchIndexInBody + 1, 0, VariableDeclaration)

const pathToRevolveArg: PathToNode = [
['body', ''],
[sketchIndexInBody + 1, 'index'],
['declarations', 'VariableDeclaration'],
[0, 'index'],
['init', 'VariableDeclarator'],
['arguments', 'CallExpression'],
[0, 'index'],
]
return {
modifiedAst: _node,
pathToNode: [...pathToNode.slice(0, -1), [-1, 'index']],
pathToRevolveArg,
}
}

export function sketchOnExtrudedFace(
node: Program,
sketchPathToNode: PathToNode,
Expand Down
8 changes: 4 additions & 4 deletions src/lang/queryAst.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
hasExtrudeSketchGroup,
findUsesOfTagInPipe,
hasSketchPipeBeenExtruded,
hasExtrudableGeometry,
doesSceneHaveSweepableSketch,
traverse,
} from './queryAst'
import { enginelessExecutor } from '../lib/testHelpers'
Expand Down Expand Up @@ -488,7 +488,7 @@ const sketch002 = startSketchOn(extrude001, $seg01)
})
})

describe('Testing hasExtrudableGeometry', () => {
describe('Testing doesSceneHaveSweepableSketch', () => {
it('finds sketch001 pipe to be extruded', async () => {
const exampleCode = `const sketch001 = startSketchOn('XZ')
|> startProfileAt([3.29, 7.86], %)
Expand All @@ -506,7 +506,7 @@ const sketch002 = startSketchOn(extrude001, $seg01)
`
const ast = parse(exampleCode)
if (err(ast)) throw ast
const extrudable = hasExtrudableGeometry(ast)
const extrudable = doesSceneHaveSweepableSketch(ast)
expect(extrudable).toBeTruthy()
})
it('find sketch002 NOT pipe to be extruded', async () => {
Expand All @@ -520,7 +520,7 @@ const extrude001 = extrude(10, sketch001)
`
const ast = parse(exampleCode)
if (err(ast)) throw ast
const extrudable = hasExtrudableGeometry(ast)
const extrudable = doesSceneHaveSweepableSketch(ast)
expect(extrudable).toBeFalsy()
})
})
Expand Down
6 changes: 3 additions & 3 deletions src/lang/queryAst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ export function hasSketchPipeBeenExtruded(selection: Selection, ast: Program) {
if (
node.type === 'CallExpression' &&
node.callee.type === 'Identifier' &&
node.callee.name === 'extrude' &&
(node.callee.name === 'extrude' || node.callee.name === 'revolve') &&
node.arguments?.[1]?.type === 'Identifier' &&
node.arguments[1].name === varDec.id.name
) {
Expand All @@ -892,7 +892,7 @@ export function hasSketchPipeBeenExtruded(selection: Selection, ast: Program) {
}

/** File must contain at least one sketch that has not been extruded already */
export function hasExtrudableGeometry(ast: Program) {
export function doesSceneHaveSweepableSketch(ast: Program) {
const theMap: any = {}
traverse(ast as any, {
enter(node) {
Expand Down Expand Up @@ -925,7 +925,7 @@ export function hasExtrudableGeometry(ast: Program) {
}
} else if (
node.type === 'CallExpression' &&
node.callee.name === 'extrude' &&
(node.callee.name === 'extrude' || node.callee.name === 'revolve') &&
node.arguments[1]?.type === 'Identifier' &&
theMap?.[node?.arguments?.[1]?.name]
) {
Expand Down
27 changes: 26 additions & 1 deletion src/lib/commandBarConfigs/modelingCommandConfig.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Models } from '@kittycad/lib'
import { StateMachineCommandSetConfig, KclCommandValue } from 'lib/commandTypes'
import { KCL_DEFAULT_LENGTH } from 'lib/constants'
import { KCL_DEFAULT_LENGTH, KCL_DEFAULT_DEGREE } from 'lib/constants'
import { components } from 'lib/machine-api'
import { Selections } from 'lib/selections'
import { machineManager } from 'lib/machineManager'
Expand Down Expand Up @@ -32,6 +32,10 @@ export type ModelingCommandSchema = {
// result: (typeof EXTRUSION_RESULTS)[number]
distance: KclCommandValue
}
Revolve: {
selection: Selections
angle: KclCommandValue
}
Fillet: {
// todo
selection: Selections
Expand Down Expand Up @@ -209,6 +213,7 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
args: {
selection: {
inputType: 'selection',
// TODO: These are products of an extrude
selectionTypes: ['extrude-wall', 'start-cap', 'end-cap'],
multiple: false, // TODO: multiple selection
required: true,
Expand All @@ -232,6 +237,26 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
},
},
},
// TODO: Update this configuration, copied from extrude for MVP of revolve, specifically the args.selection
Revolve: {
description: 'Create a 3D body by rotating a sketch region about an axis.',
icon: 'revolve',
needsReview: true,
args: {
selection: {
inputType: 'selection',
selectionTypes: ['extrude-wall', 'start-cap', 'end-cap'],
multiple: false, // TODO: multiple selection
required: true,
skip: true,
},
angle: {
inputType: 'kcl',
defaultValue: KCL_DEFAULT_DEGREE,
required: true,
},
},
},
Fillet: {
// todo
description: 'Fillet edge',
Expand Down
5 changes: 5 additions & 0 deletions src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,14 @@ export const KCL_DEFAULT_CONSTANT_PREFIXES = {
SKETCH: 'sketch',
EXTRUDE: 'extrude',
SEGMENT: 'seg',
REVOLVE: 'revolve',
} as const
/** The default KCL length expression */
export const KCL_DEFAULT_LENGTH = `5`

/** The default KCL degree expression */
export const KCL_DEFAULT_DEGREE = `360`

/** localStorage key for the playwright test-specific app settings file */
export const TEST_SETTINGS_FILE_KEY = 'playwright-test-settings'

Expand Down
1 change: 1 addition & 0 deletions src/lib/createMachineCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export function createMachineCommand<
| Command<T, typeof type, S[typeof type]>[]
| null {
const commandConfig = commandBarConfig && commandBarConfig[type]

// There may be no command config for this event type,
// or there may be multiple commands to create.
if (!commandConfig) {
Expand Down
16 changes: 11 additions & 5 deletions src/lib/selections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,16 @@ function buildCommonNodeFromSelection(selectionRanges: Selections, i: number) {
}

function nodeHasExtrude(node: CommonASTNode) {
return doesPipeHaveCallExp({
calleeName: 'extrude',
...node,
})
return (
doesPipeHaveCallExp({
calleeName: 'extrude',
...node,
}) ||
doesPipeHaveCallExp({
calleeName: 'revolve',
...node,
})
)
}

function nodeHasClose(node: CommonASTNode) {
Expand All @@ -408,7 +414,7 @@ function nodeHasClose(node: CommonASTNode) {
})
}

export function canExtrudeSelection(selection: Selections) {
export function canSweepSelection(selection: Selections) {
const commonNodes = selection.codeBasedSelections.map((_, i) =>
buildCommonNodeFromSelection(selection, i)
)
Expand Down
11 changes: 9 additions & 2 deletions src/lib/toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,16 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
},
{
id: 'revolve',
onClick: () => console.error('Revolve not yet implemented'),
onClick: ({ commandBarSend }) =>
commandBarSend({
type: 'Find and select command',
data: { name: 'Revolve', groupId: 'modeling' },
}),
// TODO: disabled
// Who's state is this?
disabled: (state) => !state.can({ type: 'Revolve' }),
icon: 'revolve',
status: 'kcl-only',
status: DEV ? 'available' : 'kcl-only',
title: 'Revolve',
hotkey: 'R',
description:
Expand Down
Loading

0 comments on commit 8c5b146

Please sign in to comment.