Skip to content

Commit f6a37a8

Browse files
adamchalmersadamchalmers
authored
Revert "Major axis of ellipse (#7953)" (#7965)
This reverts commit f606b9b. Shouldn't have been merged as the relevant engine PR isn't ready
1 parent f606b9b commit f6a37a8

File tree

9 files changed

+36
-117
lines changed

9 files changed

+36
-117
lines changed

‎rust/Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rust/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ dashmap = { version = "6.1.0" }
3636
http = "1"
3737
indexmap = "2.10.0"
3838
kittycad = { version = "0.3.37", default-features = false, features = ["js", "requests"] }
39-
kittycad-modeling-cmds = { version = "0.2.130", features = ["ts-rs", "websocket"] }
39+
kittycad-modeling-cmds = { version = "0.2.128", features = ["ts-rs", "websocket"] }
4040
lazy_static = "1.5.0"
4141
miette = "7.6.0"
4242
pyo3 = { version = "0.24.2" }

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ pub(crate) enum GetTangentialInfoFromPathsResult {
742742
Ellipse {
743743
center: [f64; 2],
744744
ccw: bool,
745-
major_axis: [f64; 2],
745+
major_radius: f64,
746746
_minor_radius: f64,
747747
},
748748
}
@@ -761,10 +761,10 @@ impl GetTangentialInfoFromPathsResult {
761761
} => [center[0] + radius, center[1] + if *ccw { -1.0 } else { 1.0 }],
762762
GetTangentialInfoFromPathsResult::Ellipse {
763763
center,
764-
major_axis,
764+
major_radius,
765765
ccw,
766766
..
767-
} => [center[0] + major_axis[0], center[1] + if *ccw { -1.0 } else { 1.0 }],
767+
} => [center[0] + major_radius, center[1] + if *ccw { -1.0 } else { 1.0 }],
768768
}
769769
}
770770
}
@@ -1272,7 +1272,7 @@ pub enum Path {
12721272
#[serde(flatten)]
12731273
base: BasePath,
12741274
center: [f64; 2],
1275-
major_axis: [f64; 2],
1275+
major_radius: f64,
12761276
minor_radius: f64,
12771277
ccw: bool,
12781278
},
@@ -1524,13 +1524,13 @@ impl Path {
15241524
// TODO: (bc) fix me
15251525
Path::Ellipse {
15261526
center,
1527-
major_axis,
1527+
major_radius,
15281528
minor_radius,
15291529
ccw,
15301530
..
15311531
} => GetTangentialInfoFromPathsResult::Ellipse {
15321532
center: *center,
1533-
major_axis: *major_axis,
1533+
major_radius: *major_radius,
15341534
_minor_radius: *minor_radius,
15351535
ccw: *ccw,
15361536
},

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

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -535,16 +535,14 @@ pub async fn ellipse(exec_state: &mut ExecState, args: Args) -> Result<KclValue,
535535
let sketch_or_surface =
536536
args.get_unlabeled_kw_arg("sketchOrSurface", &RuntimeType::sketch_or_surface(), exec_state)?;
537537
let center = args.get_kw_arg("center", &RuntimeType::point2d(), exec_state)?;
538-
let major_radius = args.get_kw_arg_opt("majorRadius", &RuntimeType::length(), exec_state)?;
539-
let major_axis = args.get_kw_arg_opt("majorAxis", &RuntimeType::point2d(), exec_state)?;
538+
let major_radius = args.get_kw_arg("majorRadius", &RuntimeType::length(), exec_state)?;
540539
let minor_radius = args.get_kw_arg("minorRadius", &RuntimeType::length(), exec_state)?;
541540
let tag = args.get_kw_arg_opt("tag", &RuntimeType::tag_decl(), exec_state)?;
542541

543542
let sketch = inner_ellipse(
544543
sketch_or_surface,
545544
center,
546545
major_radius,
547-
major_axis,
548546
minor_radius,
549547
tag,
550548
exec_state,
@@ -556,12 +554,10 @@ pub async fn ellipse(exec_state: &mut ExecState, args: Args) -> Result<KclValue,
556554
})
557555
}
558556

559-
#[allow(clippy::too_many_arguments)]
560557
async fn inner_ellipse(
561558
sketch_surface_or_group: SketchOrSurface,
562559
center: [TyF64; 2],
563-
major_radius: Option<TyF64>,
564-
major_axis: Option<[TyF64; 2]>,
560+
major_radius: TyF64,
565561
minor_radius: TyF64,
566562
tag: Option<TagNode>,
567563
exec_state: &mut ExecState,
@@ -574,27 +570,7 @@ async fn inner_ellipse(
574570
let (center_u, ty) = untype_point(center.clone());
575571
let units = ty.expect_length();
576572

577-
let major_axis = match (major_axis, major_radius) {
578-
(Some(_), Some(_)) | (None, None) => {
579-
return Err(KclError::new_type(KclErrorDetails::new(
580-
"Provide either `majorAxis` or `majorRadius` but not both.".to_string(),
581-
vec![args.source_range],
582-
)));
583-
}
584-
(Some(major_axis), None) => major_axis,
585-
(None, Some(major_radius)) => [
586-
major_radius.clone(),
587-
TyF64 {
588-
n: 0.0,
589-
ty: major_radius.ty,
590-
},
591-
],
592-
};
593-
594-
let from = [
595-
center_u[0] + major_axis[0].to_length_units(units),
596-
center_u[1] + major_axis[1].to_length_units(units),
597-
];
573+
let from = [center_u[0] + major_radius.to_length_units(units), center_u[1]];
598574
let from_t = [TyF64::new(from[0], ty), TyF64::new(from[1], ty)];
599575

600576
let sketch =
@@ -605,15 +581,14 @@ async fn inner_ellipse(
605581

606582
let id = exec_state.next_uuid();
607583

608-
let axis = KPoint2d::from(untyped_point_to_mm([major_axis[0].n, major_axis[1].n], units)).map(LengthUnit);
609584
exec_state
610585
.batch_modeling_cmd(
611586
ModelingCmdMeta::from_args_id(&args, id),
612587
ModelingCmd::from(mcmd::ExtendPath {
613588
path: sketch.id.into(),
614589
segment: PathSegment::Ellipse {
615590
center: KPoint2d::from(point_to_mm(center)).map(LengthUnit),
616-
major_axis: axis,
591+
major_radius: LengthUnit(major_radius.to_mm()),
617592
minor_radius: LengthUnit(minor_radius.to_mm()),
618593
start_angle: Angle::from_degrees(angle_start.to_degrees()),
619594
end_angle: Angle::from_degrees(angle_end.to_degrees()),
@@ -633,7 +608,7 @@ async fn inner_ellipse(
633608
metadata: args.source_range.into(),
634609
},
635610
},
636-
major_axis: major_axis.map(|x| x.to_length_units(units)),
611+
major_radius: major_radius.to_length_units(units),
637612
minor_radius: minor_radius.to_length_units(units),
638613
center: center_u,
639614
ccw: angle_start < angle_end,

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

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,8 +1997,7 @@ pub async fn elliptic(exec_state: &mut ExecState, args: Args) -> Result<KclValue
19971997
let center = args.get_kw_arg("center", &RuntimeType::point2d(), exec_state)?;
19981998
let angle_start = args.get_kw_arg("angleStart", &RuntimeType::degrees(), exec_state)?;
19991999
let angle_end = args.get_kw_arg("angleEnd", &RuntimeType::degrees(), exec_state)?;
2000-
let major_radius = args.get_kw_arg_opt("majorRadius", &RuntimeType::length(), exec_state)?;
2001-
let major_axis = args.get_kw_arg_opt("majorAxis", &RuntimeType::point2d(), exec_state)?;
2000+
let major_radius = args.get_kw_arg("majorRadius", &RuntimeType::length(), exec_state)?;
20022001
let minor_radius = args.get_kw_arg("minorRadius", &RuntimeType::length(), exec_state)?;
20032002
let tag = args.get_kw_arg_opt("tag", &RuntimeType::tag_decl(), exec_state)?;
20042003

@@ -2008,7 +2007,6 @@ pub async fn elliptic(exec_state: &mut ExecState, args: Args) -> Result<KclValue
20082007
angle_start,
20092008
angle_end,
20102009
major_radius,
2011-
major_axis,
20122010
minor_radius,
20132011
tag,
20142012
exec_state,
@@ -2026,8 +2024,7 @@ pub(crate) async fn inner_elliptic(
20262024
center: [TyF64; 2],
20272025
angle_start: TyF64,
20282026
angle_end: TyF64,
2029-
major_radius: Option<TyF64>,
2030-
major_axis: Option<[TyF64; 2]>,
2027+
major_radius: TyF64,
20312028
minor_radius: TyF64,
20322029
tag: Option<TagNode>,
20332030
exec_state: &mut ExecState,
@@ -2038,47 +2035,21 @@ pub(crate) async fn inner_elliptic(
20382035

20392036
let (center_u, _) = untype_point(center);
20402037

2041-
let major_axis = match (major_axis, major_radius) {
2042-
(Some(_), Some(_)) | (None, None) => {
2043-
return Err(KclError::new_type(KclErrorDetails::new(
2044-
"Provide either `majorAxis` or `majorRadius` but not both.".to_string(),
2045-
vec![args.source_range],
2046-
)));
2047-
}
2048-
(Some(major_axis), None) => major_axis,
2049-
(None, Some(major_radius)) => [
2050-
major_radius.clone(),
2051-
TyF64 {
2052-
n: 0.0,
2053-
ty: major_radius.ty,
2054-
},
2055-
],
2056-
};
20572038
let start_angle = Angle::from_degrees(angle_start.to_degrees());
20582039
let end_angle = Angle::from_degrees(angle_end.to_degrees());
2059-
let major_axis_magnitude = (major_axis[0].to_length_units(from.units) * major_axis[0].to_length_units(from.units)
2060-
+ major_axis[1].to_length_units(from.units) * major_axis[1].to_length_units(from.units))
2061-
.sqrt();
20622040
let to = [
2063-
major_axis_magnitude * libm::cos(end_angle.to_radians()),
2064-
minor_radius.to_length_units(from.units) * libm::sin(end_angle.to_radians()),
2041+
center_u[0] + major_radius.to_length_units(from.units) * libm::cos(end_angle.to_radians()),
2042+
center_u[1] + minor_radius.to_length_units(from.units) * libm::sin(end_angle.to_radians()),
20652043
];
2066-
let major_axis_angle = libm::atan2(major_axis[1].n, major_axis[0].n);
20672044

2068-
let point = [
2069-
center_u[0] + to[0] * libm::cos(major_axis_angle) - to[1] * libm::sin(major_axis_angle),
2070-
center_u[1] + to[0] * libm::sin(major_axis_angle) + to[1] * libm::cos(major_axis_angle),
2071-
];
2072-
2073-
let axis = major_axis.map(|x| x.to_mm());
20742045
exec_state
20752046
.batch_modeling_cmd(
20762047
ModelingCmdMeta::from_args_id(&args, id),
20772048
ModelingCmd::from(mcmd::ExtendPath {
20782049
path: sketch.id.into(),
20792050
segment: PathSegment::Ellipse {
20802051
center: KPoint2d::from(untyped_point_to_mm(center_u, from.units)).map(LengthUnit),
2081-
major_axis: axis.map(LengthUnit).into(),
2052+
major_radius: LengthUnit(major_radius.to_mm()),
20822053
minor_radius: LengthUnit(minor_radius.to_mm()),
20832054
start_angle,
20842055
end_angle,
@@ -2090,11 +2061,11 @@ pub(crate) async fn inner_elliptic(
20902061
let current_path = Path::Ellipse {
20912062
ccw: start_angle < end_angle,
20922063
center: center_u,
2093-
major_axis: axis,
2064+
major_radius: major_radius.to_mm(),
20942065
minor_radius: minor_radius.to_mm(),
20952066
base: BasePath {
20962067
from: from.ignore_units(),
2097-
to: point,
2068+
to,
20982069
tag: tag.clone(),
20992070
units: sketch.units,
21002071
geo_meta: GeoMeta {

‎rust/kcl-lib/std/sketch.kcl

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -362,12 +362,10 @@ export fn ellipse(
362362
/// The center of the ellipse.
363363
@(snippetArray = ["0", "0"])
364364
center: Point2d,
365+
/// The major radius of the ellipse.
366+
majorRadius: number(Length),
365367
/// The minor radius of the ellipse.
366368
minorRadius: number(Length),
367-
/// The major radius of the ellipse. Equivalent to majorAxis = [majorRadius, 0].
368-
majorRadius?: number(Length),
369-
/// The major axis of the ellipse.
370-
majorAxis?: Point2d,
371369
/// Create a new tag which refers to this ellipse.
372370
tag?: tag,
373371
): Sketch {}
@@ -2230,14 +2228,12 @@ export fn elliptic(
22302228
/// Where along the ellptic should this segment end?
22312229
@(includeInSnippet = true)
22322230
angleEnd: number(Angle),
2231+
/// The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1.
2232+
@(includeInSnippet = true)
2233+
majorRadius: number(Length),
22332234
/// The minor radius, b, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1.
22342235
@(includeInSnippet = true)
22352236
minorRadius: number(Length),
2236-
/// The major radius, a, of the elliptic equation x^2 / a^2 + y^2 / b^2 = 1. Equivalent to majorAxis = [majorRadius, 0].
2237-
majorRadius?: number(Length),
2238-
/// The major axis of the elliptic.
2239-
@(includeInSnippet = true)
2240-
majorAxis?: Point2d,
22412237
/// Create a new tag which refers to this arc.
22422238
tag?: tag,
22432239
): Sketch {}

‎rust/kcl-lib/tests/elliptic_curve_inches_regression/ast.snap

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -526,45 +526,22 @@ description: Result of parsing elliptic_curve_inches_regression.kcl
526526
"commentStart": 0,
527527
"end": 0,
528528
"moduleId": 0,
529-
"name": "majorAxis",
529+
"name": "majorRadius",
530530
"start": 0,
531531
"type": "Identifier"
532532
},
533533
"arg": {
534534
"commentStart": 0,
535-
"elements": [
536-
{
537-
"commentStart": 0,
538-
"end": 0,
539-
"moduleId": 0,
540-
"raw": "90.658755",
541-
"start": 0,
542-
"type": "Literal",
543-
"type": "Literal",
544-
"value": {
545-
"value": 90.658755,
546-
"suffix": "None"
547-
}
548-
},
549-
{
550-
"commentStart": 0,
551-
"end": 0,
552-
"moduleId": 0,
553-
"raw": "0",
554-
"start": 0,
555-
"type": "Literal",
556-
"type": "Literal",
557-
"value": {
558-
"value": 0.0,
559-
"suffix": "None"
560-
}
561-
}
562-
],
563535
"end": 0,
564536
"moduleId": 0,
537+
"raw": "90.658755",
565538
"start": 0,
566-
"type": "ArrayExpression",
567-
"type": "ArrayExpression"
539+
"type": "Literal",
540+
"type": "Literal",
541+
"value": {
542+
"value": 90.658755,
543+
"suffix": "None"
544+
}
568545
}
569546
},
570547
{

‎rust/kcl-lib/tests/elliptic_curve_inches_regression/input.kcl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ sketch000Profile003 = startProfile(sketch000, at = [90.65875528983305, 60.855170
1717
center = [0, 60.855171],
1818
angleStart = 0deg,
1919
angleEnd = 180deg,
20-
majorAxis = [90.658755, 0],
20+
majorRadius = 90.658755,
2121
minorRadius = 16.805477,
2222
)
2323
|> bezierCurve(end = [50.20468, -26.67249], control1 = [15.007446, -25.538491], control2 = [32.368565, -29.659071])

‎rust/kcl-lib/tests/elliptic_curve_inches_regression/unparsed.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ sketch000Profile003 = startProfile(sketch000, at = [90.65875528983305, 60.855170
2121
center = [0, 60.855171],
2222
angleStart = 0deg,
2323
angleEnd = 180deg,
24-
majorAxis = [90.658755, 0],
24+
majorRadius = 90.658755,
2525
minorRadius = 16.805477,
2626
)
2727
|> bezierCurve(end = [50.20468, -26.67249], control1 = [15.007446, -25.538491], control2 = [32.368565, -29.659071])

0 commit comments

Comments
 (0)