Skip to content

KCL: Fix do_post_extrude querying wrong body for clones/mirrors #12088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adamchalmers merged 3 commits into from
Jun 12, 2026
Merged

KCL: Fix do_post_extrude querying wrong body for clones/mirrors #12088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fced9dc
Fix bug
adamchalmers Jun 12, 2026
f7336e1
Break into function to be more readable
adamchalmers Jun 12, 2026
ccad296
Update sim tests
adamchalmers Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions rust/kcl-lib/src/simulation_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6012,3 +6012,24 @@ mod clone_a_mirror3d {
super::execute(TEST_NAME, true).await
}
}
mod sweep_mirror {
const TEST_NAME: &str = "sweep_mirror";

/// Test parsing KCL.
#[test]
fn parse() {
super::parse(TEST_NAME)
}

/// Test that parsing and unparsing KCL produces the original KCL input.
#[tokio::test(flavor = "multi_thread")]
async fn unparse() {
super::unparse(TEST_NAME).await
}

/// Test that KCL is executed correctly.
#[tokio::test(flavor = "multi_thread")]
async fn kcl_test_execute() {
super::execute(TEST_NAME, true).await
}
}
56 changes: 40 additions & 16 deletions rust/kcl-lib/src/std/extrude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,41 @@ pub enum BeingExtruded {
Face { face_id: Uuid, solid_id: Uuid },
}

/// Which edge should we use for querying Solid3dGetExtrusionInfo and GetAdjacencyInfo?
/// It can be any edge of the body, but if our body is a clone, we should use an edge of
/// the original body, not the new cloned body.
fn get_extrusion_info_edge_id(sketch: &Sketch, any_edge_id: Uuid, clone_id_map: Option<&HashMap<Uuid, Uuid>>) -> Uuid {
// If this isn't a clone, there's no old/new body distinction.
// So just use the edge.
if sketch.clone.is_none() {
return any_edge_id;
}
let Some(clone_map) = clone_id_map else {
return any_edge_id;
};

// clone_map maps old IDs -> new IDs.
// If the `any_edge_id` is an ID of the OLD body
// (we know this if it's a _key_ of the map)
// we should use it (because that's the old body we're querying).
if clone_map.contains_key(&any_edge_id) {
return any_edge_id;
}

// Otherwise, if the `any_edge_id` is an ID of the NEW body
// (we know this if it's a _value_ of the map),
// we should query the corresponding ID in the OLD body.
// i.e. if it's a hashmap value, find the corresponding key.
if let Some((old_edge_id, _)) = clone_map.iter().find(|(_, new_edge_id)| **new_edge_id == any_edge_id) {
return *old_edge_id;
}

// Fall back to this if the clone_map doesn't have the data we expect.
// Probably will fail in the engine because it means the clone map was built wrong,
// or KCL and the engine disagree about what geometry exists.
any_edge_id
}

#[allow(clippy::too_many_arguments)]
pub(crate) async fn do_post_extrude<'a>(
sketch: &Sketch,
Expand Down Expand Up @@ -771,19 +806,8 @@ pub(crate) async fn do_post_extrude<'a>(
};

// If the sketch is a clone, we will use the original info to get the extrusion face info.
let mut extrusion_info_edge_id = any_edge_id;
if sketch.clone.is_some() && clone_id_map.is_some() {
extrusion_info_edge_id = if let Some(clone_map) = clone_id_map {
if let Some(new_edge_id) = clone_map.get(&extrusion_info_edge_id) {
*new_edge_id
} else {
extrusion_info_edge_id
}
} else {
any_edge_id
};
}

// So let's find an edge of the old body.
let extrusion_info_edge_id = get_extrusion_info_edge_id(sketch, any_edge_id, clone_id_map);
let mut sketch = sketch.clone();
match body_type {
BodyType::Solid => {
Expand Down Expand Up @@ -871,8 +895,8 @@ pub(crate) async fn do_post_extrude<'a>(
ModelingCmdMeta::from_args(exec_state, args),
ModelingCmd::from(
mcmd::Solid3dGetAdjacencyInfo::builder()
.object_id(sketch.id)
.edge_id(any_edge_id)
.object_id(sketch_id)
.edge_id(extrusion_info_edge_id)
.build(),
),
)
Expand Down Expand Up @@ -1033,7 +1057,7 @@ pub(crate) async fn do_post_extrude<'a>(
})
}

#[derive(Default)]
#[derive(Debug, Default)]
struct Faces {
/// Maps curve ID to face ID for each side.
sides: HashMap<Uuid, Option<Uuid>>,
Expand Down
78 changes: 47 additions & 31 deletions rust/kcl-lib/tests/christmas_tree_mirror3d_union/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,53 +78,56 @@ flowchart LR
%% [ProgramBodyItem { index: 5 }, VariableDeclarationDeclaration, VariableDeclarationInit]
44["Sweep Extrusion<br>[1806, 1839, 0]<br>Consumed: true"]
%% [ProgramBodyItem { index: 5 }, VariableDeclarationDeclaration, VariableDeclarationInit]
45["CompositeSolid Union<br>[1851, 1880, 0]<br>Consumed: false"]
45["SweepEdge Adjacent"]
46["SweepEdge Adjacent"]
47["SweepEdge Adjacent"]
48["CompositeSolid Union<br>[1851, 1880, 0]<br>Consumed: false"]
%% [ProgramBodyItem { index: 6 }, VariableDeclarationDeclaration, VariableDeclarationInit]
46["SketchBlock<br>[12, 1316, 0]"]
49["SketchBlock<br>[12, 1316, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit]
47["SketchBlockConstraint Coincident<br>[101, 134, 0]"]
50["SketchBlockConstraint Coincident<br>[101, 134, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 1 }, ExpressionStatementExpr]
48["SketchBlockConstraint Horizontal<br>[137, 168, 0]"]
51["SketchBlockConstraint Horizontal<br>[137, 168, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 2 }, ExpressionStatementExpr]
49["SketchBlockConstraint Coincident<br>[243, 279, 0]"]
52["SketchBlockConstraint Coincident<br>[243, 279, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 4 }, ExpressionStatementExpr]
50["SketchBlockConstraint Coincident<br>[357, 393, 0]"]
53["SketchBlockConstraint Coincident<br>[357, 393, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 6 }, ExpressionStatementExpr]
51["SketchBlockConstraint Coincident<br>[471, 507, 0]"]
54["SketchBlockConstraint Coincident<br>[471, 507, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 8 }, ExpressionStatementExpr]
52["SketchBlockConstraint Coincident<br>[585, 621, 0]"]
55["SketchBlockConstraint Coincident<br>[585, 621, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 10 }, ExpressionStatementExpr]
53["SketchBlockConstraint Coincident<br>[697, 733, 0]"]
56["SketchBlockConstraint Coincident<br>[697, 733, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 12 }, ExpressionStatementExpr]
54["SketchBlockConstraint Vertical<br>[736, 765, 0]"]
57["SketchBlockConstraint Vertical<br>[736, 765, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 13 }, ExpressionStatementExpr]
55["SketchBlockConstraint Coincident<br>[838, 876, 0]"]
58["SketchBlockConstraint Coincident<br>[838, 876, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 15 }, ExpressionStatementExpr]
56["SketchBlockConstraint Coincident<br>[879, 913, 0]"]
59["SketchBlockConstraint Coincident<br>[879, 913, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 16 }, ExpressionStatementExpr]
57["SketchBlockConstraint Distance<br>[916, 955, 0]"]
60["SketchBlockConstraint Distance<br>[916, 955, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 17 }, ExpressionStatementExpr]
58["SketchBlockConstraint Distance<br>[958, 998, 0]"]
61["SketchBlockConstraint Distance<br>[958, 998, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 18 }, ExpressionStatementExpr]
59["SketchBlockConstraint HorizontalDistance<br>[1001, 1054, 0]"]
62["SketchBlockConstraint HorizontalDistance<br>[1001, 1054, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 19 }, ExpressionStatementExpr]
60["SketchBlockConstraint Horizontal<br>[1057, 1074, 0]"]
63["SketchBlockConstraint Horizontal<br>[1057, 1074, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 20 }, ExpressionStatementExpr]
61["SketchBlockConstraint Horizontal<br>[1077, 1094, 0]"]
64["SketchBlockConstraint Horizontal<br>[1077, 1094, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 21 }, ExpressionStatementExpr]
62["SketchBlockConstraint Parallel<br>[1097, 1121, 0]"]
65["SketchBlockConstraint Parallel<br>[1097, 1121, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 22 }, ExpressionStatementExpr]
63["SketchBlockConstraint Parallel<br>[1124, 1148, 0]"]
66["SketchBlockConstraint Parallel<br>[1124, 1148, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 23 }, ExpressionStatementExpr]
64["SketchBlockConstraint HorizontalDistance<br>[1151, 1204, 0]"]
67["SketchBlockConstraint HorizontalDistance<br>[1151, 1204, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 24 }, ExpressionStatementExpr]
65["SketchBlockConstraint HorizontalDistance<br>[1207, 1260, 0]"]
68["SketchBlockConstraint HorizontalDistance<br>[1207, 1260, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 25 }, ExpressionStatementExpr]
66["SketchBlockConstraint HorizontalDistance<br>[1263, 1314, 0]"]
69["SketchBlockConstraint HorizontalDistance<br>[1263, 1314, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, SketchBlockBody, SketchBlockBodyItem { index: 26 }, ExpressionStatementExpr]
1 --- 2
1 <--x 10
1 <--x 46
1 <--x 49
2 --- 3
2 --- 4
2 --- 5
Expand All @@ -133,7 +136,7 @@ flowchart LR
2 --- 8
2 --- 9
2 <--x 10
46 --- 2
49 --- 2
3 <--x 11
4 <--x 12
5 <--x 13
Expand All @@ -150,11 +153,12 @@ flowchart LR
10 <--x 17
10 ---- 18
10 <---x 44
10 --- 45
10 --- 48
11 --- 25
11 x--> 26
11 --- 40
11 --- 41
11 --- 47
12 --- 24
12 x--> 26
12 --- 38
Expand All @@ -163,22 +167,27 @@ flowchart LR
13 x--> 26
13 --- 28
13 --- 29
13 --- 31
14 --- 20
14 x--> 26
14 --- 30
14 --- 31
14 x--> 31
14 --- 45
15 --- 21
15 x--> 26
15 --- 32
15 --- 33
15 --- 35
16 --- 22
16 x--> 26
16 --- 34
16 --- 35
16 x--> 35
16 --- 46
17 --- 23
17 x--> 26
17 --- 36
17 --- 37
17 x--> 39
18 --- 19
18 --- 20
18 --- 21
Expand All @@ -202,27 +211,33 @@ flowchart LR
18 --- 39
18 --- 40
18 --- 41
18 --- 45
18 --- 46
18 --- 47
19 --- 28
19 --- 29
31 <--x 19
19 --- 31
20 --- 30
20 --- 31
33 <--x 20
20 --- 45
21 --- 32
21 --- 33
35 <--x 21
21 --- 35
22 --- 34
22 --- 35
37 <--x 22
22 --- 46
23 --- 36
23 --- 37
39 <--x 23
23 --- 39
24 --- 38
24 --- 39
41 <--x 24
29 <--x 25
25 --- 40
25 --- 41
25 --- 47
28 <--x 27
30 <--x 27
32 <--x 27
Expand All @@ -231,5 +246,6 @@ flowchart LR
38 <--x 27
40 <--x 27
29 <--x 42
44 <--x 45
47 <--x 42
44 <--x 48
```
Loading
Loading