From d584d6600334dc26c6a29c833098938ad27fc70b Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Wed, 12 Aug 2026 15:06:17 +0200 Subject: [PATCH] Mate conflicts: name what silently wins, don't call it over-constraint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/libslic3r/CadDocument.cpp | 108 ++++++++++++++++++++ src/libslic3r/CadDocument.hpp | 8 ++ tests/libslic3r/test_caddocument.cpp | 142 +++++++++++++++++++++++++++ 3 files changed, 258 insertions(+) diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index 6a31cf04d0..e601c16d8f 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -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& 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 first_driver; // body -> feature index of first mate that drives it + std::map> 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 colour; + struct Frame { int node; size_t next; }; + std::vector 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& bodies, const CadFeature& f) const { const int nc = int(features.size()); @@ -3221,6 +3328,7 @@ void CadDocument::route_feature(std::vector& bodies, const CadFeature& bool CadDocument::recompute() { error.clear(); + detect_mate_conflicts(); std::vector built; try { // Parametric pass: evaluate document variables, then each feature's expression bindings, diff --git a/src/libslic3r/CadDocument.hpp b/src/libslic3r/CadDocument.hpp index 62675f2331..2016649392 100644 --- a/src/libslic3r/CadDocument.hpp +++ b/src/libslic3r/CadDocument.hpp @@ -425,6 +425,13 @@ public: std::vector 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> 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& bodies, CadFeature& f) const; static DatumCoordSys datum_frame(const std::vector& bodies, const CadFeature& f); void apply_mate(std::vector& 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 diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index de888f2bb6..eb45aa791f 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -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;