mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-09-26 10:21:00 +00:00
Mate conflicts: name what silently wins, don't call it over-constraint
Two enabled mates driving the same body is not an error today — the later one just wins, and the earlier mate looks ignored with nothing said. A cycle in the mate graph is worse: composition still produces a result, but an arbitrary, order-dependent one. recompute() now fills a mate_conflicts vector of (feature index, reason) before the geometry pass, so it survives a throw further down. It catches a second mate on the same target body, a mate positioning a body against itself, and a cycle, via an iterative three-colour DFS over the body graph. Broken mates are skipped silently — apply_mate() already errors on those. Deliberately non-fatal: recompute() still returns true and error stays empty. Deliberately not "over-constraint" — that word promises DOF analysis from a solver this kernel does not have. Port of snaporca ec4ffeb979. Kernel half of snaporca-bioq. Suite: 2213 assertions / 160 cases green on this fork too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3eec65f2bd
commit
d584d66003
@@ -1765,6 +1765,7 @@ void CadDocument::clear()
|
||||
display_body_meshes.clear();
|
||||
display_tri_face.clear();
|
||||
error.clear();
|
||||
mate_conflicts.clear();
|
||||
// A cleared document is a fresh start with no history.
|
||||
m_undo.clear();
|
||||
m_redo.clear();
|
||||
@@ -2979,6 +2980,112 @@ void CadDocument::apply_project(const std::vector<CadBody>& bodies, CadFeature&
|
||||
if (f.entities.empty()) throw std::runtime_error("project: produced no entities");
|
||||
}
|
||||
|
||||
void CadDocument::detect_mate_conflicts()
|
||||
{
|
||||
mate_conflicts.clear();
|
||||
|
||||
const int n = int(features.size());
|
||||
std::map<int, int> first_driver; // body -> feature index of first mate that drives it
|
||||
std::map<int, std::vector<int>> graph; // dst -> list of src (body indices)
|
||||
|
||||
for (int fi = 0; fi < n; ++fi) {
|
||||
const CadFeature& f = features[fi];
|
||||
if (f.type != CadFeatureType::Mate || !f.enabled) continue;
|
||||
|
||||
if (f.mate_cs_a < 0 || f.mate_cs_a >= n) continue;
|
||||
if (f.mate_cs_b < 0 || f.mate_cs_b >= n) continue;
|
||||
const CadFeature& fa = features[f.mate_cs_a];
|
||||
const CadFeature& fb = features[f.mate_cs_b];
|
||||
if (fa.type != CadFeatureType::CoordSys || !fa.enabled) continue;
|
||||
if (fb.type != CadFeatureType::CoordSys || !fb.enabled) continue;
|
||||
|
||||
const int src = fa.coordsys_body;
|
||||
const int dst = fb.coordsys_body;
|
||||
if (dst < 0) continue; // apply_mate already reports this one
|
||||
|
||||
// (a) duplicate target: a second mate driving the same body
|
||||
auto it = first_driver.find(dst);
|
||||
if (it != first_driver.end()) {
|
||||
int first_fi = it->second;
|
||||
std::string first_name = features[first_fi].name.empty() ? "Mate" : features[first_fi].name;
|
||||
std::string this_name = f.name.empty() ? "Mate" : f.name;
|
||||
mate_conflicts.push_back({fi,
|
||||
"Body " + std::to_string(dst + 1) + " is already positioned by '" +
|
||||
first_name + "' (feature " + std::to_string(first_fi) +
|
||||
") — '" + this_name + "' overrides it; suppress one"});
|
||||
} else {
|
||||
first_driver[dst] = fi;
|
||||
}
|
||||
|
||||
// (b) self-mate or cycle detection
|
||||
if (src >= 0 && dst >= 0) {
|
||||
if (src == dst) {
|
||||
mate_conflicts.push_back({fi,
|
||||
"this mate positions Body " + std::to_string(dst + 1) + " against itself"});
|
||||
} else {
|
||||
graph[dst].push_back(src);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// DFS cycle detection on the graph built above.
|
||||
// ponytail: iterative DFS avoids recursion depth issues on long chains.
|
||||
// Colour: 0 = unvisited, 1 = in-progress (grey), 2 = done (black).
|
||||
std::map<int, int> colour;
|
||||
struct Frame { int node; size_t next; };
|
||||
std::vector<Frame> stack;
|
||||
|
||||
for (const auto& [start, _] : graph) {
|
||||
if (colour[start] == 2) continue;
|
||||
stack.clear();
|
||||
stack.push_back({start, 0});
|
||||
colour[start] = 1;
|
||||
while (!stack.empty()) {
|
||||
Frame& top = stack.back();
|
||||
auto git = graph.find(top.node);
|
||||
if (git == graph.end() || top.next >= git->second.size()) {
|
||||
colour[top.node] = 2;
|
||||
stack.pop_back();
|
||||
continue;
|
||||
}
|
||||
int child = git->second[top.next++];
|
||||
if (colour[child] == 1) {
|
||||
// Back edge found — find the mate whose (dst==top.node, src==child).
|
||||
for (int fi = 0; fi < n; ++fi) {
|
||||
const CadFeature& f = features[fi];
|
||||
if (f.type != CadFeatureType::Mate || !f.enabled) continue;
|
||||
if (f.mate_cs_a < 0 || f.mate_cs_a >= n) continue;
|
||||
if (f.mate_cs_b < 0 || f.mate_cs_b >= n) continue;
|
||||
const CadFeature& fa2 = features[f.mate_cs_a];
|
||||
const CadFeature& fb2 = features[f.mate_cs_b];
|
||||
if (fa2.type != CadFeatureType::CoordSys || !fa2.enabled) continue;
|
||||
if (fb2.type != CadFeatureType::CoordSys || !fb2.enabled) continue;
|
||||
int sd = fb2.coordsys_body;
|
||||
int ss = fa2.coordsys_body;
|
||||
if (sd == top.node && ss == child) {
|
||||
mate_conflicts.push_back({fi,
|
||||
// Worded for ANY cycle length: "leads back" is true transitively,
|
||||
// where "depends back on" would be a lie for a 3+ body chain.
|
||||
"circular mate chain: Body " + std::to_string(top.node + 1) +
|
||||
" depends on Body " + std::to_string(child + 1) +
|
||||
", which leads back to Body " + std::to_string(top.node + 1) +
|
||||
" — the result depends on feature order"});
|
||||
break;
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (colour[child] == 0) {
|
||||
colour[child] = 1;
|
||||
stack.push_back({child, 0});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Deliberately NOT in scope: computing the numeric disagreement between two mates
|
||||
// ("Mate3 puts it at X=10, Mate7 at X=15"). That needs speculative per-mate evaluation.
|
||||
}
|
||||
|
||||
void CadDocument::apply_mate(std::vector<CadBody>& bodies, const CadFeature& f) const
|
||||
{
|
||||
const int nc = int(features.size());
|
||||
@@ -3221,6 +3328,7 @@ void CadDocument::route_feature(std::vector<CadBody>& bodies, const CadFeature&
|
||||
bool CadDocument::recompute()
|
||||
{
|
||||
error.clear();
|
||||
detect_mate_conflicts();
|
||||
std::vector<CadBody> built;
|
||||
try {
|
||||
// Parametric pass: evaluate document variables, then each feature's expression bindings,
|
||||
|
||||
@@ -425,6 +425,13 @@ public:
|
||||
std::vector<int> display_tri_body; // per-triangle source body index (into bodies)
|
||||
std::string error; // last recompute error ("" = ok)
|
||||
|
||||
// Mate diagnostics, refilled by every recompute(). Non-fatal by design: the
|
||||
// document still evaluates — this only names what the user should look at.
|
||||
// .first = index of the offending Mate feature, .second = human-readable reason.
|
||||
// NOT "over-constraint" — this kernel has no solver, so there is no DOF analysis
|
||||
// behind these; they are graph facts about which mate drives which body.
|
||||
std::vector<std::pair<int, std::string>> mate_conflicts;
|
||||
|
||||
// Modeling origin: the world point the default XY/XZ/YZ planes pass through. The GUI sets this
|
||||
// to the bed centre so sketches/datums land in the middle of the bed (not the bed corner =
|
||||
// world 0). Not serialized — the GUI re-applies it from the live bed on every tab show.
|
||||
@@ -680,6 +687,7 @@ private:
|
||||
void apply_project(const std::vector<CadBody>& bodies, CadFeature& f) const;
|
||||
static DatumCoordSys datum_frame(const std::vector<CadBody>& bodies, const CadFeature& f);
|
||||
void apply_mate(std::vector<CadBody>& bodies, const CadFeature& f) const;
|
||||
void detect_mate_conflicts(); // refills mate_conflicts from the feature list alone
|
||||
|
||||
// Undo/redo stacks of recipe snapshots. checkpoint() pushes onto m_undo and clears
|
||||
// m_redo; undo()/redo() shuffle the current state between them. Capped so a long
|
||||
|
||||
@@ -5585,6 +5585,148 @@ TEST_CASE("mate error: disabled connector", "[CadDocument][mate]")
|
||||
REQUIRE_CONTAINS(doc.error, "not a valid CoordSys");
|
||||
}
|
||||
|
||||
TEST_CASE("mate conflicts: a clean assembly reports none", "[CadDocument][mate]")
|
||||
{
|
||||
using Catch::Matchers::WithinAbs;
|
||||
|
||||
CadDocument doc;
|
||||
int sk_box = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "Box");
|
||||
doc.add_extrude(sk_box, 5.0, false, BooleanMode::New, "BoxExt");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int cs_fixed = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(5, 5, 5), "CS_Fixed");
|
||||
doc.features[cs_fixed].coordsys_body = 0;
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int sk_cyl = doc.add_sketch(SketchShape::Circle, SketchPlane::XY(), 0, 0, 3, "Cyl");
|
||||
doc.add_extrude(sk_cyl, 10.0, false, BooleanMode::New, "CylExt");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.bodies.size() == 2);
|
||||
|
||||
int cs_moving = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 5), "CS_Moving");
|
||||
doc.features[cs_moving].coordsys_body = 1;
|
||||
REQUIRE(doc.recompute());
|
||||
|
||||
int mi = doc.add_mate(0, cs_fixed, cs_moving, 0.0, 0.0, false, "Mate1");
|
||||
REQUIRE(mi >= 0);
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
REQUIRE(doc.mate_conflicts.empty());
|
||||
|
||||
GProp_GProps props;
|
||||
BRepGProp::VolumeProperties(doc.bodies[1].shape, props);
|
||||
gp_Pnt com = props.CentreOfMass();
|
||||
REQUIRE_THAT(double(com.X()), WithinAbs(5.0, 1e-4));
|
||||
REQUIRE_THAT(double(com.Y()), WithinAbs(5.0, 1e-4));
|
||||
REQUIRE_THAT(double(com.Z()), WithinAbs(5.0, 1e-4));
|
||||
}
|
||||
|
||||
TEST_CASE("mate conflicts: two mates driving the same body", "[CadDocument][mate]")
|
||||
{
|
||||
CadDocument doc;
|
||||
int sk_box = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "Box");
|
||||
doc.add_extrude(sk_box, 5.0, false, BooleanMode::New, "BoxExt");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int sk_cyl = doc.add_sketch(SketchShape::Circle, SketchPlane::XY(), 0, 0, 3, "Cyl");
|
||||
doc.add_extrude(sk_cyl, 10.0, false, BooleanMode::New, "CylExt");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.bodies.size() == 2);
|
||||
|
||||
int cs_a = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 0), "CS_A");
|
||||
doc.features[cs_a].coordsys_body = 0;
|
||||
int cs_b = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 0), "CS_B");
|
||||
doc.features[cs_b].coordsys_body = 1;
|
||||
int cs_c = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(10, 10, 10), "CS_C");
|
||||
doc.features[cs_c].coordsys_body = 0;
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int mate1 = doc.add_mate(0, cs_c, cs_b, 0, 0, false, "Mate1");
|
||||
REQUIRE(mate1 >= 0);
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int mate2 = doc.add_mate(0, cs_a, cs_b, 0, 0, false, "Mate2");
|
||||
REQUIRE(mate2 >= 0);
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
REQUIRE(doc.mate_conflicts.size() == 1);
|
||||
REQUIRE(doc.mate_conflicts[0].first == mate2);
|
||||
REQUIRE(doc.mate_conflicts[0].second.find("Mate1") != std::string::npos);
|
||||
REQUIRE(doc.mate_conflicts[0].second.find("Mate2") != std::string::npos);
|
||||
}
|
||||
|
||||
TEST_CASE("mate conflicts: a circular mate chain", "[CadDocument][mate]")
|
||||
{
|
||||
CadDocument doc;
|
||||
int sk_a = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "BoxA");
|
||||
doc.add_extrude(sk_a, 5.0, false, BooleanMode::New, "ExtA");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.bodies.size() == 1);
|
||||
|
||||
int sk_b = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "BoxB");
|
||||
doc.add_extrude(sk_b, 5.0, false, BooleanMode::New, "ExtB");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.bodies.size() == 2);
|
||||
|
||||
int cs_a = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(5, 5, 5), "CS_A");
|
||||
doc.features[cs_a].coordsys_body = 0;
|
||||
int cs_b = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 5), "CS_B");
|
||||
doc.features[cs_b].coordsys_body = 1;
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int cs_c = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(5, 5, 10), "CS_C");
|
||||
doc.features[cs_c].coordsys_body = 0;
|
||||
int cs_d = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 10), "CS_D");
|
||||
doc.features[cs_d].coordsys_body = 1;
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int mate1 = doc.add_mate(0, cs_a, cs_b, 0, 0, false, "MateAB");
|
||||
REQUIRE(mate1 >= 0);
|
||||
REQUIRE(doc.recompute());
|
||||
|
||||
int mate2 = doc.add_mate(0, cs_d, cs_c, 0, 0, false, "MateBA");
|
||||
REQUIRE(mate2 >= 0);
|
||||
REQUIRE(doc.recompute());
|
||||
|
||||
REQUIRE(doc.error.empty());
|
||||
REQUIRE(doc.mate_conflicts.size() >= 1);
|
||||
bool found_cycle = false;
|
||||
for (const auto& c : doc.mate_conflicts) {
|
||||
if (c.second.find("circular") != std::string::npos) {
|
||||
found_cycle = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
REQUIRE(found_cycle);
|
||||
}
|
||||
|
||||
TEST_CASE("mate conflicts: a broken mate is left to apply_mate", "[CadDocument][mate]")
|
||||
{
|
||||
CadDocument doc;
|
||||
int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "Box");
|
||||
doc.add_extrude(sk, 5.0, false, BooleanMode::New, "E");
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
int cs = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(5, 5, 5), "CS");
|
||||
doc.features[cs].coordsys_body = 0;
|
||||
REQUIRE(doc.recompute());
|
||||
REQUIRE(doc.error.empty());
|
||||
|
||||
doc.add_mate(0, 999, cs, 0, 0, false, "Bad");
|
||||
REQUIRE_FALSE(doc.recompute());
|
||||
REQUIRE_CONTAINS(doc.error, "mate_cs_a out of range");
|
||||
REQUIRE(doc.mate_conflicts.empty());
|
||||
}
|
||||
|
||||
TEST_CASE("ordering: fillet after mate resolves face ids", "[CadDocument][mate]")
|
||||
{
|
||||
using Catch::Matchers::WithinAbs;
|
||||
|
||||
Reference in New Issue
Block a user