From 9ee8ba5dd752229eaf2be8c9883a6adfd79496fd Mon Sep 17 00:00:00 2001 From: bherbruck Date: Thu, 8 Oct 2026 14:19:04 +0000 Subject: [PATCH] Imported looks outlive later features: a body's imported colour, opacity and face colours (kernel paint, dropped by operations) are carried to the next version of the body, its faces by persistent name (split faces keep the colour, new faces take the body's); assigned face appearances already follow their faces by name Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL --- crates/doc/src/appearance.rs | 48 +++++++++++++++--- crates/doc/src/document.rs | 8 ++- crates/doc/src/eval.rs | 11 ++++- crates/engine/src/cmd/motion_tests.rs | 3 +- crates/engine/src/cmd/patterns_tests.rs | 6 +-- crates/engine/src/cmd/sketch.rs | 16 ++---- .../src/cmd/sketch_constraints_tests.rs | 3 +- crates/engine/src/cmd/sketch_modify_tests.rs | 7 +-- crates/engine/src/cmd/surface.rs | 6 ++- crates/engine/src/cmd/surface_analysis.rs | 4 +- crates/engine/src/frames.rs | 4 +- crates/engine/src/tests.rs | 49 ++++++++++++++++--- 12 files changed, 120 insertions(+), 45 deletions(-) diff --git a/crates/doc/src/appearance.rs b/crates/doc/src/appearance.rs index 6d377b2a..f5f8b431 100644 --- a/crates/doc/src/appearance.rs +++ b/crates/doc/src/appearance.rs @@ -47,6 +47,33 @@ pub struct FaceLook { pub name: Option, } +/// The look a body was imported with, carried through later features (which build new kernel +/// bodies without it): its colour and opacity, and its faces' colours by persistent face name. +#[derive(Clone, Debug, Default, PartialEq)] +pub struct ImportedLook { + pub color: Option<[f32; 3]>, + pub opacity: Option, + pub faces: BTreeMap, +} + +impl ImportedLook { + /// A body's imported look: its own kernel paint, else what it carries. + pub fn of(b: &crate::ModelBody) -> Option> { + if b.body.paint().is_none() && b.body.color().is_none() { + return b.imported.clone(); + } + let names = crate::naming::face_names(b); + let faces = b + .body + .paint() + .map(|p| { + p.faces.iter().filter_map(|f| names.get(f.face).map(|n| (crate::naming::strip_piece(n).to_string(), (f.color, f.opacity)))).collect() + }) + .unwrap_or_default(); + Some(std::sync::Arc::new(ImportedLook { color: b.body.color(), opacity: b.body.paint().map(|p| p.opacity), faces })) + } +} + /// The design's appearances. #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct Appearances { @@ -126,15 +153,20 @@ impl Document { /// assignment to the same face wins. pub fn face_colors(&self, b: &crate::ModelBody) -> Vec<(usize, Look)> { // Face colours the body was imported with, under the design's own. - let mut out: Vec<(usize, Look)> = b - .body - .paint() - .map(|p| { - let to8 = |x: f32| (x.clamp(0.0, 1.0) * 255.0).round() as u8; - p.faces.iter().map(|f| (f.face, Look::custom(f.color.map(to8), f.opacity as f64))).collect() - }) - .unwrap_or_default(); + let to8 = |x: f32| (x.clamp(0.0, 1.0) * 255.0).round() as u8; let names = crate::naming::face_names(b); + let mut out: Vec<(usize, Look)> = match b.body.paint() { + Some(p) => p.faces.iter().map(|f| (f.face, Look::custom(f.color.map(to8), f.opacity as f64))).collect(), + // Carried from an earlier version of the body: by face name. + None => match &b.imported { + Some(imp) => names + .iter() + .enumerate() + .filter_map(|(i, n)| imp.faces.get(crate::naming::strip_piece(n)).map(|(c, o)| (i, Look::custom(c.map(to8), *o as f64)))) + .collect(), + None => Vec::new(), + }, + }; for f in self.appearances.faces.iter().filter(|f| f.body == b.name) { let (p, look) = (f.point, f.look.clone()); // By name (every piece of a split face), else by the point. diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 184b2079..3e7faa41 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -1329,7 +1329,7 @@ impl Document { /// else its component's, else its material's look, else the colour it was imported with. /// (Faces may have their own: `face_looks`.) pub fn body_color(&self, b: &crate::ModelBody) -> Option<[f32; 3]> { - self.body_look(&b.name, b.feature).map(|l| l.rgb()).or_else(|| b.body.color()) + self.body_look(&b.name, b.feature).map(|l| l.rgb()).or_else(|| b.body.color()).or_else(|| b.imported.as_ref().and_then(|i| i.color)) } /// The model with each body carrying the colour, opacity and face colours it is shown with @@ -1344,7 +1344,11 @@ impl Document { .into_iter() .map(|(face, l)| solvecraft_kernel::FacePaint { face, color: l.rgb(), opacity: l.opacity as f32 }) .collect(); - let opacity = look.map(|l| l.opacity as f32).or_else(|| b.body.paint().map(|p| p.opacity)).unwrap_or(1.0); + let opacity = look + .map(|l| l.opacity as f32) + .or_else(|| b.body.paint().map(|p| p.opacity)) + .or_else(|| b.imported.as_ref().and_then(|i| i.opacity)) + .unwrap_or(1.0); let paint = solvecraft_kernel::Paint { opacity, faces }; if c != b.body.color() || b.body.paint() != Some(&paint) { b.body = b.body.clone().with_color(c).with_paint(Some(paint)); diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 70dac4de..ffd93785 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -27,11 +27,13 @@ pub struct ModelBody { pub(crate) names: crate::naming::NameCell, /// The component the body belongs to (set after each feature). pub component: u64, + /// The look it was imported with, carried through later features. + pub imported: Option>, } impl ModelBody { pub fn new(name: String, body: Body, feature: u64) -> Self { - ModelBody { name, body, feature, mesh: OnceLock::new(), key: 0, names: Default::default(), component: 0 } + ModelBody { name, body, feature, mesh: OnceLock::new(), key: 0, names: Default::default(), component: 0, imported: None } } /// Display mesh (chord tolerance 1/1000 of the body size), computed once. pub fn mesh(&self) -> Arc { @@ -236,6 +238,13 @@ impl ModelState { (key, &b.name).hash(&mut h); b.key = h.finish(); b.names = crate::naming::NameCell::new(Some(origin.clone())); + // An imported look outlives the operations on the body. + if b.body.paint().is_none() + && b.body.color().is_none() + && let Some(old) = before.bodies.iter().find(|x| x.name == b.name) + { + b.imported = crate::appearance::ImportedLook::of(old); + } } } } diff --git a/crates/engine/src/cmd/motion_tests.rs b/crates/engine/src/cmd/motion_tests.rs index dbf1f062..e91c900a 100644 --- a/crates/engine/src/cmd/motion_tests.rs +++ b/crates/engine/src/cmd/motion_tests.rs @@ -74,7 +74,8 @@ fn contact_sets_stop_a_driven_joint_at_touching() { #[test] fn motion_study_plays_and_exports() { let mut s = rail(); - let r = run(&mut s, "motion.study", json!({"name": "Push", "steps": 100, "keys": [{"joint": "Rail", "points": [[0, 10], [100, -5]]}]})); + let r = + run(&mut s, "motion.study", json!({"name": "Push", "steps": 100, "keys": [{"joint": "Rail", "points": [[0, 10], [100, -5]]}]})); assert_eq!(r["keys"], 1); run(&mut s, "motion.play", json!({"study": "Push", "step": 50})); assert!((b_x(&s) - 12.5).abs() < 1e-6, "{}", b_x(&s)); diff --git a/crates/engine/src/cmd/patterns_tests.rs b/crates/engine/src/cmd/patterns_tests.rs index 09eb681e..1d4e8381 100644 --- a/crates/engine/src/cmd/patterns_tests.rs +++ b/crates/engine/src/cmd/patterns_tests.rs @@ -136,9 +136,7 @@ fn patterns_of_a_blind_hole_drill_every_copy() { }; let r = patterned(("solid.pattern.rectangular", json!({"features": ["Hole1"], "dir1": [1, 0, 0], "count1": 3, "spacing1": "15 mm"}))); assert!((r - 3.0 * one).abs() < 1e-3 * one, "{r} vs 3 × {one}"); - let c = patterned(( - "solid.pattern.circular", - json!({"features": ["Hole1"], "axis": {"origin": [25, 12, 0], "dir": [0, 0, 1]}, "count": 2, "angle": 360}), - )); + let c = + patterned(("solid.pattern.circular", json!({"features": ["Hole1"], "axis": {"origin": [25, 12, 0], "dir": [0, 0, 1]}, "count": 2, "angle": 360}))); assert!((c - 2.0 * one).abs() < 1e-3 * one, "{c} vs 2 × {one}"); } diff --git a/crates/engine/src/cmd/sketch.rs b/crates/engine/src/cmd/sketch.rs index 5467800d..92195575 100644 --- a/crates/engine/src/cmd/sketch.rs +++ b/crates/engine/src/cmd/sketch.rs @@ -1076,24 +1076,16 @@ fn c_equal(s: &mut Session, p: &Value) -> Result { constrain(s, p, "sketch.constraint.equal", |sk| two_curves(sk, p, "sketch.constraint.equal").map(|(a, b)| ConstraintKind::Equal { a, b })) } fn c_parallel(s: &mut Session, p: &Value) -> Result { - constrain(s, p, "sketch.constraint.parallel", |sk| { - two_curves(sk, p, "sketch.constraint.parallel").map(|(a, b)| ConstraintKind::Parallel { a, b }) - }) + constrain(s, p, "sketch.constraint.parallel", |sk| two_curves(sk, p, "sketch.constraint.parallel").map(|(a, b)| ConstraintKind::Parallel { a, b })) } fn c_perpendicular(s: &mut Session, p: &Value) -> Result { - constrain(s, p, "sketch.constraint.perpendicular", |sk| { - two_curves(sk, p, "sketch.constraint.perpendicular").map(|(a, b)| ConstraintKind::Perpendicular { a, b }) - }) + constrain(s, p, "sketch.constraint.perpendicular", |sk| two_curves(sk, p, "sketch.constraint.perpendicular").map(|(a, b)| ConstraintKind::Perpendicular { a, b })) } fn c_concentric(s: &mut Session, p: &Value) -> Result { - constrain(s, p, "sketch.constraint.concentric", |sk| { - two_curves(sk, p, "sketch.constraint.concentric").map(|(a, b)| ConstraintKind::Concentric { a, b }) - }) + constrain(s, p, "sketch.constraint.concentric", |sk| two_curves(sk, p, "sketch.constraint.concentric").map(|(a, b)| ConstraintKind::Concentric { a, b })) } fn c_collinear(s: &mut Session, p: &Value) -> Result { - constrain(s, p, "sketch.constraint.collinear", |sk| { - two_curves(sk, p, "sketch.constraint.collinear").map(|(a, b)| ConstraintKind::Collinear { a, b }) - }) + constrain(s, p, "sketch.constraint.collinear", |sk| two_curves(sk, p, "sketch.constraint.collinear").map(|(a, b)| ConstraintKind::Collinear { a, b })) } fn c_midpoint(s: &mut Session, p: &Value) -> Result { let cmd = "sketch.constraint.midpoint"; diff --git a/crates/engine/src/cmd/sketch_constraints_tests.rs b/crates/engine/src/cmd/sketch_constraints_tests.rs index 9aaf25b6..8149ef00 100644 --- a/crates/engine/src/cmd/sketch_constraints_tests.rs +++ b/crates/engine/src/cmd/sketch_constraints_tests.rs @@ -54,8 +54,7 @@ fn curvature_polygon_arc_length_linear_diameter() { // A horizontal line ending where a control-point spline starts: G2 keeps the spline's // start straight (zero curvature) and tangent. let l = ids(&run(&mut s, "sketch.line", json!({"points": [[-10, 0], [0, 0]], "infer": true}))["curves"])[0].clone(); - let sp = - ids(&run(&mut s, "sketch.spline.control_point", json!({"points": [format!("{l}.end"), [5, 2], [10, 6], [15, 15]]}))["curves"])[0].clone(); + let sp = ids(&run(&mut s, "sketch.spline.control_point", json!({"points": [format!("{l}.end"), [5, 2], [10, 6], [15, 15]]}))["curves"])[0].clone(); run(&mut s, "sketch.constraint.curvature", json!({"a": l, "b": sp})); let sk = sketch(&s); let pts: Vec = sk.curves[sk.curve_index(&sp).unwrap()].kind.point_ids().iter().map(|i| sk.point(*i).unwrap()).collect(); diff --git a/crates/engine/src/cmd/sketch_modify_tests.rs b/crates/engine/src/cmd/sketch_modify_tests.rs index 51916014..4417fc74 100644 --- a/crates/engine/src/cmd/sketch_modify_tests.rs +++ b/crates/engine/src/cmd/sketch_modify_tests.rs @@ -348,9 +348,7 @@ fn degree_five_and_cut_free_form_profiles_extrude_exactly() { let vol = |s: &mut Session| run(s, "inspect.measure", json!({}))["total"]["volume_mm3"].as_f64().unwrap(); // A degree-5 control spline closed by a line: one B-spline side face. let mut s = new_sketch(); - let sp = - ids(&run(&mut s, "sketch.spline.control_point_5", json!({"points": [[0, 0], [4, 10], [10, 12], [16, 12], [22, 10], [26, 0]]}))["curves"])[0] - .clone(); + let sp = ids(&run(&mut s, "sketch.spline.control_point_5", json!({"points": [[0, 0], [4, 10], [10, 12], [16, 12], [22, 10], [26, 0]]}))["curves"])[0].clone(); run(&mut s, "sketch.line", json!({"points": [format!("{sp}.end"), format!("{sp}.start")]})); let a = profiles(&s)[0]; run(&mut s, "sketch.finish", json!({})); @@ -429,8 +427,7 @@ fn fillet_a_line_and_an_arc() { let mut s = new_sketch(); // A line into a quarter arc meeting at a sharp corner. let l = ids(&run(&mut s, "sketch.line", json!({"points": [[0, 0], [20, 0]]}))["curves"])[0].clone(); - let a = - ids(&run(&mut s, "sketch.arc.center_point", json!({"center": [30, 0], "start": format!("{l}.end"), "end": [30, -10]}))["curves"])[0].clone(); + let a = ids(&run(&mut s, "sketch.arc.center_point", json!({"center": [30, 0], "start": format!("{l}.end"), "end": [30, -10]}))["curves"])[0].clone(); let _ = a; let f = run(&mut s, "sketch.fillet", json!({"point": format!("{l}.end"), "radius": 2})); let arc = ids(&f["curves"])[0].clone(); diff --git a/crates/engine/src/cmd/surface.rs b/crates/engine/src/cmd/surface.rs index 23ff4ada..1ba7cdeb 100644 --- a/crates/engine/src/cmd/surface.rs +++ b/crates/engine/src/cmd/surface.rs @@ -146,7 +146,11 @@ mod tests { assert!((v - 240.0).abs() < 1e-9, "{v}"); // Stitch the patch of a box's open top back on. run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "corner": [20, 0, 0], "body_name": "Cup"})); - run(&mut s, "surface.patch", json!({"edges": [[25, 0, 10], [30, 5, 10], [25, 10, 10], [20, 5, 10]], "body": "Cup", "body_name": "Lid"})); + run( + &mut s, + "surface.patch", + json!({"edges": [[25, 0, 10], [30, 5, 10], [25, 10, 10], [20, 5, 10]], "body": "Cup", "body_name": "Lid"}), + ); assert!(s.world_state().body("Lid").unwrap().body.is_surface()); assert!(s.execute("surface.thicken", &json!({"bodies": ["Nope"], "thickness": 1})).is_err()); assert!(s.execute("surface.trim", &json!({"body": "Sheet"})).is_err()); diff --git a/crates/engine/src/cmd/surface_analysis.rs b/crates/engine/src/cmd/surface_analysis.rs index d917534d..20b88362 100644 --- a/crates/engine/src/cmd/surface_analysis.rs +++ b/crates/engine/src/cmd/surface_analysis.rs @@ -69,7 +69,9 @@ fn draft(s: &mut Session, p: &Value) -> Result { fn accessibility(s: &mut Session, p: &Value) -> Result { let dir = match p.get("direction") { - Some(v) => vec3(v).and_then(Vec3::normalized).ok_or_else(|| bad("inspect.accessibility", "`direction` must be a non-zero [x,y,z]"))?, + Some(v) => { + vec3(v).and_then(Vec3::normalized).ok_or_else(|| bad("inspect.accessibility", "`direction` must be a non-zero [x,y,z]"))? + } None => Vec3::Z, }; set(s, p, SurfaceAnalysis::Access { dir }) diff --git a/crates/engine/src/frames.rs b/crates/engine/src/frames.rs index a4b2de07..2c3bc4f3 100644 --- a/crates/engine/src/frames.rs +++ b/crates/engine/src/frames.rs @@ -61,7 +61,9 @@ fn table(id: &str) -> &'static [(&'static str, K)] { "sketch.intersect" => &[("refs", Picks)], "sketch.project_to_surface" | "sketch.isoparametric_curve" => &[("face", Pick), ("body", Body)], "sketch.intersection_curve" => &[("a", Picks), ("b", Picks)], - "inspect.curvature_comb" | "inspect.minimum_radius" | "inspect.isocurve" => &[("edges", Picks), ("faces", Picks)], + "inspect.curvature_comb" | "inspect.minimum_radius" | "inspect.isocurve" => { + &[("edges", Picks), ("faces", Picks)] + } "parts.insert" | "parts.fastener" => &[("at", Pick), ("point", Make), ("direction", Dir)], "appearance.assign" => &[("faces", Picks), ("bodies", Body), ("body", Body)], "solid.move" => &[("bodies", Body), ("translate", Dir), ("axis", Dir), ("origin", Make)], diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3d3ed5d1..0420342e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -299,7 +299,10 @@ fn hostile_params_never_panic() { ]; let mut internal = Vec::new(); for spec in command_specs() { - if matches!(spec.id, "doc.open" | "file.save" | "file.save_as" | "file.export" | "file.save_mesh" | "sketch.export_dxf") { + if matches!( + spec.id, + "doc.open" | "file.save" | "file.save_as" | "file.export" | "file.save_mesh" | "sketch.export_dxf" + ) { continue; // file system side effects are covered by their own tests } for v in hostile_values() { @@ -402,7 +405,11 @@ fn timeline_edits_reresolve_references() { assert_eq!(names, ["Base", "Bore", "Round"]); assert!(rel(volume(&mut s), expect(60.0, 25.0)) < 1e-3); // Redefine the fillet in place with a new radius; it keeps its name and position. - run(&mut s, "timeline.redefine", json!({"feature": "Round", "command": "solid.fillet", "params": {"edges": [[30, 0, 25]], "radius": 4}})); + run( + &mut s, + "timeline.redefine", + json!({"feature": "Round", "command": "solid.fillet", "params": {"edges": [[30, 0, 25]], "radius": 4}}), + ); let f4 = (16.0 - 16.0 * PI / 4.0) * 60.0; assert!(rel(volume(&mut s), 60.0 * 30.0 * 25.0 - f4 - PI * 9.0 * 25.0) < 1e-3); assert_eq!(s.doc.features.iter().map(|f| f.name.as_str()).collect::>(), ["Base", "Bore", "Round"]); @@ -1110,11 +1117,7 @@ fn face_extrude_follows_edits() { let mut s = Session::default(); run(&mut s, "solid.box", json!({"length": 40, "width": 30, "height": 20})); let before = volume(&mut s); - run( - &mut s, - "solid.revolve", - json!({"face": [40, 15, 10], "axis": {"origin": [40, 0, 30], "dir": [0, 1, 0]}, "angle": "90 deg", "operation": "new"}), - ); + run(&mut s, "solid.revolve", json!({"face": [40, 15, 10], "axis": {"origin": [40, 0, 30], "dir": [0, 1, 0]}, "angle": "90 deg", "operation": "new"})); let added = volume(&mut s) - before; let want = 30.0 * 20.0 * 20.0 * PI / 2.0; assert!(rel(added, want) < 1e-4, "{added} vs {want}"); @@ -1619,4 +1622,36 @@ fn hole_as_deep_as_the_plate() { assert!(r.is_ok(), "{i} {p}: {r:?}"); assert!(volume(&mut s) < before - 1.0, "{i} {p}"); } +/// Appearances outlive later features: a coloured face keeps its colour after a fillet on a +/// neighbouring edge; an imported STEP's body and face colours survive a hole. +#[test] +fn appearances_survive_later_features() { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 40, "width": 30, "height": 20, "body_name": "B"})); + run(&mut s, "appearance.assign", json!({"faces": [[20, 15, 20]], "color": "#ff0000"})); + run(&mut s, "solid.fillet", json!({"edges": [[40, 15, 0]], "radius": 3})); + let red = |s: &mut Session, body: &str| { + let l = run(s, "appearance.list", json!({ "body": body })); + l["body"]["faces"].as_array().into_iter().flatten().any(|f| f["appearance"]["color"] == "#ff0000") + }; + assert!(red(&mut s, "B"), "the top face is still red after the fillet"); + // Exported with a body colour and the red face, then opened: the colours come in as the + // body's own; a hole afterwards keeps them. + run(&mut s, "appearance.assign", json!({"bodies": ["B"], "color": "#2050c0"})); + let dir = std::env::temp_dir().join(format!("solvecraft-paint-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let step = dir.join("painted.step"); + run(&mut s, "file.export", json!({"path": step.to_string_lossy()})); + let mut t = Session::default(); + run(&mut t, "doc.open", json!({"path": step.to_string_lossy()})); + let body = t.model.state().bodies[0].name.clone(); + let before = t.doc.body_color(&t.model.state().bodies[0]); + assert!(before.is_some()); + run(&mut t, "solid.hole", json!({"position": [10, 10, 20], "diameter": 4})); + let st = t.model.state(); + let b = st.body(&body).unwrap(); + assert!(b.body.paint().is_none(), "the hole made a new kernel body"); + assert_eq!(t.doc.body_color(b), before, "the imported body colour carries on"); + assert!(red(&mut t, &body), "the imported red face carries on"); + let _ = std::fs::remove_dir_all(&dir); }