-
Notifications
You must be signed in to change notification settings - Fork 88
#8162 Predictive ids #8332
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.
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
base: main
Are you sure you want to change the base?
#8162 Predictive ids #8332
Changes from 5 commits
123a937
c908cb2
d344c3d
8fc4c3e
3f98c2b
23bf3d3
d268f62
be20d04
4dbd484
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ use uuid::Uuid; | |
use crate::{ | ||
KclError, NodePath, SourceRange, | ||
errors::KclErrorDetails, | ||
execution::ArtifactId, | ||
execution::{ArtifactId, id_generator::EngineIdGenerator}, | ||
parsing::ast::types::{Node, Program}, | ||
}; | ||
|
||
|
@@ -594,6 +594,8 @@ pub(super) fn build_artifact_graph( | |
fill_in_node_paths(exec_artifact, ast, item_count); | ||
} | ||
|
||
let mut id_generator = EngineIdGenerator::new(Uuid::new_v4()); | ||
|
||
for artifact_command in artifact_commands { | ||
if let ModelingCmd::EnableSketchMode(EnableSketchMode { entity_id, .. }) = artifact_command.command { | ||
current_plane_id = Some(entity_id); | ||
|
@@ -610,6 +612,9 @@ pub(super) fn build_artifact_graph( | |
if let ModelingCmd::SketchModeDisable(_) = artifact_command.command { | ||
current_plane_id = None; | ||
} | ||
if let ModelingCmd::StartPath(_) = artifact_command.command { | ||
id_generator = EngineIdGenerator::new(artifact_command.cmd_id); | ||
} | ||
|
||
let flattened_responses = flatten_modeling_command_responses(responses); | ||
let artifact_updates = artifacts_to_update( | ||
|
@@ -620,7 +625,13 @@ pub(super) fn build_artifact_graph( | |
ast, | ||
item_count, | ||
exec_artifacts, | ||
&id_generator, | ||
)?; | ||
|
||
if let ModelingCmd::ExtendPath(_) = artifact_command.command { | ||
id_generator.next_edge(); | ||
} | ||
|
||
for artifact in artifact_updates { | ||
// Merge with existing artifacts. | ||
merge_artifact_into_map(&mut map, artifact); | ||
|
@@ -741,6 +752,7 @@ fn artifacts_to_update( | |
ast: &Node<Program>, | ||
cached_body_items: usize, | ||
exec_artifacts: &IndexMap<ArtifactId, Artifact>, | ||
id_generator: &EngineIdGenerator, | ||
) -> Result<Vec<Artifact>, KclError> { | ||
let uuid = artifact_command.cmd_id; | ||
let response = responses.get(&uuid); | ||
|
@@ -881,8 +893,9 @@ fn artifacts_to_update( | |
), | ||
}); | ||
let mut return_arr = Vec::new(); | ||
let curve_id = id_generator.get_curve_id(); | ||
return_arr.push(Artifact::Segment(Segment { | ||
id, | ||
id: curve_id, | ||
path_id, | ||
surface_id: None, | ||
edge_ids: Vec::new(), | ||
|
@@ -893,7 +906,7 @@ fn artifacts_to_update( | |
let path = artifacts.get(&path_id); | ||
if let Some(Artifact::Path(path)) = path { | ||
let mut new_path = path.clone(); | ||
new_path.seg_ids = vec![id]; | ||
new_path.seg_ids = vec![curve_id]; | ||
return_arr.push(Artifact::Path(new_path)); | ||
} | ||
if let Some(OkModelingCmdResponse::ClosePath(close_path)) = response { | ||
|
@@ -997,14 +1010,15 @@ fn artifacts_to_update( | |
}; | ||
let mut return_arr = Vec::new(); | ||
let target = ArtifactId::from(target); | ||
return_arr.push(Artifact::Sweep(Sweep { | ||
let mut sweep = Sweep { | ||
id, | ||
sub_type, | ||
path_id: target, | ||
surface_ids: Vec::new(), | ||
edge_ids: Vec::new(), | ||
code_ref, | ||
})); | ||
}; | ||
|
||
let path = artifacts.get(&target); | ||
if let Some(Artifact::Path(path)) = path { | ||
let mut new_path = path.clone(); | ||
|
@@ -1017,7 +1031,111 @@ fn artifacts_to_update( | |
inner_path_artifact.sweep_id = Some(id); | ||
return_arr.push(Artifact::Path(inner_path_artifact)) | ||
} | ||
|
||
if let ModelingCmd::Extrude(kcmc::Extrude { .. }) = cmd { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is where the artifact graph is populated with the extrusion related artifacts. |
||
// Note: target.0 === path.id | ||
let mut id_generator = EngineIdGenerator::new(target.0); | ||
let sweep_id = id; // command id is the sweep id | ||
|
||
let start_cap_id = id_generator.get_start_cap_id(); | ||
let end_cap_id = id_generator.get_end_cap_id(); | ||
|
||
// Go through segments and add walls, opposite and adj edges | ||
for index in 0..path.seg_ids.len() - 1 { | ||
let face_id = id_generator.get_face_id(); | ||
let next_face_id = id_generator.get_next_face_id(path.seg_ids.len() as u32); | ||
let curve_id = id_generator.get_curve_id(); | ||
let opposite_edge_id = id_generator.get_opposite_edge_id(); | ||
|
||
let adjacent_edge_id = id_generator.get_adjacent_edge_id(); | ||
Comment on lines
+1045
to
+1050
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the order these are called important, does it have to generate them in the same order as the engine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I answered my own question. They are all derived on the current segment index, so the order of these don't mater. And later in the loop There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly! It would be cleaner to use something like an Iterator in this particular code location but there are others where it was much less disruptive to the existing code to have a manual way to step the index: |
||
|
||
let wall_artifact = Artifact::Wall(Wall { | ||
id: face_id, | ||
seg_id: curve_id, | ||
edge_cut_edge_ids: vec![opposite_edge_id, adjacent_edge_id], | ||
sweep_id, | ||
path_ids: Vec::new(), | ||
face_code_ref: find_sketch_on_face_code_ref(exec_artifacts, face_id), | ||
cmd_id: artifact_command.cmd_id, | ||
}); | ||
return_arr.push(wall_artifact); | ||
sweep.surface_ids.push(face_id); | ||
|
||
return_arr.push(Artifact::SweepEdge(SweepEdge { | ||
id: opposite_edge_id, | ||
sub_type: SweepEdgeSubType::Opposite, | ||
seg_id: curve_id, | ||
cmd_id: artifact_command.cmd_id, | ||
index, | ||
sweep_id, | ||
common_surface_ids: vec![face_id, end_cap_id], | ||
})); | ||
|
||
return_arr.push(Artifact::SweepEdge(SweepEdge { | ||
id: adjacent_edge_id, | ||
sub_type: SweepEdgeSubType::Adjacent, | ||
seg_id: curve_id, | ||
cmd_id: artifact_command.cmd_id, | ||
index, | ||
sweep_id, | ||
common_surface_ids: vec![face_id, next_face_id], | ||
})); | ||
|
||
// Add opposite and adjacent edges to segment, sweep and wall. | ||
if let Some(Artifact::Segment(segment)) = artifacts.get(&curve_id) { | ||
let mut new_segment = segment.clone(); | ||
new_segment.edge_ids = vec![opposite_edge_id, adjacent_edge_id]; | ||
new_segment.common_surface_ids = vec![face_id, end_cap_id]; | ||
return_arr.push(Artifact::Segment(new_segment)); | ||
} | ||
|
||
sweep.edge_ids.push(opposite_edge_id); | ||
sweep.edge_ids.push(adjacent_edge_id); | ||
|
||
// TODO is this ever used? | ||
// if let Some(artifact) = artifacts.get(&edge_id) { | ||
// match artifact { | ||
// Artifact::SweepEdge(_sweep_edge) => { | ||
|
||
// // let mut new_sweep_edge = sweep_edge.clone(); | ||
// // new_sweep_edge.common_surface_ids = | ||
// // original_info.faces.iter().map(|face| ArtifactId::new(*face)).collect(); | ||
// // return_arr.push(Artifact::SweepEdge(new_sweep_edge)); | ||
// } | ||
// _ => {} | ||
// }; | ||
// }; | ||
|
||
id_generator.next_edge(); | ||
} | ||
|
||
// Add end caps | ||
|
||
return_arr.push(Artifact::Cap(Cap { | ||
id: start_cap_id, | ||
sub_type: CapSubType::Start, | ||
edge_cut_edge_ids: Vec::new(), | ||
sweep_id, | ||
path_ids: Vec::new(), | ||
face_code_ref: find_sketch_on_face_code_ref(exec_artifacts, start_cap_id), | ||
cmd_id: artifact_command.cmd_id, | ||
})); | ||
sweep.surface_ids.push(start_cap_id); | ||
|
||
return_arr.push(Artifact::Cap(Cap { | ||
id: end_cap_id, | ||
sub_type: CapSubType::End, | ||
edge_cut_edge_ids: Vec::new(), | ||
sweep_id, | ||
path_ids: Vec::new(), | ||
face_code_ref: find_sketch_on_face_code_ref(exec_artifacts, end_cap_id), | ||
cmd_id: artifact_command.cmd_id, | ||
})); | ||
sweep.surface_ids.push(end_cap_id); | ||
} | ||
} | ||
return_arr.push(Artifact::Sweep(sweep)); | ||
|
||
return Ok(return_arr); | ||
} | ||
ModelingCmd::Loft(loft_cmd) => { | ||
|
@@ -1465,3 +1583,25 @@ fn artifacts_to_update( | |
|
||
Ok(Vec::new()) | ||
} | ||
|
||
// Find sketch_on_face_code_ref (code from Solid3dGetExtrusionFaceInfo handling) | ||
fn find_sketch_on_face_code_ref(exec_artifacts: &IndexMap<ArtifactId, Artifact>, face_id: ArtifactId) -> CodeRef { | ||
let extra_artifact = exec_artifacts.values().find(|a| { | ||
if let Artifact::StartSketchOnFace(s) = a { | ||
s.face_id == face_id | ||
} else if let Artifact::StartSketchOnPlane(s) = a { | ||
s.plane_id == face_id | ||
} else { | ||
false | ||
} | ||
}); | ||
let sketch_on_face_code_ref = extra_artifact | ||
.and_then(|a| match a { | ||
Artifact::StartSketchOnFace(s) => Some(s.code_ref.clone()), | ||
Artifact::StartSketchOnPlane(s) => Some(s.code_ref.clone()), | ||
_ => None, | ||
}) | ||
// TODO: If we didn't find it, it's probably a bug. | ||
.unwrap_or_default(); | ||
sketch_on_face_code_ref | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,6 +154,7 @@ async fn fix_tags_and_references( | |
exec_state, | ||
args, | ||
None, | ||
false, | ||
) | ||
.await?; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is passed to
artifacts_to_update
so the original Segments can be created with the new ids.