diff --git a/docs/CAD/design_tab.md b/docs/CAD/design_tab.md index 65f455bc3c..87ef8fe7a4 100644 --- a/docs/CAD/design_tab.md +++ b/docs/CAD/design_tab.md @@ -52,6 +52,11 @@ tool"*. It is also where a refusal explains itself. - Sketching happens on the face you clicked, first click. - A sketch whose entities form no wire **fails** instead of extruding a default box. A subtraction that removes nothing is reported as an error instead of a silent success. +- A loop whose ends all meet can still fail to bound one region. It may cross itself, or turn + straight back along itself at a joint (an arc leaving a line tangent to it but heading the + other way). The sketch tints such a loop red, marks the point, and says so once on the status + line. The extrude refuses a crossing loop with the same reason; before this check it produced a + body with no caps. --- diff --git a/src/libslic3r/CAD/SketchEngine.cpp b/src/libslic3r/CAD/SketchEngine.cpp index 8133de1174..812e6fa703 100644 --- a/src/libslic3r/CAD/SketchEngine.cpp +++ b/src/libslic3r/CAD/SketchEngine.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -881,6 +882,18 @@ std::vector sketch_open_ends(const std::vector& entities, return out; } +// A closed wire can still fail to bound a region: a loop that crosses itself, or one that +// doubles back along itself (a cusp, e.g. an arc leaving a line tangent to it but in the +// opposite direction). MakeFace reports success on both and the prism built from the face is +// an invalid solid with no caps. Refuse it here, with the reason, instead of shipping that. +static TopoDS_Face checked_profile_face(const TopoDS_Face& f) +{ + BRepCheck_Analyzer an(f); + if (!an.IsValid()) + throw std::runtime_error("the profile crosses or folds back on itself, so it does not bound one region"); + return f; +} + TopoDS_Face SketchEngine::wires_to_face(const std::vector& wires, const SketchPlane& plane) { @@ -900,7 +913,7 @@ TopoDS_Face SketchEngine::wires_to_face(const std::vector& wires, if (wires.size() == 1) { BRepBuilderAPI_MakeFace fm(wires[0]); if (!fm.IsDone()) throw std::runtime_error("sketch loop does not bound a face"); - return fm.Face(); + return checked_profile_face(fm.Face()); } // Two or more loops: build a face per wire and let the largest area be the outer @@ -956,7 +969,7 @@ TopoDS_Face SketchEngine::wires_to_face(const std::vector& wires, // while a holed SKETCH did not. ShapeFix_Face sff(fm.Face()); sff.FixOrientation(); - return sff.Face(); + return checked_profile_face(sff.Face()); } std::vector SketchEngine::mirror_entities( @@ -2107,6 +2120,153 @@ int sketch_entity_ends(const SketchEntity& e, std::pair return 0; } +namespace { + +// An arc's signed angular offset of `ang` from its start, within its sweep: true when the angle +// lies on the arc (with `tol` radians of slack at either end). +bool angle_on_arc(const SketchEntity& e, double ang, double tol) +{ + const double TWO_PI = 2.0 * M_PI; + const double sweep = e.end_angle - e.start_angle; + double d = (sweep >= 0.0) ? ang - e.start_angle : e.start_angle - ang; + d = std::fmod(d, TWO_PI); + if (d < 0.0) d += TWO_PI; + if (d > TWO_PI - tol) d -= TWO_PI; // just before the start counts as the start + return d >= -tol && d <= std::abs(sweep) + tol; +} + +// Every point where two lines/arcs meet (tangent contact included). Exact, no sampling. +void entity_intersections(const SketchEntity& A, const SketchEntity& B, std::vector& out) +{ + using T = SketchEntity::Type; + const double eps = 1e-9; + auto on_seg = [](const SketchEntity& L, const Vec2d& p) { + const Vec2d d = L.p1 - L.p0; + const double l2 = d.squaredNorm(); + if (l2 < 1e-24) return false; + const double t = (p - L.p0).dot(d) / l2; + return t >= -1e-9 && t <= 1.0 + 1e-9; + }; + if (A.type == T::Line && B.type == T::Line) { + const Vec2d r = A.p1 - A.p0, q = B.p1 - B.p0, w = B.p0 - A.p0; + const double den = r.x() * q.y() - r.y() * q.x(); + if (std::abs(den) < eps * r.norm() * q.norm()) { + // Parallel. Collinear overlap is a crossing wherever it is: report the overlap's + // nearest end, which the caller then tests against the shared joints. + if (std::abs(r.x() * w.y() - r.y() * w.x()) > 1e-9 * std::max(1.0, r.norm())) return; + for (const Vec2d& p : { B.p0, B.p1 }) if (on_seg(A, p)) out.push_back(p); + for (const Vec2d& p : { A.p0, A.p1 }) if (on_seg(B, p)) out.push_back(p); + return; + } + const double t = (w.x() * q.y() - w.y() * q.x()) / den; + const double u = (w.x() * r.y() - w.y() * r.x()) / den; + if (t >= -1e-9 && t <= 1.0 + 1e-9 && u >= -1e-9 && u <= 1.0 + 1e-9) out.push_back(A.p0 + t * r); + return; + } + if (A.type == T::Arc && B.type == T::Line) { entity_intersections(B, A, out); return; } + if (A.type == T::Line && B.type == T::Arc) { + const Vec2d d = A.p1 - A.p0, f = A.p0 - B.center; + const double a = d.squaredNorm(), b = 2.0 * f.dot(d), c = f.squaredNorm() - B.radius * B.radius; + if (a < 1e-24) return; + double disc = b * b - 4.0 * a * c; + if (disc < -1e-9 * a * B.radius * B.radius) return; + disc = std::sqrt(std::max(0.0, disc)); + for (double t : { (-b - disc) / (2.0 * a), (-b + disc) / (2.0 * a) }) { + if (t < -1e-9 || t > 1.0 + 1e-9) continue; + const Vec2d p = A.p0 + t * d; + if (angle_on_arc(B, std::atan2(p.y() - B.center.y(), p.x() - B.center.x()), 1e-9)) + out.push_back(p); + } + return; + } + if (A.type == T::Arc && B.type == T::Arc) { + const Vec2d dc = B.center - A.center; + const double dd = dc.norm(); + if (dd < 1e-12) { + // Concentric: they overlap only on the same circle, and then everywhere they share. + if (std::abs(A.radius - B.radius) > 1e-9) return; + for (const Vec2d& p : { B.p0, B.p1 }) + if (angle_on_arc(A, std::atan2(p.y() - A.center.y(), p.x() - A.center.x()), 1e-9)) out.push_back(p); + for (const Vec2d& p : { A.p0, A.p1 }) + if (angle_on_arc(B, std::atan2(p.y() - B.center.y(), p.x() - B.center.x()), 1e-9)) out.push_back(p); + return; + } + if (dd > A.radius + B.radius + 1e-9 || dd < std::abs(A.radius - B.radius) - 1e-9) return; + const double x = (dd * dd + A.radius * A.radius - B.radius * B.radius) / (2.0 * dd); + const double h = std::sqrt(std::max(0.0, A.radius * A.radius - x * x)); + const Vec2d u = dc / dd, m = A.center + x * u, n(-u.y(), u.x()); + for (const Vec2d& p : { Vec2d(m + h * n), Vec2d(m - h * n) }) { + if (angle_on_arc(A, std::atan2(p.y() - A.center.y(), p.x() - A.center.x()), 1e-9) && + angle_on_arc(B, std::atan2(p.y() - B.center.y(), p.x() - B.center.x()), 1e-9)) + out.push_back(p); + if (h < 1e-12) break; + } + } +} + +} // namespace + +bool sketch_loop_defect(const std::vector& ents, const std::vector& order, Vec2d& at) +{ + using T = SketchEntity::Type; + const size_t n = order.size(); + if (n < 2) return false; + for (int ei : order) + if (ei < 0 || ei >= int(ents.size()) || (ents[ei].type != T::Line && ents[ei].type != T::Arc)) + return false; + const double tol = std::max(1e-6, sketch_join_tol()); + + // Traversal direction of each entity: the end it shares with the NEXT one is where it + // finishes. start[k]/end[k] are the traversal's ends. + std::vector start(n), end(n); + std::vector rev(n, false); + for (size_t k = 0; k < n; ++k) { + const SketchEntity& e = ents[order[k]]; + const SketchEntity& nx = ents[order[(k + 1) % n]]; + const double to_next_p1 = std::min((e.p1 - nx.p0).norm(), (e.p1 - nx.p1).norm()); + const double to_next_p0 = std::min((e.p0 - nx.p0).norm(), (e.p0 - nx.p1).norm()); + rev[k] = to_next_p0 < to_next_p1; + start[k] = rev[k] ? e.p1 : e.p0; + end[k] = rev[k] ? e.p0 : e.p1; + } + auto tangent = [&](size_t k, bool at_end) { + const SketchEntity& e = ents[order[k]]; + Vec2d t; + if (e.type == T::Line) { + t = e.p1 - e.p0; + } else { + const double a = at_end ? (rev[k] ? e.start_angle : e.end_angle) + : (rev[k] ? e.end_angle : e.start_angle); + const double s = (e.end_angle >= e.start_angle) ? 1.0 : -1.0; + t = s * Vec2d(-std::sin(a), std::cos(a)); + } + if (rev[k]) t = -t; + const double l = t.norm(); + return l > 1e-15 ? Vec2d(t / l) : Vec2d(0, 0); + }; + + // Cusps: the curve arriving at a joint and the one leaving it point opposite ways. + for (size_t k = 0; k < n; ++k) { + const size_t j = (k + 1) % n; + if (tangent(k, true).dot(tangent(j, false)) < -0.9999) { at = end[k]; return true; } + } + + // Crossings: any contact between two entities away from the joints they share. + for (size_t i = 0; i < n; ++i) + for (size_t j = i + 1; j < n; ++j) { + std::vector hits; + entity_intersections(ents[order[i]], ents[order[j]], hits); + const bool next = (j == i + 1), prev = (i == 0 && j == n - 1); + for (const Vec2d& p : hits) { + bool joint = false; + if (next && (p - end[i]).norm() < tol) joint = true; + if (prev && (p - start[i]).norm() < tol) joint = true; + if (!joint) { at = p; return true; } + } + } + return false; +} + bool sketch_closest_ends(const SketchEntity& A, const SketchEntity& B, SketchPointRole& ra, SketchPointRole& rb, Vec2d& pa, Vec2d& pb) { diff --git a/src/libslic3r/CAD/SketchEngine.hpp b/src/libslic3r/CAD/SketchEngine.hpp index f433b90bce..785102c205 100644 --- a/src/libslic3r/CAD/SketchEngine.hpp +++ b/src/libslic3r/CAD/SketchEngine.hpp @@ -164,6 +164,15 @@ int sketch_entity_ends(const SketchEntity& e, std::pair bool sketch_closest_ends(const SketchEntity& A, const SketchEntity& B, SketchPointRole& ra, SketchPointRole& rb, Vec2d& pa, Vec2d& pb); +// Where a CLOSED loop fails to bound one region, although every joint meets: two of its +// entities touch somewhere other than a joint they share (the loop crosses itself), or a joint +// where the curve turns straight back along itself (a cusp: an arc leaving a line tangent to it +// but heading the other way). Either makes a face OCCT accepts and then builds an invalid solid +// from. `order` lists the loop's entity indices in traversal order, as the chainer found them. +// Lines and arcs are judged exactly; a loop holding any other kind is not judged (false). +// On true, `at` is the offending point in sketch coordinates. +bool sketch_loop_defect(const std::vector& ents, const std::vector& order, Vec2d& at); + // Why an entity-constraint pick is refused. The caller maps a reason to a localized string; // the planner itself stays translation-free. enum class ConstraintReject { diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.cpp b/src/slic3r/GUI/CAD/DesignSketchTool.cpp index 31beb1e407..1f581561e2 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.cpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.cpp @@ -889,6 +889,20 @@ void DesignSketchTool::resolve_live_drag(int dragged_ei, SketchPointRole dragged m_bad_dims.clear(); } if (on_solve_state) on_solve_state(m_dof, m_solve_ok, has); + announce_loop_defects(); +} + +// The red tint and marker on a loop that crosses or folds back explain nothing on their own, so +// the first time one appears the status line says what they mean. Once, on the transition: this +// runs on every solve, drags included. +void DesignSketchTool::announce_loop_defects() +{ + bool any = false; + for (const RegionLoop& r : region_loops(m_entities)) + if (r.defect) { any = true; break; } + if (any && !m_loop_defect_shown) + notify(_u8L("This profile crosses or folds back on itself at the red mark, so it does not bound one region")); + m_loop_defect_shown = any; } // ---- Onshape-style visual editing: feature grouping + handles ----------------- @@ -6757,6 +6771,12 @@ DesignSketchTool::region_loops(const std::vector& ents) const } if (best >= 0) regions[best].holes.push_back(int(i)); } + // Closed is not the same as bounding one region: the chainer only asks whether the ends + // meet. A loop that crosses itself, or turns straight back along itself, meets at every + // joint and still cannot be built — the kernel refuses a crossing, and a fold back is never + // what was meant. Say so here, where the region is drawn, instead of after an extrude. + for (RegionLoop& r : regions) + r.defect = sketch_loop_defect(ents, r.ents, r.defect_at); return regions; } @@ -9049,14 +9069,21 @@ void DesignSketchTool::render(GLCanvas3D& canvas) for (const RegionLoop& L : loops) for (int h : L.holes) if (h >= 0 && h < int(is_hole.size())) is_hole[h] = 1; + std::vector defects; for (size_t r = 0; r < loops.size(); ++r) { + if (loops[r].defect) defects.push_back(loops[r].defect_at); if (is_hole[r]) continue; std::vector> hp; for (int h : loops[r].holes) if (h >= 0 && h < int(loops.size())) hp.push_back(loops[h].poly); - draw_fill_holed(m_fill_model, loops[r].poly, hp, design_idle_face_color()); + // A loop that crosses or folds back is tinted red, not offered as a face. + draw_fill_holed(m_fill_model, loops[r].poly, hp, + loops[r].defect ? ColorRGBA(1.0f, 0.22f, 0.22f, 0.18f) : design_idle_face_color()); } glsafe(::glDisable(GL_BLEND)); + if (!defects.empty()) // and the place it goes wrong gets a screen-constant red marker + draw_vertices(m_vertex_model, defects, ColorRGBA(1.0f, 0.22f, 0.22f, 1.0f), + 5.0 / std::max(camera.get_zoom(), 1e-6)); } } @@ -9741,6 +9768,8 @@ DesignSketchTool::LoopReport DesignSketchTool::loop_report() const li.ents = r.ents; li.holes = r.holes; li.closed = true; + li.defect = r.defect; + li.defect_at = r.defect_at; // Analytic where the loop IS one closed curve; shoelace only where it is a chain. // region_loops hands back the render polyline, and a circle's is a 64-gon whose area is // 0.3% short — a number reported as "area" must not be the faceting error. diff --git a/src/slic3r/GUI/CAD/DesignSketchTool.hpp b/src/slic3r/GUI/CAD/DesignSketchTool.hpp index 60a7fa25dd..88c5e1fee9 100644 --- a/src/slic3r/GUI/CAD/DesignSketchTool.hpp +++ b/src/slic3r/GUI/CAD/DesignSketchTool.hpp @@ -523,6 +523,8 @@ public: std::vector holes; // indices into LoopReport::loops that this loop encloses bool closed{false}; double area{0.0}; // signed shoelace area of the loop polyline + bool defect{false}; // crosses or folds back on itself (see RegionLoop) + Vec2d defect_at{0, 0}; }; struct LoopReport { std::vector loops; @@ -701,6 +703,8 @@ private: bool selection_valid() const; // all selection indices in range void record_dimension_constraint(double v); // append the driving def for the selection void resolve_live(); // solve accumulated constraints on m_entities now + void announce_loop_defects(); // status line, once, when a loop starts crossing itself + bool m_loop_defect_shown{false}; // Drag-aware re-solve: pins the dragged point at its current coord and lets the // solver move the rest (Slvs dragged[]). Used live while a point grab is active. void resolve_live_drag(int dragged_ei, SketchPointRole dragged_role); @@ -965,6 +969,10 @@ private: std::vector poly; std::vector ents; std::vector holes; // indices into the same vector; one nesting level + // Closed, but crossing itself or turning straight back somewhere (sketch_loop_defect): + // it does not bound one region, whatever the chainer says. defect_at names the place. + bool defect{false}; + Vec2d defect_at{0, 0}; }; std::vector region_loops(const std::vector& ents) const; // Index of the closed region containing plane-point p (point-in-polygon), or -1. diff --git a/src/slic3r/GUI/CAD/McpControl.cpp b/src/slic3r/GUI/CAD/McpControl.cpp index 90148abee1..8f9f161d76 100644 --- a/src/slic3r/GUI/CAD/McpControl.cpp +++ b/src/slic3r/GUI/CAD/McpControl.cpp @@ -1500,9 +1500,13 @@ json sketch_report(DesignSketchTool& t) } json open_ends = json::array(); for (const Vec2d& p : rep.open_ends) open_ends.push_back(json::array({p.x(), p.y()})); - // A profile is buildable when at least one loop closed and nothing is left dangling. - const bool buildable = !rep.loops.empty() && rep.open_ends.empty(); - return json{{"closed_loops", loops}, {"open_ends", open_ends}, {"buildable", buildable}}; + // A profile is buildable when at least one loop closed, nothing is left dangling and no loop + // crosses or folds back on itself. + json defects = json::array(); + for (const auto& l : rep.loops) + if (l.defect) defects.push_back(json::array({l.defect_at.x(), l.defect_at.y()})); + const bool buildable = !rep.loops.empty() && rep.open_ends.empty() && defects.empty(); + return json{{"closed_loops", loops}, {"open_ends", open_ends}, {"defects", defects}, {"buildable", buildable}}; } json action_sketch_describe(DesignPanel* panel, const json& params) diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index 135905b116..5ce265e06d 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -7987,6 +7987,98 @@ TEST_CASE("dressup: four chamfer ids captured up-front drift as earlier chamfers REQUIRE(ok); } +// A closed chain that fails to bound one region although every joint meets, recorded on the rig: +// a top line, an arc that leaves its end heading back over it (crossing it again 2.5 mm on), and +// a 241 deg clockwise arc that leaves a 0.28 mm line tangent to it but the other way (a cusp). +// MakeFace accepted it and the prism came out as walls with no caps. The loop analysis must name +// the point, and the extrude must refuse it with a reason; the same arcs swept the other way round +// are an ordinary profile. +namespace { +enum class Fold { None, Crossing, Cusp, Both }; +std::vector folding_profile(Fold fold) +{ + auto line = [](Vec2d a, Vec2d b) { SketchEntity e; e.type = SketchEntity::Type::Line; e.p0 = a; e.p1 = b; return e; }; + auto arc = [](Vec2d a, Vec2d b, Vec2d c, double r, double a0, double a1) { + SketchEntity e; e.type = SketchEntity::Type::Arc; e.p0 = a; e.p1 = b; e.center = c; e.radius = r; + e.start_angle = a0; e.end_angle = a1; return e; }; + const Vec2d A(3.745882, 40.003810), B(54.449392, 40.003810), C(52.800871, -34.610853), + D(7.730833, -51.667501), E(-32.425978, -102.110463), F(-48.295291, -47.364094), + G(-48.017198, -47.364094); + const double TWO_PI = 2.0 * M_PI; + const bool cross = fold == Fold::Crossing || fold == Fold::Both; + const bool cusp = fold == Fold::Cusp || fold == Fold::Both; + // B->C: clockwise it bulges out to the right; counter-clockwise (recorded) it leaves B back + // over the top line and crosses it. G->A, centred right above G so it is tangent to F->G: + // counter-clockwise it carries on from F->G; clockwise (recorded) it turns straight back. + const double b0 = 1.536417, b1 = 4.702588; + const double g0 = -M_PI / 2.0, g1 = std::atan2(A.y() - 11.653958, A.x() + 48.017198); + return { + line(A, B), + arc(B, C, Vec2d(53.166645, 2.706608), 37.319254, b0, cross ? b1 : b1 - TWO_PI), + line(C, D), + line(D, E), + line(E, F), + line(F, G), + arc(G, A, Vec2d(-48.017198, 11.653958), 59.018053, g0, cusp ? g1 - TWO_PI : g1), + }; +} +} // namespace + +TEST_CASE("loop analysis: a closed loop that crosses or folds back names the point", "[sketch]") +{ + const std::vector order{ 0, 1, 2, 3, 4, 5, 6 }; + Vec2d at; + REQUIRE_FALSE(sketch_loop_defect(folding_profile(Fold::None), order, at)); + + REQUIRE(sketch_loop_defect(folding_profile(Fold::Cusp), order, at)); + INFO("cusp at " << at.transpose()); + REQUIRE((at - Vec2d(-48.017198, -47.364094)).norm() < 1e-3); // the joint G + + REQUIRE(sketch_loop_defect(folding_profile(Fold::Crossing), order, at)); + INFO("crossing at " << at.transpose()); + REQUIRE(std::abs(at.y() - 40.003810) < 1e-3); // on the top line... + REQUIRE(at.x() > 50.0); // ...a little before B + REQUIRE(at.x() < 54.4); + + // Traversal order does not matter: the same loop walked backwards. + const std::vector back{ 6, 5, 4, 3, 2, 1, 0 }; + REQUIRE(sketch_loop_defect(folding_profile(Fold::Cusp), back, at)); + REQUIRE_FALSE(sketch_loop_defect(folding_profile(Fold::None), back, at)); + + // Ordinary joints are not defects: a fillet-like tangent join and a straight continuation. + SketchEntity l1; l1.type = SketchEntity::Type::Line; l1.p0 = Vec2d(0, 0); l1.p1 = Vec2d(10, 0); + SketchEntity l2; l2.type = SketchEntity::Type::Line; l2.p0 = Vec2d(10, 0); l2.p1 = Vec2d(20, 0); + SketchEntity a1; a1.type = SketchEntity::Type::Arc; a1.center = Vec2d(20, 5); a1.radius = 5; + a1.start_angle = -M_PI / 2; a1.end_angle = M_PI / 2; a1.p0 = Vec2d(20, 0); a1.p1 = Vec2d(20, 10); + SketchEntity l3; l3.type = SketchEntity::Type::Line; l3.p0 = Vec2d(20, 10); l3.p1 = Vec2d(0, 10); + SketchEntity l4; l4.type = SketchEntity::Type::Line; l4.p0 = Vec2d(0, 10); l4.p1 = Vec2d(0, 0); + REQUIRE_FALSE(sketch_loop_defect({ l1, l2, a1, l3, l4 }, { 0, 1, 2, 3, 4 }, at)); +} + +TEST_CASE("extrude: a profile that folds back on itself is refused with a reason", "[CadDocument][sketch]") +{ + // A cusp alone still makes a face OCCT calls valid (the spike it leaves is 0.009 mm wide), so + // the kernel builds it; the viewport's loop analysis is what flags it. A crossing does not. + for (Fold f : { Fold::Crossing, Fold::Both }) { + CadDocument bad; + const int sk = bad.add_sketch_entities(folding_profile(f), SketchPlane::XY(), "Sketch"); + bad.add_extrude(sk, 69.42, false, BooleanMode::New, "Extrude"); + INFO("fold kind " << int(f) << ": " << bad.error); + REQUIRE_FALSE(bad.recompute()); + REQUIRE(bad.error.find("folds back") != std::string::npos); + } + + CadDocument good; + const int sk2 = good.add_sketch_entities(folding_profile(Fold::None), SketchPlane::XY(), "Sketch"); + good.add_extrude(sk2, 69.42, false, BooleanMode::New, "Extrude"); + REQUIRE(good.recompute()); + REQUIRE(good.error.empty()); + REQUIRE(good.bodies.size() == 1); + const auto mp = GeometryEngine::mass_properties(good.bodies[0].shape); + REQUIRE(mp.is_solid); + REQUIRE(mp.volume > 0.0); +} + // Several picked edges dressed by ONE feature: every id is resolved against the same body, so // capturing them up-front is correct here (unlike the chain of single-edge features above). TEST_CASE("dressup: one fillet on four picked edges equals the Top face group", "[CadDocument][dressup]")