ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Show offset planes in the scene, let user select them (#4481)

Browse Source 
* Update offset_plane to actually create and show the plane in-engine

* Fix broken ability to use offsetPlanes in startSketchOn

* Make the newly-visible offset planes usable for sketching via UI

* Add a playwright test for sketching on an offset plane via point-and-click

* cargo clippy & cargo fmt

* Make `PlaneData` the first item of `SketchData` so autocomplete continues to work well for `startSketchOn`

* @nadr0 feedback re: `offsetIndex`

* From @jtran: "Need to call the ID generator so that IDs are stable."

* More feedback from @jtran and fix incomplete use of `id_generator` in last commit

* Oops I missed saving `isPathToNodeNumber` earlier 🤦🏻

* Make the distinction between `Plane` and `PlaneOrientationData` more clear per @nadr0 and @lf94's feedback

* Make `newPathToNode` less hardcoded, per @lf94's feedback

* Don't need to unbox and rebox `plane`

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* Rearranging of enums and structs, but the offsetPlanes are still not used by their sketches

* A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores)

* Revert all my little newtype fiddling it's a waste of time.

* Update docs

* cargo fmt

* Remove log

* Print the unexpected diagnostics

* Undo renaming of `PlaneData`

* Remove generated PlaneRientationData docs page

* Redo doc generation after undoing `PlaneData` rename

* Impl FromKclValue for the new plane datatypes

* Clippy lint

* When starting a sketch, only hide the plane if it's a custom plane

* Fix FromKclValue and macro use since merge

* Fix to not convert Plane to PlaneData

* Make sure offset planes are `Custom` type

* SketchData actually doesn't need to be in a certain order
This avoids the autocompletion issue I was having.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Adam Chalmers <adam.chalmers@zoo.dev>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
This commit is contained in:
Frank Noirot
2024-11-18 16:25:25 -05:00
committed by GitHub
parent 97b9529c81
commit 24bc4fcd8c
21 changed files with 762 additions and 243 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/components/ModelingMachineProvider.tsx
View File

@ -63,6 +63,7 @@ import {
import {
moveValueIntoNewVariablePath,
sketchOnExtrudedFace,
sketchOnOffsetPlane,
startSketchOnDefault,
} from 'lang/modifyAst'
import { Program, parse, recast } from 'lang/wasm'
@ -636,13 +637,16 @@ export const ModelingMachineProvider = ({
),
'animate-to-face': fromPromise(async ({ input }) => {
if (!input) return undefined
if (input.type === 'extrudeFace') {
const sketched = sketchOnExtrudedFace(
kclManager.ast,
input.sketchPathToNode,
input.extrudePathToNode,
input.faceInfo
)
if (input.type === 'extrudeFace' || input.type === 'offsetPlane') {
const sketched =
input.type === 'extrudeFace'
? sketchOnExtrudedFace(
kclManager.ast,
input.sketchPathToNode,
input.extrudePathToNode,
input.faceInfo
)
: sketchOnOffsetPlane(kclManager.ast, input.pathToNode)
if (err(sketched)) {
const sketchedError = new Error(
'Incompatible face, please try another'
@ -654,10 +658,9 @@ export const ModelingMachineProvider = ({
await kclManager.executeAstMock(modifiedAst)
await letEngineAnimateAndSyncCamAfter(
engineCommandManager,
input.faceId
)
const id =
input.type === 'extrudeFace' ? input.faceId : input.planeId
await letEngineAnimateAndSyncCamAfter(engineCommandManager, id)
sceneInfra.camControls.syncDirection = 'clientToEngine'
return {
sketchPathToNode: pathToNewSketchNode,

40
src/hooks/useEngineConnectionSubscriptions.ts
View File

@ -88,6 +88,10 @@ export function useEngineConnectionSubscriptions() {
? [codeRef.range]
: [codeRef.range, consumedCodeRef.range]
)
} else if (artifact?.type === 'plane') {
const codeRef = artifact.codeRef
if (err(codeRef)) return
editorManager.setHighlightRange([codeRef.range])
} else {
editorManager.setHighlightRange([[0, 0]])
}
@ -186,8 +190,42 @@ export function useEngineConnectionSubscriptions() {
})
return
}
const artifact =
engineCommandManager.artifactGraph.get(planeOrFaceId)
if (artifact?.type === 'plane') {
const planeInfo = await getFaceDetails(planeOrFaceId)
sceneInfra.modelingSend({
type: 'Select default plane',
data: {
type: 'offsetPlane',
zAxis: [
planeInfo.z_axis.x,
planeInfo.z_axis.y,
planeInfo.z_axis.z,
],
yAxis: [
planeInfo.y_axis.x,
planeInfo.y_axis.y,
planeInfo.y_axis.z,
],
position: [
planeInfo.origin.x,
planeInfo.origin.y,
planeInfo.origin.z,
].map((num) => num / sceneInfra._baseUnitMultiplier) as [
number,
number,
number
],
planeId: planeOrFaceId,
pathToNode: artifact.codeRef.pathToNode,
},
})
}
// Artifact is likely an extrusion face
const faceId = planeOrFaceId
const artifact = engineCommandManager.artifactGraph.get(faceId)
const extrusion = getSweepFromSuspectedSweepSurface(
faceId,
engineCommandManager.artifactGraph

55
src/lang/modifyAst.ts
View File

@ -19,6 +19,7 @@ import {
ProgramMemory,
SourceRange,
sketchFromKclValue,
isPathToNodeNumber,
} from './wasm'
import {
isNodeSafeToReplacePath,
@ -526,6 +527,60 @@ export function sketchOnExtrudedFace(
}
}
/**
* Modify the AST to create a new sketch using the variable declaration
* of an offset plane. The new sketch just has to come after the offset
* plane declaration.
*/
export function sketchOnOffsetPlane(
node: Node<Program>,
offsetPathToNode: PathToNode
) {
let _node = { ...node }
// Find the offset plane declaration
const offsetPlaneDeclarator = getNodeFromPath<VariableDeclarator>(
_node,
offsetPathToNode,
'VariableDeclarator',
true
)
if (err(offsetPlaneDeclarator)) return offsetPlaneDeclarator
const { node: offsetPlaneNode } = offsetPlaneDeclarator
const offsetPlaneName = offsetPlaneNode.id.name
// Create a new sketch declaration
const newSketchName = findUniqueName(
node,
KCL_DEFAULT_CONSTANT_PREFIXES.SKETCH
)
const newSketch = createVariableDeclaration(
newSketchName,
createCallExpressionStdLib('startSketchOn', [
createIdentifier(offsetPlaneName),
]),
undefined,
'const'
)
// Decide where to insert the new sketch declaration
const offsetIndex = offsetPathToNode[1][0]
if (!isPathToNodeNumber(offsetIndex)) {
return new Error('Expected offsetIndex to be a number')
}
// and insert it
_node.body.splice(offsetIndex + 1, 0, newSketch)
const newPathToNode = structuredClone(offsetPathToNode)
newPathToNode[1][0] = offsetIndex + 1
// Return the modified AST and the path to the new sketch declaration
return {
modifiedAst: _node,
pathToNode: newPathToNode,
}
}
export const getLastIndex = (pathToNode: PathToNode): number =>
splitPathAtLastIndex(pathToNode).index

56
src/lang/std/artifactGraph.test.ts
View File

@ -98,12 +98,22 @@ sketch004 = startSketchOn(extrude003, seg02)
|> close(%)
extrude004 = extrude(3, sketch004)
`
const exampleCodeOffsetPlanes = `
offsetPlane001 = offsetPlane("XY", 20)
offsetPlane002 = offsetPlane("XZ", -50)
offsetPlane003 = offsetPlane("YZ", 10)
sketch002 = startSketchOn(offsetPlane001)
|> startProfileAt([0, 0], %)
|> line([6.78, 15.01], %)
`
// add more code snippets here and use `getCommands` to get the orderedCommands and responseMap for more tests
const codeToWriteCacheFor = {
exampleCode1,
sketchOnFaceOnFaceEtc,
exampleCodeNo3D,
exampleCodeOffsetPlanes,
} as const
type CodeKey = keyof typeof codeToWriteCacheFor
@ -165,6 +175,52 @@ afterAll(() => {
})
describe('testing createArtifactGraph', () => {
describe('code with offset planes and a sketch:', () => {
let ast: Program
let theMap: ReturnType<typeof createArtifactGraph>
it('setup', () => {
// putting this logic in here because describe blocks runs before beforeAll has finished
const {
orderedCommands,
responseMap,
ast: _ast,
} = getCommands('exampleCodeOffsetPlanes')
ast = _ast
theMap = createArtifactGraph({ orderedCommands, responseMap, ast })
})
it(`there should be one sketch`, () => {
const sketches = [...filterArtifacts({ types: ['path'] }, theMap)].map(
(path) => expandPath(path[1], theMap)
)
expect(sketches).toHaveLength(1)
sketches.forEach((path) => {
if (err(path)) throw path
expect(path.type).toBe('path')
})
})
it(`there should be three offsetPlanes`, () => {
const offsetPlanes = [
...filterArtifacts({ types: ['plane'] }, theMap),
].map((plane) => expandPlane(plane[1], theMap))
expect(offsetPlanes).toHaveLength(3)
offsetPlanes.forEach((path) => {
expect(path.type).toBe('plane')
})
})
it(`Only one offset plane should have a path`, () => {
const offsetPlanes = [
...filterArtifacts({ types: ['plane'] }, theMap),
].map((plane) => expandPlane(plane[1], theMap))
const offsetPlaneWithPaths = offsetPlanes.filter(
(plane) => plane.paths.length
)
expect(offsetPlaneWithPaths).toHaveLength(1)
})
})
describe('code with an extrusion, fillet and sketch of face:', () => {
let ast: Program
let theMap: ReturnType<typeof createArtifactGraph>

15
src/lang/std/artifactGraph.ts
View File

@ -249,7 +249,20 @@ export function getArtifactsToUpdate({
const cmd = command.cmd
const returnArr: ReturnType<typeof getArtifactsToUpdate> = []
if (!response) return returnArr
if (cmd.type === 'enable_sketch_mode') {
if (cmd.type === 'make_plane' && range[1] !== 0) {
// If we're calling `make_plane` and the code range doesn't end at `0`
// it's not a default plane, but a custom one from the offsetPlane standard library function
return [
{
id,
artifact: {
type: 'plane',
pathIds: [],
codeRef: { range, pathToNode },
},
},
]
} else if (cmd.type === 'enable_sketch_mode') {
const plane = getArtifact(currentPlaneId)
const pathIds = plane?.type === 'plane' ? plane?.pathIds : []
const codeRef =

6
src/lang/wasm.ts
View File

@ -144,6 +144,12 @@ export const parse = (code: string | Error): Node<Program> | Error => {
export type PathToNode = [string | number, string][]
export const isPathToNodeNumber = (
pathToNode: string | number
): pathToNode is number => {
return typeof pathToNode === 'number'
}
export interface ExecState {
memory: ProgramMemory
idGenerator: IdGenerator

13
src/machines/modelingMachine.ts
View File

@ -159,6 +159,15 @@ export type DefaultPlane = {
yAxis: [number, number, number]
}
export type OffsetPlane = {
type: 'offsetPlane'
position: [number, number, number]
planeId: string
pathToNode: PathToNode
zAxis: [number, number, number]
yAxis: [number, number, number]
}
export type SegmentOverlayPayload =
| {
type: 'set-one'
@ -198,7 +207,7 @@ export type ModelingMachineEvent =
| { type: 'Sketch On Face' }
| {
type: 'Select default plane'
data: DefaultPlane | ExtrudeFacePlane
data: DefaultPlane | ExtrudeFacePlane | OffsetPlane
}
| {
type: 'Set selection'
@ -1394,7 +1403,7 @@ export const modelingMachine = setup({
}
),
'animate-to-face': fromPromise(
async (_: { input?: ExtrudeFacePlane | DefaultPlane }) => {
async (_: { input?: ExtrudeFacePlane | DefaultPlane | OffsetPlane }) => {
return {} as
| undefined
| {

19
src/wasm-lib/kcl/src/executor.rs
View File

@ -801,6 +801,17 @@ impl Plane {
},
}
}
/// The standard planes are XY, YZ and XZ (in both positive and negative)
pub fn is_standard(&self) -> bool {
!self.is_custom()
}
/// The standard planes are XY, YZ and XZ (in both positive and negative)
/// Custom planes are any other plane that the user might specify.
pub fn is_custom(&self) -> bool {
matches!(self.value, PlaneType::Custom)
}
}
#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
@ -1049,6 +1060,14 @@ impl KclValue {
}
}
pub fn as_plane(&self) -> Option<&Plane> {
if let KclValue::Plane(value) = &self {
Some(value)
} else {
None
}
}
pub fn as_solid(&self) -> Option<&Solid> {
if let KclValue::Solid(value) = &self {
Some(value)

11
src/wasm-lib/kcl/src/lsp/tests.rs
View File

@ -2,7 +2,7 @@ use std::collections::BTreeMap;
use pretty_assertions::assert_eq;
use tower_lsp::{
lsp_types::{SemanticTokenModifier, SemanticTokenType},
lsp_types::{Diagnostic, SemanticTokenModifier, SemanticTokenType},
LanguageServer,
};
@ -2369,7 +2369,14 @@ async fn kcl_test_kcl_lsp_diagnostics_on_execution_error() {
// Get the diagnostics.
let diagnostics = server.diagnostics_map.get("file:///test.kcl");
assert!(diagnostics.is_none());
if let Some(diagnostics) = diagnostics {
let ds: Vec<Diagnostic> = diagnostics.to_owned();
eprintln!("Expected no diagnostics, but found some.");
for d in ds {
eprintln!("{:?}: {}", d.severity, d.message);
}
panic!();
}
}
#[tokio::test(flavor = "multi_thread")]

49
src/wasm-lib/kcl/src/std/args.rs
View File

@ -794,6 +794,45 @@ impl<'a> FromKclValue<'a> for crate::std::planes::StandardPlane {
}
}
impl<'a> FromKclValue<'a> for crate::executor::Plane {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
if let Some(plane) = arg.as_plane() {
return Some(plane.clone());
}
let obj = arg.as_object()?;
let_field_of!(obj, id);
let_field_of!(obj, value);
let_field_of!(obj, origin);
let_field_of!(obj, x_axis "xAxis");
let_field_of!(obj, y_axis "yAxis");
let_field_of!(obj, z_axis "zAxis");
let_field_of!(obj, meta "__meta");
Some(Self {
id,
value,
origin,
x_axis,
y_axis,
z_axis,
meta,
})
}
}
impl<'a> FromKclValue<'a> for crate::executor::PlaneType {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
let plane_type = match arg.as_str()? {
"XY" | "xy" => Self::XY,
"XZ" | "xz" => Self::XZ,
"YZ" | "yz" => Self::YZ,
"Custom" => Self::Custom,
_ => return None,
};
Some(plane_type)
}
}
impl<'a> FromKclValue<'a> for kittycad_modeling_cmds::units::UnitLength {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
let s = arg.as_str()?;
@ -1264,11 +1303,15 @@ impl<'a> FromKclValue<'a> for crate::executor::Solid {
impl<'a> FromKclValue<'a> for super::sketch::SketchData {
fn from_kcl_val(arg: &'a KclValue) -> Option<Self> {
let case1 = super::sketch::PlaneData::from_kcl_val;
let case2 = crate::executor::Solid::from_kcl_val;
// Order is critical since PlaneData is a subset of Plane.
let case1 = crate::executor::Plane::from_kcl_val;
let case2 = super::sketch::PlaneData::from_kcl_val;
let case3 = crate::executor::Solid::from_kcl_val;
case1(arg)
.map(Box::new)
.map(Self::Plane)
.or_else(|| case2(arg).map(Box::new).map(Self::Solid))
.or_else(|| case2(arg).map(Self::PlaneOrientation))
.or_else(|| case3(arg).map(Box::new).map(Self::Solid))
}
}

62
src/wasm-lib/kcl/src/std/planes.rs
View File

@ -1,17 +1,20 @@
//! Standard library plane helpers.
use derive_docs::stdlib;
use kcmc::{each_cmd as mcmd, length_unit::LengthUnit, shared::Color, ModelingCmd};
use kittycad_modeling_cmds as kcmc;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use crate::{
errors::KclError,
executor::{ExecState, KclValue, Plane},
executor::{ExecState, KclValue, Plane, PlaneType},
std::{sketch::PlaneData, Args},
};
/// One of the standard planes.
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, JsonSchema)]
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
#[ts(export)]
#[serde(rename_all = "camelCase")]
pub enum StandardPlane {
/// The XY plane.
@ -50,8 +53,8 @@ impl From<StandardPlane> for PlaneData {
/// Offset a plane by a distance along its normal.
pub async fn offset_plane(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
let (std_plane, offset): (StandardPlane, f64) = args.get_data_and_float()?;
let plane_data = inner_offset_plane(std_plane, offset, exec_state).await?;
let plane = Plane::from_plane_data(plane_data, exec_state);
let plane = inner_offset_plane(std_plane, offset, exec_state).await?;
make_offset_plane_in_engine(&plane, exec_state, &args).await?;
Ok(KclValue::Plane(Box::new(plane)))
}
@ -144,11 +147,14 @@ async fn inner_offset_plane(
std_plane: StandardPlane,
offset: f64,
exec_state: &mut ExecState,
) -> Result<PlaneData, KclError> {
) -> Result<Plane, KclError> {
// Convert to the plane type.
let plane_data: PlaneData = std_plane.into();
// Convert to a plane.
let mut plane = Plane::from_plane_data(plane_data, exec_state);
// Though offset planes are derived from standard planes, they are not
// standard planes themselves.
plane.value = PlaneType::Custom;
match std_plane {
StandardPlane::XY => {
@ -171,10 +177,44 @@ async fn inner_offset_plane(
}
}
Ok(PlaneData::Plane {
origin: Box::new(plane.origin),
x_axis: Box::new(plane.x_axis),
y_axis: Box::new(plane.y_axis),
z_axis: Box::new(plane.z_axis),
})
Ok(plane)
}
// Engine-side effectful creation of an actual plane object.
// offset planes are shown by default, and hidden by default if they
// are used as a sketch plane. That hiding command is sent within inner_start_profile_at
async fn make_offset_plane_in_engine(plane: &Plane, exec_state: &mut ExecState, args: &Args) -> Result<(), KclError> {
// Create new default planes.
let default_size = 100.0;
let color = Color {
r: 0.6,
g: 0.6,
b: 0.6,
a: 0.3,
};
args.batch_modeling_cmd(
plane.id,
ModelingCmd::from(mcmd::MakePlane {
clobber: false,
origin: plane.origin.into(),
size: LengthUnit(default_size),
x_axis: plane.x_axis.into(),
y_axis: plane.y_axis.into(),
hide: Some(false),
}),
)
.await?;
// Set the color.
args.batch_modeling_cmd(
exec_state.id_generator.next_uuid(),
ModelingCmd::from(mcmd::PlaneSetColor {
color,
plane_id: plane.id,
}),
)
.await?;
Ok(())
}

41
src/wasm-lib/kcl/src/std/sketch.rs
View File

@ -894,7 +894,7 @@ pub async fn start_sketch_at(exec_state: &mut ExecState, args: Args) -> Result<K
async fn inner_start_sketch_at(data: [f64; 2], exec_state: &mut ExecState, args: Args) -> Result<Sketch, KclError> {
// Let's assume it's the XY plane for now, this is just for backwards compatibility.
let xy_plane = PlaneData::XY;
let sketch_surface = inner_start_sketch_on(SketchData::Plane(xy_plane), None, exec_state, &args).await?;
let sketch_surface = inner_start_sketch_on(SketchData::PlaneOrientation(xy_plane), None, exec_state, &args).await?;
let sketch = inner_start_profile_at(data, sketch_surface, None, exec_state, args).await?;
Ok(sketch)
}
@ -905,11 +905,12 @@ async fn inner_start_sketch_at(data: [f64; 2], exec_state: &mut ExecState, args:
#[ts(export)]
#[serde(rename_all = "camelCase", untagged)]
pub enum SketchData {
Plane(PlaneData),
PlaneOrientation(PlaneData),
Plane(Box<Plane>),
Solid(Box<Solid>),
}
/// Data for a plane.
/// Orientation data that can be used to construct a plane, not a plane in itself.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
#[ts(export)]
#[serde(rename_all = "camelCase")]
@ -1069,10 +1070,11 @@ async fn inner_start_sketch_on(
args: &Args,
) -> Result<SketchSurface, KclError> {
match data {
SketchData::Plane(plane_data) => {
let plane = start_sketch_on_plane(plane_data, exec_state, args).await?;
SketchData::PlaneOrientation(plane_data) => {
let plane = make_sketch_plane_from_orientation(plane_data, exec_state, args).await?;
Ok(SketchSurface::Plane(plane))
}
SketchData::Plane(plane) => Ok(SketchSurface::Plane(plane)),
SketchData::Solid(solid) => {
let Some(tag) = tag else {
return Err(KclError::Type(KclErrorDetails {
@ -1106,7 +1108,7 @@ async fn start_sketch_on_face(
}))
}
async fn start_sketch_on_plane(
async fn make_sketch_plane_from_orientation(
data: PlaneData,
exec_state: &mut ExecState,
args: &Args,
@ -1122,10 +1124,10 @@ async fn start_sketch_on_plane(
plane.id = match data {
PlaneData::XY => default_planes.xy,
PlaneData::XZ => default_planes.xz,
PlaneData::YZ => default_planes.yz,
PlaneData::NegXY => default_planes.neg_xy,
PlaneData::XZ => default_planes.xz,
PlaneData::NegXZ => default_planes.neg_xz,
PlaneData::YZ => default_planes.yz,
PlaneData::NegYZ => default_planes.neg_yz,
PlaneData::Plane {
origin,
@ -1210,11 +1212,26 @@ pub(crate) async fn inner_start_profile_at(
exec_state: &mut ExecState,
args: Args,
) -> Result<Sketch, KclError> {
if let SketchSurface::Face(face) = &sketch_surface {
// Flush the batch for our fillets/chamfers if there are any.
// If we do not do these for sketch on face, things will fail with face does not exist.
args.flush_batch_for_solid_set(exec_state, face.solid.clone().into())
match &sketch_surface {
SketchSurface::Face(face) => {
// Flush the batch for our fillets/chamfers if there are any.
// If we do not do these for sketch on face, things will fail with face does not exist.
args.flush_batch_for_solid_set(exec_state, face.solid.clone().into())
.await?;
}
SketchSurface::Plane(plane) if !plane.is_standard() => {
// Hide whatever plane we are sketching on.
// This is especially helpful for offset planes, which would be visible otherwise.
args.batch_end_cmd(
exec_state.id_generator.next_uuid(),
ModelingCmd::from(mcmd::ObjectVisible {
object_id: plane.id,
hidden: true,
}),
)
.await?;
}
_ => {}
}
// Enter sketch mode on the surface.