Deleting a sketch deletes every feature built from it: a pipe whose path sketch was deleted stayed in the timeline as a broken feature ("unknown sketch"), and so did holes drilled at sketch points and patches filling sketch regions. Which sketches a feature uses now comes from one place, FeatureKind::input_sketches, a match over every kind with no wildcard arm, so a new feature kind does not compile until it says which sketches it uses; a test reads the FeatureKind definition and fails when a kind with a sketch field is listed as using none (it catches a sketch field added to an existing kind). timeline.dependents reports the same features

This commit is contained in:
CypherNaugh_0x authored and bherbruck committed 2026-10-10 17:21:25 -04:00
1 parent acf4669f73
commit 54a86dc6f5
3 files changed
+127 -12

No files matched your search

+61 -12
View File
@@ -939,6 +939,66 @@ fn z_axis() -> Vec3 {
}
impl FeatureKind {
/// Sketches the feature is built from (profiles, paths, points): deleting one of them
/// deletes the feature. Every kind is listed, so a new one has to say what it uses.
pub fn input_sketches(&self) -> Vec<u64> {
use FeatureKind as K;
match self {
K::Extrude { sketch, .. }
| K::Revolve { sketch, .. }
| K::Emboss { sketch, .. }
| K::Rib { sketch, .. }
| K::SheetBase { sketch, .. }
| K::SheetContour { sketch, .. } => vec![*sketch],
K::Sweep { sketch, path_sketch, .. } => vec![*sketch, *path_sketch],
K::Pipe { path_sketch, .. } | K::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } => vec![*path_sketch],
K::Loft { sections, .. } => sections.iter().map(|s| s.sketch).collect(),
K::SheetFold { sketch, .. } | K::SplitFace { sketch, .. } | K::Patch { sketch, .. } => sketch.iter().copied().collect(),
K::Hole { points, .. } => points.iter().map(|p| p.sketch).collect(),
K::Sketch { .. }
| K::Pattern { .. }
| K::Fillet { .. }
| K::Chamfer { .. }
| K::Box { .. }
| K::Cylinder { .. }
| K::Sphere { .. }
| K::Torus { .. }
| K::Combine { .. }
| K::Mirror { .. }
| K::Thread { .. }
| K::Shell { .. }
| K::Draft { .. }
| K::ConstructionPlane { .. }
| K::Split { .. }
| K::ConstructionAxis { .. }
| K::ConstructionPoint { .. }
| K::Scale { .. }
| K::OffsetFace { .. }
| K::DeleteFace { .. }
| K::BoundingSolid { .. }
| K::ReplaceFace { .. }
| K::Align { .. }
| K::Coil { .. }
| K::SheetFlange { .. }
| K::SheetHem { .. }
| K::SheetUnfold { .. }
| K::SheetConvert { .. }
| K::Boss { .. }
| K::Lip { .. }
| K::SnapFit { .. }
| K::Rest { .. }
| K::BoundaryFill { .. }
| K::Remove { .. }
| K::Stitch { .. }
| K::Thicken { .. }
| K::SurfaceExtend { .. }
| K::SurfaceTrim { .. }
| K::Move { .. }
| K::Import { .. }
| K::MeshImport { .. } => Vec::new(),
}
}
/// A sheet metal feature (its result depends on the sheet metal rules).
pub fn is_plastic(&self) -> bool {
matches!(self, FeatureKind::Boss { .. } | FeatureKind::Lip { .. } | FeatureKind::SnapFit { .. } | FeatureKind::Rest { .. })
@@ -1620,18 +1680,7 @@ impl Document {
pub fn delete_feature(&mut self, id: u64) -> Result<Vec<u64>> {
let idx = self.feature_index(id).ok_or_else(|| DocError::Unknown(format!("feature {id}")))?;
let mut gone = vec![id];
for f in &self.features {
match &f.kind {
FeatureKind::Extrude { sketch, .. } | FeatureKind::Revolve { sketch, .. } if *sketch == id => gone.push(f.id),
FeatureKind::Sweep { sketch, path_sketch, .. } if *sketch == id || *path_sketch == id => gone.push(f.id),
FeatureKind::Loft { sections, .. } if sections.iter().any(|s| s.sketch == id) => gone.push(f.id),
FeatureKind::Pattern { pattern: PatternKind::Path { path_sketch, .. }, .. } if *path_sketch == id => gone.push(f.id),
FeatureKind::Emboss { sketch, .. } | FeatureKind::Rib { sketch, .. } if *sketch == id => gone.push(f.id),
FeatureKind::SheetBase { sketch, .. } | FeatureKind::SheetContour { sketch, .. } if *sketch == id => gone.push(f.id),
FeatureKind::SheetFold { sketch: Some(s), .. } | FeatureKind::SplitFace { sketch: Some(s), .. } if *s == id => gone.push(f.id),
_ => {}
}
}
gone.extend(self.features.iter().filter(|f| f.kind.input_sketches().contains(&id)).map(|f| f.id));
self.features.retain(|f| !gone.contains(&f.id));
// Configuration columns of deleted features go with them.
let keep: Vec<bool> = self.configs.columns.iter().map(|c| !matches!(c, crate::config::Column::Suppress(id) if gone.contains(id))).collect();
+50
View File
@@ -295,3 +295,53 @@ fn parameter_cycles_are_found() {
assert!(errs["a"].contains("circular reference: a → b → c → a"), "{}", errs["a"]);
assert!(vals.contains_key("d") && errs.contains_key("a") && errs.contains_key("e"));
}
/// Features that use a sketch's points or regions go with it (not only profile and path users).
#[test]
fn input_sketches_cover_points_and_patches() {
let hole: FeatureKind = serde_json::from_value(serde_json::json!({
"type": "hole", "position": [0, 0, 0], "direction": [0, 0, -1], "diameter": "3",
"points": {"sketch": 7, "ids": ["p1"]},
}))
.unwrap();
assert_eq!(hole.input_sketches(), vec![7]);
let patch: FeatureKind = serde_json::from_value(serde_json::json!({"type": "patch", "sketch": 9})).unwrap();
assert_eq!(patch.input_sketches(), vec![9]);
let pipe: FeatureKind = serde_json::from_value(serde_json::json!({"type": "pipe", "path_sketch": 72, "path": ["l1"], "diameter": "4"})).unwrap();
assert_eq!(pipe.input_sketches(), vec![72]);
}
/// Guard for `FeatureKind::input_sketches`: a variant with a field that refers to a sketch
/// (a field named `*sketch*` holding an id, sketch points or loft sections) must not sit in the
/// arm that returns no sketches. New variants are caught by the compiler (the match has no
/// wildcard); this catches a sketch field added to an existing variant.
#[test]
fn input_sketches_lists_every_variant_with_a_sketch_field() {
let src = include_str!("document.rs");
let body = src.split("pub enum FeatureKind {").nth(1).and_then(|s| s.split("\n}\n").next()).unwrap();
let mut with_sketch = Vec::new();
let mut variant = "";
for line in body.lines() {
let t = line.trim();
if line.starts_with(" ") && !line.starts_with(" ") && t.chars().next().is_some_and(|c| c.is_ascii_uppercase()) {
variant = t.split(|c: char| !c.is_alphanumeric()).next().unwrap_or("");
continue;
}
let Some((name, ty)) = t.strip_prefix("pub ").unwrap_or(t).split_once(':') else { continue };
let refers = (name.contains("sketch") && (ty.contains("u64") || ty.contains("SketchPoints")))
|| ty.contains("SketchPoints")
|| ty.contains("LoftSection");
if refers && !line.trim_start().starts_with("//") && !with_sketch.contains(&variant) {
with_sketch.push(variant);
}
}
assert!(with_sketch.len() >= 10, "the FeatureKind parse found too few sketch users: {with_sketch:?}");
let f = src.split("pub fn input_sketches(&self)").nth(1).unwrap();
let empty_arm = f.split("=> Vec::new()").next().and_then(|s| s.rsplit("=> ").next()).unwrap();
for v in with_sketch {
assert!(
!empty_arm.contains(&format!("K::{v} {{")),
"FeatureKind::{v} has a sketch field but input_sketches says it uses no sketch: deleting the sketch would leave it broken"
);
}
}
+16
View File
@@ -839,6 +839,22 @@ fn pipe_scale_offset_bounding_and_materials() {
assert!(s.execute("material.assign", &json!({"bodies": ["Cube"], "material": "unobtainium"})).is_err());
}
/// Regression: deleting a pipe's path sketch left the pipe in the timeline ("unknown sketch").
#[test]
fn deleting_a_path_sketch_deletes_its_pipe() {
let mut s = Session::default();
run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Path"}));
run(&mut s, "sketch.line", json!({"points": [[0, 0], [30, 0]], "ids": ["l1"]}));
run(&mut s, "sketch.finish", json!({}));
let pipe = run(&mut s, "solid.pipe", json!({"path_sketch": "Path", "path": ["l1"], "diameter": 4}));
let deps = run(&mut s, "timeline.dependents", json!({"feature": "Path"}));
assert_eq!(deps["deleted_with_it"].as_array().map(Vec::len), Some(1), "{deps} (pipe {pipe})");
let r = run(&mut s, "timeline.delete", json!({"features": ["Path"]}));
assert_eq!(r["deleted"].as_array().map(Vec::len), Some(2), "{r}");
assert!(s.doc.features.is_empty(), "{:?}", s.doc.features.iter().map(|f| &f.name).collect::<Vec<_>>());
assert!(s.model.results.iter().all(|r| r.error.is_none()));
}
#[test]
fn components_occurrences_and_world_placement() {
let mut s = Session::default();