fix circle sketch on face (#1668)

* fix circle sketch on face

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

* cargo.lock

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

* fix tests

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>

docs/kcl/std.json

@@ -18097,8 +18097,31 @@
     "tags": [],
     "args": [
       {
-        "name": "plane",
+        "name": "center",
-        "type": "SketchData",
+        "type": "[number, number]",
+        "schema": {
+          "type": "array",
+          "items": {
+            "type": "number",
+            "format": "double"
+          },
+          "maxItems": 2,
+          "minItems": 2
+        },
+        "required": true
+      },
+      {
+        "name": "radius",
+        "type": "number",
+        "schema": {
+          "type": "number",
+          "format": "double"
+        },
+        "required": true
+      },
+      {
+        "name": "surface",
+        "type": "SketchSurface",
         "schema": {
           "description": "Data for start sketch on. You can start a sketch on a plane or an extrude group.",
           "anyOf": [
@@ -19016,29 +19039,6 @@
           ]
         },
         "required": true
-      },
-      {
-        "name": "center",
-        "type": "[number, number]",
-        "schema": {
-          "type": "array",
-          "items": {
-            "type": "number",
-            "format": "double"
-          },
-          "maxItems": 2,
-          "minItems": 2
-        },
-        "required": true
-      },
-      {
-        "name": "radius",
-        "type": "number",
-        "schema": {
-          "type": "number",
-          "format": "double"
-        },
-        "required": true
       }
     ],
     "returnValue": {

docs/kcl/std.md

@@ -3449,12 +3449,14 @@ Sketch a circle on the given plane
 
 
 ```
-circle(plane: SketchData, center: [number, number], radius: number) -> SketchGroup
+circle(center: [number, number], radius: number, surface: SketchSurface) -> SketchGroup
 ```
 
 #### Arguments
 
-* `plane`: `SketchData` - Data for start sketch on. You can start a sketch on a plane or an extrude group. (REQUIRED)
+* `center`: `[number, number]` (REQUIRED)
+* `radius`: `number` (REQUIRED)
+* `surface`: `SketchSurface` - Data for start sketch on. You can start a sketch on a plane or an extrude group. (REQUIRED)
 ```
 "XY" |
 "-XY" |
@@ -3619,8 +3621,6 @@ circle(plane: SketchData, center: [number, number], radius: number) -> SketchGro
 },
 }
 ```
-* `center`: `[number, number]` (REQUIRED)
-* `radius`: `number` (REQUIRED)
 
 #### Returns
 

src/wasm-lib/Cargo.lock

@@ -1897,7 +1897,7 @@ dependencies = [
 
 [[package]]
 name = "kcl-lib"
-version = "0.1.43"
+version = "0.1.44"
 dependencies = [
  "anyhow",
  "approx 0.5.1",

src/wasm-lib/kcl/Cargo.toml

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

src/wasm-lib/kcl/src/parser/snapshots/kcl_lib__parser__parser_impl__snapshot_tests__au.snap

@@ -4,18 +4,18 @@ expression: actual
 ---
 {
   "start": 0,
-  "end": 59,
+  "end": 87,
   "body": [
     {
       "type": "VariableDeclaration",
       "type": "VariableDeclaration",
       "start": 0,
-      "end": 58,
+      "end": 86,
       "declarations": [
         {
           "type": "VariableDeclarator",
           "start": 6,
-          "end": 58,
+          "end": 86,
           "id": {
             "type": "Identifier",
             "start": 6,
@@ -26,59 +26,27 @@ expression: actual
             "type": "PipeExpression",
             "type": "PipeExpression",
             "start": 17,
-            "end": 58,
+            "end": 86,
             "body": [
               {
                 "type": "CallExpression",
                 "type": "CallExpression",
                 "start": 17,
-                "end": 40,
+                "end": 36,
                 "callee": {
                   "type": "Identifier",
                   "start": 17,
-                  "end": 23,
+                  "end": 30,
-                  "name": "circle"
+                  "name": "startSketchOn"
                 },
                 "arguments": [
                   {
                     "type": "Literal",
                     "type": "Literal",
-                    "start": 24,
+                    "start": 31,
-                    "end": 28,
+                    "end": 35,
                     "value": "XY",
                     "raw": "'XY'"
-                  },
-                  {
-                    "type": "ArrayExpression",
-                    "type": "ArrayExpression",
-                    "start": 30,
-                    "end": 35,
-                    "elements": [
-                      {
-                        "type": "Literal",
-                        "type": "Literal",
-                        "start": 31,
-                        "end": 32,
-                        "value": 0,
-                        "raw": "0"
-                      },
-                      {
-                        "type": "Literal",
-                        "type": "Literal",
-                        "start": 33,
-                        "end": 34,
-                        "value": 0,
-                        "raw": "0"
-                      }
-                    ]
-                  },
-                  {
-                    "type": "Literal",
-                    "type": "Literal",
-                    "start": 37,
-                    "end": 39,
-                    "value": 22,
-                    "raw": "22"
                   }
                 ],
                 "optional": false
@@ -87,27 +55,80 @@ expression: actual
                 "type": "CallExpression",
                 "type": "CallExpression",
                 "start": 44,
-                "end": 58,
+                "end": 64,
                 "callee": {
                   "type": "Identifier",
                   "start": 44,
-                  "end": 51,
+                  "end": 50,
+                  "name": "circle"
+                },
+                "arguments": [
+                  {
+                    "type": "ArrayExpression",
+                    "type": "ArrayExpression",
+                    "start": 51,
+                    "end": 56,
+                    "elements": [
+                      {
+                        "type": "Literal",
+                        "type": "Literal",
+                        "start": 52,
+                        "end": 53,
+                        "value": 0,
+                        "raw": "0"
+                      },
+                      {
+                        "type": "Literal",
+                        "type": "Literal",
+                        "start": 54,
+                        "end": 55,
+                        "value": 0,
+                        "raw": "0"
+                      }
+                    ]
+                  },
+                  {
+                    "type": "Literal",
+                    "type": "Literal",
+                    "start": 58,
+                    "end": 60,
+                    "value": 22,
+                    "raw": "22"
+                  },
+                  {
+                    "type": "PipeSubstitution",
+                    "type": "PipeSubstitution",
+                    "start": 62,
+                    "end": 63
+                  }
+                ],
+                "optional": false
+              },
+              {
+                "type": "CallExpression",
+                "type": "CallExpression",
+                "start": 72,
+                "end": 86,
+                "callee": {
+                  "type": "Identifier",
+                  "start": 72,
+                  "end": 79,
                   "name": "extrude"
                 },
                 "arguments": [
                   {
                     "type": "Literal",
                     "type": "Literal",
-                    "start": 52,
+                    "start": 80,
-                    "end": 54,
+                    "end": 82,
                     "value": 14,
                     "raw": "14"
                   },
                   {
                     "type": "PipeSubstitution",
                     "type": "PipeSubstitution",
-                    "start": 56,
+                    "start": 84,
-                    "end": 57
+                    "end": 85
                   }
                 ],
                 "optional": false

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

@@ -8,9 +8,8 @@ use crate::{
 };
 
 pub const CIRCLE_FN: &str = r#"
-(plane, center, radius) => {
+(center, radius, surface) => {
-  const sg = startSketchOn(plane)
+const sg = startProfileAt([center[0] + radius, center[1]], surface)
-    |> startProfileAt([center[0] + radius, center[1]], %)
     |> arc({
        angle_end: 360,
        angle_start: 0,
@@ -70,14 +69,6 @@ impl StdLibFn for Circle {
         let mut args = Vec::new();
         for parameter in &self.function.params {
             match parameter.identifier.name.as_str() {
-                "plane" => {
-                    args.push(crate::docs::StdLibFnArg {
-                        name: parameter.identifier.name.to_owned(),
-                        type_: "SketchData".to_string(),
-                        schema: <crate::std::sketch::SketchData>::json_schema(&mut generator),
-                        required: true,
-                    });
-                }
                 "center" => {
                     args.push(crate::docs::StdLibFnArg {
                         name: parameter.identifier.name.to_owned(),
@@ -94,6 +85,14 @@ impl StdLibFn for Circle {
                         required: true,
                     });
                 }
+                "surface" => {
+                    args.push(crate::docs::StdLibFnArg {
+                        name: parameter.identifier.name.to_owned(),
+                        type_: "SketchSurface".to_string(),
+                        schema: <crate::std::sketch::SketchData>::json_schema(&mut generator),
+                        required: true,
+                    });
+                }
                 _ => panic!("Unknown parameter: {:?}", parameter.identifier.name),
             }
         }

src/wasm-lib/tests/executor/inputs/cylinder.kcl

@@ -1 +1,3 @@
-const cylinder = circle('XY', [0,0], 22) |> extrude(14, %)
+const cylinder = startSketchOn('XY')
+    |> circle([0,0], 22, %)
+    |> extrude(14, %)

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

@@ -732,8 +732,8 @@ async fn serial_test_holes() {
   |> line([10, 0], %)
   |> line([0, -10], %)
   |> close(%)
-  |> hole(circle('XY', [2, 2], .5), %)
+  |> hole(circle([2, 2], .5, startSketchOn('XY')), %)
-  |> hole(circle('XY', [2, 8], .5), %)
+  |> hole(circle([2, 8], .5, startSketchOn('XY')), %)
   |> extrude(2, %)
 "#;
 
@@ -788,10 +788,10 @@ const holeRadius = 1
 const holeIndex = 6
 
 const part = roundedRectangle([0, 0], 20, 20, 4)
-  |> hole(circle('XY', [-holeIndex, holeIndex], holeRadius), %)
+  |> hole(circle([-holeIndex, holeIndex], holeRadius,  startSketchOn('XY')), %)
-  |> hole(circle('XY', [holeIndex, holeIndex], holeRadius), %)
+  |> hole(circle([holeIndex, holeIndex], holeRadius,  startSketchOn('XY')), %)
-  |> hole(circle('XY', [-holeIndex, -holeIndex], holeRadius), %)
+  |> hole(circle([-holeIndex, -holeIndex], holeRadius,  startSketchOn('XY')), %)
-  |> hole(circle('XY', [holeIndex, -holeIndex], holeRadius), %)
+  |> hole(circle([holeIndex, -holeIndex], holeRadius,  startSketchOn('XY')), %)
   |> extrude(2, %)
 "#;
 
@@ -803,7 +803,7 @@ const part = roundedRectangle([0, 0], 20, 20, 4)
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_top_level_expression() {
-    let code = r#"circle('XY', [0,0], 22) |> extrude(14, %)"#;
+    let code = r#"circle([0,0], 22,  startSketchOn('XY')) |> extrude(14, %)"#;
 
     let result = execute_and_snapshot(code, kittycad::types::UnitLength::Mm)
         .await
@@ -813,7 +813,8 @@ async fn serial_test_top_level_expression() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_patterns_linear_basic() {
-    let code = r#"const part = circle('XY', [0,0], 2)
+    let code = r#"const part =  startSketchOn('XY')
+    |> circle([0,0], 2, %)
     |> patternLinear({axis: [0,1], repetitions: 12, distance: 2}, %)
 "#;
 
@@ -843,7 +844,8 @@ async fn serial_test_patterns_linear_basic_3d() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_patterns_linear_basic_negative_distance() {
-    let code = r#"const part = circle('XY', [0,0], 2)
+    let code = r#"const part = startSketchOn('XY')
+    |> circle([0,0], 2, %)
     |> patternLinear({axis: [0,1], repetitions: 12, distance: -2}, %)
 "#;
 
@@ -859,7 +861,8 @@ async fn serial_test_patterns_linear_basic_negative_distance() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_patterns_linear_basic_negative_axis() {
-    let code = r#"const part = circle('XY', [0,0], 2)
+    let code = r#"const part = startSketchOn('XY')
+    |> circle([0,0], 2, %)
     |> patternLinear({axis: [0,-1], repetitions: 12, distance: 2}, %)
 "#;
 
@@ -875,7 +878,8 @@ async fn serial_test_patterns_linear_basic_negative_axis() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_patterns_linear_basic_holes() {
-    let code = r#"const circles = circle('XY', [5, 5], 1)
+    let code = r#"const circles = startSketchOn('XY')
+    |> circle([5, 5], 1, %)
     |> patternLinear({axis: [1,1], repetitions: 12, distance: 3}, %)
 
 const rectangle = startSketchOn('XY')
@@ -896,7 +900,8 @@ const rectangle = startSketchOn('XY')
 
 #[tokio::test(flavor = "multi_thread")]
 async fn serial_test_patterns_circular_basic_2d() {
-    let code = r#"const part = circle('XY', [0,0], 2)
+    let code = r#"const part = startSketchOn('XY')
+    |> circle([0,0], 2, %)
     |> patternCircular({axis: [0,1], center: [20, 20, 20], repetitions: 12, arcDegrees: 210, rotateDuplicates: true}, %)
 "#;
 
@@ -1248,7 +1253,7 @@ async fn serial_test_stdlib_kcl_error_right_code_path() {
   |> line([0, -10], %)
   |> close(%)
   |> hole(circle([2, 2], .5), %)
-  |> hole(circle('XY', [2, 8], .5), %)
+  |> hole(circle([2, 8], .5, startSketchOn('XY')), %)
   |> extrude(2, %)
 "#;
 
@@ -1259,3 +1264,29 @@ async fn serial_test_stdlib_kcl_error_right_code_path() {
         r#"semantic: KclErrorDetails { source_ranges: [SourceRange([157, 175])], message: "this function expected 3 arguments, got 2" }"#
     );
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn serial_test_sketch_on_face_circle() {
+    let code = r#"fn cube = (pos, scale) => {
+  const sg = startSketchOn('XY')
+    |> startProfileAt(pos, %)
+    |> line([0, scale], %)
+    |> line([scale, 0], %)
+    |> line([0, -scale], %)
+
+  return sg
+}
+const part001 = cube([0,0], 20)
+    |> close(%)
+    |> extrude(20, %)
+
+const part002 = startSketchOn(part001, "end")
+  |> circle([0, 0], 5, %) 
+  |> extrude(5, %)
+"#;
+
+    let result = execute_and_snapshot(code, kittycad::types::UnitLength::Mm)
+        .await
+        .unwrap();
+    twenty_twenty::assert_image("tests/executor/outputs/sketch_on_face_circle.png", &result, 1.0);
+}