Fix: false "projected face changed shape" warnings after a rollback
Re-reading an unchanged projected face could walk its loops in another order or direction, or (from some meshes) return the same half of a split circle twice. The link was then rebuilt, warning and dropping constraints on it. Projected geometry now compares by position regardless of order and direction, and a duplicated half arc is turned into the missing half. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL
This commit is contained in:
1 parent
08cf223ced
commit
0271919d52
3 files changed
+110
-1
No files matched your search
@@ -147,7 +147,7 @@ pub fn resolve(doc: &Document, vals: &BTreeMap<String, Value>, st: &ModelState,
|
||||
if geom.is_empty() {
|
||||
return Err(DocError::Invalid(format!("the {} projects to nothing on this sketch plane", src.describe())));
|
||||
}
|
||||
Ok(Resolved { geom, source, moved })
|
||||
Ok(Resolved { geom: merge_split_circles(geom), source, moved })
|
||||
}
|
||||
|
||||
/// Re-resolve every link of a sketch (in place). Returns warnings for lost, moved or rebuilt
|
||||
@@ -257,6 +257,29 @@ pub(crate) fn nearest_face(m: &Mesh, p: Vec3) -> Option<(u32, Vec3, f64)> {
|
||||
best
|
||||
}
|
||||
|
||||
/// A full circle split by a seam comes out as two arcs between the same two points, but some
|
||||
/// meshes give the same half twice. That is a circle too: turn the duplicate into the other half
|
||||
/// so re-reading an unchanged face always gives the same two complementary arcs.
|
||||
fn merge_split_circles(geom: Vec<LinkGeom>) -> Vec<LinkGeom> {
|
||||
const TOL: f64 = 1e-6;
|
||||
let near = |p: Vec2, q: Vec2| p.dist(q) <= TOL * (1.0 + p.len().max(q.len()));
|
||||
let mut out = geom.clone();
|
||||
for i in 0..geom.len() {
|
||||
let Some(LinkGeom::Arc { c, a, b }) = geom.get(i).copied() else { continue };
|
||||
for j in i + 1..geom.len() {
|
||||
if let Some(LinkGeom::Arc { c: c2, a: a2, b: b2 }) = geom.get(j).copied()
|
||||
&& near(c, c2)
|
||||
&& near(a, a2)
|
||||
&& near(b, b2)
|
||||
&& let Some(slot) = out.get_mut(j)
|
||||
{
|
||||
*slot = LinkGeom::Arc { c, a: b, b: a };
|
||||
}
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------------------------
|
||||
// Projection
|
||||
|
||||
|
||||
@@ -841,3 +841,18 @@ fn feature_inputs_are_parameters() {
|
||||
let back = solvecraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
|
||||
assert_eq!(back.features[0].param_names, s.doc.features[0].param_names);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rolling_back_and_forward_leaves_no_projection_warnings() {
|
||||
let mut s = Session::default();
|
||||
s.run_script(&crate::sample::script()).unwrap();
|
||||
let warnings = |s: &Session| -> Vec<String> {
|
||||
s.doc.features.iter().filter_map(|f| s.model.result(f.id).and_then(|r| r.warning.clone()).map(|w| format!("{}: {w}", f.name))).collect()
|
||||
};
|
||||
assert!(warnings(&s).is_empty(), "{:?}", warnings(&s));
|
||||
for pos in 0..s.doc.features.len() {
|
||||
run(&mut s, "timeline.rollTo", json!({ "position": pos }));
|
||||
run(&mut s, "timeline.rollTo", json!({}));
|
||||
assert!(warnings(&s).is_empty(), "after rolling to {pos}: {:?}", warnings(&s));
|
||||
}
|
||||
}
|
||||
@@ -215,6 +215,54 @@ fn same_shape(a: &LCurve, b: &LCurve) -> bool {
|
||||
)
|
||||
}
|
||||
|
||||
/// Do two curve sets describe the same geometry, regardless of the order the curves come in or
|
||||
/// the direction a loop was walked? Lines match with either end first; circles by centre and
|
||||
/// radius; arcs by centre and their (start, end) pair; conics by all their points and rho.
|
||||
fn same_geometry(a: &[(LCurve, Vec<Vec2>)], b: &[(LCurve, Vec<Vec2>)]) -> bool {
|
||||
const TOL: f64 = 1e-6;
|
||||
let near = |p: Vec2, q: Vec2| p.dist(q) <= TOL * (1.0 + p.len().max(q.len()));
|
||||
let close = |x: f64, y: f64| (x - y).abs() <= TOL * (1.0 + x.abs().max(y.abs()));
|
||||
let matches = |(ca, pa): &(LCurve, Vec<Vec2>), (cb, pb): &(LCurve, Vec<Vec2>)| -> bool {
|
||||
if pa.len() != pb.len() {
|
||||
return false;
|
||||
}
|
||||
let all = || pa.iter().zip(pb).all(|(p, q)| near(*p, *q));
|
||||
match (ca, cb) {
|
||||
(LCurve::Line(..), LCurve::Line(..)) => {
|
||||
all() || matches!((pa.as_slice(), pb.as_slice()), ([a0, a1], [b0, b1]) if near(*a0, *b1) && near(*a1, *b0))
|
||||
}
|
||||
(LCurve::Circle(_, ra), LCurve::Circle(_, rb)) => all() && close(*ra, *rb),
|
||||
(LCurve::Arc(..), LCurve::Arc(..)) => all(),
|
||||
(LCurve::Conic(.., ra), LCurve::Conic(.., rb)) => all() && close(*ra, *rb),
|
||||
_ => false,
|
||||
}
|
||||
};
|
||||
if a.len() != b.len() {
|
||||
return false;
|
||||
}
|
||||
let mut used = vec![false; b.len()];
|
||||
a.iter().all(|x| {
|
||||
let hit = b.iter().enumerate().find(|(j, y)| !used.get(*j).copied().unwrap_or(true) && matches(x, y)).map(|(j, _)| j);
|
||||
match hit.and_then(|j| used.get_mut(j)) {
|
||||
Some(u) => {
|
||||
*u = true;
|
||||
true
|
||||
}
|
||||
None => false,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// The point indices a layout curve uses, in order.
|
||||
fn curve_points(c: &LCurve) -> Vec<usize> {
|
||||
match *c {
|
||||
LCurve::Line(a, b) => vec![a, b],
|
||||
LCurve::Circle(c, _) => vec![c],
|
||||
LCurve::Arc(c, a, b) => vec![c, a, b],
|
||||
LCurve::Conic(a, b, x, _) => vec![a, b, x],
|
||||
}
|
||||
}
|
||||
|
||||
impl Sketch {
|
||||
pub fn link(&self, id: &str) -> Option<&Link> {
|
||||
self.links.iter().find(|l| l.id == id)
|
||||
@@ -395,6 +443,29 @@ impl Sketch {
|
||||
}
|
||||
return Ok(false);
|
||||
}
|
||||
// The same geometry walked in another order or direction (e.g. a face loop re-read after
|
||||
// a rollback) is not a change: keep the entities, and the constraints on them.
|
||||
if let Some(cur) = &cur {
|
||||
let pos = |pi: usize| self.points.get(pi).map(|p| p.pos);
|
||||
let old: Option<Vec<(LCurve, Vec<Vec2>)>> = cur
|
||||
.iter()
|
||||
.map(|c| curve_points(c).into_iter().map(|li| points.get(li).and_then(|pi| pos(*pi))).collect::<Option<Vec<_>>>().map(|ps| (*c, ps)))
|
||||
.collect();
|
||||
let fresh: Option<Vec<(LCurve, Vec<Vec2>)>> = lay
|
||||
.curves
|
||||
.iter()
|
||||
.map(|c| curve_points(c).into_iter().map(|li| lay.pts.get(li).copied()).collect::<Option<Vec<_>>>().map(|ps| (*c, ps)))
|
||||
.collect();
|
||||
let lone_old: Vec<Vec2> = points.iter().filter_map(|pi| pos(*pi)).collect();
|
||||
if let (Some(old), Some(fresh)) = (old, fresh)
|
||||
&& lay.lone.is_empty()
|
||||
&& points.len() == lay.pts.len()
|
||||
&& lone_old.len() == points.len()
|
||||
&& same_geometry(&old, &fresh)
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
}
|
||||
// Rebuild: drop the old entities (constraints on them go too) and make new ones.
|
||||
self.drop_link_entities(id);
|
||||
self.build_link(id, &lay)?;
|
||||
|
||||
Reference in new issue
Block a user