Skip to content

Commit d96a4d3

Browse files
authored
Improvements to axis handling (KittyCAD#6530)
Signed-off-by: Nick Cameron <nrc@ncameron.org>
1 parent 94452cc commit d96a4d3

File tree

93 files changed

+3216
-3209
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

93 files changed

+3216
-3209
lines changed

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

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -308,16 +308,14 @@ impl Plane {
308308
x: 1.0,
309309
y: 0.0,
310310
z: 0.0,
311-
// TODO axes must be normalized, so maybe these should all be count
312-
// rather than mm?
313-
units: UnitLen::Mm,
311+
units: _,
314312
},
315313
y_axis:
316314
Point3d {
317315
x: 0.0,
318316
y: 1.0,
319317
z: 0.0,
320-
units: UnitLen::Mm,
318+
units: _,
321319
},
322320
..
323321
} => return PlaneData::XY,
@@ -334,14 +332,14 @@ impl Plane {
334332
x: -1.0,
335333
y: 0.0,
336334
z: 0.0,
337-
units: UnitLen::Mm,
335+
units: _,
338336
},
339337
y_axis:
340338
Point3d {
341339
x: 0.0,
342340
y: 1.0,
343341
z: 0.0,
344-
units: UnitLen::Mm,
342+
units: _,
345343
},
346344
..
347345
} => return PlaneData::NegXY,
@@ -358,14 +356,14 @@ impl Plane {
358356
x: 1.0,
359357
y: 0.0,
360358
z: 0.0,
361-
units: UnitLen::Mm,
359+
units: _,
362360
},
363361
y_axis:
364362
Point3d {
365363
x: 0.0,
366364
y: 0.0,
367365
z: 1.0,
368-
units: UnitLen::Mm,
366+
units: _,
369367
},
370368
..
371369
} => return PlaneData::XZ,
@@ -382,14 +380,14 @@ impl Plane {
382380
x: -1.0,
383381
y: 0.0,
384382
z: 0.0,
385-
units: UnitLen::Mm,
383+
units: _,
386384
},
387385
y_axis:
388386
Point3d {
389387
x: 0.0,
390388
y: 0.0,
391389
z: 1.0,
392-
units: UnitLen::Mm,
390+
units: _,
393391
},
394392
..
395393
} => return PlaneData::NegXZ,
@@ -406,14 +404,14 @@ impl Plane {
406404
x: 0.0,
407405
y: 1.0,
408406
z: 0.0,
409-
units: UnitLen::Mm,
407+
units: _,
410408
},
411409
y_axis:
412410
Point3d {
413411
x: 0.0,
414412
y: 0.0,
415413
z: 1.0,
416-
units: UnitLen::Mm,
414+
units: _,
417415
},
418416
..
419417
} => return PlaneData::YZ,
@@ -430,14 +428,14 @@ impl Plane {
430428
x: 0.0,
431429
y: -1.0,
432430
z: 0.0,
433-
units: UnitLen::Mm,
431+
units: _,
434432
},
435433
y_axis:
436434
Point3d {
437435
x: 0.0,
438436
y: 0.0,
439437
z: 1.0,
440-
units: UnitLen::Mm,
438+
units: _,
441439
},
442440
..
443441
} => return PlaneData::NegYZ,
@@ -460,8 +458,8 @@ impl Plane {
460458
#[cfg(feature = "artifact-graph")]
461459
artifact_id: id.into(),
462460
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
463-
x_axis: Point3d::new(1.0, 0.0, 0.0, UnitLen::Mm),
464-
y_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Mm),
461+
x_axis: Point3d::new(1.0, 0.0, 0.0, UnitLen::Unknown),
462+
y_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Unknown),
465463
value: PlaneType::XY,
466464
meta: vec![],
467465
},
@@ -470,8 +468,8 @@ impl Plane {
470468
#[cfg(feature = "artifact-graph")]
471469
artifact_id: id.into(),
472470
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
473-
x_axis: Point3d::new(-1.0, 0.0, 0.0, UnitLen::Mm),
474-
y_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Mm),
471+
x_axis: Point3d::new(-1.0, 0.0, 0.0, UnitLen::Unknown),
472+
y_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Unknown),
475473
value: PlaneType::XY,
476474
meta: vec![],
477475
},
@@ -480,8 +478,8 @@ impl Plane {
480478
#[cfg(feature = "artifact-graph")]
481479
artifact_id: id.into(),
482480
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
483-
x_axis: Point3d::new(1.0, 0.0, 0.0, UnitLen::Mm),
484-
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Mm),
481+
x_axis: Point3d::new(1.0, 0.0, 0.0, UnitLen::Unknown),
482+
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Unknown),
485483
value: PlaneType::XZ,
486484
meta: vec![],
487485
},
@@ -490,8 +488,8 @@ impl Plane {
490488
#[cfg(feature = "artifact-graph")]
491489
artifact_id: id.into(),
492490
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
493-
x_axis: Point3d::new(-1.0, 0.0, 0.0, UnitLen::Mm),
494-
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Mm),
491+
x_axis: Point3d::new(-1.0, 0.0, 0.0, UnitLen::Unknown),
492+
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Unknown),
495493
value: PlaneType::XZ,
496494
meta: vec![],
497495
},
@@ -500,8 +498,8 @@ impl Plane {
500498
#[cfg(feature = "artifact-graph")]
501499
artifact_id: id.into(),
502500
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
503-
x_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Mm),
504-
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Mm),
501+
x_axis: Point3d::new(0.0, 1.0, 0.0, UnitLen::Unknown),
502+
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Unknown),
505503
value: PlaneType::YZ,
506504
meta: vec![],
507505
},
@@ -510,8 +508,8 @@ impl Plane {
510508
#[cfg(feature = "artifact-graph")]
511509
artifact_id: id.into(),
512510
origin: Point3d::new(0.0, 0.0, 0.0, UnitLen::Mm),
513-
x_axis: Point3d::new(0.0, -1.0, 0.0, UnitLen::Mm),
514-
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Mm),
511+
x_axis: Point3d::new(0.0, -1.0, 0.0, UnitLen::Unknown),
512+
y_axis: Point3d::new(0.0, 0.0, 1.0, UnitLen::Unknown),
515513
value: PlaneType::YZ,
516514
meta: vec![],
517515
},
@@ -631,7 +629,7 @@ impl Sketch {
631629
adjust_camera: false,
632630
planar_normal: if let SketchSurface::Plane(plane) = &self.on {
633631
// We pass in the normal for the plane here.
634-
let normal = plane.x_axis.cross(&plane.y_axis);
632+
let normal = plane.x_axis.axes_cross_product(&plane.y_axis);
635633
Some(normal.into())
636634
} else {
637635
None
@@ -918,24 +916,26 @@ impl Point3d {
918916
self.x == 0.0 && self.y == 0.0 && self.z == 0.0
919917
}
920918

921-
/// Calculate the cross product of this vector with another
922-
pub fn cross(&self, other: &Self) -> Self {
923-
let other = if other.units == self.units {
924-
other
925-
} else {
926-
&Point3d {
927-
x: self.units.adjust_to(other.x, self.units).0,
928-
y: self.units.adjust_to(other.y, self.units).0,
929-
z: self.units.adjust_to(other.z, self.units).0,
930-
units: self.units,
931-
}
932-
};
933-
919+
/// Calculate the cross product of this vector with another.
920+
///
921+
/// This should only be applied to axes or other vectors which represent only a direction (and
922+
/// no magnitude) since units are ignored.
923+
pub fn axes_cross_product(&self, other: &Self) -> Self {
934924
Self {
935925
x: self.y * other.z - self.z * other.y,
936926
y: self.z * other.x - self.x * other.z,
937927
z: self.x * other.y - self.y * other.x,
938-
units: self.units,
928+
units: UnitLen::Unknown,
929+
}
930+
}
931+
932+
pub fn normalize(&self) -> Self {
933+
let len = f64::sqrt(self.x * self.x + self.y * self.y + self.z * self.z);
934+
Point3d {
935+
x: self.x / len,
936+
y: self.y / len,
937+
z: self.z / len,
938+
units: UnitLen::Unknown,
939939
}
940940
}
941941
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,14 +1056,21 @@ impl KclValue {
10561056
.and_then(Point3d::from_kcl_val)
10571057
.ok_or(CoercionError::from(self))?;
10581058

1059+
if value.get("zAxis").is_some() {
1060+
exec_state.warn(CompilationError::err(
1061+
self.into(),
1062+
"Object with a zAxis field is being coerced into a plane, but the zAxis is ignored.",
1063+
));
1064+
}
1065+
10591066
let id = exec_state.mod_local.id_generator.next_uuid();
10601067
let plane = Plane {
10611068
id,
10621069
#[cfg(feature = "artifact-graph")]
10631070
artifact_id: id.into(),
10641071
origin,
1065-
x_axis,
1066-
y_axis,
1072+
x_axis: x_axis.normalize(),
1073+
y_axis: y_axis.normalize(),
10671074
value: super::PlaneType::Uninit,
10681075
meta: meta.clone(),
10691076
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ async fn inner_offset_plane(
2929
// standard planes themselves.
3030
plane.value = PlaneType::Custom;
3131

32-
let normal = plane.x_axis.cross(&plane.y_axis);
32+
let normal = plane.x_axis.axes_cross_product(&plane.y_axis);
3333
plane.origin += normal * offset.to_length_units(plane.origin.units);
3434
make_offset_plane_in_engine(&plane, exec_state, args).await?;
3535

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,7 @@ pub(crate) async fn inner_start_profile(
13681368
adjust_camera: false,
13691369
planar_normal: if let SketchSurface::Plane(plane) = &sketch_surface {
13701370
// We pass in the normal for the plane here.
1371-
let normal = plane.x_axis.cross(&plane.y_axis);
1371+
let normal = plane.x_axis.axes_cross_product(&plane.y_axis);
13721372
Some(normal.into())
13731373
} else {
13741374
None

rust/kcl-lib/tests/angled_line/program_memory.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,15 +202,15 @@ description: Variables in memory after executing angled_line.kcl
202202
"y": 0.0,
203203
"z": 0.0,
204204
"units": {
205-
"type": "Mm"
205+
"type": "Unknown"
206206
}
207207
},
208208
"yAxis": {
209209
"x": 0.0,
210210
"y": 1.0,
211211
"z": 0.0,
212212
"units": {
213-
"type": "Mm"
213+
"type": "Unknown"
214214
}
215215
}
216216
},

rust/kcl-lib/tests/artifact_graph_example_code1/program_memory.snap

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,15 +181,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
181181
"y": 0.0,
182182
"z": 0.0,
183183
"units": {
184-
"type": "Mm"
184+
"type": "Unknown"
185185
}
186186
},
187187
"yAxis": {
188188
"x": 0.0,
189189
"y": 1.0,
190190
"z": 0.0,
191191
"units": {
192-
"type": "Mm"
192+
"type": "Unknown"
193193
}
194194
}
195195
},
@@ -376,15 +376,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
376376
"y": 0.0,
377377
"z": 0.0,
378378
"units": {
379-
"type": "Mm"
379+
"type": "Unknown"
380380
}
381381
},
382382
"yAxis": {
383383
"x": 0.0,
384384
"y": 1.0,
385385
"z": 0.0,
386386
"units": {
387-
"type": "Mm"
387+
"type": "Unknown"
388388
}
389389
},
390390
"solid": {
@@ -563,15 +563,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
563563
"y": 0.0,
564564
"z": 0.0,
565565
"units": {
566-
"type": "Mm"
566+
"type": "Unknown"
567567
}
568568
},
569569
"yAxis": {
570570
"x": 0.0,
571571
"y": 1.0,
572572
"z": 0.0,
573573
"units": {
574-
"type": "Mm"
574+
"type": "Unknown"
575575
}
576576
}
577577
},
@@ -816,15 +816,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
816816
"y": 0.0,
817817
"z": 0.0,
818818
"units": {
819-
"type": "Mm"
819+
"type": "Unknown"
820820
}
821821
},
822822
"yAxis": {
823823
"x": 0.0,
824824
"y": 1.0,
825825
"z": 0.0,
826826
"units": {
827-
"type": "Mm"
827+
"type": "Unknown"
828828
}
829829
}
830830
},
@@ -956,15 +956,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
956956
"y": 0.0,
957957
"z": 0.0,
958958
"units": {
959-
"type": "Mm"
959+
"type": "Unknown"
960960
}
961961
},
962962
"yAxis": {
963963
"x": 0.0,
964964
"y": 1.0,
965965
"z": 0.0,
966966
"units": {
967-
"type": "Mm"
967+
"type": "Unknown"
968968
}
969969
},
970970
"solid": {
@@ -1143,15 +1143,15 @@ description: Variables in memory after executing artifact_graph_example_code1.kc
11431143
"y": 0.0,
11441144
"z": 0.0,
11451145
"units": {
1146-
"type": "Mm"
1146+
"type": "Unknown"
11471147
}
11481148
},
11491149
"yAxis": {
11501150
"x": 0.0,
11511151
"y": 1.0,
11521152
"z": 0.0,
11531153
"units": {
1154-
"type": "Mm"
1154+
"type": "Unknown"
11551155
}
11561156
}
11571157
},

0 commit comments

Comments
 (0)