Fix sketchOnFace point&click for booleans

e2e/playwright/fixtures/toolbarFixture.ts

@@ -241,6 +241,12 @@ export class ToolbarFixture {
   async openFeatureTreePane() {
     return this.openPane(this.featureTreeId)
   }
+
+  async waitForFeatureTreeToBeBuilt() {
+    await this.openFeatureTreePane()
+    await expect(this.page.getByText('Building feature tree')).not.toBeVisible()
+    await this.closeFeatureTreePane()
+  }
   async closeFeatureTreePane() {
     await this.closePane(this.featureTreeId)
   }

e2e/playwright/point-click.spec.ts

@@ -138,6 +138,7 @@ test.describe('Point-and-click tests', () => {
         await scene.moveCameraTo(cameraPos, cameraTarget)
 
         await test.step('check chamfer selection changes cursor position', async () => {
+          await toolbar.waitForFeatureTreeToBeBuilt()
           await expect(async () => {
             // sometimes initial click doesn't register
             await clickChamfer()
@@ -177,6 +178,7 @@ test.describe('Point-and-click tests', () => {
             highlightedCode: '',
             diagnostics: [],
           })
+          await toolbar.waitForFeatureTreeToBeBuilt()
         })
       }
     test('works on all edge selections and can break up multi edges in a chamfer array', async ({
@@ -296,6 +298,7 @@ test.describe('Point-and-click tests', () => {
       await test.step('verify at the end of the test that final code is what is expected', async () => {
         await editor.expectEditor.toContain(
           `@settings(defaultLengthUnit = in)
+
 sketch001 = startSketchOn(XZ)
   |> startProfile(at = [75.8, 317.2]) // [$startCapTag, $EndCapTag]
   |> angledLine(angle = 0, length = 268.43, tag = $rectangleSegmentA001)
@@ -308,18 +311,11 @@ extrude001 = extrude(sketch001, length = 100)
   |> chamfer(length = 30, tags = [seg01], tag = $seg04)
   |> chamfer(length = 30, tags = [getNextAdjacentEdge(seg02)], tag = $seg05)
   |> chamfer(length = 30, tags = [getNextAdjacentEdge(yo)], tag = $seg06)
-sketch005 = startSketchOn(extrude001, face = seg06)
-profile004=startProfile(sketch005, at = [-23.43,19.69])
-  |> angledLine(angle = 0, length = 9.1, tag = $rectangleSegmentA005)
-  |> angledLine(angle = segAng(rectangleSegmentA005) - 90, length = 84.07)
-  |> angledLine(angle = segAng(rectangleSegmentA005), length = -segLen(rectangleSegmentA005))
-  |> line(endAbsolute=[profileStartX(%), profileStartY(%)])
-  |> close()
-sketch004 = startSketchOn(extrude001, face = seg05)
-profile003 = startProfile(sketch004, at = [82.57, 322.96])
-  |> angledLine(angle = 0, length = 11.16, tag = $rectangleSegmentA004)
-  |> angledLine(angle = segAng(rectangleSegmentA004) - 90, length = 103.07)
-  |> angledLine(angle = segAng(rectangleSegmentA004), length = -segLen(rectangleSegmentA004))
+sketch002 = startSketchOn(extrude001, face = seg03)
+profile001 = startProfile(sketch002, at = [205.96, 254.59])
+  |> angledLine(angle = 0, length = 11.39, tag = $rectangleSegmentA002)
+  |> angledLine(angle = segAng(rectangleSegmentA002) - 90, length = 105.26)
+  |> angledLine(angle = segAng(rectangleSegmentA002), length = -segLen(rectangleSegmentA002))
   |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
   |> close()
 sketch003 = startSketchOn(extrude001, face = seg04)
@@ -329,11 +325,18 @@ profile002 = startProfile(sketch003, at = [-209.64, 255.28])
   |> angledLine(angle = segAng(rectangleSegmentA003), length = -segLen(rectangleSegmentA003))
   |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
   |> close()
-sketch002 = startSketchOn(extrude001, face = seg03)
-profile001 = startProfile(sketch002, at = [205.96, 254.59])
-  |> angledLine(angle = 0, length = 11.39, tag = $rectangleSegmentA002)
-  |> angledLine(angle = segAng(rectangleSegmentA002) - 90, length = 105.26)
-  |> angledLine(angle = segAng(rectangleSegmentA002), length = -segLen(rectangleSegmentA002))
+sketch004 = startSketchOn(extrude001, face = seg05)
+profile003 = startProfile(sketch004, at = [82.57, 322.96])
+  |> angledLine(angle = 0, length = 11.16, tag = $rectangleSegmentA004)
+  |> angledLine(angle = segAng(rectangleSegmentA004) - 90, length = 103.07)
+  |> angledLine(angle = segAng(rectangleSegmentA004), length = -segLen(rectangleSegmentA004))
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+  |> close()
+sketch005 = startSketchOn(extrude001, face = seg06)
+profile004 = startProfile(sketch005, at = [-23.43, 19.69])
+  |> angledLine(angle = 0, length = 9.1, tag = $rectangleSegmentA005)
+  |> angledLine(angle = segAng(rectangleSegmentA005) - 90, length = 84.07)
+  |> angledLine(angle = segAng(rectangleSegmentA005), length = -segLen(rectangleSegmentA005))
   |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
   |> close()
 `,

e2e/playwright/sketch-tests.spec.ts

@@ -1256,6 +1256,97 @@ profile001 = startProfile(sketch001, at = [299.72, 230.82])
     }).toPass({ timeout: 40_000, intervals: [1_000] })
   })
 
+  test('sketch on face of a boolean works', async ({
+    page,
+    homePage,
+    scene,
+    cmdBar,
+    toolbar,
+    editor,
+  }) => {
+    await page.setBodyDimensions({ width: 1000, height: 500 })
+
+    await page.addInitScript(async () => {
+      localStorage.setItem(
+        'persistCode',
+        `@settings(defaultLengthUnit = mm)
+
+myVar = 50
+sketch001 = startSketchOn(XZ)
+profile001 = circle(sketch001, center = [myVar, 43.9], radius = 41.05)
+extrude001 = extrude(profile001, length = 200)
+  |> translate(x = 3.14, y = 3.14, z = -50.154)
+sketch002 = startSketchOn(XY)
+profile002 = startProfile(sketch002, at = [72.2, -52.05])
+  |> angledLine(angle = 0, length = 181.26, tag = $rectangleSegmentA001)
+  |> angledLine(angle = segAng(rectangleSegmentA001) - 90, length = 21.54)
+  |> angledLine(angle = segAng(rectangleSegmentA001), length = -segLen(rectangleSegmentA001), tag = $mySeg)
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)], tag = $seg01)
+  |> close()
+
+extrude002 = extrude(profile002, length = 151)
+  |> chamfer(
+       %,
+       length = 15,
+       tags = [mySeg],
+       tag = $seg02,
+     )
+solid001 = subtract([extrude001], tools = [extrude002])
+`
+      )
+    })
+
+    const [selectChamferFaceClk] = scene.makeMouseHelpers(671, 283)
+    const [circleCenterClk] = scene.makeMouseHelpers(700, 272)
+    const [circleRadiusClk] = scene.makeMouseHelpers(694, 264)
+
+    await test.step('Setup', async () => {
+      await homePage.goToModelingScene()
+      await scene.settled(cmdBar)
+
+      await scene.moveCameraTo(
+        { x: 180, y: -75, z: 116 },
+        { x: 67, y: -114, z: -15 }
+      )
+    })
+
+    await test.step('sketch on chamfer face that is part of a boolean', async () => {
+      await toolbar.startSketchPlaneSelection()
+      await selectChamferFaceClk()
+
+      await expect
+        .poll(async () => {
+          const lineBtn = page.getByRole('button', { name: 'line Line' })
+          return lineBtn.getAttribute('aria-pressed')
+        })
+        .toBe('true')
+
+      await editor.expectEditor.toContain(
+        'startSketchOn(solid001, face = seg02)'
+      )
+    })
+
+    await test.step('verify sketching still works', async () => {
+      await toolbar.circleBtn.click()
+      await expect
+        .poll(async () => {
+          const circleBtn = page.getByRole('button', { name: 'circle Circle' })
+          return circleBtn.getAttribute('aria-pressed')
+        })
+        .toBe('true')
+
+      await circleCenterClk()
+      await editor.expectEditor.toContain(
+        'profile003 = circle(sketch003, center'
+      )
+
+      await circleRadiusClk()
+      await editor.expectEditor.toContain(
+        'profile003 = circle(sketch003, center = [119.41, -56.05], radius = 1.82)'
+      )
+    })
+  })
+
   test('Can sketch on face when user defined function was used in the sketch', async ({
     page,
     homePage,

rust/kcl-lib/src/execution/artifact.rs

@@ -150,6 +150,10 @@ pub struct CompositeSolid {
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub tool_ids: Vec<ArtifactId>,
     pub code_ref: CodeRef,
+    /// This is the ID of the composite solid that this is part of, if any, as a
+    /// composite solid can be used as input for another composite solid.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub composite_solid_id: Option<ArtifactId>,
 }
 
 #[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq, ts_rs::TS)]
@@ -182,6 +186,10 @@ pub struct Path {
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub solid2d_id: Option<ArtifactId>,
     pub code_ref: CodeRef,
+    /// This is the ID of the composite solid that this is part of, if any, as
+    /// this can be used as input for another composite solid.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub composite_solid_id: Option<ArtifactId>,
 }
 
 #[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)]
@@ -585,6 +593,7 @@ impl CompositeSolid {
         };
         merge_ids(&mut self.solid_ids, new.solid_ids);
         merge_ids(&mut self.tool_ids, new.tool_ids);
+        merge_opt_id(&mut self.composite_solid_id, new.composite_solid_id);
 
         None
     }
@@ -609,6 +618,7 @@ impl Path {
         merge_opt_id(&mut self.sweep_id, new.sweep_id);
         merge_ids(&mut self.seg_ids, new.seg_ids);
         merge_opt_id(&mut self.solid2d_id, new.solid2d_id);
+        merge_opt_id(&mut self.composite_solid_id, new.composite_solid_id);
 
         None
     }
@@ -921,6 +931,7 @@ fn artifacts_to_update(
                 sweep_id: None,
                 solid2d_id: None,
                 code_ref,
+                composite_solid_id: None,
             }));
             let plane = artifacts.get(&ArtifactId::new(*current_plane_id));
             if let Some(Artifact::Plane(plane)) = plane {
@@ -1359,20 +1370,59 @@ fn artifacts_to_update(
                     .for_each(|id| new_solid_ids.push(id)),
                 _ => {}
             }
-            let return_arr = new_solid_ids
-                .into_iter()
-                .map(|solid_id| {
-                    Artifact::CompositeSolid(CompositeSolid {
-                        id: solid_id,
-                        sub_type,
-                        solid_ids: solid_ids.clone(),
-                        tool_ids: tool_ids.clone(),
-                        code_ref: code_ref.clone(),
-                    })
-                })
-                .collect::<Vec<_>>();
 
-            // TODO: Should we add the reverse graph edges?
+            let mut return_arr = Vec::new();
+
+            // Create the new composite solids and update their linked artifacts
+            for solid_id in &new_solid_ids {
+                // Create the composite solid
+                return_arr.push(Artifact::CompositeSolid(CompositeSolid {
+                    id: *solid_id,
+                    sub_type,
+                    solid_ids: solid_ids.clone(),
+                    tool_ids: tool_ids.clone(),
+                    code_ref: code_ref.clone(),
+                    composite_solid_id: None,
+                }));
+
+                // Update the artifacts that were used as input for this composite solid
+                for input_id in &solid_ids {
+                    if let Some(artifact) = artifacts.get(input_id) {
+                        match artifact {
+                            Artifact::CompositeSolid(comp) => {
+                                let mut new_comp = comp.clone();
+                                new_comp.composite_solid_id = Some(*solid_id);
+                                return_arr.push(Artifact::CompositeSolid(new_comp));
+                            }
+                            Artifact::Path(path) => {
+                                let mut new_path = path.clone();
+                                new_path.composite_solid_id = Some(*solid_id);
+                                return_arr.push(Artifact::Path(new_path));
+                            }
+                            _ => {}
+                        }
+                    }
+                }
+
+                // Update the tool artifacts if this is a subtract operation
+                for tool_id in &tool_ids {
+                    if let Some(artifact) = artifacts.get(tool_id) {
+                        match artifact {
+                            Artifact::CompositeSolid(comp) => {
+                                let mut new_comp = comp.clone();
+                                new_comp.composite_solid_id = Some(*solid_id);
+                                return_arr.push(Artifact::CompositeSolid(new_comp));
+                            }
+                            Artifact::Path(path) => {
+                                let mut new_path = path.clone();
+                                new_path.composite_solid_id = Some(*solid_id);
+                                return_arr.push(Artifact::Path(new_path));
+                            }
+                            _ => {}
+                        }
+                    }
+                }
+            }
 
             return Ok(return_arr);
         }

rust/kcl-lib/src/execution/artifact/mermaid_tests.rs

@@ -92,10 +92,14 @@ impl Artifact {
     /// the graph.
     pub(crate) fn child_ids(&self) -> Vec<ArtifactId> {
         match self {
-            Artifact::CompositeSolid(_) => {
+            Artifact::CompositeSolid(a) => {
                 // Note: Don't include these since they're parents: solid_ids,
                 // tool_ids.
-                Vec::new()
+                let mut ids = Vec::new();
+                if let Some(composite_solid_id) = a.composite_solid_id {
+                    ids.push(composite_solid_id);
+                }
+                ids
             }
             Artifact::Plane(a) => a.path_ids.clone(),
             Artifact::Path(a) => {
@@ -107,6 +111,9 @@ impl Artifact {
                 if let Some(solid2d_id) = a.solid2d_id {
                     ids.push(solid2d_id);
                 }
+                if let Some(composite_solid_id) = a.composite_solid_id {
+                    ids.push(composite_solid_id);
+                }
                 ids
             }
             Artifact::Segment(a) => {

rust/kcl-lib/tests/intersect_cubes/artifact_graph_flowchart.snap.md

@@ -57,14 +57,14 @@ flowchart LR
   3 --- 11
   3 --- 13
   3 ---- 16
-  3 <--x 17
+  3 --- 17
   4 --- 6
   4 --- 7
   4 --- 9
   4 --- 12
   4 --- 14
   4 ---- 15
-  4 <--x 17
+  4 --- 17
   5 --- 24
   5 x--> 26
   5 --- 35