diff --git a/apps/solidcraft-cli/tests/action_matrix.rs b/apps/solidcraft-cli/tests/action_matrix.rs index 9e634be1bf..6a21e2dcdb 100644 --- a/apps/solidcraft-cli/tests/action_matrix.rs +++ b/apps/solidcraft-cli/tests/action_matrix.rs @@ -1665,7 +1665,6 @@ fn a_tangent_plane_on_a_drill_tip() { } #[test] -#[ignore = "fails until #568 is fixed"] fn a_plane_on_a_patterned_counterbore_after_a_thickness_change() { let mut c = vec![json!({"command": "parameters.change", "params": {"name": "thick", "expression": "10 mm"}})]; c.extend(rectangle(json!("XY"), [0., 0.], [80., 30.])); diff --git a/crates/doc/src/clipboard.rs b/crates/doc/src/clipboard.rs index 5581892cb3..ee2d2b616a 100644 --- a/crates/doc/src/clipboard.rs +++ b/crates/doc/src/clipboard.rs @@ -95,6 +95,11 @@ fn remap_geo( } } GeoRef::Face { name: Some(n), .. } | GeoRef::Edge { name: Some(n), .. } => *n = remap_topology(n, ids, bodies), + GeoRef::Vertex { edges: Some(names), .. } => { + for n in names { + *n = remap_topology(n, ids, bodies); + } + } GeoRef::ConstructPoint { name } | GeoRef::Axis { name } => mapped_name(name, names), GeoRef::Plane { plane } => remap_plane(plane, ids, names, bodies, depth + 1)?, _ => {} diff --git a/crates/doc/src/construct.rs b/crates/doc/src/construct.rs index c6abe88673..6c441bf3e9 100644 --- a/crates/doc/src/construct.rs +++ b/crates/doc/src/construct.rs @@ -2,8 +2,9 @@ //! points and sketch entities, resolved on each evaluation, and the axes, points and planes //! built from them (the CONSTRUCT panel). //! -//! Face and edge references carry the persistent name of what was picked (see `naming`), so -//! they follow the model when earlier features change; the picked point is the fallback. +//! Face and edge references carry the persistent name of what was picked (see `naming`), and a +//! vertex the names of the edges that meet there, so they follow the model when earlier features +//! change; the picked point is the fallback. use std::collections::BTreeMap; @@ -23,8 +24,13 @@ pub enum GeoRef { Point { p: Vec3 }, /// The design origin. Origin, - /// A body vertex (the one nearest `at`). - Vertex { at: Vec3 }, + /// A body vertex: where the edges named `edges` meet (the end they share), else the one + /// nearest `at`. + Vertex { + at: Vec3, + #[serde(default, skip_serializing_if = "Option::is_none")] + edges: Option>, + }, /// A body edge: the one named `name`, else the one nearest `at`. Edge { at: Vec3, @@ -170,6 +176,33 @@ fn edge_points(st: &ModelState, at: Vec3, name: &Option) -> Result Option { + if names.len() < 2 || names.len() > MAX_VERTEX_EDGES { + return None; + } + let ends: Vec<[Vec3; 2]> = names + .iter() + .map(|n| { + // The edge with that name itself, never the one nearest the old pick. + let on = bodies(st).find_map(|b| crate::naming::find_edge(b, n, at).ok())?.0; + let pts = edge_points(st, on, &None).ok()?; + Some([*pts.first()?, *pts.last()?]) + }) + .collect::>()?; + let (first, rest) = ends.split_first()?; + let tol = |p: Vec3| 1e-6 * (1.0 + p.len()); + first + .iter() + .copied() + .filter(|p| p.is_finite() && rest.iter().all(|e| e.iter().any(|q| q.dist(*p) <= tol(*p)))) + .min_by(|p, q| p.dist(at).total_cmp(&q.dist(at))) +} + /// A polyline that is straight: its line. fn as_line(pts: &[Vec3]) -> Option<(Vec3, Vec3)> { let (a, b) = (*pts.first()?, *pts.last()?); @@ -228,7 +261,10 @@ pub(crate) fn point(ctx: &Ctx, g: &GeoRef) -> Result { match g { GeoRef::Point { p } => Ok(*p), GeoRef::Origin => Ok(Vec3::ZERO), - GeoRef::Vertex { at } => { + GeoRef::Vertex { at, edges } => { + if let Some(v) = edges.as_deref().and_then(|names| named_vertex(ctx.st, *at, names)) { + return Ok(v); + } let mut best: Option<(f64, Vec3)> = None; for b in bodies(ctx.st) { let tol = (b.body.size() * 1e-3).max(1e-3); @@ -553,13 +589,14 @@ impl GeoRef { pub fn map_points(&mut self, pt: &mut dyn FnMut(&mut Vec3), plane: &mut dyn FnMut(&mut PlaneRef)) { match self { GeoRef::Point { p } => pt(p), - GeoRef::Vertex { at } | GeoRef::Edge { at, .. } | GeoRef::Face { at, .. } => pt(at), + GeoRef::Vertex { at, .. } | GeoRef::Edge { at, .. } | GeoRef::Face { at, .. } => pt(at), GeoRef::Plane { plane: p } => plane(p), _ => {} } } - /// Picked faces and edges get their persistent names (and only those still missing one). + /// Picked faces and edges get their persistent names, and picked vertices the names of the + /// edges that meet there (only those still missing them). pub fn name_picks(&mut self, st: &ModelState) { match self { GeoRef::Face { at, name: name @ None } => { @@ -574,6 +611,24 @@ impl GeoRef { } } } + GeoRef::Vertex { at, edges: edges @ None } => { + for b in bodies(st) { + let tol = (b.body.size() * 1e-3).max(1e-3); + let m = b.mesh(); + let ends_here = |e: &Vec| [e.first(), e.last()].into_iter().flatten().any(|q| q.dist(*at) <= tol); + let mut meet: Vec = + m.edges.iter().zip(crate::naming::edge_names(b)).filter(|(e, n)| !n.is_empty() && ends_here(e)).map(|(_, n)| n).collect(); + if meet.is_empty() { + continue; + } + meet.sort(); + meet.dedup(); + if (2..=MAX_VERTEX_EDGES).contains(&meet.len()) { + *edges = Some(meet); + } + break; + } + } GeoRef::Plane { plane } => plane.name_picks(st), _ => {} } diff --git a/crates/doc/src/format/pending/vertex-edge-names.rs b/crates/doc/src/format/pending/vertex-edge-names.rs new file mode 100644 index 0000000000..536f4944b7 --- /dev/null +++ b/crates/doc/src/format/pending/vertex-edge-names.rs @@ -0,0 +1,4 @@ +//! A construction reference to a body vertex saves the names of the edges that meet there +//! (`edges` beside `at`), so a plane, axis or point on a vertex follows it when an earlier change +//! moves it. Older designs need no transformation: a vertex reference without `edges` is the +//! vertex nearest its point, as before. 0.15.x asks for an update instead of reading `edges`. diff --git a/crates/engine/src/cmd/construct.rs b/crates/engine/src/cmd/construct.rs index 4c42c43f19..12ed05e372 100644 --- a/crates/engine/src/cmd/construct.rs +++ b/crates/engine/src/cmd/construct.rs @@ -99,7 +99,7 @@ fn geo(s: &Session, v: Option<&Value>, cmd: &str, what: &str) -> Result } else if let Some(at) = p3("edge") { GeoRef::Edge { at, name: None } } else if let Some(at) = p3("vertex") { - GeoRef::Vertex { at } + GeoRef::Vertex { at, edges: None } } else if let Some(sk) = v.get("sketch") { let sketch = sketch_id(s, Some(sk), cmd, what)?; match (str_(v, "point"), str_(v, "curve")) { diff --git a/crates/engine/src/construct_tests.rs b/crates/engine/src/construct_tests.rs index 373fe2b817..157324a053 100644 --- a/crates/engine/src/construct_tests.rs +++ b/crates/engine/src/construct_tests.rs @@ -282,6 +282,30 @@ fn construction_follows_the_model() { assert!(on_line(o, d, Vec3::new(40.0, 30.0, 50.0)) && on_line(o, d, Vec3::ZERO)); } +/// #568: a plane through three vertices follows them when a parameter moves them. Two of them +/// sit on the box's end at x = 40; the box grows to 100 mm, so the vertices nearest the old +/// picks are the ones at x = 0, on a line with the first: found by the names of the edges that +/// meet there, they are the moved ones. +#[test] +fn a_plane_through_three_vertices_follows_them() { + let mut s = Session::default(); + run(&mut s, "parameters.add", json!({"name": "len", "expression": "40 mm"})); + run(&mut s, "solid.box", json!({"length": "len", "width": 30, "height": 20, "body_name": "Box"})); + run( + &mut s, + "construct.plane.three_points", + json!({"a": {"vertex": [0, 0, 20]}, "b": {"vertex": [40, 0, 20]}, "c": {"vertex": [40, 30, 0]}, "name": "T"}), + ); + run(&mut s, "parameters.change", json!({"name": "len", "expression": "100 mm"})); + let id = s.doc.features.iter().find(|f| f.name == "T").map(|f| f.id).unwrap_or_else(|| panic!("feature T")); + let error = s.model.result(id).and_then(|r| r.error.clone()); + assert_eq!(error, None); + let t = plane(&s, "T"); + for p in [Vec3::new(0.0, 0.0, 20.0), Vec3::new(100.0, 0.0, 20.0), Vec3::new(100.0, 30.0, 0.0)] { + assert!(on_plane(&t, p), "{p:?} is off {t:?}"); + } +} + /// In a component moved 50 mm along X, picks given in the world land on its geometry, and the /// axis and point move with the occurrence. #[test] diff --git a/docs/action-matrix.md b/docs/action-matrix.md index b15c385527..543fdec224 100644 --- a/docs/action-matrix.md +++ b/docs/action-matrix.md @@ -283,10 +283,10 @@ make a legacy face sketch there). | 56 | #563 | Draft and tapered Extrude drop the faces' sources | `face.extrude` 14, `face.offset_plane` 14, `face.press_pull` 14, `face.sketch` 14 | 3 | oracle/05-taper-extrude `Frustum/F2:start` | | 48 | #564 | The rim a Shell leaves has no source | `face.extrude` 12, `face.offset_plane` 12, `face.press_pull` 12, `face.sketch` 12 | 8 | demo/pi-case `Body1/F2:top` | | 18 | #567 | Axis or tangent plane on a drill tip: no face at NaN | `curved.axis` 9, `curved.tangent_plane` 9 | 3 | oracle/18-hole-features `Plate/F5:face:15` | -| 9 | #568 | Construction geometry on model edges, faces and vertices is lost when a parameter moves it | `vertex.plane_three_points+edit` 5, `edge.normal_plane+edit` 2, `face.midplane+edit` 1, `face.shell+edit` 1 | 4 | demo/enclosure_steps `Enclosure/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l` | | 9 | | Axis through a face the kernel sees as a cylinder that the command doesn't (sensor enclosure slot ends; not filed yet) | `curved.axis` 9 | 1 | examples/sensor_enclosure `Lid/F15:blend:F14:side:l2\|F14:side:l3` | | 7 | | Other failures in the edit round: the hole cut and blend sizes above, and an extrude from the M0 box's top face whose profile isn't found again (not filed yet) | `face.hole+edit` 3, `face.extrude+edit` 1, `face.press_pull+edit` 1, `edge.chamfer+edit` 1 | 4 | corpus/m0-box-fillet-hole `Body1/F2:top` | | 2 | | A helix edge projected along its axis is a point (expected) | `edge.project` 2 | 1 | oracle/50-sweep-tight-bend `Bend/F3:side:c1#1\|F3:side:c1#2~2` | +| 1 | #566 | Shell after the edit round: a corner whose faces don't meet the same way after the move | `face.shell+edit` 1 | 1 | demo/pi-case `Body1/F4:inner:F2:start#[…]#[F6:side:c2@F7.2]` | Where the face history stands (`crates/kernel/src/history.rs`, [face attachments](face-attachments.md)): a flat face takes an attached sketch when the body carries an exact source certificate for it diff --git a/tests/action_matrix/baseline.json b/tests/action_matrix/baseline.json index 15b924f143..51d3af3c95 100644 --- a/tests/action_matrix/baseline.json +++ b/tests/action_matrix/baseline.json @@ -1704,7 +1704,7 @@ "Enclosure/v[F2:top & F4:inner:F2:side:l3 & F4:inner:F3:blend:F2:side:l2|F2:side:l3]": "vertex.plane_three_points vertex.point", "Enclosure/v[F2:top & F4:inner:F2:side:l3 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points vertex.point", "Enclosure/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l1|F2:side:l4]": "vertex.plane_three_points vertex.point", - "Enclosure/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points !vertex.plane_three_points+edit vertex.point", + "Enclosure/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points vertex.plane_three_points+edit vertex.point", "Enclosure/v[F4:inner:F2:side:l1 & F4:inner:F2:start & F4:inner:F3:blend:F2:side:l1|F2:side:l2]": "vertex.plane_three_points vertex.point", "Enclosure/v[F4:inner:F2:side:l1 & F4:inner:F2:start & F4:inner:F3:blend:F2:side:l1|F2:side:l4]": "vertex.plane_three_points vertex.point", "Enclosure/v[F4:inner:F2:side:l2 & F4:inner:F2:start & F4:inner:F3:blend:F2:side:l1|F2:side:l2]": "vertex.plane_three_points vertex.point", @@ -2082,7 +2082,7 @@ "Body1/v[F11:side:l5 & F11:side:l8 & F2:side:l2]": "vertex.plane_three_points vertex.point", "Body1/v[F11:side:l5 & F11:side:l8 & F4:inner:F2:side:l2]": "vertex.plane_three_points vertex.point", "Body1/v[F11:side:l6 & F11:side:l7 & F2:side:l2]": "vertex.plane_three_points vertex.point", - "Body1/v[F11:side:l6 & F11:side:l7 & F4:inner:F2:side:l2]": "vertex.plane_three_points !vertex.plane_three_points+edit vertex.point", + "Body1/v[F11:side:l6 & F11:side:l7 & F4:inner:F2:side:l2]": "vertex.plane_three_points vertex.plane_three_points+edit vertex.point", "Body1/v[F11:side:l7 & F11:side:l8 & F2:side:l2]": "vertex.plane_three_points vertex.point", "Body1/v[F11:side:l7 & F11:side:l8 & F4:inner:F2:side:l2]": "vertex.plane_three_points vertex.point", "Body1/v[F13:side:l5 & F13:side:l6 & F2:start]": "vertex.plane_three_points vertex.point", @@ -2158,7 +2158,7 @@ "Body1/v[F2:top & F4:inner:F2:side:l2 & F4:inner:F3:blend:F2:side:l1|F2:side:l2]": "vertex.plane_three_points vertex.point", "Body1/v[F2:top & F4:inner:F2:side:l2 & F4:inner:F3:blend:F2:side:l2|F2:side:l3]": "vertex.plane_three_points vertex.point", "Body1/v[F2:top & F4:inner:F2:side:l3 & F4:inner:F3:blend:F2:side:l2|F2:side:l3]": "vertex.plane_three_points vertex.point", - "Body1/v[F2:top & F4:inner:F2:side:l3 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points !vertex.plane_three_points+edit vertex.point", + "Body1/v[F2:top & F4:inner:F2:side:l3 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points vertex.plane_three_points+edit vertex.point", "Body1/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l1|F2:side:l4]": "vertex.plane_three_points vertex.point vertex.point+edit", "Body1/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l3|F2:side:l4]": "vertex.plane_three_points vertex.point", "Body1/v[F4:inner:F2:side:l1 & F4:inner:F2:start#[F4:inner:F2:side:l1,F4:inner:F2:side:l2,F4:inner:F2:side:l3,F4:inner:F2:side:l4,F4:inner:F3:blend:F2:side:l1|F2:side:l2,F4:inner:F3:blend:F2:side:l1|F2:side:l4,F4:inner:F3:blend:F2:side:l2|F2:side:l3,F4:inner:F3:blend:F2:side:l3|F2:side:l4,F6:side:c1]#[F4:inner:F2:side:l1,F4:inner:F2:side:l2,F4:inner:F2:side:l3,F4:inner:F2:side:l4,F4:inner:F3:blend:F2:side:l1|F2:side:l2,F4:inner:F3:blend:F2:side:l1|F2:side:l4,F4:inner:F3:blend:F2:side:l2|F2:side:l3,F4:inner:F3:blend:F2:side:l3|F2:side:l4,F6:side:c1,F6:side:c1@F7.1,F6:side:c1@F7.2,F6:side:c1@F7.3] & F4:inner:F3:blend:F2:side:l1|F2:side:l2]": "vertex.plane_three_points vertex.point", @@ -3591,7 +3591,7 @@ "Lid/F14:side:l4|F15:blend:F14:side:l1|F14:side:l4": "edge.axis edge.axis+edit edge.normal_plane edge.project", "Lid/F14:side:l4|F15:blend:F14:side:l3|F14:side:l4": "edge.axis edge.normal_plane edge.project", "Lid/F14:side:l4|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Lid/F14:start": "face.angle_plane face.angle_plane+edit !face.draft_neutral !face.extrude !face.hole face.midplane face.offset_plane !face.press_pull face.shell !face.shell+edit face.sketch face.sketch+edit", + "Lid/F14:start": "face.angle_plane face.angle_plane+edit !face.draft_neutral !face.extrude !face.hole face.midplane face.offset_plane !face.press_pull face.shell face.shell+edit face.sketch face.sketch+edit", "Lid/F14:start|F18:side:a17": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Lid/F14:start|F18:side:a18": "edge.chamfer edge.chamfer+edit edge.fillet edge.normal_plane edge.project", "Lid/F14:start|F18:side:a19": "edge.chamfer edge.fillet edge.normal_plane edge.project edge.project+edit", @@ -3758,7 +3758,7 @@ "Lid/v[F18:side:a18 & F18:side:l19 & F18:top]": "vertex.plane_three_points vertex.point", "Lid/v[F18:side:a19 & F18:side:l19 & F18:top]": "vertex.plane_three_points vertex.point", "Lid/v[F18:side:a19 & F18:side:l20 & F18:top]": "vertex.plane_three_points vertex.point", - "Lid/v[F18:side:a20 & F18:side:l17 & F18:top]": "vertex.plane_three_points !vertex.plane_three_points+edit vertex.point", + "Lid/v[F18:side:a20 & F18:side:l17 & F18:top]": "vertex.plane_three_points vertex.plane_three_points+edit vertex.point", "Lid/v[F18:side:a20 & F18:side:l20 & F18:top]": "vertex.plane_three_points vertex.point", "design": "load" },