Major axis 2 (#7971)

* change major radius to axis

* working sketch with major axis

* change docs

* make trig consistent

* clippy fix

* more clippy

* graphite fixes

* major_axis encodes major radius length and direction

* update modeling-api to v130

* fix sim-test with local engine

* fix docs with local engine

---------

Co-authored-by: Adam Sunderland <[email protected]>

Author: benjamaan476

docs/kcl-std/functions/std-sketch-ellipse.md

@@ -11,8 +11,9 @@ Construct a 2-dimensional ellipse, of the specified major/minor radius, centered
 ellipse(
   @sketchOrSurface: Sketch | Plane | Face,
   center: Point2d,
-  majorRadius: number(Length),
   minorRadius: number(Length),
+  majorRadius?: number(Length),
+  majorAxis?: Point2d,
   tag?: tag,
 ): Sketch
 ```
@@ -25,8 +26,9 @@ ellipse(
 |----------|------|-------------|----------|
 | `sketchOrSurface` | [`Sketch`](/docs/kcl-std/types/std-types-Sketch) or [`Plane`](/docs/kcl-std/types/std-types-Plane) or [`Face`](/docs/kcl-std/types/std-types-Face) | Sketch to extend, or plane or surface to sketch on. | Yes |
 | `center` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The center of the ellipse. | Yes |
-| `majorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The major radius of the ellipse. | Yes |
 | `minorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The minor radius of the ellipse. | Yes |
+| `majorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The major radius of the ellipse. Equivalent to majorAxis = [majorRadius, 0]. | No |
+| `majorAxis` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The major axis of the ellipse. | No |
 | `tag` | `tag` | Create a new tag which refers to this ellipse. | No |
 
 ### Returns

docs/kcl-std/functions/std-sketch-elliptic.md

@@ -13,8 +13,9 @@ elliptic(
   center: Point2d,
   angleStart: number(Angle),
   angleEnd: number(Angle),
-  majorRadius: number(Length),
   minorRadius: number(Length),
+  majorRadius?: number(Length),
+  majorAxis?: Point2d,
   tag?: tag,
 ): Sketch
 ```
@@ -29,8 +30,9 @@ elliptic(
 | `center` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The center of the ellipse. | Yes |
 | `angleStart` | [`number(Angle)`](/docs/kcl-std/types/std-types-number) | Where along the ellptic should this segment start? | Yes |
 | `angleEnd` | [`number(Angle)`](/docs/kcl-std/types/std-types-number) | Where along the ellptic should this segment end? | Yes |
-| `majorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1. | Yes |
 | `minorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The minor radius, b, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1. | Yes |
+| `majorRadius` | [`number(Length)`](/docs/kcl-std/types/std-types-number) | The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1. Equivalent to majorAxis = [majorRadius, 0]. | No |
+| `majorAxis` | [`Point2d`](/docs/kcl-std/types/std-types-Point2d) | The major axis of the elliptic. | No |
 | `tag` | `tag` | Create a new tag which refers to this arc. | No |
 
 ### Returns

rust/Cargo.lock

@@ -36,7 +36,7 @@ dashmap = { version = "6.1.0" }
 http = "1"
 indexmap = "2.10.0"
 kittycad = { version = "0.3.37", default-features = false, features = ["js", "requests"] }
-kittycad-modeling-cmds = { version = "=0.2.129", features = ["ts-rs", "websocket"] }
+kittycad-modeling-cmds = { version = "0.2.130", features = ["ts-rs", "websocket"] }
 lazy_static = "1.5.0"
 miette = "7.6.0"
 pyo3 = { version = "0.25.1" }

rust/Cargo.toml

@@ -742,7 +742,7 @@ pub(crate) enum GetTangentialInfoFromPathsResult {
     Ellipse {
         center: [f64; 2],
         ccw: bool,
-        major_radius: f64,
+        major_axis: [f64; 2],
         _minor_radius: f64,
     },
 }
@@ -761,10 +761,10 @@ impl GetTangentialInfoFromPathsResult {
             } => [center[0] + radius, center[1] + if *ccw { -1.0 } else { 1.0 }],
             GetTangentialInfoFromPathsResult::Ellipse {
                 center,
-                major_radius,
+                major_axis,
                 ccw,
                 ..
-            } => [center[0] + major_radius, center[1] + if *ccw { -1.0 } else { 1.0 }],
+            } => [center[0] + major_axis[0], center[1] + if *ccw { -1.0 } else { 1.0 }],
         }
     }
 }
@@ -1272,7 +1272,7 @@ pub enum Path {
         #[serde(flatten)]
         base: BasePath,
         center: [f64; 2],
-        major_radius: f64,
+        major_axis: [f64; 2],
         minor_radius: f64,
         ccw: bool,
     },
@@ -1524,13 +1524,13 @@ impl Path {
             // TODO: (bc) fix me
             Path::Ellipse {
                 center,
-                major_radius,
+                major_axis,
                 minor_radius,
                 ccw,
                 ..
             } => GetTangentialInfoFromPathsResult::Ellipse {
                 center: *center,
-                major_radius: *major_radius,
+                major_axis: *major_axis,
                 _minor_radius: *minor_radius,
                 ccw: *ccw,
             },

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

@@ -538,14 +538,16 @@ pub async fn ellipse(exec_state: &mut ExecState, args: Args) -> Result<KclValue,
     let sketch_or_surface =
         args.get_unlabeled_kw_arg("sketchOrSurface", &RuntimeType::sketch_or_surface(), exec_state)?;
     let center = args.get_kw_arg("center", &RuntimeType::point2d(), exec_state)?;
-    let major_radius = args.get_kw_arg("majorRadius", &RuntimeType::length(), exec_state)?;
+    let major_radius = args.get_kw_arg_opt("majorRadius", &RuntimeType::length(), exec_state)?;
+    let major_axis = args.get_kw_arg_opt("majorAxis", &RuntimeType::point2d(), exec_state)?;
     let minor_radius = args.get_kw_arg("minorRadius", &RuntimeType::length(), exec_state)?;
     let tag = args.get_kw_arg_opt("tag", &RuntimeType::tag_decl(), exec_state)?;
 
     let sketch = inner_ellipse(
         sketch_or_surface,
         center,
         major_radius,
+        major_axis,
         minor_radius,
         tag,
         exec_state,
@@ -557,10 +559,12 @@ pub async fn ellipse(exec_state: &mut ExecState, args: Args) -> Result<KclValue,
     })
 }
 
+#[allow(clippy::too_many_arguments)]
 async fn inner_ellipse(
     sketch_surface_or_group: SketchOrSurface,
     center: [TyF64; 2],
-    major_radius: TyF64,
+    major_radius: Option<TyF64>,
+    major_axis: Option<[TyF64; 2]>,
     minor_radius: TyF64,
     tag: Option<TagNode>,
     exec_state: &mut ExecState,
@@ -573,7 +577,27 @@ async fn inner_ellipse(
     let (center_u, ty) = untype_point(center.clone());
     let units = ty.expect_length();
 
-    let from = [center_u[0] + major_radius.to_length_units(units), center_u[1]];
+    let major_axis = match (major_axis, major_radius) {
+        (Some(_), Some(_)) | (None, None) => {
+            return Err(KclError::new_type(KclErrorDetails::new(
+                "Provide either `majorAxis` or `majorRadius`.".to_string(),
+                vec![args.source_range],
+            )));
+        }
+        (Some(major_axis), None) => major_axis,
+        (None, Some(major_radius)) => [
+            major_radius.clone(),
+            TyF64 {
+                n: 0.0,
+                ty: major_radius.ty,
+            },
+        ],
+    };
+
+    let from = [
+        center_u[0] + major_axis[0].to_length_units(units),
+        center_u[1] + major_axis[1].to_length_units(units),
+    ];
     let from_t = [TyF64::new(from[0], ty), TyF64::new(from[1], ty)];
 
     let sketch =
@@ -584,14 +608,15 @@ async fn inner_ellipse(
 
     let id = exec_state.next_uuid();
 
+    let axis = KPoint2d::from(untyped_point_to_mm([major_axis[0].n, major_axis[1].n], units)).map(LengthUnit);
     exec_state
         .batch_modeling_cmd(
             ModelingCmdMeta::from_args_id(&args, id),
             ModelingCmd::from(mcmd::ExtendPath {
                 path: sketch.id.into(),
                 segment: PathSegment::Ellipse {
                     center: KPoint2d::from(point_to_mm(center)).map(LengthUnit),
-                    major_radius: LengthUnit(major_radius.to_mm()),
+                    major_axis: axis,
                     minor_radius: LengthUnit(minor_radius.to_mm()),
                     start_angle: Angle::from_degrees(angle_start.to_degrees()),
                     end_angle: Angle::from_degrees(angle_end.to_degrees()),
@@ -611,7 +636,7 @@ async fn inner_ellipse(
                 metadata: args.source_range.into(),
             },
         },
-        major_radius: major_radius.to_length_units(units),
+        major_axis: major_axis.map(|x| x.to_length_units(units)),
         minor_radius: minor_radius.to_length_units(units),
         center: center_u,
         ccw: angle_start < angle_end,

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

@@ -2003,7 +2003,8 @@ pub async fn elliptic(exec_state: &mut ExecState, args: Args) -> Result<KclValue
     let center = args.get_kw_arg("center", &RuntimeType::point2d(), exec_state)?;
     let angle_start = args.get_kw_arg("angleStart", &RuntimeType::degrees(), exec_state)?;
     let angle_end = args.get_kw_arg("angleEnd", &RuntimeType::degrees(), exec_state)?;
-    let major_radius = args.get_kw_arg("majorRadius", &RuntimeType::length(), exec_state)?;
+    let major_radius = args.get_kw_arg_opt("majorRadius", &RuntimeType::length(), exec_state)?;
+    let major_axis = args.get_kw_arg_opt("majorAxis", &RuntimeType::point2d(), exec_state)?;
     let minor_radius = args.get_kw_arg("minorRadius", &RuntimeType::length(), exec_state)?;
     let tag = args.get_kw_arg_opt("tag", &RuntimeType::tag_decl(), exec_state)?;
 
@@ -2013,6 +2014,7 @@ pub async fn elliptic(exec_state: &mut ExecState, args: Args) -> Result<KclValue
         angle_start,
         angle_end,
         major_radius,
+        major_axis,
         minor_radius,
         tag,
         exec_state,
@@ -2030,7 +2032,8 @@ pub(crate) async fn inner_elliptic(
     center: [TyF64; 2],
     angle_start: TyF64,
     angle_end: TyF64,
-    major_radius: TyF64,
+    major_radius: Option<TyF64>,
+    major_axis: Option<[TyF64; 2]>,
     minor_radius: TyF64,
     tag: Option<TagNode>,
     exec_state: &mut ExecState,
@@ -2041,21 +2044,47 @@ pub(crate) async fn inner_elliptic(
 
     let (center_u, _) = untype_point(center);
 
+    let major_axis = match (major_axis, major_radius) {
+        (Some(_), Some(_)) | (None, None) => {
+            return Err(KclError::new_type(KclErrorDetails::new(
+                "Provide either `majorAxis` or `majorRadius`.".to_string(),
+                vec![args.source_range],
+            )));
+        }
+        (Some(major_axis), None) => major_axis,
+        (None, Some(major_radius)) => [
+            major_radius.clone(),
+            TyF64 {
+                n: 0.0,
+                ty: major_radius.ty,
+            },
+        ],
+    };
     let start_angle = Angle::from_degrees(angle_start.to_degrees());
     let end_angle = Angle::from_degrees(angle_end.to_degrees());
+    let major_axis_magnitude = (major_axis[0].to_length_units(from.units) * major_axis[0].to_length_units(from.units)
+        + major_axis[1].to_length_units(from.units) * major_axis[1].to_length_units(from.units))
+    .sqrt();
     let to = [
-        center_u[0] + major_radius.to_length_units(from.units) * libm::cos(end_angle.to_radians()),
-        center_u[1] + minor_radius.to_length_units(from.units) * libm::sin(end_angle.to_radians()),
+        major_axis_magnitude * libm::cos(end_angle.to_radians()),
+        minor_radius.to_length_units(from.units) * libm::sin(end_angle.to_radians()),
     ];
+    let major_axis_angle = libm::atan2(major_axis[1].n, major_axis[0].n);
 
+    let point = [
+        center_u[0] + to[0] * libm::cos(major_axis_angle) - to[1] * libm::sin(major_axis_angle),
+        center_u[1] + to[0] * libm::sin(major_axis_angle) + to[1] * libm::cos(major_axis_angle),
+    ];
+
+    let axis = major_axis.map(|x| x.to_mm());
     exec_state
         .batch_modeling_cmd(
             ModelingCmdMeta::from_args_id(&args, id),
             ModelingCmd::from(mcmd::ExtendPath {
                 path: sketch.id.into(),
                 segment: PathSegment::Ellipse {
                     center: KPoint2d::from(untyped_point_to_mm(center_u, from.units)).map(LengthUnit),
-                    major_radius: LengthUnit(major_radius.to_mm()),
+                    major_axis: axis.map(LengthUnit).into(),
                     minor_radius: LengthUnit(minor_radius.to_mm()),
                     start_angle,
                     end_angle,
@@ -2067,11 +2096,11 @@ pub(crate) async fn inner_elliptic(
     let current_path = Path::Ellipse {
         ccw: start_angle < end_angle,
         center: center_u,
-        major_radius: major_radius.to_mm(),
+        major_axis: axis,
         minor_radius: minor_radius.to_mm(),
         base: BasePath {
             from: from.ignore_units(),
-            to,
+            to: point,
             tag: tag.clone(),
             units: sketch.units,
             geo_meta: GeoMeta {

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

@@ -362,10 +362,12 @@ export fn ellipse(
   /// The center of the ellipse.
   @(snippetArray = ["0", "0"])
   center: Point2d,
-  /// The major radius of the ellipse.
-  majorRadius: number(Length),
   /// The minor radius of the ellipse.
   minorRadius: number(Length),
+  /// The major radius of the ellipse. Equivalent to majorAxis = [majorRadius, 0].
+  majorRadius?: number(Length),
+  /// The major axis of the ellipse.
+  majorAxis?: Point2d,
   /// Create a new tag which refers to this ellipse.
   tag?: tag,
 ): Sketch {}
@@ -2228,12 +2230,14 @@ export fn elliptic(
   /// Where along the ellptic should this segment end?
   @(includeInSnippet = true)
   angleEnd: number(Angle),
-  /// The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1.
-  @(includeInSnippet = true)
-  majorRadius: number(Length),
   /// The minor radius, b, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1.
   @(includeInSnippet = true)
   minorRadius: number(Length),
+  /// The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1. Equivalent to majorAxis = [majorRadius, 0].
+  majorRadius?: number(Length),
+  /// The major axis of the elliptic.
+  @(includeInSnippet = true)
+  majorAxis?: Point2d,
   /// Create a new tag which refers to this arc.
   tag?: tag,
 ): Sketch {}

rust/kcl-lib/std/sketch.kcl

@@ -93,7 +93,10 @@ description: Artifact commands elliptic_curve_inches_regression.kcl
             "x": 0.0,
             "y": 1545.7213433999998
           },
-          "major_radius": 2302.732377,
+          "major_axis": {
+            "x": 2302.732377,
+            "y": 0.0
+          },
           "minor_radius": 426.8591158,
           "start_angle": {
             "unit": "degrees",

rust/kcl-lib/tests/elliptic_curve_inches_regression/artifact_commands.snap

@@ -3,17 +3,17 @@ flowchart LR
   subgraph path2 [Path]
     2["Path<br>[322, 390, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 0 }]
-    3["Segment<br>[396, 560, 0]"]
+    3["Segment<br>[396, 563, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 1 }]
-    4["Segment<br>[566, 678, 0]"]
+    4["Segment<br>[569, 681, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 2 }]
-    5["Segment<br>[684, 793, 0]"]
+    5["Segment<br>[687, 796, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 3 }]
-    6["Segment<br>[799, 910, 0]"]
+    6["Segment<br>[802, 913, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 4 }]
-    7["Segment<br>[916, 1026, 0]"]
+    7["Segment<br>[919, 1029, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 5 }]
-    8["Segment<br>[1032, 1039, 0]"]
+    8["Segment<br>[1035, 1042, 0]"]
       %% [ProgramBodyItem { index: 2 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 6 }]
     9[Solid2d]
   end