save specific commands like fillet and chamfer for last (#2753)

* save specific commands like fillet and chamfer for last

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* bump kcl-lib

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* more tests

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* more images

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>

src/wasm-lib/Cargo.lock

@@ -1375,7 +1375,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-lib"
-version = "0.1.62"
+version = "0.1.63"
 dependencies = [
  "anyhow",
  "approx",

src/wasm-lib/kcl/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "kcl-lib"
 description = "KittyCAD Language implementation and tools"
-version = "0.1.62"
+version = "0.1.63"
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/KittyCAD/modeling-app"

src/wasm-lib/kcl/src/engine/conn.rs

@@ -31,6 +31,7 @@ pub struct EngineConnection {
     tcp_read_handle: Arc<TcpReadHandle>,
     socket_health: Arc<Mutex<SocketHealth>>,
     batch: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
+    batch_end: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
 
     /// The default planes for the scene.
     default_planes: Arc<RwLock<Option<DefaultPlanes>>>,
@@ -236,6 +237,7 @@ impl EngineConnection {
             responses,
             socket_health,
             batch: Arc::new(Mutex::new(Vec::new())),
+            batch_end: Arc::new(Mutex::new(Vec::new())),
             default_planes: Default::default(),
         })
     }
@@ -247,6 +249,10 @@ impl EngineManager for EngineConnection {
         self.batch.clone()
     }
 
+    fn batch_end(&self) -> Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>> {
+        self.batch_end.clone()
+    }
+
     async fn default_planes(&self, source_range: crate::executor::SourceRange) -> Result<DefaultPlanes, KclError> {
         {
             let opt = self.default_planes.read().await.as_ref().cloned();

src/wasm-lib/kcl/src/engine/conn_mock.rs

@@ -14,12 +14,14 @@ use crate::{errors::KclError, executor::DefaultPlanes};
 #[derive(Debug, Clone)]
 pub struct EngineConnection {
     batch: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
+    batch_end: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
 }
 
 impl EngineConnection {
     pub async fn new() -> Result<EngineConnection> {
         Ok(EngineConnection {
             batch: Arc::new(Mutex::new(Vec::new())),
+            batch_end: Arc::new(Mutex::new(Vec::new())),
         })
     }
 }
@@ -30,6 +32,10 @@ impl crate::engine::EngineManager for EngineConnection {
         self.batch.clone()
     }
 
+    fn batch_end(&self) -> Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>> {
+        self.batch_end.clone()
+    }
+
     async fn default_planes(&self, _source_range: crate::executor::SourceRange) -> Result<DefaultPlanes, KclError> {
         Ok(DefaultPlanes::default())
     }

src/wasm-lib/kcl/src/engine/conn_wasm.rs

@@ -39,6 +39,7 @@ extern "C" {
 pub struct EngineConnection {
     manager: Arc<EngineCommandManager>,
     batch: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
+    batch_end: Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>>,
 }
 
 // Safety: WebAssembly will only ever run in a single-threaded context.
@@ -50,6 +51,7 @@ impl EngineConnection {
         Ok(EngineConnection {
             manager: Arc::new(manager),
             batch: Arc::new(Mutex::new(Vec::new())),
+            batch_end: Arc::new(Mutex::new(Vec::new())),
         })
     }
 }
@@ -60,6 +62,10 @@ impl crate::engine::EngineManager for EngineConnection {
         self.batch.clone()
     }
 
+    fn batch_end(&self) -> Arc<Mutex<Vec<(WebSocketRequest, crate::executor::SourceRange)>>> {
+        self.batch_end.clone()
+    }
+
     async fn default_planes(&self, source_range: crate::executor::SourceRange) -> Result<DefaultPlanes, KclError> {
         // Get the default planes.
         let promise = self.manager.get_default_planes().map_err(|e| {

src/wasm-lib/kcl/src/engine/mod.rs

@@ -13,7 +13,7 @@ use std::{
     sync::{Arc, Mutex},
 };
 
-use kittycad::types::{Color, ModelingCmd, OkWebSocketResponseData, WebSocketRequest};
+use kittycad::types::{Color, ModelingCmd, ModelingCmdReq, OkWebSocketResponseData, WebSocketRequest};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
@@ -27,6 +27,9 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
     /// Get the batch of commands to be sent to the engine.
     fn batch(&self) -> Arc<Mutex<Vec<(kittycad::types::WebSocketRequest, crate::executor::SourceRange)>>>;
 
+    /// Get the batch of end commands to be sent to the engine.
+    fn batch_end(&self) -> Arc<Mutex<Vec<(kittycad::types::WebSocketRequest, crate::executor::SourceRange)>>>;
+
     /// Get the default planes.
     async fn default_planes(
         &self,
@@ -59,7 +62,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
 
         // Flush the batch queue, so clear is run right away.
         // Otherwise the hooks below won't work.
-        self.flush_batch(source_range).await?;
+        self.flush_batch(false, source_range).await?;
 
         // Do the after clear scene hook.
         self.clear_scene_post_hook(source_range).await?;
@@ -85,6 +88,25 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         Ok(())
     }
 
+    /// Add a command to the batch that needs to be executed at the very end.
+    /// This for stuff like fillets or chamfers where if we execute too soon the
+    /// engine will eat the ID and we can't reference it for other commands.
+    async fn batch_end_cmd(
+        &self,
+        id: uuid::Uuid,
+        source_range: crate::executor::SourceRange,
+        cmd: &kittycad::types::ModelingCmd,
+    ) -> Result<(), crate::errors::KclError> {
+        let req = WebSocketRequest::ModelingCmdReq {
+            cmd: cmd.clone(),
+            cmd_id: id,
+        };
+
+        // Add cmd to the batch end.
+        self.batch_end().lock().unwrap().push((req, source_range));
+        Ok(())
+    }
+
     /// Send the modeling cmd and wait for the response.
     async fn send_modeling_cmd(
         &self,
@@ -95,25 +117,33 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         self.batch_modeling_cmd(id, source_range, &cmd).await?;
 
         // Flush the batch queue.
-        self.flush_batch(source_range).await
+        self.flush_batch(false, source_range).await
     }
 
     /// Force flush the batch queue.
     async fn flush_batch(
         &self,
+        // Whether or not to flush the end commands as well.
+        // We only do this at the very end of the file.
+        batch_end: bool,
         source_range: crate::executor::SourceRange,
     ) -> Result<kittycad::types::OkWebSocketResponseData, crate::errors::KclError> {
+        let all_requests = if batch_end {
+            let mut requests = self.batch().lock().unwrap().clone();
+            requests.extend(self.batch_end().lock().unwrap().clone());
+            requests
+        } else {
+            self.batch().lock().unwrap().clone()
+        };
+
         // Return early if we have no commands to send.
-        if self.batch().lock().unwrap().is_empty() {
+        if all_requests.is_empty() {
             return Ok(OkWebSocketResponseData::Modeling {
                 modeling_response: kittycad::types::OkModelingCmdResponse::Empty {},
             });
         }
 
-        let requests = self
-            .batch()
-            .lock()
-            .unwrap()
+        let requests: Vec<ModelingCmdReq> = all_requests
             .iter()
             .filter_map(|(val, _)| match val {
                 WebSocketRequest::ModelingCmdReq { cmd, cmd_id } => Some(kittycad::types::ModelingCmdReq {
@@ -123,15 +153,16 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
                 _ => None,
             })
             .collect();
+
         let batched_requests = WebSocketRequest::ModelingCmdBatchReq {
             requests,
             batch_id: uuid::Uuid::new_v4(),
             responses: true,
         };
 
-        let final_req = if self.batch().lock().unwrap().len() == 1 {
+        let final_req = if all_requests.len() == 1 {
             // We can unwrap here because we know the batch has only one element.
-            self.batch().lock().unwrap().first().unwrap().0.clone()
+            all_requests.first().unwrap().0.clone()
         } else {
             batched_requests
         };
@@ -139,7 +170,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         // Create the map of original command IDs to source range.
         // This is for the wasm side, kurt needs it for selections.
         let mut id_to_source_range = std::collections::HashMap::new();
-        for (req, range) in self.batch().lock().unwrap().iter() {
+        for (req, range) in all_requests.iter() {
             match req {
                 WebSocketRequest::ModelingCmdReq { cmd: _, cmd_id } => {
                     id_to_source_range.insert(*cmd_id, *range);
@@ -155,6 +186,9 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
 
         // Throw away the old batch queue.
         self.batch().lock().unwrap().clear();
+        if batch_end {
+            self.batch_end().lock().unwrap().clear();
+        }
 
         // We pop off the responses to cleanup our mappings.
         match final_req {
@@ -321,7 +355,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         }
 
         // Flush the batch queue, so these planes are created right away.
-        self.flush_batch(source_range).await?;
+        self.flush_batch(false, source_range).await?;
 
         Ok(DefaultPlanes {
             xy: planes[&PlaneName::Xy],

src/wasm-lib/kcl/src/executor.rs

@@ -1248,7 +1248,14 @@ impl ExecutorContext {
 
         if BodyType::Root == body_type {
             // Flush the batch queue.
-            self.engine.flush_batch(SourceRange([program.end, program.end])).await?;
+            self.engine
+                .flush_batch(
+                    // True here tells the engine to flush all the end commands as well like fillets
+                    // and chamfers where the engine would otherwise eat the ID of the segments.
+                    true,
+                    SourceRange([program.end, program.end]),
+                )
+                .await?;
         }
 
         Ok(memory.clone())

src/wasm-lib/kcl/src/std/chamfer.rs

@@ -110,7 +110,7 @@ async fn inner_chamfer(
             }
         };
 
-        args.batch_modeling_cmd(
+        args.batch_end_cmd(
             uuid::Uuid::new_v4(),
             ModelingCmd::Solid3DFilletEdge {
                 edge_id,

src/wasm-lib/kcl/src/std/fillet.rs

@@ -111,7 +111,7 @@ async fn inner_fillet(
             }
         };
 
-        args.batch_modeling_cmd(
+        args.batch_end_cmd(
             uuid::Uuid::new_v4(),
             ModelingCmd::Solid3DFilletEdge {
                 edge_id,

src/wasm-lib/kcl/src/std/mod.rs

@@ -224,6 +224,17 @@ impl Args {
         self.ctx.engine.batch_modeling_cmd(id, self.source_range, &cmd).await
     }
 
+    // Add a modeling command to the batch that gets executed at the end of the file.
+    // This is good for something like fillet or chamfer where the engine would
+    // eat the path id if we executed it right away.
+    pub async fn batch_end_cmd(
+        &self,
+        id: uuid::Uuid,
+        cmd: kittycad::types::ModelingCmd,
+    ) -> Result<(), crate::errors::KclError> {
+        self.ctx.engine.batch_end_cmd(id, self.source_range, &cmd).await
+    }
+
     /// Send the modeling cmd and wait for the response.
     pub async fn send_modeling_cmd(
         &self,
@@ -233,6 +244,16 @@ impl Args {
         self.ctx.engine.send_modeling_cmd(id, self.source_range, cmd).await
     }
 
+    /// Flush the batch for our fillets/chamfers if there are any.
+    pub async fn flush_batch(&self) -> Result<(), KclError> {
+        if self.ctx.engine.batch_end().lock().unwrap().is_empty() {
+            return Ok(());
+        }
+        self.ctx.engine.flush_batch(true, SourceRange::default()).await?;
+
+        Ok(())
+    }
+
     fn make_user_val_from_json(&self, j: serde_json::Value) -> Result<MemoryItem, KclError> {
         Ok(MemoryItem::UserVal(crate::executor::UserVal {
             value: j,

src/wasm-lib/kcl/src/std/sketch.rs

@@ -1173,6 +1173,12 @@ pub(crate) async fn inner_start_profile_at(
     tag: Option<String>,
     args: Args,
 ) -> Result<Box<SketchGroup>, KclError> {
+    if let SketchSurface::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().await?;
+    }
+
     // Enter sketch mode on the surface.
     // We call this here so you can reuse the sketch surface for multiple sketches.
     let id = uuid::Uuid::new_v4();

src/wasm-lib/tests/executor/main.rs

@@ -1873,7 +1873,7 @@ const bracket = startSketchOn('XY')
     assert!(result.is_err());
     assert_eq!(
         result.err().unwrap().to_string(),
-        r#"engine: KclErrorDetails { source_ranges: [SourceRange([1443, 1443])], message: "Modeling command failed: [ApiError { error_code: BadRequest, message: \"Fillet failed\" }]" }"#
+        r#"engine: KclErrorDetails { source_ranges: [SourceRange([1336, 1442])], message: "Modeling command failed: [ApiError { error_code: BadRequest, message: \"Fillet failed\" }]" }"#
     );
 }
 
@@ -2039,6 +2039,73 @@ extrude(10, sketch001)
     twenty_twenty::assert_image("tests/executor/outputs/array_of_sketches.png", &result, 1.0);
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn serial_test_sketch_on_face_after_fillets_referencing_face() {
+    let code = r#"// Shelf Bracket
+// This is a shelf bracket made out of 6061-T6 aluminum sheet metal. The required thickness is calculated based on a point load of 300 lbs applied to the end of the shelf. There are two brackets holding up the shelf, so the moment experienced is divided by 2. The shelf is 1 foot long from the wall.
+
+
+// Define our bracket feet lengths
+const shelfMountL = 8 // The length of the bracket holding up the shelf is 6 inches
+const wallMountL = 6 // the length of the bracket
+
+
+// Define constants required to calculate the thickness needed to support 300 lbs
+const sigmaAllow = 35000 // psi
+const width = 6 // inch
+const p = 300 // Force on shelf - lbs
+const L = 12 // inches
+const M = L * p / 2 // Moment experienced at fixed end of bracket
+const FOS = 2 // Factor of safety of 2 to be conservative
+
+
+// Calculate the thickness off the bending stress and factor of safety
+const thickness = sqrt(6 * M * FOS / (width * sigmaAllow))
+
+// 0.25 inch fillet radius
+const filletR = 0.25
+
+// Sketch the bracket and extrude with fillets
+const bracket = startSketchOn('XY')
+  |> startProfileAt([0, 0], %)
+  |> line([0, wallMountL], %, 'outerEdge')
+  |> line([-shelfMountL, 0], %, 'seg01')
+  |> line([0, -thickness], %)
+  |> line([shelfMountL - thickness, 0], %, 'innerEdge')
+  |> line([0, -wallMountL + thickness], %)
+  |> close(%)
+  |> extrude(width, %)
+  |> fillet({
+       radius: filletR,
+       tags: [
+         getPreviousAdjacentEdge('innerEdge', %)
+       ]
+     }, %)
+  |> fillet({
+       radius: filletR + thickness,
+       tags: [
+         getPreviousAdjacentEdge('outerEdge', %)
+       ]
+     }, %)
+
+const sketch001 = startSketchOn(bracket, 'seg01')
+  |> startProfileAt([4.28, 3.83], %)
+  |> line([2.17, -0.03], %)
+  |> line([-0.07, -1.8], %)
+  |> line([-2.07, 0.05], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+  |> extrude(10, %)
+"#;
+
+    let result = execute_and_snapshot(code, UnitLength::Mm).await.unwrap();
+    twenty_twenty::assert_image(
+        "tests/executor/outputs/sketch_on_face_after_fillets_referencing_face.png",
+        &result,
+        1.0,
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_circular_pattern3d_array_of_extrudes() {
     let code = r#"const plane001 = startSketchOn('XZ')
@@ -2074,3 +2141,107 @@ const pattn1 = patternLinear3d({
     let result = execute_and_snapshot(code, UnitLength::Mm).await.unwrap();
     twenty_twenty::assert_image("tests/executor/outputs/pattern3d_array_of_extrudes.png", &result, 1.0);
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn serial_test_fillets_referencing_other_fillets() {
+    let code = r#"// Z-Bracket
+
+// Z-brackets are designed to affix or hang objects from a wall by securing them to the wall's studs. These brackets offer support and mounting solutions for bulky or heavy items that may be challenging to attach directly. Serving as a protective feature, Z-brackets help prevent heavy loads from moving or toppling, enhancing safety in the environment where they are used.
+
+// Define constants
+const foot1Length = 4
+const height = 4
+const foot2Length = 5
+const width = 4
+const filletRad = 0.25
+const thickness = 0.125
+
+const cornerFilletRad = 0.5
+
+const holeDia = 0.5
+
+const sketch001 = startSketchOn("XZ")
+  |> startProfileAt([-foot1Length, 0], %)
+  |> line([0, thickness], %, 'cornerFillet1')
+  |> line([foot1Length, 0], %)
+  |> line([0, height], %, 'fillet1')
+  |> line([foot2Length, 0], %)
+  |> line([0, -thickness], %, 'cornerFillet2')
+  |> line([-foot2Length+thickness, 0], %)
+  |> line([0, -height], %, 'fillet2')
+  |> close(%)
+
+const baseExtrusion = extrude(width, sketch001)
+  |> fillet({
+    radius: cornerFilletRad,
+    tags: ["cornerFillet1", "cornerFillet2", getOppositeEdge("cornerFillet1", %), getOppositeEdge("cornerFillet2", %)],
+  }, %)
+  |> fillet({
+    radius: filletRad,
+    tags: [getPreviousAdjacentEdge("fillet1", %), getPreviousAdjacentEdge("fillet2", %)]
+  }, %)
+  |> fillet({
+   radius: filletRad + thickness,
+   tags: [getNextAdjacentEdge("fillet1", %), getNextAdjacentEdge("fillet2", %)],
+ }, %)
+"#;
+
+    let result = execute_and_snapshot(code, UnitLength::Mm).await.unwrap();
+    twenty_twenty::assert_image(
+        "tests/executor/outputs/fillets_referencing_other_fillets.png",
+        &result,
+        1.0,
+    );
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn serial_test_chamfers_referencing_other_chamfers() {
+    let code = r#"// Z-Bracket
+
+// Z-brackets are designed to affix or hang objects from a wall by securing them to the wall's studs. These brackets offer support and mounting solutions for bulky or heavy items that may be challenging to attach directly. Serving as a protective feature, Z-brackets help prevent heavy loads from moving or toppling, enhancing safety in the environment where they are used.
+
+// Define constants
+const foot1Length = 4
+const height = 4
+const foot2Length = 5
+const width = 4
+const chamferRad = 0.25
+const thickness = 0.125
+
+const cornerChamferRad = 0.5
+
+const holeDia = 0.5
+
+const sketch001 = startSketchOn("XZ")
+  |> startProfileAt([-foot1Length, 0], %)
+  |> line([0, thickness], %, 'cornerChamfer1')
+  |> line([foot1Length, 0], %)
+  |> line([0, height], %, 'chamfer1')
+  |> line([foot2Length, 0], %)
+  |> line([0, -thickness], %, 'cornerChamfer2')
+  |> line([-foot2Length+thickness, 0], %)
+  |> line([0, -height], %, 'chamfer2')
+  |> close(%)
+
+const baseExtrusion = extrude(width, sketch001)
+  |> chamfer({
+    length: cornerChamferRad,
+    tags: ["cornerChamfer1", "cornerChamfer2", getOppositeEdge("cornerChamfer1", %), getOppositeEdge("cornerChamfer2", %)],
+  }, %)
+  |> chamfer({
+    length: chamferRad,
+    tags: [getPreviousAdjacentEdge("chamfer1", %), getPreviousAdjacentEdge("chamfer2", %)]
+  }, %)
+  |> chamfer({
+   length: chamferRad + thickness,
+   tags: [getNextAdjacentEdge("chamfer1", %), getNextAdjacentEdge("chamfer2", %)],
+ }, %)
+"#;
+
+    let result = execute_and_snapshot(code, UnitLength::Mm).await.unwrap();
+    twenty_twenty::assert_image(
+        "tests/executor/outputs/chamfers_referencing_other_chamfers.png",
+        &result,
+        1.0,
+    );
+}