From 6b3642fa534ec355ebc3fe744324db1d4bb6ce00 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Wed, 12 Aug 2026 23:45:37 +0200 Subject: [PATCH] Mate viability: which of the five apply, and why the others do not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit snaporca-lukg wants a palette offering all five mate types with the non-viable ones DIMMED AND EXPLAINED rather than hidden — its reasoning being that a menu changing shape between invocations destroys the motor memory experts rely on. That needs an answer this document could not give. This is that answer, and nothing else: mate_options(cs_a, cs_b) returns five MateOption{kind, viable, reason}, always five, always in kind order, never filtered. The geometry test rides on the fingerprint added for snaporca-kqih, which is why it costs no new serialized field: coordsys_face_kind already records the surface type. Revolute and Cylindrical need a cylindrical face at both ends because they need an axis to turn about; Planar needs flat faces; Fastened and Slider constrain frames rather than surfaces, so no geometry test applies to them. UNKNOWN IS PERMISSIVE. A fingerprint of -1 means PointWorld or a connector that has not resolved yet, and it does NOT make a type non-viable. Refusing on missing information is the false-alarm behaviour that gets a whole feature ignored — the same reasoning already recorded on kqih for the drift warning, applied again because it is the same trade. Reasons name WHICH connector is the problem when only one is. "needs a cylindrical face at both ends" tells the user what the rule is; "connector A is on a flat face" tells them where to look, and the second half is the one that saves the time. The stability contract has its own test, asserting five entries in kind order even for a completely invalid pair. That matters more than any individual verdict: the palette addresses rows by position, so a shorter list would move every row below it. Golden fixture unchanged — this is a pure query. Suite 167 -> 171 cases, 2284 -> 2348 assertions, green. snaporca-lukg part A; the palette is part B. --- src/libslic3r/CadDocument.cpp | 56 ++++++++++++++++ src/libslic3r/CadDocument.hpp | 12 ++++ tests/libslic3r/test_caddocument.cpp | 97 ++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+) diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index 38825e8208..8a348c1213 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -1315,6 +1315,62 @@ int CadDocument::add_mate(int kind, int cs_a, int cs_b, double offset, double an return int(features.size()) - 1; } +std::vector CadDocument::mate_options(int cs_a, int cs_b) const +{ + std::vector out(5); + for (int k = 0; k < 5; ++k) out[k].kind = k; + + auto is_connector = [&](int idx) -> bool { + return idx >= 0 && idx < int(features.size()) && + features[idx].type == CadFeatureType::CoordSys && + features[idx].enabled; + }; + if (!is_connector(cs_a) || !is_connector(cs_b)) { + const char side = is_connector(cs_a) ? 'B' : 'A'; + for (auto& o : out) { o.viable = false; o.reason = std::string("connector ") + side + " is not a coordinate system"; } + return out; + } + if (cs_a == cs_b) { + for (auto& o : out) { o.viable = false; o.reason = "a mate needs two different connectors"; } + return out; + } + + const int ka = features[cs_a].coordsys_face_kind; + const int kb = features[cs_b].coordsys_face_kind; + auto face_desc = [](int kind) -> std::string { return kind == GeomAbs_Plane ? "a flat face" : "a curved face"; }; + + // Planar: needs a flat face at both ends. + const bool plan_bad_a = ka >= 0 && ka != GeomAbs_Plane; + const bool plan_bad_b = kb >= 0 && kb != GeomAbs_Plane; + if (plan_bad_a || plan_bad_b) { + out[1].viable = false; + if (plan_bad_a && plan_bad_b) + out[1].reason = "needs a flat face at both ends — both connectors are on curved faces"; + else if (plan_bad_a) + out[1].reason = "needs a flat face at both ends — connector A is on " + face_desc(ka); + else + out[1].reason = "needs a flat face at both ends — connector B is on " + face_desc(kb); + } + + // Revolute and Cylindrical: need a cylindrical face at both ends. + for (int k : {2, 4}) { + const bool ax_bad_a = ka >= 0 && ka != GeomAbs_Cylinder; + const bool ax_bad_b = kb >= 0 && kb != GeomAbs_Cylinder; + if (!ax_bad_a && !ax_bad_b) continue; + out[k].viable = false; + if (ax_bad_a && ax_bad_b) + out[k].reason = (ka == GeomAbs_Plane && kb == GeomAbs_Plane) + ? "needs a cylindrical face at both ends — both connectors are on flat faces" + : "needs a cylindrical face at both ends — both connectors are on non-cylindrical faces"; + else if (ax_bad_a) + out[k].reason = "needs a cylindrical face at both ends — connector A is on " + face_desc(ka); + else + out[k].reason = "needs a cylindrical face at both ends — connector B is on " + face_desc(kb); + } + + return out; +} + int CadDocument::add_helix(const SketchPlane& plane, double radius, double pitch, double height, bool left_handed, double taper_deg, const std::string& name) { diff --git a/src/libslic3r/CadDocument.hpp b/src/libslic3r/CadDocument.hpp index 83798931d2..13d112c7c6 100644 --- a/src/libslic3r/CadDocument.hpp +++ b/src/libslic3r/CadDocument.hpp @@ -578,6 +578,18 @@ public: int add_coordsys(CoordSysType type, const Vec3d& point, const std::string& name); int add_mate(int kind, int cs_a, int cs_b, double offset, double angle_deg, bool flip, const std::string& name); + + // Which mate types apply to a connector pair, as reported to the viewport palette. + struct MateOption { + int kind{0}; // 0..4, the five mate types in CadDocument.hpp:308-314 + bool viable{true}; + std::string reason; // empty when viable; why not, when not + }; + // ALWAYS all five entries, ALWAYS in kind order. Never filtered: the caller dims what is + // not viable rather than hiding it, so the list must be stable in length and order between + // calls. Pure query over existing data — records nothing, mutates nothing. + std::vector mate_options(int cs_a, int cs_b) const; + int add_helix(const SketchPlane& plane, double radius, double pitch, double height, bool left_handed, double taper_deg, const std::string& name); // Build the helix wire from a Helix feature's params (exposed for tests). diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index edf1e26853..ddab69db1b 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -7429,3 +7429,100 @@ TEST_CASE("a resized body does not report drift", "[CadDocument][mate]") REQUIRE(doc.error.empty()); REQUIRE(doc.mate_conflicts.empty()); } + +TEST_CASE("mate_options always returns five entries in kind order", "[CadDocument][mate]") +{ + CadDocument doc; + // Completely invalid pair: the stability contract (always five, always in order) must hold. + auto opts = doc.mate_options(-1, -1); + REQUIRE(opts.size() == 5); + for (int i = 0; i < 5; ++i) REQUIRE(opts[i].kind == i); + + // A valid, distinct pair must return the same shape. + int a = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 0), "A"); + int b = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(1, 0, 0), "B"); + auto opts2 = doc.mate_options(a, b); + REQUIRE(opts2.size() == 5); + for (int i = 0; i < 5; ++i) REQUIRE(opts2[i].kind == i); +} + +TEST_CASE("a flat-face pair offers Fastened, Planar and Slider but not the axial types", "[CadDocument][mate]") +{ + CadDocument doc; + int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 0, "Box"); + doc.add_extrude(sk, 10.0, false, BooleanMode::New, "Extrude"); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + + int top = find_face_by_normal(doc, 0, Vec3d(0, 0, 1)); + int side = find_face_by_normal(doc, 0, Vec3d(1, 0, 0)); + REQUIRE(top >= 0); + REQUIRE(side >= 0); + + int a = doc.add_coordsys(CoordSysType::FaceAndDirection, Vec3d(0, 0, 0), "A"); + doc.features[a].coordsys_body = 0; + doc.features[a].coordsys_face = top; + int b = doc.add_coordsys(CoordSysType::FaceAndDirection, Vec3d(0, 0, 0), "B"); + doc.features[b].coordsys_body = 0; + doc.features[b].coordsys_face = side; + + // Recompute so the face fingerprints record (both planar => GeomAbs_Plane). + REQUIRE(doc.recompute()); + REQUIRE(doc.features[a].coordsys_face_kind >= 0); + REQUIRE(doc.features[b].coordsys_face_kind >= 0); + + auto opts = doc.mate_options(a, b); + REQUIRE(opts.size() == 5); + REQUIRE(opts[0].viable); // Fastened: frame-only, always viable + REQUIRE(opts[1].viable); // Planar: both faces planar + REQUIRE_FALSE(opts[2].viable); // Revolute: needs a cylindrical face + REQUIRE(opts[3].viable); // Slider: frame-only, always viable + REQUIRE_FALSE(opts[4].viable); // Cylindrical: needs a cylindrical face + + REQUIRE(opts[0].reason.empty()); + REQUIRE(opts[1].reason.empty()); + REQUIRE(opts[3].reason.empty()); + REQUIRE_FALSE(opts[2].reason.empty()); + REQUIRE_FALSE(opts[4].reason.empty()); + REQUIRE_CONTAINS(opts[2].reason, "connector"); + REQUIRE_CONTAINS(opts[4].reason, "connector"); +} + +TEST_CASE("an unrecorded fingerprint does not make a type non-viable", "[CadDocument][mate]") +{ + CadDocument doc; + int a = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 0), "A"); + int b = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(1, 0, 0), "B"); + // PointWorld connectors never record a face fingerprint: face_kind stays -1. + REQUIRE(doc.features[a].coordsys_face_kind == -1); + REQUIRE(doc.features[b].coordsys_face_kind == -1); + + auto opts = doc.mate_options(a, b); + REQUIRE(opts.size() == 5); + for (const auto& o : opts) { + REQUIRE(o.viable); + REQUIRE(o.reason.empty()); + } +} + +TEST_CASE("an invalid pair names which side is wrong", "[CadDocument][mate]") +{ + CadDocument doc; + int a = doc.add_coordsys(CoordSysType::PointWorld, Vec3d(0, 0, 0), "A"); + int sketch = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 0, "S"); + + // cs_b points at a Sketch feature, not a CoordSys: all five non-viable, reason names B. + auto opts = doc.mate_options(a, sketch); + REQUIRE(opts.size() == 5); + for (const auto& o : opts) { + REQUIRE_FALSE(o.viable); + REQUIRE_CONTAINS(o.reason, "connector B"); + REQUIRE(o.reason.find("connector A") == std::string::npos); + } + + // Out of range on B names B; out of range on A names A. + for (const auto& o : doc.mate_options(a, 9999)) + REQUIRE_CONTAINS(o.reason, "connector B"); + for (const auto& o : doc.mate_options(-5, a)) + REQUIRE_CONTAINS(o.reason, "connector A"); +}