Skip to content

Commit 989c48a

Browse files
adamchalmersadamchalmers
authored
KCL: Sketches should track their inner edges (#7753)
Consider cutting a triangle out of another triangle, using subtract2d. The resulting sketch should have 6 path segments: 3 on the outside, and 3 on the inside (which resulted from the subtract2d). Currently, sketches only track the 3 outer edges. They should track the inner ones too.
1 parent 6f6cb6c commit 989c48a

File tree

33 files changed

+8266
-702
lines changed

33 files changed

+8266
-702
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,12 @@ pub struct Sketch {
614614
/// The id of the sketch (this will change when the engine's reference to it changes).
615615
pub id: uuid::Uuid,
616616
/// The paths in the sketch.
617+
/// Only paths on the "outside" i.e. the perimeter.
618+
/// Does not include paths "inside" the profile (for example, edges made by subtracting a profile)
617619
pub paths: Vec<Path>,
620+
/// Inner paths, resulting from subtract2d to carve profiles out of the sketch.
621+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
622+
pub inner_paths: Vec<Path>,
618623
/// What the sketch is on (can be a plane or a face).
619624
pub on: SketchSurface,
620625
/// The starting path.

‎rust/kcl-lib/src/parsing/ast/types/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2432,6 +2432,12 @@ pub struct TagDeclarator {
24322432

24332433
pub type TagNode = Node<TagDeclarator>;
24342434

2435+
impl std::fmt::Display for TagNode {
2436+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
2437+
self.inner.name.fmt(f)
2438+
}
2439+
}
2440+
24352441
impl From<&BoxNode<TagDeclarator>> for KclValue {
24362442
fn from(tag: &BoxNode<TagDeclarator>) -> Self {
24372443
KclValue::TagDeclarator(tag.clone())

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

Lines changed: 81 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -260,74 +260,31 @@ pub(crate) async fn do_post_extrude<'a>(
260260
start_cap_id,
261261
end_cap_id,
262262
} = analyze_faces(exec_state, args, face_infos).await;
263-
264263
// Iterate over the sketch.value array and add face_id to GeoMeta
265264
let no_engine_commands = args.ctx.no_engine_commands().await;
266-
let mut new_value: Vec<ExtrudeSurface> = sketch
267-
.paths
268-
.iter()
269-
.flat_map(|path| {
270-
if let Some(Some(actual_face_id)) = face_id_map.get(&path.get_base().geo_meta.id) {
271-
match path {
272-
Path::Arc { .. }
273-
| Path::TangentialArc { .. }
274-
| Path::TangentialArcTo { .. }
275-
// TODO: (bc) fix me
276-
| Path::Ellipse { .. }
277-
| Path::Conic {.. }
278-
| Path::Circle { .. }
279-
| Path::CircleThreePoint { .. } => {
280-
let extrude_surface = ExtrudeSurface::ExtrudeArc(crate::execution::ExtrudeArc {
281-
face_id: *actual_face_id,
282-
tag: path.get_base().tag.clone(),
283-
geo_meta: GeoMeta {
284-
id: path.get_base().geo_meta.id,
285-
metadata: path.get_base().geo_meta.metadata,
286-
},
287-
});
288-
Some(extrude_surface)
289-
}
290-
Path::Base { .. } | Path::ToPoint { .. } | Path::Horizontal { .. } | Path::AngledLineTo { .. } => {
291-
let extrude_surface = ExtrudeSurface::ExtrudePlane(crate::execution::ExtrudePlane {
292-
face_id: *actual_face_id,
293-
tag: path.get_base().tag.clone(),
294-
geo_meta: GeoMeta {
295-
id: path.get_base().geo_meta.id,
296-
metadata: path.get_base().geo_meta.metadata,
297-
},
298-
});
299-
Some(extrude_surface)
300-
}
301-
Path::ArcThreePoint { .. } => {
302-
let extrude_surface = ExtrudeSurface::ExtrudeArc(crate::execution::ExtrudeArc {
303-
face_id: *actual_face_id,
304-
tag: path.get_base().tag.clone(),
305-
geo_meta: GeoMeta {
306-
id: path.get_base().geo_meta.id,
307-
metadata: path.get_base().geo_meta.metadata,
308-
},
309-
});
310-
Some(extrude_surface)
311-
}
312-
}
313-
} else if no_engine_commands {
314-
// Only pre-populate the extrude surface if we are in mock mode.
315-
316-
let extrude_surface = ExtrudeSurface::ExtrudePlane(crate::execution::ExtrudePlane {
317-
// pushing this values with a fake face_id to make extrudes mock-execute safe
318-
face_id: exec_state.next_uuid(),
319-
tag: path.get_base().tag.clone(),
320-
geo_meta: GeoMeta {
321-
id: path.get_base().geo_meta.id,
322-
metadata: path.get_base().geo_meta.metadata,
323-
},
324-
});
325-
Some(extrude_surface)
326-
} else {
327-
None
328-
}
329-
})
330-
.collect();
265+
let mut new_value: Vec<ExtrudeSurface> = Vec::with_capacity(sketch.paths.len() + sketch.inner_paths.len() + 2);
266+
let outer_surfaces = sketch.paths.iter().flat_map(|path| {
267+
if let Some(Some(actual_face_id)) = face_id_map.get(&path.get_base().geo_meta.id) {
268+
surface_of(path, *actual_face_id)
269+
} else if no_engine_commands {
270+
// Only pre-populate the extrude surface if we are in mock mode.
271+
fake_extrude_surface(exec_state, path)
272+
} else {
273+
None
274+
}
275+
});
276+
new_value.extend(outer_surfaces);
277+
let inner_surfaces = sketch.inner_paths.iter().flat_map(|path| {
278+
if let Some(Some(actual_face_id)) = face_id_map.get(&path.get_base().geo_meta.id) {
279+
surface_of(path, *actual_face_id)
280+
} else if no_engine_commands {
281+
// Only pre-populate the extrude surface if we are in mock mode.
282+
fake_extrude_surface(exec_state, path)
283+
} else {
284+
None
285+
}
286+
});
287+
new_value.extend(inner_surfaces);
331288

332289
// Add the tags for the start or end caps.
333290
if let Some(tag_start) = named_cap_tags.start {
@@ -426,3 +383,61 @@ async fn analyze_faces(exec_state: &mut ExecState, args: &Args, face_infos: Vec<
426383
}
427384
faces
428385
}
386+
fn surface_of(path: &Path, actual_face_id: Uuid) -> Option<ExtrudeSurface> {
387+
match path {
388+
Path::Arc { .. }
389+
| Path::TangentialArc { .. }
390+
| Path::TangentialArcTo { .. }
391+
// TODO: (bc) fix me
392+
| Path::Ellipse { .. }
393+
| Path::Conic {.. }
394+
| Path::Circle { .. }
395+
| Path::CircleThreePoint { .. } => {
396+
let extrude_surface = ExtrudeSurface::ExtrudeArc(crate::execution::ExtrudeArc {
397+
face_id: actual_face_id,
398+
tag: path.get_base().tag.clone(),
399+
geo_meta: GeoMeta {
400+
id: path.get_base().geo_meta.id,
401+
metadata: path.get_base().geo_meta.metadata,
402+
},
403+
});
404+
Some(extrude_surface)
405+
}
406+
Path::Base { .. } | Path::ToPoint { .. } | Path::Horizontal { .. } | Path::AngledLineTo { .. } => {
407+
let extrude_surface = ExtrudeSurface::ExtrudePlane(crate::execution::ExtrudePlane {
408+
face_id: actual_face_id,
409+
tag: path.get_base().tag.clone(),
410+
geo_meta: GeoMeta {
411+
id: path.get_base().geo_meta.id,
412+
metadata: path.get_base().geo_meta.metadata,
413+
},
414+
});
415+
Some(extrude_surface)
416+
}
417+
Path::ArcThreePoint { .. } => {
418+
let extrude_surface = ExtrudeSurface::ExtrudeArc(crate::execution::ExtrudeArc {
419+
face_id: actual_face_id,
420+
tag: path.get_base().tag.clone(),
421+
geo_meta: GeoMeta {
422+
id: path.get_base().geo_meta.id,
423+
metadata: path.get_base().geo_meta.metadata,
424+
},
425+
});
426+
Some(extrude_surface)
427+
}
428+
}
429+
}
430+
431+
/// Create a fake extrude surface to report for mock execution, when there's no engine response.
432+
fn fake_extrude_surface(exec_state: &mut ExecState, path: &Path) -> Option<ExtrudeSurface> {
433+
let extrude_surface = ExtrudeSurface::ExtrudePlane(crate::execution::ExtrudePlane {
434+
// pushing this values with a fake face_id to make extrudes mock-execute safe
435+
face_id: exec_state.next_uuid(),
436+
tag: path.get_base().tag.clone(),
437+
geo_meta: GeoMeta {
438+
id: path.get_base().geo_meta.id,
439+
metadata: path.get_base().geo_meta.metadata,
440+
},
441+
});
442+
Some(extrude_surface)
443+
}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,7 @@ pub(crate) async fn inner_start_profile(
10391039
artifact_id: path_id.into(),
10401040
on: sketch_surface.clone(),
10411041
paths: vec![],
1042+
inner_paths: vec![],
10421043
units,
10431044
mirror: Default::default(),
10441045
meta: vec![args.source_range.into()],
@@ -1748,7 +1749,7 @@ pub async fn subtract_2d(exec_state: &mut ExecState, args: Args) -> Result<KclVa
17481749
}
17491750

17501751
async fn inner_subtract_2d(
1751-
sketch: Sketch,
1752+
mut sketch: Sketch,
17521753
tool: Vec<Sketch>,
17531754
exec_state: &mut ExecState,
17541755
args: Args,
@@ -1764,8 +1765,8 @@ async fn inner_subtract_2d(
17641765
)
17651766
.await?;
17661767

1767-
// suggestion (mike)
1768-
// we also hide the source hole since its essentially "consumed" by this operation
1768+
// Hide the source hole since it's no longer its own profile,
1769+
// it's just used to modify some other profile.
17691770
exec_state
17701771
.batch_modeling_cmd(
17711772
ModelingCmdMeta::from(&args),
@@ -1775,8 +1776,16 @@ async fn inner_subtract_2d(
17751776
}),
17761777
)
17771778
.await?;
1779+
1780+
// NOTE: We don't look at the inner paths of the hole/tool sketch.
1781+
// So if you have circle A, and it has a circular hole cut out (B),
1782+
// then you cut A out of an even bigger circle C, we will lose that info.
1783+
// Not really sure what to do about this.
1784+
sketch.inner_paths.extend_from_slice(&hole_sketch.paths);
17781785
}
17791786

1787+
// Returns the input sketch, exactly as it was, zero modifications.
1788+
// This means the edges from `tool` are basically ignored, they're not in the output.
17801789
Ok(sketch)
17811790
}
17821791

‎rust/kcl-lib/tests/flush_batch_on_end/program_memory.snap

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,40 @@ description: Variables in memory after executing flush_batch_on_end.kcl
327327
}
328328
}
329329
],
330+
"innerPaths": [
331+
{
332+
"__geoMeta": {
333+
"id": "[uuid]",
334+
"sourceRange": []
335+
},
336+
"ccw": true,
337+
"center": [
338+
0.0,
339+
0.0
340+
],
341+
"from": [
342+
0.182,
343+
0.0
344+
],
345+
"radius": 0.182,
346+
"tag": {
347+
"commentStart": 515,
348+
"end": 522,
349+
"moduleId": 0,
350+
"start": 515,
351+
"type": "TagDeclarator",
352+
"value": "arc001"
353+
},
354+
"to": [
355+
0.182,
356+
0.0
357+
],
358+
"type": "Circle",
359+
"units": {
360+
"type": "Inches"
361+
}
362+
}
363+
],
330364
"on": {
331365
"artifactId": "[uuid]",
332366
"id": "[uuid]",
@@ -443,6 +477,40 @@ description: Variables in memory after executing flush_batch_on_end.kcl
443477
}
444478
}
445479
],
480+
"innerPaths": [
481+
{
482+
"__geoMeta": {
483+
"id": "[uuid]",
484+
"sourceRange": []
485+
},
486+
"ccw": true,
487+
"center": [
488+
0.0,
489+
0.0
490+
],
491+
"from": [
492+
0.182,
493+
0.0
494+
],
495+
"radius": 0.182,
496+
"tag": {
497+
"commentStart": 515,
498+
"end": 522,
499+
"moduleId": 0,
500+
"start": 515,
501+
"type": "TagDeclarator",
502+
"value": "arc001"
503+
},
504+
"to": [
505+
0.182,
506+
0.0
507+
],
508+
"type": "Circle",
509+
"units": {
510+
"type": "Inches"
511+
}
512+
}
513+
],
446514
"on": {
447515
"artifactId": "[uuid]",
448516
"id": "[uuid]",

0 commit comments

Comments
 (0)