diff --git a/src/libslic3r/AppConfig.cpp b/src/libslic3r/AppConfig.cpp index 17b86ac13e..2f2fb4a0a1 100644 --- a/src/libslic3r/AppConfig.cpp +++ b/src/libslic3r/AppConfig.cpp @@ -316,6 +316,12 @@ void AppConfig::set_defaults() if (get("enable_cad_feature").empty()) set_bool("enable_cad_feature", false); + // Auto-weld sketch endpoints within kSketchJoinTol when building closed loops. + // Default ON: it is what the ~90% case wants; OFF makes the kernel demand an exact + // joint. The GUI pushes it into SketchEngine via set_sketch_auto_close(). + if (get("auto_close_sketch_loops").empty()) + set_bool("auto_close_sketch_loops", true); + // Design tab: draw a mate connector as a face rather than as the abstract disc + roll // quadrant. Defaults ON — face orientation is hardwired perception, so the roll and the // verse read without being learned, which no abstract glyph achieves. Turning it off diff --git a/src/libslic3r/CAD/SketchEngine.cpp b/src/libslic3r/CAD/SketchEngine.cpp index 835bb7a5fc..213bbab3e7 100644 --- a/src/libslic3r/CAD/SketchEngine.cpp +++ b/src/libslic3r/CAD/SketchEngine.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -51,6 +52,14 @@ namespace Slic3r { +// Single source of truth for the weld tolerance the viewport and the kernel share. +// Defaults ON so headless/kernel-only callers keep welding; the GUI pushes the +// "auto_close_sketch_loops" preference in via set_sketch_auto_close(). +static bool s_auto_close = true; + +double sketch_join_tol() { return s_auto_close ? kSketchJoinTol : 0.0; } +void set_sketch_auto_close(bool on) { s_auto_close = on; } + // ---- SketchPlane ---- gp_Pln SketchPlane::to_occt() const @@ -535,6 +544,11 @@ TriangleMesh SketchEngine::tessellate(const TopoDS_Shape& shape, std::vector SketchEngine::entities_to_wires(const std::vector& entities, const SketchPlane& plane) { + // Effective weld tolerance: kSketchJoinTol when auto-close is on, 0.0 when off. + // Read ONCE so the union-find, the node weld and the vertex tolerance below all + // agree. With 0.0 the comparisons use <= so exactly coincident endpoints still join. + const double tol = sketch_join_tol(); + struct Item { const SketchEntity* e; size_t idx; }; std::vector valid; valid.reserve(entities.size()); @@ -600,12 +614,12 @@ std::vector SketchEngine::entities_to_wires(const std::vector parent(valid.size()); @@ -687,7 +701,7 @@ std::vector SketchEngine::entities_to_wires(const std::vector SketchEngine::entities_to_wires(const std::vector node_deg; // endpoint count per node auto node_id = [&](const Vec2d& p) -> int { for (size_t i = 0; i < node_pt.size(); ++i) - if ((node_pt[i] - p).norm() < kSketchWeldTol) return int(i); + if ((node_pt[i] - p).norm() <= tol) return int(i); node_pt.push_back(p); node_deg.push_back(0); return int(node_pt.size()) - 1; @@ -736,15 +750,16 @@ std::vector SketchEngine::entities_to_wires(const std::vector verts(node_pt.size()); BRep_Builder B; for (size_t i = 0; i < node_pt.size(); ++i) { Vec3d w = plane.to_world(node_pt[i]); verts[i] = BRepBuilderAPI_MakeVertex(gp_Pnt(w.x(), w.y(), w.z())).Vertex(); - B.UpdateVertex(verts[i], kSketchWeldTol); + B.UpdateVertex(verts[i], std::max(tol, Precision::Confusion())); } for (size_t o : order) { diff --git a/src/libslic3r/CAD/SketchEngine.hpp b/src/libslic3r/CAD/SketchEngine.hpp index 7ccc807103..41ec5123f5 100644 --- a/src/libslic3r/CAD/SketchEngine.hpp +++ b/src/libslic3r/CAD/SketchEngine.hpp @@ -78,6 +78,24 @@ struct SketchProfile { void serialize(Archive& ar) { ar(points, closed); } }; +// Two sketch endpoints this close are ONE joint. Shared deliberately by the viewport +// (region_loops / connected_loop / open-end detection) and by the kernel +// (entities_to_wires): the viewport is what shades a region closed and offers it for +// extrude, so the kernel MUST be able to build every loop the viewport shades. When +// these two numbers disagreed the viewport promised a closed region at 1e-3 and the +// kernel refused it at 1e-4, which extruded a solid the user never drew. +// Nothing legitimate in a mm-scale sketch is 1 um apart. +inline constexpr double kSketchJoinTol = 1e-3; // mm + +// Effective sketch joint tolerance. ONE value for the viewport (region_loops / +// loop_report / connected_loop) and the kernel (entities_to_wires): if these ever +// disagree again, the viewport shades a region closed that the kernel refuses to +// build, which is how a sketch got extruded into the wrong solid. The GUI pushes +// the "auto_close_sketch_loops" preference in via set_sketch_auto_close(); the +// kernel defaults to ON so headless/kernel-only callers keep welding. +double sketch_join_tol(); +void set_sketch_auto_close(bool on); + enum class SketchConstraintType { Fix, Coincident, Horizontal, Vertical, Distance, LockX, LockY, EqualLength, Parallel, Perpendicular, diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index 9ef927cd42..4d4aa02fec 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -62,8 +62,19 @@ static double ray_segment_dist3(const Vec3d& ro, const Vec3d& rd, const Vec3d& a return wxPoint(int(sx + 0.5), int(sy + 0.5)); } +// The kernel's weld tolerance follows the app preference, and it must be pushed at EVERY +// point that starts a sketch session: a Constrain session never passes through begin(), and +// it uses region_loops()/connected_loop(), which read the same tolerance. Pushing in one +// place only would leave those sessions on whatever the previous session set. +static void push_auto_close_pref() +{ + Slic3r::set_sketch_auto_close(wxGetApp().is_auto_close_sketch_loops()); +} + void DesignSketchTool::begin(const SketchPlane& plane, Mode mode) { + push_auto_close_pref(); + m_plane = plane; m_mode = mode; m_step_mode_last = -1; // a new session re-announces its step, even if it repeats the last @@ -2221,6 +2232,7 @@ void DesignSketchTool::finish() void DesignSketchTool::begin_constrain(const SketchProfile& prof, const SketchPlane& plane) { + push_auto_close_pref(); m_plane = plane; m_mode = Mode::Constrain; m_points = prof.points; @@ -2235,6 +2247,7 @@ void DesignSketchTool::begin_constrain(const SketchProfile& prof, const SketchPl void DesignSketchTool::begin_constrain_entities(const std::vector& ents, const SketchPlane& plane) { + push_auto_close_pref(); m_plane = plane; m_mode = Mode::Constrain; m_constrain_entities = true; @@ -6333,7 +6346,7 @@ std::vector DesignSketchTool::region_loops(const std::vector& ents) const { std::vector regions; - const double eps2 = 1e-3 * 1e-3; + const double eps2 = sketch_join_tol() * sketch_join_tol(); auto is_near = [&](const Vec2d& a, const Vec2d& b) { return (a - b).squaredNorm() < eps2; }; // Circles are self-closed regions; lines/arcs are open segments to be chained. Each @@ -9420,7 +9433,7 @@ DesignSketchTool::LoopReport DesignSketchTool::loop_report() const ends.push_back({ e.p1 }); } } - const double eps = 1e-3; + const double eps = sketch_join_tol(); for (size_t i = 0; i < ends.size(); ++i) { int met = 0; for (size_t j = 0; j < ends.size(); ++j) { @@ -9553,7 +9566,7 @@ std::vector DesignSketchTool::connected_loop(int seed) const { std::vector out; if (seed < 0 || seed >= int(m_entities.size())) return out; - const double eps2 = 1e-6; + const double eps2 = sketch_join_tol() * sketch_join_tol(); std::vector vis(m_entities.size(), false); std::vector stack = { seed }; vis[seed] = true; diff --git a/src/slic3r/GUI/GUI_App.hpp b/src/slic3r/GUI/GUI_App.hpp index 68074894ee..f4c20af60c 100644 --- a/src/slic3r/GUI/GUI_App.hpp +++ b/src/slic3r/GUI/GUI_App.hpp @@ -351,6 +351,8 @@ public: inline bool is_enable_multi_machine() { return this->app_config&& this->app_config->get("enable_multi_machine") == "true"; } #ifdef SLIC3R_CAD inline bool is_enable_cad_feature() { return this->app_config && this->app_config->get_bool("enable_cad_feature"); } + inline bool is_auto_close_sketch_loops() { return !this->app_config + || this->app_config->get_bool("auto_close_sketch_loops"); } #endif std::map test_url_state; diff --git a/src/slic3r/GUI/Preferences.cpp b/src/slic3r/GUI/Preferences.cpp index 39acb05d83..77574fdd25 100644 --- a/src/slic3r/GUI/Preferences.cpp +++ b/src/slic3r/GUI/Preferences.cpp @@ -8,6 +8,7 @@ #include "I18N.hpp" #include "libslic3r/AppConfig.hpp" #include "libslic3r/Format/DRC.hpp" +#include "libslic3r/CAD/SketchEngine.hpp" #include #include "OG_CustomCtrl.hpp" #include "wx/graphics.h" @@ -1747,6 +1748,13 @@ void PreferencesDialog::create_items() "parametrically. This feature is experimental and still under development."), "enable_cad_feature", _L("(Requires restart)")); g_sizer->Add(item_cad_feature); + + auto item_auto_close_sketch_loops = create_item_checkbox(_L("Auto-close sketch loops"), + _L("Treat sketch endpoints within 0.001 mm as one joint and weld the loop shut. " + "Off: only exactly coincident endpoints join, so a loop with a tiny gap is " + "shown as open instead of being closed for you."), + "auto_close_sketch_loops"); + g_sizer->Add(item_auto_close_sketch_loops); #endif #if 0 @@ -1834,6 +1842,10 @@ void PreferencesDialog::create_items() "Turn this off for the conventional CAD representation."), "design_connector_face_glyph"); g_sizer->Add(item_connector_face_glyph); } + + // Push the weld preference into the kernel now so toggling it takes effect without + // a restart (the sketch tool also re-pushes on activation, see DesignSketchTool::begin). + Slic3r::set_sketch_auto_close(wxGetApp().is_auto_close_sketch_loops()); #endif std::vector ButtonDragActions = {_L("None"), _L("Pan"), _L("Rotate")}; diff --git a/tests/libslic3r/test_sketchedit.cpp b/tests/libslic3r/test_sketchedit.cpp index 022aaa12e0..973785e812 100644 --- a/tests/libslic3r/test_sketchedit.cpp +++ b/tests/libslic3r/test_sketchedit.cpp @@ -555,3 +555,100 @@ TEST_CASE("entities_to_wires keeps every edge of a loop drawn out of order", "[S REQUIRE(wires[0].Closed()); } + +// Regression guard: this fails at 1e-4 (the wire builder refuses a joint the viewport had +// already shaded closed) and passes at kSketchJoinTol. A 20x10 quad with one joint left open +// by 9e-4 mm — just inside kSketchJoinTol, exactly the case the viewport shades closed — given +// in an order that is NOT traversal order, so the ordering path is covered too. +TEST_CASE("a loop the viewport shades closed is buildable by the kernel", "[SketchEngine]") +{ + std::vector ents(4); + + // (0,0) -> (20,0) -> (20,10) -> (0,10) -> (0.0009, 0): last endpoint misses (0,0) by 9e-4. + ents[0].type = SketchEntity::Type::Line; + ents[0].p0 = Vec2d(0, 0); + ents[0].p1 = Vec2d(20, 0); + + // Index 1 is the FAR side, not the neighbour of index 0: creation order here is + // deliberately not traversal order, so a partial wire would reject it without the + // traversal walk. + ents[1].type = SketchEntity::Type::Line; + ents[1].p0 = Vec2d(20, 10); + ents[1].p1 = Vec2d(0, 10); + + ents[2].type = SketchEntity::Type::Line; + ents[2].p0 = Vec2d(20, 0); + ents[2].p1 = Vec2d(20, 10); + + ents[3].type = SketchEntity::Type::Line; + ents[3].p0 = Vec2d(0, 10); + ents[3].p1 = Vec2d(0.0009, 0); + + auto wires = SketchEngine::entities_to_wires(ents, SketchPlane::XY()); + + REQUIRE(wires.size() == 1); + + int edge_count = 0; + for (TopExp_Explorer ex(wires[0], TopAbs_EDGE); ex.More(); ex.Next()) + ++edge_count; + REQUIRE(edge_count == 4); + + REQUIRE(wires[0].Closed()); +} + +// Regression guard for the auto-close preference. Same 20x10 quad, one joint open by 9e-4 mm +// and given out of traversal order, as "a loop the viewport shades closed is buildable by the +// kernel". With auto-close ON the gap welds (one closed wire); with auto-close OFF it must not. +TEST_CASE("auto-close off makes the kernel demand an exact joint", "[SketchEngine]") +{ + std::vector ents(4); + + ents[0].type = SketchEntity::Type::Line; + ents[0].p0 = Vec2d(0, 0); + ents[0].p1 = Vec2d(20, 0); + + ents[1].type = SketchEntity::Type::Line; + ents[1].p0 = Vec2d(20, 10); + ents[1].p1 = Vec2d(0, 10); + + ents[2].type = SketchEntity::Type::Line; + ents[2].p0 = Vec2d(20, 0); + ents[2].p1 = Vec2d(20, 10); + + ents[3].type = SketchEntity::Type::Line; + ents[3].p0 = Vec2d(0, 10); + ents[3].p1 = Vec2d(0.0009, 0); + + auto edge_count = [](const TopoDS_Wire& w) { + int n = 0; + for (TopExp_Explorer ex(w, TopAbs_EDGE); ex.More(); ex.Next()) ++n; + return n; + }; + + // ON: the 9e-4 mm gap is inside kSketchJoinTol, so the loop welds into one closed wire. + Slic3r::set_sketch_auto_close(true); + auto wires_on = SketchEngine::entities_to_wires(ents, SketchPlane::XY()); + REQUIRE(wires_on.size() == 1); + REQUIRE(edge_count(wires_on[0]) == 4); + REQUIRE(wires_on[0].Closed()); + + // OFF: the joint is not exact, so the gap is NOT welded. entities_to_wires legitimately + // returns open chains (a sweep path is open), so the observable is an OPEN wire — the + // kernel no longer hands back the closed loop the viewport would have shaded. + Slic3r::set_sketch_auto_close(false); + auto wires_off = SketchEngine::entities_to_wires(ents, SketchPlane::XY()); + REQUIRE(wires_off.size() == 1); + REQUIRE(edge_count(wires_off[0]) == 4); + REQUIRE_FALSE(wires_off[0].Closed()); + + // OFF + an EXACT joint (last endpoint exactly (0,0)): the quad still builds closed, + // proving "off" means exact rather than broken. + ents[3].p1 = Vec2d(0, 0); + auto wires_exact = SketchEngine::entities_to_wires(ents, SketchPlane::XY()); + REQUIRE(wires_exact.size() == 1); + REQUIRE(edge_count(wires_exact[0]) == 4); + REQUIRE(wires_exact[0].Closed()); + + // Restore the default so test order cannot leak OFF into the other cases. + Slic3r::set_sketch_auto_close(true); +}