From fac3cf44dfb51c5a22c4f10c3edb4dc80ca547d7 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Mon, 31 Aug 2026 03:43:08 +0200 Subject: [PATCH] Infer parallel, perpendicular, equal radius and tangent while drawing Port of snaporca 2d36d28770. Parity OK: 17 files identical, 8 diverging at their expected counts. infer_axis_constraint returned only Horizontal or Vertical. On the CAD-1000-hours corpus the top two transitions are sketch_dim -> sketch_draw (5896) and back (5756): the signature of geometry that does not self-constrain as it is drawn. Every rule requires the relation to be ALREADY TRUE within tolerance, so nothing the user drew is moved; parallel/perpendicular and tangent additionally require a shared endpoint. TWO LIMITS THE CORPUS RUNG FORCED, neither visible to the unit tests: 1. At most ONE constraint per rule per new entity, not one per PAIR, and no one-at-a-time fallback for the relations batch. EqualRadius has no locality restriction, so 200 equal holes produced ~20000 candidates; the rejected batch then cost a solve per constraint and pinned the app at 95% of a core with the MCP socket unresponsive. 2. Relations only for gesture-sized batches. "A scripted add is not a drawn gesture" is already this file's rule at its bulk call site (snaporca-8xg1), and EqualRadius also couples geometrically distant entities, merging independent connected components and defeating the partitioning that makes large sketches solvable (snaporca-yww4). With the cap alone geometry stayed correct (32/32 sheets clean) but seven of the largest timed out, including MPD681 -- the sheet that call site's own comment names. Also fixes the tolerance leak behind 2: the bulk path asks for exact inference with ang_tol_rad = 0 but len_tol_frac kept its 0.01 default. ALSO independent of this feature: run-kernel-tests.sh defaulted to TAGS=[CadDocument] while four CAD test files carry their own tags and nothing selected them (2624 assertions / 206 cases reported, 7648 / 264 actual). All 58 dark cases were passing; the coverage was never exercised. VERIFICATION LIMIT, as with the previous three commits: this fork's kernel suite still cannot run (find_package(assimp) at configure time, snaporca-w80c). Shared sources are byte-identical to snaporca's, where kernel is 7651 assertions / 265 cases and ALL LADDERS HELD 7/7. --- scripts/CAD/run-kernel-tests.sh | 12 ++- src/libslic3r/CAD/SketchInference.cpp | 108 +++++++++++++++++++++++ src/libslic3r/CAD/SketchInference.hpp | 11 +++ src/slic3r/GUI/CAD/DesignSketchTool.cpp | 33 +++++++ tests/libslic3r/test_sketchinference.cpp | 96 ++++++++++++++++++++ 5 files changed, 259 insertions(+), 1 deletion(-) diff --git a/scripts/CAD/run-kernel-tests.sh b/scripts/CAD/run-kernel-tests.sh index f8c8b18153..56a60e8636 100755 --- a/scripts/CAD/run-kernel-tests.sh +++ b/scripts/CAD/run-kernel-tests.sh @@ -36,7 +36,17 @@ VOL="${BUILD_VOL:-orcacad_kerneltest}" # circle-line tangency is fixed (snaporca-tkz), and the internal-thread case turned out to have # correct geometry and a wrong reference in the test (snaporca-kzy). A green run here now means # the whole CAD suite passed, not "everything except the two we gave up on". -TAGS="${TAGS:-[CadDocument]}" +# +# ...and that claim was still not true, because the default tag was [CadDocument] alone while +# four CAD test files carry their own tags and NOTHING ELSE selected them. test_sketchinference +# ([inference], 15), test_sketchedit ([SketchEdit], 23), test_sketchconstraints +# ([SketchConstraints], 8) and test_sketchimport ([SketchImport], 4) never ran here, nor did the +# older [slvs]-only cases in test_slvs_constraints. Measured 2026-08-31: the default reported +# 2624 assertions / 206 cases, the full set 7648 / 264 -- so the gate was speaking for about a +# third of the assertions, and a whole file could be added, tagged by its own convention, and +# stay dark while the suite printed green. All 58 were passing; the coverage was simply never +# exercised. Adding a tag here is now part of adding a test file. +TAGS="${TAGS:-[CadDocument],[inference],[SketchEdit],[SketchConstraints],[SketchImport],[slvs]}" HOST="" while [[ $# -gt 0 ]]; do diff --git a/src/libslic3r/CAD/SketchInference.cpp b/src/libslic3r/CAD/SketchInference.cpp index b2d3487eb1..cfeb2cd509 100644 --- a/src/libslic3r/CAD/SketchInference.cpp +++ b/src/libslic3r/CAD/SketchInference.cpp @@ -1,5 +1,6 @@ #include "libslic3r/CAD/SketchInference.hpp" +#include #include namespace Slic3r { @@ -123,4 +124,111 @@ infer_axis_constraint(const Vec2d& anchor, const Vec2d& tip, double ang_tol_rad) return std::nullopt; } +// Unsigned angle between two (unnormalized) direction vectors, in [0, pi]. 0 = same +// direction, pi = opposite, pi/2 = perpendicular. Inputs must be non-degenerate. +// static: this is a file-local helper, not part of the module's interface -- at namespace +// scope with external linkage it would be a link-time collision waiting to happen. +static double unsigned_angle(const Vec2d& a, const Vec2d& b) +{ + const double cross = a.x() * b.y() - a.y() * b.x(); + const double dot = a.x() * b.x() + a.y() * b.y(); + return std::atan2(std::abs(cross), dot); +} + +std::vector +infer_relations(const std::vector& entities, int new_ei, + double ang_tol_rad, double len_tol_frac) +{ + std::vector out; + if (new_ei <= 0 || new_ei >= int(entities.size())) return out; + + // AT MOST ONE constraint per rule per new entity, not one per PAIR. Without this the + // function is quadratic in the sketch: a drawing with 200 equal holes yields ~20000 + // EqualRadius candidates, the batch is rejected as over-constrained, and the caller's + // one-at-a-time fallback then runs a solve per constraint. Measured 2026-08-31: that + // pinned the app at 95% of a core with the MCP socket unresponsive -- the same failure + // the axes batch above already carries a warning about. Keep the best candidate only. + int best_ang_j = -1, best_rad_j = -1, best_tan_j = -1; + double best_ang_err = 1e30, best_rad_err = 1e30, best_tan_err = 1e30; + SketchConstraintType best_ang_type = SketchConstraintType::Parallel; + + const SketchEntity& n = entities[new_ei]; + const bool n_line = n.type == SketchEntity::Type::Line; + const bool n_curve = n.type == SketchEntity::Type::Arc || n.type == SketchEntity::Type::Circle; + if (!n_line && !n_curve) return out; // not a Line / Arc / Circle + if (n_line && (n.p1 - n.p0).squaredNorm() < 1e-18) return out; // degenerate + if (n_curve && n.radius < 1e-9) return out; + + for (int j = 0; j < new_ei; ++j) { + const SketchEntity& o = entities[j]; + const bool o_line = o.type == SketchEntity::Type::Line; + const bool o_curve = o.type == SketchEntity::Type::Arc || o.type == SketchEntity::Type::Circle; + if (!o_line && !o_curve) continue; + if (o_line && (o.p1 - o.p0).squaredNorm() < 1e-18) continue; + if (o_curve && o.radius < 1e-9) continue; + + if (n_line && o_line) { + // R1 — parallel / perpendicular, restricted to CONNECTED lines. Connection is + // what keeps this from firing on every distant line that is roughly parallel. + const bool connected = (n.p0 - o.p0).squaredNorm() <= 1e-14 || + (n.p0 - o.p1).squaredNorm() <= 1e-14 || + (n.p1 - o.p0).squaredNorm() <= 1e-14 || + (n.p1 - o.p1).squaredNorm() <= 1e-14; + if (!connected) continue; + const double ang = unsigned_angle(n.p1 - n.p0, o.p1 - o.p0); + const double par_err = std::min(ang, M_PI - ang); + const double per_err = std::abs(ang - M_PI / 2.0); + if (par_err <= ang_tol_rad && par_err < best_ang_err) { + best_ang_err = par_err; best_ang_j = j; + best_ang_type = SketchConstraintType::Parallel; + } else if (per_err <= ang_tol_rad && per_err < best_ang_err) { + best_ang_err = per_err; best_ang_j = j; + best_ang_type = SketchConstraintType::Perpendicular; + } + } else if (n_curve && o_curve) { + // R2 — equal radius between circles / arcs, relative to the larger. + const double larger = n.radius > o.radius ? n.radius : o.radius; + const double err = std::abs(n.radius - o.radius) / larger; + if (err <= len_tol_frac && err < best_rad_err) { best_rad_err = err; best_rad_j = j; } + } else { + // R3 — tangent where a line meets a circle / arc at a shared endpoint, and only + // when the line is ALREADY perpendicular to the radius at that point. + const SketchEntity& ln = n_line ? n : o; + const SketchEntity& cv = n_line ? o : n; + const Vec2d ldir = ln.p1 - ln.p0; + bool tangent = false; + const Vec2d le[2] = { ln.p0, ln.p1 }; + for (int k = 0; k < 2 && !tangent; ++k) { + if (cv.type == SketchEntity::Type::Arc) { + const Vec2d ce[2] = { cv.p0, cv.p1 }; + for (int m = 0; m < 2; ++m) { + if ((le[k] - ce[m]).squaredNorm() > 1e-14) continue; + const Vec2d r = ce[m] - cv.center; + if (r.squaredNorm() < 1e-18) continue; + tangent = std::abs(unsigned_angle(ldir, r) - M_PI / 2.0) <= ang_tol_rad; + if (tangent) break; + } + } else { // Circle: shared point is a line endpoint on the rim. + const Vec2d r = le[k] - cv.center; + if (std::abs(r.norm() - cv.radius) > 1e-7) continue; + if (r.squaredNorm() < 1e-18) continue; + tangent = std::abs(unsigned_angle(ldir, r) - M_PI / 2.0) <= ang_tol_rad; + } + } + if (tangent && best_tan_err > 0.0) { best_tan_err = 0.0; best_tan_j = j; } + } + } + + auto emit = [&](SketchConstraintType t, int j) { + if (j < 0) return; + SketchEntityConstraintDef c; + c.type = t; c.ea = j; c.eb = new_ei; + out.push_back(c); + }; + emit(best_ang_type, best_ang_j); // R1 + emit(SketchConstraintType::EqualRadius, best_rad_j); // R2 + emit(SketchConstraintType::Tangent, best_tan_j); // R3 + return out; +} + } // namespace Slic3r diff --git a/src/libslic3r/CAD/SketchInference.hpp b/src/libslic3r/CAD/SketchInference.hpp index d520041ab7..be51f0240e 100644 --- a/src/libslic3r/CAD/SketchInference.hpp +++ b/src/libslic3r/CAD/SketchInference.hpp @@ -39,6 +39,17 @@ InferenceSnap infer_point_snap(const std::vector& entities, std::optional infer_axis_constraint(const Vec2d& anchor, const Vec2d& tip, double ang_tol_rad = 3.0 * M_PI / 180.0); +// Relational constraints to auto-emit for a newly drawn entity `new_ei` against the +// entities already in the sketch. Pure, no GUI/GL dependencies, unit-testable. +// +// Deliberately conservative: every rule requires the relation to be ALREADY TRUE within +// tolerance, so an inferred constraint never moves geometry the user drew — it only pins a +// relation that is visibly there. Returns an empty vector when nothing qualifies. +std::vector +infer_relations(const std::vector& entities, int new_ei, + double ang_tol_rad = 2.0 * M_PI / 180.0, + double len_tol_frac = 0.01); + } // namespace Slic3r #endif // slic3r_SketchInference_hpp_ diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index 361bf848d9..83260099a0 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -2411,6 +2411,39 @@ void DesignSketchTool::infer_auto_constraints(int base, double ang_tol_rad, doub if (!try_add_constraints(axes)) for (const auto& c : axes) try_add_constraints({ c }); + // 3) Relational constraints on the new entities (parallel/perpendicular on connected + // lines, equal radius, tangent). Same batch-then-one-at-a-time fallback as above: + // try_add_constraints rolls a conflicting batch back, so a single rejected relation + // never costs the others. Every rule only pins a relation that is ALREADY true, so + // nothing the user drew is moved by this. + // A SCRIPTED ADD IS NOT A DRAWN GESTURE — the rule this function already states at its + // bulk call site, which passes zero tolerances for exactly that reason (snaporca-8xg1). + // Relational inference must obey it too, and for a second reason beyond tolerance: + // EqualRadius couples entities that are geometrically far apart, so on a real drawing it + // merges independent connected components into one huge system and defeats the + // component partitioning that makes large sketches solvable at all (snaporca-yww4). + // Measured 2026-08-31 on the corpus rung: geometry stayed correct (32/32 sheets clean) + // but seven of the largest sheets hit main-thread timeout — MPD681 among them, the very + // sheet named in the comment at the bulk call site. Exact-equality would not save it + // either: patterned holes in real drawings ARE exactly equal. + // So: relations only for batches the size of a human gesture. A polyline segment is 1, a + // rectangle 4, a polygon a dozen; a scripted or imported add is hundreds. + constexpr int kRelInferMaxBatch = 16; + std::vector rels; + if (n - base <= kRelInferMaxBatch) { + const double rel_len_tol = ang_tol_rad > 0.0 ? 0.01 : 0.0; // exact-only when the caller asked for exact + for (int i = base; i < n; ++i) { + auto r = infer_relations(m_entities, i, ang_tol_rad, rel_len_tol); + rels.insert(rels.end(), r.begin(), r.end()); + } + } + // NO one-at-a-time fallback here, unlike the two batches above. Relations are a + // convenience: nothing is incorrect without them. The fallback costs one SOLVE PER + // CONSTRAINT, which is what the warning on the axes batch is about, and paying it for + // optional constraints is how a bulk add pins the app at 100% CPU with the MCP socket + // unresponsive (measured 2026-08-31). If the batch conflicts, drop the batch. + if (!rels.empty()) try_add_constraints(rels); + resolve_live(); } diff --git a/tests/libslic3r/test_sketchinference.cpp b/tests/libslic3r/test_sketchinference.cpp index dfbfefdbc3..cbdd90628b 100644 --- a/tests/libslic3r/test_sketchinference.cpp +++ b/tests/libslic3r/test_sketchinference.cpp @@ -76,3 +76,99 @@ TEST_CASE("inference: axis inference flags horizontal / vertical segments", "[in CHECK_FALSE(infer_axis_constraint({0, 0}, {10, 10}).has_value()); // 45 deg CHECK_FALSE(infer_axis_constraint({0, 0}, {0, 0}).has_value()); // degenerate } + +TEST_CASE("inference: perpendicular inferred for a connected square corner", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({10, 0}, {10, 7}) }; + auto r = infer_relations(ents, 1); + REQUIRE(r.size() == 1); + CHECK(r[0].type == SketchConstraintType::Perpendicular); + CHECK(r[0].ea == 0); + CHECK(r[0].eb == 1); +} + +TEST_CASE("inference: parallel inferred for connected collinear-ish lines", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({10, 0}, {21, 0.1}) }; + auto r = infer_relations(ents, 1); + REQUIRE(r.size() == 1); + CHECK(r[0].type == SketchConstraintType::Parallel); + CHECK(r[0].ea == 0); + CHECK(r[0].eb == 1); +} + +TEST_CASE("inference: two unconnected parallel lines infer nothing", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({0, 5}, {10, 5}) }; + auto r = infer_relations(ents, 1); + CHECK(r.empty()); +} + +TEST_CASE("inference: a corner outside tolerance infers nothing", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({10, 0}, {15, 7}) }; + auto r = infer_relations(ents, 1); + CHECK(r.empty()); +} + +TEST_CASE("inference: equal radius inferred for near-equal circles", "[inference]") +{ + auto r = infer_relations({ circle({0, 0}, 5.0), circle({30, 0}, 5.02) }, 1); + REQUIRE(r.size() == 1); + CHECK(r[0].type == SketchConstraintType::EqualRadius); + CHECK(r[0].ea == 0); + CHECK(r[0].eb == 1); + + auto r2 = infer_relations({ circle({0, 0}, 5.0), circle({30, 0}, 6.0) }, 1); + CHECK(r2.empty()); +} + +TEST_CASE("inference: tangent inferred for a line meeting a circle tangentially", "[inference]") +{ + std::vector ents = { circle({0, 0}, 5.0), line({0, 5}, {10, 5}) }; + auto r = infer_relations(ents, 1); + REQUIRE(r.size() == 1); + CHECK(r[0].type == SketchConstraintType::Tangent); + CHECK(r[0].ea == 0); + CHECK(r[0].eb == 1); + + std::vector off = { circle({0, 0}, 5.0), line({0, 5}, {10, 9}) }; + CHECK(infer_relations(off, 1).empty()); +} + +TEST_CASE("inference: nothing inferred against a higher index", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({10, 0}, {10, 7}) }; + auto r = infer_relations(ents, 0); + CHECK(r.empty()); +} + +TEST_CASE("inference: degenerate entities are ignored", "[inference]") +{ + std::vector ents = { line({0, 0}, {10, 0}), line({10, 0}, {10, 0}) }; + auto r = infer_relations(ents, 1); + CHECK(r.empty()); +} + +// The cap that keeps infer_relations linear rather than quadratic. Without it a drawing with +// many equal holes yields a constraint per PAIR: 200 equal circles produced ~20000 candidates, +// the batch was rejected as over-constrained, and the caller's one-at-a-time fallback then ran +// a solve per constraint -- which pinned the app at 95% of a core with the MCP socket +// unresponsive, and is what the corpus rung caught. +TEST_CASE("inference: at most one relation per rule per new entity", "[inference]") +{ + // 40 circles of the same radius; the 41st must not produce 40 EqualRadius constraints. + std::vector ents; + for (int i = 0; i < 41; ++i) { + SketchEntity c; + c.type = SketchEntity::Type::Circle; + c.center = Vec2d(i * 20.0, 0.0); + c.p0 = c.center; + c.radius = 5.0; + ents.push_back(c); + } + auto rels = infer_relations(ents, 40); + CHECK(rels.size() == 1); + CHECK(rels[0].type == SketchConstraintType::EqualRadius); + CHECK(rels[0].eb == 40); +}