Skip to content

Commit 6fa24dc

Browse files
authored
KCL: Revolve should create new solids (#7937)
When you extrude a sketch-on-face, you could either modify the original solid, or create a new one. These are known as extrude methods (Merge and New respectively). Revolve, on the engine side, always does the equivalent of New for revolve, but the app wrongly thought it was doing a Merge. This meant KCL got confused about whether a revolved SoF should be treated as its own object or part of something else's. Here's an easy way to visualize the problem: Before: <img width="1840" height="1037" alt="Screenshot 2025-07-30 at 2 07 42 PM" src="https://github.com/user-attachments/assets/f82f5107-1017-4b12-91f6-baf6ea3a6797" /> After: <img width="1840" height="1151" alt="Screenshot 2025-07-30 at 2 07 10 PM" src="https://github.com/user-attachments/assets/1be72b33-a76b-47b0-9357-8583f86204fc" /> Fixes #6925
1 parent c82f231 commit 6fa24dc

15 files changed

+3058
-214
lines changed

rust/kcl-lib/src/simulation_tests.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3838,3 +3838,24 @@ mod tag_inner_face {
38383838
super::execute(TEST_NAME, true).await
38393839
}
38403840
}
3841+
mod revolve_on_face {
3842+
const TEST_NAME: &str = "revolve_on_face";
3843+
3844+
/// Test parsing KCL.
3845+
#[test]
3846+
fn parse() {
3847+
super::parse(TEST_NAME)
3848+
}
3849+
3850+
/// Test that parsing and unparsing KCL produces the original KCL input.
3851+
#[tokio::test(flavor = "multi_thread")]
3852+
async fn unparse() {
3853+
super::unparse(TEST_NAME).await
3854+
}
3855+
3856+
/// Test that KCL is executed correctly.
3857+
#[tokio::test(flavor = "multi_thread")]
3858+
async fn kcl_test_execute() {
3859+
super::execute(TEST_NAME, true).await
3860+
}
3861+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,9 @@ pub(crate) async fn do_post_extrude<'a>(
332332
// Ok so you would think that the id would be the id of the solid,
333333
// that we passed in to the function, but it's actually the id of the
334334
// sketch.
335+
//
336+
// Why? Because when you extrude a sketch, the engine lets the solid absorb the
337+
// sketch's ID. So the solid should take over the sketch's ID.
335338
id: sketch.id,
336339
artifact_id: solid_id,
337340
value: new_value,

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,14 @@ async fn inner_revolve(
127127

128128
let mut solids = Vec::new();
129129
for sketch in &sketches {
130-
let id = exec_state.next_uuid();
130+
let new_solid_id = exec_state.next_uuid();
131131
let tolerance = tolerance.as_ref().map(|t| t.to_mm()).unwrap_or(DEFAULT_TOLERANCE_MM);
132132

133133
let direction = match &axis {
134134
Axis2dOrEdgeReference::Axis { direction, origin } => {
135135
exec_state
136136
.batch_modeling_cmd(
137-
ModelingCmdMeta::from_args_id(&args, id),
137+
ModelingCmdMeta::from_args_id(&args, new_solid_id),
138138
ModelingCmd::from(mcmd::Revolve {
139139
angle,
140140
target: sketch.id.into(),
@@ -160,7 +160,7 @@ async fn inner_revolve(
160160
let edge_id = edge.get_engine_id(exec_state, &args)?;
161161
exec_state
162162
.batch_modeling_cmd(
163-
ModelingCmdMeta::from_args_id(&args, id),
163+
ModelingCmdMeta::from_args_id(&args, new_solid_id),
164164
ModelingCmd::from(mcmd::RevolveAboutEdge {
165165
angle,
166166
target: sketch.id.into(),
@@ -198,14 +198,14 @@ async fn inner_revolve(
198198
solids.push(
199199
do_post_extrude(
200200
sketch,
201-
id.into(),
201+
new_solid_id.into(),
202202
TyF64::new(0.0, NumericType::mm()),
203203
false,
204204
&super::extrude::NamedCapTags {
205205
start: tag_start.as_ref(),
206206
end: tag_end.as_ref(),
207207
},
208-
kittycad_modeling_cmds::shared::ExtrudeMethod::Merge,
208+
kittycad_modeling_cmds::shared::ExtrudeMethod::New,
209209
exec_state,
210210
&args,
211211
edge_id,

0 commit comments

Comments
 (0)