From a043979f44fc43814a7b50d1003245d0f402f972 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 17:11:30 +0000 Subject: [PATCH] CAD kernel: fix the sketch solver, trim/offset/mirror, and history references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sketch layer - Trim/extend of an arc (and a circle opened into an arc) rewrites p0/p1 from the new angles; the wire builder, the solver and snapping read them. - Tangency binds the arc end that touches the line (or the other arc) instead of always the start, so a fillet tangent to both legs solves; circle-circle / circle-arc tangency uses the centre distance instead of CURVE_CURVE_TANGENT, which aborts on a circle. - Point-on-line distances and circle-line tangency keep the side the geometry is on; a point on an arc's rim uses PT_ON_CIRCLE. - The partitioned solve keeps constraints onto the origin/axes and counts free entities' DOF. - Zero-radius circles get no solver primitive and build no wire; constraints the solver cannot apply are reported in SketchSolveResult::skipped. - EllipseArc: mirror no longer yields the complement; after a solve its angles and ends are re-derived; on the XZ plane it is no longer built mirrored. - Offset: a circle follows the "+d = left of travel" rule (it shrinks, like a CCW arc chain); chains are joined at the weld tolerance. Bridge end pole fixed (G1, no cusp). Negative-scale transforms keep arcs and ellipses on their ends. Inference tolerances aligned with the weld. Model layer - Body references follow the body across delete / reorder / hide (resolved by the feature that made it); datum-plane ordinals are re-pointed; an index past the end is an error, not "the last body". New set_feature_enabled(). A move that puts a consumer above its input is refused. - Threads made from now on read thread_radius as the nominal major radius (internal: bore to minor, groove to major; external: groove cut into the rod); older recipes build as before. Bad thread parameters say why. Circular patterns span their angle end to end (new ones); add_pattern pivots on the modeling origin. - clear() drops variables; expression fields the GUI offers are bindable (thread_diameter, helix_*, thicken_thickness, ...); deg()/rad() in expressions; the recipe saves the modeling origin and body colours; names/colours follow bodies by identity. - Boolean and dress-up failures throw instead of returning the input; one produces_body(); hole standards corrected (82° inch countersinks, UNC names, #10-24); legacy profile solve validates indices and writes back only on success; v4 recipes read with a frozen field list. Tests cover each of the above. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QK4VgguuCAk2hZLWgcjJb9 --- src/libslic3r/CAD/CadDocument.cpp | 625 ++++++++++++++++++++------ src/libslic3r/CAD/CadDocument.hpp | 99 +++- src/libslic3r/CAD/GeometryEngine.cpp | 34 +- src/libslic3r/CAD/GeometryEngine.hpp | 7 +- src/libslic3r/CAD/SketchEngine.cpp | 108 +++-- src/libslic3r/CAD/SketchEngine.hpp | 12 +- src/libslic3r/CAD/SketchInference.cpp | 27 +- src/libslic3r/CAD/SketchInference.hpp | 8 +- src/libslic3r/CAD/SketchSolver.cpp | 161 ++++++- src/libslic3r/CAD/SketchSolver.hpp | 4 + src/libslic3r/CAD/ThreadStandards.hpp | 2 + tests/libslic3r/test_caddocument.cpp | 370 +++++++++++++-- tests/libslic3r/test_sketchedit.cpp | 113 ++++- 13 files changed, 1291 insertions(+), 279 deletions(-) diff --git a/src/libslic3r/CAD/CadDocument.cpp b/src/libslic3r/CAD/CadDocument.cpp index ea20ffd966..22b84e0099 100644 --- a/src/libslic3r/CAD/CadDocument.cpp +++ b/src/libslic3r/CAD/CadDocument.cpp @@ -69,11 +69,14 @@ #include #include #include +#include +#include #include #include #include #include +#include #include namespace Slic3r { @@ -153,6 +156,21 @@ static TopoDS_Wire make_helix_spine(const CadFeature& f, std::string& err) // - internal: the V is CUT from the wall -> a sunken helical groove. The cut MUST // go outward into the wall to be visible; an inward V (the old behaviour) only // sweeps already-empty bore space and removes nothing. +// The V between two radii: its base (the axial edge) at `base_r`, its apex at `apex_r`. The +// base is meant to sit INSIDE the material it is fused to or cut from, the apex out in the space +// the thread reaches into. make_thread_profile below is the legacy layout. +static TopoDS_Wire make_thread_v(const gp_Pnt& origin, const gp_Dir& xdir, const gp_Dir& zdir, + double base_r, double apex_r, double pitch) +{ + gp_Vec vx(xdir), vz(zdir); + const double half = 0.42 * pitch; // < pitch/2: adjacent turns must not touch (see below) + gp_Pnt top (origin.XYZ() + (vx * base_r).XYZ() + (vz * ( half)).XYZ()); + gp_Pnt bot (origin.XYZ() + (vx * base_r).XYZ() + (vz * (-half)).XYZ()); + gp_Pnt apex(origin.XYZ() + (vx * apex_r).XYZ()); + BRepBuilderAPI_MakePolygon poly(top, bot, apex, Standard_True); + return poly.Wire(); +} + static TopoDS_Wire make_thread_profile(const gp_Pnt& origin, const gp_Dir& xdir, const gp_Dir& zdir, double radius, double pitch, double depth, bool internal) @@ -187,7 +205,10 @@ static TopoDS_Wire make_thread_profile(const gp_Pnt& origin, const gp_Dir& xdir, // Evaluate an arithmetic expression to a double. Supports + - * /, parentheses, // unary minus, decimal literals, and identifiers resolved via `vars`. Functions: -// sqrt, abs, sin, cos, tan (degrees), min, max, and the constant `pi`. +// sqrt, abs, sin, cos, tan, min, max, deg, rad, and the constant `pi`. +// ANGLES ARE DEGREES — every angle field in a feature is, so sin(30) is 0.5. `pi` is the number +// (for 2 * pi * r); to use it as an angle convert it: sin(deg(pi / 2)) = 1. deg() turns radians +// into degrees and rad() degrees into radians. // Throws std::runtime_error on any parse or lookup error, div-by-zero, bad arity. static double eval_expr(const std::string& src, const std::map& vars) { @@ -344,6 +365,8 @@ static double eval_expr(const std::string& src, const std::map& variables) std::string id = expr.substr(start, i - start); // skip "pi" and function names — they're built-ins, not variables if (id == "pi" || id == "sqrt" || id == "abs" || id == "sin" || id == "cos" || - id == "tan" || id == "min" || id == "max") continue; + id == "tan" || id == "min" || id == "max" || id == "deg" || id == "rad") continue; if (!variables.count(id)) throw std::runtime_error("unknown identifier: " + id); scope[id] = resolve(id); continue; @@ -411,11 +434,17 @@ evaluate_variables(const std::map& variables) static void assign_field(CadFeature& f, const std::string& field, double value) { // double fields (alphabetical) + if (field == "bool_tolerance") { f.bool_tolerance = value; return; } + if (field == "cut_offset") { f.cut_offset = value; return; } if (field == "distance") { f.distance = value; return; } if (field == "distance2") { f.distance2 = value; return; } if (field == "draft_angle") { f.draft_angle = value; return; } if (field == "dressup_size") { f.dressup_size = value; return; } if (field == "height") { f.height = value; return; } + if (field == "helix_height") { f.helix_height = value; return; } + if (field == "helix_pitch") { f.helix_pitch = value; return; } + if (field == "helix_radius") { f.helix_radius = value; return; } + if (field == "helix_taper_deg") { f.helix_taper_deg = value; return; } if (field == "hole_cbore_diameter") { f.hole_cbore_diameter = value; return; } if (field == "hole_cbore_depth") { f.hole_cbore_depth = value; return; } if (field == "hole_csink_angle") { f.hole_csink_angle = value; return; } @@ -426,28 +455,55 @@ static void assign_field(CadFeature& f, const std::string& field, double value) if (field == "hole_y") { f.hole_y = value; return; } if (field == "import_scale_x") { f.import_scale_x = value; return; } if (field == "import_scale_y") { f.import_scale_y = value; return; } + if (field == "mate_angle") { f.mate_angle = value; return; } + if (field == "mate_offset") { f.mate_offset = value; return; } if (field == "pattern_angle") { f.pattern_angle = value; return; } if (field == "pattern_spacing") { f.pattern_spacing = value; return; } if (field == "plane_angle_tilt") { f.plane_angle_tilt = value; return; } if (field == "plane_offset") { f.plane_offset = value; return; } + if (field == "plane_u_size") { f.plane_u_size = value; return; } + if (field == "plane_v_size") { f.plane_v_size = value; return; } if (field == "radius") { f.radius = value; return; } if (field == "revolve_angle") { f.revolve_angle = value; return; } if (field == "rib_depth") { f.rib_depth = value; return; } if (field == "rib_thickness") { f.rib_thickness = value; return; } if (field == "shell_thickness") { f.shell_thickness = value; return; } if (field == "taper_deg") { f.taper_deg = value; return; } + if (field == "thicken_thickness") { f.thicken_thickness = value; return; } if (field == "thread_depth") { f.thread_depth = value; return; } if (field == "thread_height") { f.thread_height = value; return; } if (field == "thread_pitch") { f.thread_pitch = value; return; } if (field == "thread_radius") { f.thread_radius = value; return; } + // The Thread card shows a DIAMETER (the size a thread is named by: M6, 1/4"), so that is + // the name it offers for binding; thread_radius stays for recipes that already use it. + if (field == "thread_diameter") { f.thread_radius = 0.5 * value; return; } if (field == "thread_x") { f.thread_x = value; return; } if (field == "thread_y") { f.thread_y = value; return; } if (field == "width") { f.width = value; return; } + if (field == "xf_angle_deg") { f.xf_angle_deg = value; return; } // int fields (rounded) if (field == "pattern_count") { f.pattern_count = (int)std::lround(value); return; } throw std::runtime_error("unknown parameter: " + field); } +bool CadDocument::produces_body(CadFeatureType t) +{ + switch (t) { + case CadFeatureType::Sketch: case CadFeatureType::Helix: case CadFeatureType::Plane: + case CadFeatureType::Axis: case CadFeatureType::CoordSys: case CadFeatureType::Project: + return false; + default: + return true; + } +} + +bool CadDocument::is_bindable_field(const std::string& field) +{ + CadFeature probe; + try { assign_field(probe, field, 0.0); return true; } + catch (const std::exception&) { return false; } +} + // --------------------------------------------------------------------------- // Sample a sketch entity at parameter t in [0,1]. Handles Line, Arc, BSpline (cubic, 4 poles). @@ -758,29 +814,46 @@ bool CadDocument::solve_sketch_feature(int index) if (f.constraints.empty()) return true; + // Legacy point-profile path. Every index is checked before the solver sees it: an unset + // (-1) or stale index made get_point/residuals read m_vars[2*-1]. A constraint type this + // solver cannot express is a failure, not a silent no-op, and the points are written back + // only when the solve succeeded — the entity path's rule (SketchSolver.cpp) too. + const int np = int(f.profile.points.size()); + auto ok_idx = [np](std::initializer_list ids) { + for (int i : ids) if (i < 0 || i >= np) return false; + return true; + }; SketchConstraints sc; for (const Vec2d& p : f.profile.points) sc.add_point(p.x(), p.y()); for (const SketchConstraintDef& c : f.constraints) { + bool valid = true; switch (c.type) { - case SketchConstraintType::Fix: sc.fix_point(c.a); break; - case SketchConstraintType::Coincident: sc.coincident(c.a, c.b); break; - case SketchConstraintType::Horizontal: sc.horizontal(c.a, c.b); break; - case SketchConstraintType::Vertical: sc.vertical(c.a, c.b); break; - case SketchConstraintType::Distance: sc.distance(c.a, c.b, c.value); break; - case SketchConstraintType::LockX: sc.lock_x(c.a, c.value); break; - case SketchConstraintType::LockY: sc.lock_y(c.a, c.value); break; - case SketchConstraintType::EqualLength: sc.equal_length(c.a, c.b, c.c, c.d); break; - case SketchConstraintType::Parallel: sc.parallel(c.a, c.b, c.c, c.d); break; - case SketchConstraintType::Perpendicular:sc.perpendicular(c.a, c.b, c.c, c.d); break; + case SketchConstraintType::Fix: if ((valid = ok_idx({c.a}))) sc.fix_point(c.a); break; + case SketchConstraintType::Coincident: if ((valid = ok_idx({c.a, c.b}))) sc.coincident(c.a, c.b); break; + case SketchConstraintType::Horizontal: if ((valid = ok_idx({c.a, c.b}))) sc.horizontal(c.a, c.b); break; + case SketchConstraintType::Vertical: if ((valid = ok_idx({c.a, c.b}))) sc.vertical(c.a, c.b); break; + case SketchConstraintType::Distance: if ((valid = ok_idx({c.a, c.b}))) sc.distance(c.a, c.b, c.value); break; + case SketchConstraintType::LockX: if ((valid = ok_idx({c.a}))) sc.lock_x(c.a, c.value); break; + case SketchConstraintType::LockY: if ((valid = ok_idx({c.a}))) sc.lock_y(c.a, c.value); break; + case SketchConstraintType::EqualLength: if ((valid = ok_idx({c.a, c.b, c.c, c.d}))) sc.equal_length(c.a, c.b, c.c, c.d); break; + case SketchConstraintType::Parallel: if ((valid = ok_idx({c.a, c.b, c.c, c.d}))) sc.parallel(c.a, c.b, c.c, c.d); break; + case SketchConstraintType::Perpendicular:if ((valid = ok_idx({c.a, c.b, c.c, c.d}))) sc.perpendicular(c.a, c.b, c.c, c.d); break; + // The legacy solver implements these too; they used to be dropped without a word. + case SketchConstraintType::Midpoint: if ((valid = ok_idx({c.a, c.b, c.c}))) sc.midpoint(c.a, c.b, c.c); break; + case SketchConstraintType::Symmetric: if ((valid = ok_idx({c.a, c.b, c.c, c.d}))) sc.symmetric(c.a, c.b, c.c, c.d); break; + case SketchConstraintType::Angle: if ((valid = ok_idx({c.a, c.b, c.c, c.d}))) sc.angle(c.a, c.b, c.c, c.d, c.value); break; + case SketchConstraintType::PointOnLine: if ((valid = ok_idx({c.a, c.b, c.c}))) sc.point_line_distance(c.a, c.b, c.c, c.value); break; + default: valid = false; break; // entity-only types } + if (!valid) return false; } - const bool ok = sc.solve(); + if (!sc.solve()) return false; for (size_t i = 0; i < f.profile.points.size(); ++i) f.profile.points[i] = sc.get_point(int(i)); - return ok; + return true; } int CadDocument::add_extrude(int sketch_ref, double distance, bool symmetric, @@ -893,34 +966,38 @@ int CadDocument::add_hole(double diameter, double depth, bool through, return int(features.size()) - 1; } -// Hole standards lookup table (representative ISO 273 medium / ISO 4762 / ANSI unified). -struct HoleStdEntry { const char* desig; double clearance; double cbore_d; double cbore_depth; double csink_d; }; +// Hole standards: clearance hole, counterbore for a socket head cap screw, countersink for a +// flat head screw. Metric: ISO 273 medium clearance, DIN 974-1 counterbores for ISO 4762 (depth +// = head height + ~0.4 mm), 90° countersinks sized to the ISO 10642 head. Inch: ASME B18.3 / +// B18.6.3 — the counterbore is the head plus 1/32", the countersink is 82°, the angle an inch +// flat head is made with (a 90° seat under an 82° head bears only on its rim). +// Designations are the ones ThreadStandards uses ("1/4-20 UNC"); the bare form without the +// series ("1/4-20") is still accepted, since recipes and scripts were written with it. +struct HoleStdEntry { const char* desig; double clearance; double cbore_d; double cbore_depth; + double csink_d; double csink_angle; }; static const HoleStdEntry kHoleStdTable[] = { - {"M3", 3.4, 6.0, 3.4, 6.3}, - {"M4", 4.5, 8.0, 4.4, 8.4}, - {"M5", 5.5, 10.0, 5.4, 10.4}, - {"M6", 6.6, 11.0, 6.8, 12.6}, - {"M8", 9.0, 15.0, 8.8, 17.3}, - {"M10", 11.0, 18.0, 11.0, 20.0}, - {"#6-32", 3.7, 8.8, 4.2, 8.7}, - {"#8-32", 4.4, 9.9, 5.1, 10.2}, - {"1/4-20", 6.9, 14.4, 7.2, 14.7}, - {"5/16-18", 8.8, 17.0, 8.2, 17.3}, - {"3/8-16", 10.5, 19.6, 9.5, 19.8}, + {"M3", 3.4, 6.5, 3.4, 6.7, 90}, + {"M4", 4.5, 8.0, 4.4, 9.0, 90}, + {"M5", 5.5, 10.0, 5.4, 11.3, 90}, + {"M6", 6.6, 11.0, 6.4, 13.4, 90}, + {"M8", 9.0, 15.0, 8.6, 17.9, 90}, + {"M10", 11.0, 18.0, 10.6, 22.4, 90}, + {"#6-32 UNC", 3.7, 6.4, 3.8, 7.1, 82}, + {"#8-32 UNC", 4.5, 7.9, 4.6, 8.4, 82}, + {"#10-24 UNC", 5.1, 9.5, 5.3, 9.8, 82}, + {"1/4-20 UNC", 6.8, 11.1, 6.9, 12.9, 82}, + {"5/16-18 UNC", 8.4, 13.5, 8.6, 16.1, 82}, + {"3/8-16 UNC", 10.1, 15.9, 10.2, 19.4, 82}, }; -static bool hole_std_lookup(const std::string& desig, double& clearance, - double& cbore_d, double& cbore_depth, double& csink_d) +static const HoleStdEntry* hole_std_lookup(const std::string& desig) { for (const auto& e : kHoleStdTable) { - if (e.desig == desig) { - clearance = e.clearance; - cbore_d = e.cbore_d; - cbore_depth = e.cbore_depth; - csink_d = e.csink_d; - return true; - } + const std::string d(e.desig); + if (d == desig) return &e; + const size_t sp = d.find(' '); + if (sp != std::string::npos && d.compare(0, sp, desig) == 0 && desig.size() == sp) return &e; } - return false; + return nullptr; } int CadDocument::add_hole_styled(double diameter, double depth, bool through, @@ -952,11 +1029,11 @@ int CadDocument::add_hole_standard(const std::string& designation, int style, bo double depth, double x, double y, const SketchPlane& plane, const std::string& name) { - double clearance, cbore_d, cbore_depth, csink_d; - if (!hole_std_lookup(designation, clearance, cbore_d, cbore_depth, csink_d)) + const HoleStdEntry* e = hole_std_lookup(designation); + if (e == nullptr) throw std::runtime_error("unknown hole standard \"" + designation + "\""); - return add_hole_styled(clearance, depth, through, x, y, plane, style, - cbore_d, cbore_depth, csink_d, 90, designation, name); + return add_hole_styled(e->clearance, depth, through, x, y, plane, style, + e->cbore_d, e->cbore_depth, e->csink_d, e->csink_angle, e->desig, name); } int CadDocument::add_thread(double radius, double pitch, double height, double depth, @@ -974,6 +1051,7 @@ int CadDocument::add_thread(double radius, double pitch, double height, double d f.thread_internal = internal; f.thread_x = x; f.thread_y = y; + f.thread_major_nominal = true; // `radius` is the major (nominal) radius features.push_back(f); return int(features.size()) - 1; } @@ -1048,7 +1126,13 @@ int CadDocument::add_pattern(bool circular, int count, double spacing, int dir, f.pattern_spacing = spacing; f.pattern_dir = dir; f.pattern_angle = angle_deg; + f.pattern_inclusive = true; f.target_body = target_body; + // The pattern's plane — its linear axes and the circular pattern's pivot — defaults to the + // XY plane through the MODELING origin (the bed centre), like every other default plane in + // the tab. Left at SketchPlane::XY() it pivoted about the bed's corner. + f.plane = SketchPlane::XY(); + f.plane.origin += modeling_origin; features.push_back(f); return int(features.size()) - 1; } @@ -1518,11 +1602,19 @@ std::vector> CadDocument::resolve_datum_plan // Resolve base reference plane. The default XY/XZ/YZ planes pass through the modeling // origin (bed centre); datum bases (>=3) are already in world coords from earlier passes. SketchPlane base; + bool base_missing = false; if (f.plane_base == 1) { base = SketchPlane::XZ(); base.origin += modeling_origin; } else if (f.plane_base == 2) { base = SketchPlane::YZ(); base.origin += modeling_origin; } else if (f.plane_base >= 3) { const int di = f.plane_base - 3; if (di < int(out.size())) base = out[di].second; + else { + // The datum it was built on is gone or hidden. Fall back to XY through the + // modeling origin like the other defaults, and SAY so in the plane's name — + // this list is what the GUI shows — instead of quietly becoming world XY. + base = SketchPlane::XY(); base.origin += modeling_origin; + base_missing = true; + } } else { base = SketchPlane::XY(); base.origin += modeling_origin; } @@ -1658,7 +1750,7 @@ std::vector> CadDocument::resolve_datum_plan } - out.emplace_back(f.name, result); + out.emplace_back(base_missing ? f.name + " (base plane missing)" : f.name, result); } return out; } @@ -1881,8 +1973,14 @@ void CadDocument::clear() display_mesh = TriangleMesh{}; display_body_meshes.clear(); display_tri_face.clear(); + display_tri_body.clear(); + origin_from_recipe = false; + // Variables are document state like the features: left behind, the previous project's + // variables were written into the next project's recipe. + variables.clear(); error.clear(); mate_conflicts.clear(); + ++topo_generation; // every face/edge id handed out before this is stale now // A cleared document is a fresh start with no history. m_undo.clear(); m_redo.clear(); @@ -1926,6 +2024,146 @@ bool CadDocument::redo() return true; } + +// A feature's body index: -1 means "the last body", anything else must exist. An index past +// the end used to fall back to the last body in silence, so a feature whose body had gone away +// went on to work on a different one. +static int pick_body(int ref, int nb) +{ + if (ref == -1) return nb - 1; + if (ref < 0 || ref >= nb) + throw std::runtime_error("a feature works on body " + std::to_string(ref + 1) + + ", which does not exist at that point in the history"); + return ref; +} + +// ---- body and datum-plane references across a change of history --------------------------- +// +// A feature names a body by its INDEX and a datum plane by its ORDINAL (3 + N = the Nth enabled +// Plane feature). Both are positions, and a position only means something against the history +// it was taken in: delete, reorder or hide a feature that makes a body or a datum plane and every +// later reference silently lands on a different one. The fix-up for features[] indices +// (for_each_feature_ref) never covered these two bases. + +// Every field holding a BODY index, and which list it indexes: `true` = the bodies as they stand +// while the feature is applied (recompute's running list), `false` = the finished list (datums +// are resolved against it afterwards). +template +static void for_each_body_ref(CadFeature& f, Visit&& visit) +{ + visit(f.target_body, true); + visit(f.bool_tool_body, true); + visit(f.cut_face_body, true); + visit(f.project_source_body, true); + visit(f.coordsys_body, false); + visit(f.axis_body, false); + visit(f.plane_face_body, false); + visit(f.plane_face2_body, false); + visit(f.plane_edge_body, false); + visit(f.plane_edge2_body, false); +} +static constexpr int kBodyRefCount = 10; +// A body reference whose body was made by a feature that has since been deleted or hidden. +static constexpr int kBodyGone = -1000; + +// Identity of bodies[i]: the feature that made it, and which of that feature's bodies it is. +static CadFeature::BodyId body_id_of(const std::vector& v, int i) +{ + CadFeature::BodyId id; + if (i < 0 || i >= int(v.size())) return id; + id.src = v[i].source_feature; + for (int j = 0; j < i; ++j) + if (v[j].source_feature == id.src) ++id.ord; + return id; +} +static int find_body(const std::vector& v, const CadFeature::BodyId& id) +{ + int ord = 0; + for (int i = 0; i < int(v.size()); ++i) + if (v[i].source_feature == id.src && ord++ == id.ord) return i; + return -1; +} + +// Re-point `f`'s body fields of one kind (`running`) from the identities recorded last time, +// then record the identities they resolve to now. Throws when a pending reference cannot be +// found: the body it named is gone, and working on some other body instead is the bug this +// exists to prevent. +static void settle_body_refs(CadFeature& f, const std::vector& v, bool running) +{ + if (f.body_ref_ids.size() != size_t(kBodyRefCount)) f.body_ref_ids.assign(kBodyRefCount, {}); + int k = 0; + for_each_body_ref(f, [&](int& ref, bool kind) { + CadFeature::BodyId& id = f.body_ref_ids[k++]; + if (kind != running) return; + if (ref == kBodyGone) + throw std::runtime_error("\"" + f.name + "\" works on a body made by a feature that was deleted or hidden"); + if (f.body_refs_pending && id.src >= 0) { + const int i = find_body(v, id); + if (i < 0) + throw std::runtime_error("\"" + f.name + "\" works on a body that does not exist at that point in the history"); + ref = i; + } + id = ref >= 0 ? body_id_of(v, ref) : CadFeature::BodyId{}; + }); +} + +// Datum-plane ordinals: plane_base, axis_plane_a, axis_plane_b hold 3 + N. +template +static void for_each_datum_ref(CadFeature& f, Visit&& visit) +{ + visit(f.plane_base); + visit(f.axis_plane_a); + visit(f.axis_plane_b); +} + +// Prepare every reference for a change of history in which feature `i` ends up at +// `map_feature(i)` (-1 = removed) and enabled as `enabled_after(i)`. Runs on the OLD history and +// writes into `features` in the old order; the caller then performs the change. +static void stage_history_change(std::vector& features, + const std::function& map_feature, + const std::function& enabled_after) +{ + const int n = int(features.size()); + auto alive = [&](int i) { return i >= 0 && i < n && map_feature(i) >= 0 && enabled_after(i); }; + + // Datum planes: ordinal -> old feature index, then -> new ordinal. + std::vector planes_before; + for (int i = 0; i < n; ++i) + if (features[i].type == CadFeatureType::Plane && features[i].enabled) planes_before.push_back(i); + std::vector> after; // (new index, old index) of planes enabled after + for (int i = 0; i < n; ++i) + if (features[i].type == CadFeatureType::Plane && alive(i)) after.emplace_back(map_feature(i), i); + std::sort(after.begin(), after.end()); + auto new_ordinal = [&](int old_feature) { + for (int k = 0; k < int(after.size()); ++k) + if (after[k].second == old_feature) return k; + return -1; + }; + + for (int j = 0; j < n; ++j) { + if (map_feature(j) < 0) continue; + CadFeature& f = features[j]; + for_each_datum_ref(f, [&](int& ref) { + if (ref < 3) return; + const int N = ref - 3; + const int oldf = (N < int(planes_before.size())) ? planes_before[N] : -1; + const int nn = oldf >= 0 ? new_ordinal(oldf) : -1; + // A plane that is gone or hidden stays pointed PAST the end, where resolution + // reports it, rather than quietly sliding onto the next plane. + ref = nn >= 0 ? 3 + nn : 3 + int(after.size()) + 1000; + }); + if (f.body_ref_ids.size() != size_t(kBodyRefCount)) continue; // never resolved: nothing to follow + int k = 0; + for_each_body_ref(f, [&](int& ref, bool) { + CadFeature::BodyId& id = f.body_ref_ids[k++]; + if (ref < 0 || id.src < 0) return; + if (!alive(id.src)) { ref = kBodyGone; id = {}; return; } + id.src = map_feature(id.src); + }); + f.body_refs_pending = true; + } +} + // Re-run recompute(); if it fails for a GENUINE geometry error, restore `snapshot` // and recompute that instead, so a rejected edit leaves the document exactly as it // was. recompute() also returns false for the BENIGN case where the edit simply @@ -1938,7 +2176,7 @@ static bool commit_or_rollback(CadDocument& doc, std::vector& snapsh bool has_solid_feature = false; for (const auto& f : doc.features) - if (f.enabled && f.type != CadFeatureType::Sketch) { has_solid_feature = true; break; } + if (f.enabled && CadDocument::produces_body(f.type)) { has_solid_feature = true; break; } if (!has_solid_feature) { doc.bodies.clear(); doc.body = TopoDS_Shape(); @@ -2012,6 +2250,15 @@ bool CadDocument::remove_feature(int index) std::sort(remove.begin(), remove.end()); remove.erase(std::unique(remove.begin(), remove.end()), remove.end()); + stage_history_change(features, + [&remove](int i) { + if (std::binary_search(remove.begin(), remove.end(), i)) return -1; + int shift = 0; + for (int r : remove) if (r < i) ++shift; + return i - shift; + }, + [this](int i) { return features[i].enabled; }); + // Erase high-to-low so earlier indices stay valid. for (auto it = remove.rbegin(); it != remove.rend(); ++it) features.erase(features.begin() + *it); @@ -2050,6 +2297,9 @@ bool CadDocument::move_feature(int index, int delta) return true; // clamped at the ends — no-op, not a failure std::vector snapshot = features; + stage_history_change(features, + [index, target](int i) { return i == index ? target : i == target ? index : i; }, + [this](int i) { return features[i].enabled; }); std::swap(features[index], features[target]); // The two slots traded places: fix every feature reference that pointed at either — @@ -2061,6 +2311,33 @@ bool CadDocument::move_feature(int index, int delta) for (auto& f : features) for_each_feature_ref(f, swap_ref); + // History runs top to bottom: a feature may only consume what comes BEFORE it. Moving a + // consumer above its sketch (or a sketch below its consumer) would make it read whatever the + // input held the last time round — or, for a Project, nothing. Refuse the move and say so. + for (int fi = 0; fi < int(features.size()); ++fi) { + CadFeature& f = features[fi]; + std::string bad; + auto check = [&](int& ref) { if (ref >= fi && bad.empty()) bad = features[ref].name; }; + check(f.sketch_ref); check(f.sweep_path_ref); check(f.pattern_curve_sketch); check(f.rib_sketch_ref); + for (int& r : f.loft_profile_refs) check(r); + if (!bad.empty()) { + error = "\"" + f.name + "\" uses \"" + bad + "\", so it cannot come before it"; + features.swap(snapshot); + return false; + } + } + + return commit_or_rollback(*this, snapshot); +} + +bool CadDocument::set_feature_enabled(int index, bool enabled) +{ + if (index < 0 || index >= int(features.size())) return false; + if (features[index].enabled == enabled) return true; + std::vector snapshot = features; + stage_history_change(features, [](int i) { return i; }, + [this, index, enabled](int i) { return i == index ? enabled : features[i].enabled; }); + features[index].enabled = enabled; return commit_or_rollback(*this, snapshot); } @@ -2370,7 +2647,11 @@ void CadDocument::apply_feature(TopoDS_Shape& result, bool& have_body, gp_Pnt o(sk.plane.origin.x(), sk.plane.origin.y(), sk.plane.origin.z()); gp_Dir xd(adir.x(), adir.y(), adir.z()); gp_Ax1 axis(o, xd); - const double ang = f.revolve_angle * M_PI / 180.0; + // Same angle rules as the solid Revolve (flip reverses it, and a negative sweep is a + // positive one about the reversed axis — MakeRevol wants (0, 2 pi]); this surface + // version used to ignore both. + double ang = (f.flip ? -f.revolve_angle : f.revolve_angle) * M_PI / 180.0; + if (ang < 0) { axis.Reverse(); ang = -ang; } BRepPrimAPI_MakeRevol rev(wire, axis, ang, false); if (!rev.IsDone()) throw std::runtime_error("surface-revolve: revolve failed"); result = rev.Shape(); have_body = true; @@ -2527,7 +2808,9 @@ void CadDocument::apply_feature(TopoDS_Shape& result, bool& have_body, Vec3d o = f.plane.to_world(Vec2d(0, 0)); gp_Ax1 ax(gp_Pnt(o.x(), o.y(), o.z()), gp_Dir(f.plane.normal.x(), f.plane.normal.y(), f.plane.normal.z())); - const double step = (f.pattern_angle * M_PI / 180.0) / double(n); + const bool full = std::abs(std::abs(f.pattern_angle) - 360.0) < 1e-9; + const double div = (f.pattern_inclusive && !full && n > 1) ? double(n - 1) : double(n); + const double step = (f.pattern_angle * M_PI / 180.0) / div; trsf.SetRotation(ax, step * i); } else { const Vec3d& d = (f.pattern_dir == 1) ? f.plane.y_axis : f.plane.x_axis; @@ -2634,15 +2917,27 @@ void CadDocument::apply_feature(TopoDS_Shape& result, bool& have_body, } case CadFeatureType::Thread: { // Reject degenerate parameters that make OCCT's helical sweep / boolean unstable (a tiny - // pitch, depth >= half-pitch, an enormous turn count, depth eating the whole wall). Better - // a no-op than a crash. Leave the body unchanged when the spec can't be built safely. + // pitch, depth >= half-pitch, an enormous turn count, depth eating the whole wall), and + // SAY which: a thread that quietly did nothing read, for an internal one, as "the tool + // does not intersect the body", and for an external one as nothing at all. { const double R = f.thread_radius, P = f.thread_pitch, H = f.thread_height, D = f.thread_depth; // ISO external thread depth is ~0.61*P, so allow up to 0.7*P (0.49 wrongly rejected // every real thread -> nothing rendered). Still bound it well under a full pitch. - const bool ok = R > 0.5 && P > 0.1 && D > 1e-3 && D < 0.7 * P && D < 0.45 * R - && H > 0.5 * P && (H / P) < 400.0; - if (!ok) break; // result/have_body untouched + // R >= 0.5 (not >): M1, the smallest size in the standards table, is exactly 0.5. + const char* why = R < 0.5 ? "thread: the diameter must be at least 1 mm" + : P <= 0.1 ? "thread: the pitch must be more than 0.1 mm" + : (D <= 1e-3 || D >= 0.7 * P) ? "thread: the depth must be between 0 and 0.7 x the pitch" + : D >= 0.45 * R ? "thread: the depth is too large for this diameter" + : H <= 0.5 * P ? "thread: the length must be more than half a pitch" + : H / P >= 400.0 ? "thread: too many turns (length / pitch must stay under 400)" + : nullptr; + // Legacy recipes keep their old outcome (the body left as it was) so a project that + // opened before still opens; every thread made now reports the reason instead. + if (why != nullptr) { + if (f.thread_major_nominal) throw std::runtime_error(why); + break; + } } // Axis at the positioned point on the plane; +normal = thread rise. Vec3d c3 = f.plane.to_world(Vec2d(f.thread_x, f.thread_y)); @@ -2651,63 +2946,79 @@ void CadDocument::apply_feature(TopoDS_Shape& result, bool& have_body, gp_Dir xdir(f.plane.x_axis.x(), f.plane.x_axis.y(), f.plane.x_axis.z()); gp_Ax3 ax3(c, zdir, xdir); gp_Ax2 ax2(c, zdir, xdir); + const double R = f.thread_radius, D = f.thread_depth; + // Root the V CLEARLY inside the material (a real overlap, not a tangency) so the boolean + // has clean intersections — near-coincident faces are what make OCCT's fuse/cut unstable. + const double over = std::min(std::max(D, 0.25), R * 0.4); - // Build the swept helical ridge (guarded — never fatal). - TopoDS_Shape ridge; - bool have_ridge = false; - try { - TopoDS_Wire spine = make_helix_wire(ax3, f.thread_radius, - f.thread_pitch, f.thread_height); - TopoDS_Wire prof = make_thread_profile(c, xdir, zdir, f.thread_radius, - f.thread_pitch, f.thread_depth, - f.thread_internal); - // MakePipeShell with a FIXED BINORMAL = cylinder axis keeps the V-profile's orientation - // constant along the helix (axial edge always parallel to the axis, V always pointing - // radially out). The plain MakePipe used a Frenet frame that TWISTED the profile around - // the helix -> the wedge inclination varied and looked mirrored. - BRepOffsetAPI_MakePipeShell pipe(spine); - pipe.SetMode(zdir); - pipe.Add(prof); - pipe.Build(); - if (pipe.IsDone() && pipe.MakeSolid()) { - ridge = pipe.Shape(); - have_ridge = !ridge.IsNull(); + // Sweep a V profile along the helix (guarded — never fatal). `helix_r` is the radius the + // helix runs at: the profile is placed relative to it. + auto sweep = [&](double helix_r, const TopoDS_Wire& prof, TopoDS_Shape& out) { + try { + TopoDS_Wire spine = make_helix_wire(ax3, helix_r, f.thread_pitch, f.thread_height); + // MakePipeShell with a FIXED BINORMAL = cylinder axis keeps the V-profile's + // orientation constant along the helix (axial edge always parallel to the axis). + // The plain MakePipe used a Frenet frame that TWISTED the profile around the helix. + BRepOffsetAPI_MakePipeShell pipe(spine); + pipe.SetMode(zdir); + pipe.Add(prof); + pipe.Build(); + if (pipe.IsDone() && pipe.MakeSolid()) out = pipe.Shape(); + } catch (const Standard_Failure&) { // on OCCT >= 8 it derives from std::exception: first + } catch (const std::exception&) { } - } catch (const Standard_Failure&) { - have_ridge = false; // OCCT failure — on OCCT >= 8 Standard_Failure derives from std::exception, so this handler must come first - } catch (const std::exception&) { - have_ridge = false; // fall back to the bare cylinder/bore below + return !out.IsNull(); + }; + auto cut_from = [](TopoDS_Shape& target, const TopoDS_Shape& tool) { + BRepAlgoAPI_Cut cut(target, tool); + if (cut.IsDone() && !cut.Shape().IsNull()) target = cut.Shape(); + }; + + if (f.thread_major_nominal) { + if (f.thread_internal) { + // Tapped hole: bore to the minor radius R - D, then cut the groove out to the + // major radius R. The bore sits 10 um inside the minor radius so that, threading + // an existing tap-drill hole, it does not coincide with that hole's wall. + if (!have_body) throw std::runtime_error("internal thread needs a body"); + const double bore_r = R - D - 0.01; + cut_from(result, BRepPrimAPI_MakeCylinder(ax2, bore_r, f.thread_height).Shape()); + TopoDS_Shape groove; + if (sweep(R, make_thread_v(c, xdir, zdir, R - D - over, R, f.thread_pitch), groove)) + cut_from(result, groove); + } else { + // External thread: the crests are at R. A groove is cut INTO the rod down to + // R - D; with no body yet, the rod is made first. + if (!have_body || result.IsNull()) { + result = BRepPrimAPI_MakeCylinder(ax2, R, f.thread_height).Shape(); + have_body = true; + } + TopoDS_Shape groove; + if (sweep(R, make_thread_v(c, xdir, zdir, R + over, R - D, f.thread_pitch), groove)) + cut_from(result, groove); + } + break; } + // ---- legacy recipes (thread_major_nominal == false): built exactly as they always were. + TopoDS_Shape ridge; + const bool have_ridge = sweep(R, make_thread_profile(c, xdir, zdir, f.thread_radius, + f.thread_pitch, f.thread_depth, + f.thread_internal), ridge); if (f.thread_internal) { if (!have_body) throw std::runtime_error("internal thread needs a body"); - // Tapped bore: ensure a clean cylindrical pocket, then carve the - // OUTWARD helical groove into its wall. When the thread is invoked on - // an existing hole the bore cut is coincident (a no-op that may report - // !IsDone) — tolerate it so the visible groove cut below still runs. - // Cut the pocket at the MINOR diameter (radius - depth), not the nominal radius. - // A nominal-radius bore that coincides with an existing hole's wall creates - // coincident faces that foul the following groove boolean (the groove then removes - // ~nothing -> invisible thread). The minor bore stays strictly inside any existing - // hole wall, leaving it clean for the groove; on solid stock it forms the tap-drill. + // Tapped bore: cut a clean pocket at radius - depth, then carve the helical groove + // outward into its wall. const double bore_r = std::max(0.5, f.thread_radius - f.thread_depth); - TopoDS_Shape bore = BRepPrimAPI_MakeCylinder(ax2, bore_r, - f.thread_height).Shape(); + TopoDS_Shape bore = BRepPrimAPI_MakeCylinder(ax2, bore_r, f.thread_height).Shape(); try { BRepAlgoAPI_Cut cut_bore(result, bore); if (cut_bore.IsDone() && !cut_bore.Shape().IsNull()) result = cut_bore.Shape(); } catch (const std::exception&) { /* keep existing bore */ } - if (have_ridge) { - BRepAlgoAPI_Cut cut_ridge(result, ridge); - if (cut_ridge.IsDone() && !cut_ridge.Shape().IsNull()) - result = cut_ridge.Shape(); - } + if (have_ridge) cut_from(result, ridge); } else { - // External thread: FUSE the helical ridge ONTO the existing body (the picked cylinder), - // leaving the rest of the part intact. Replacing the body with a bare rod — the old - // behaviour — wiped whatever the user picked; that was the "mess". With no body yet - // (a thread from scratch on a dropdown plane), fall back to a standalone threaded rod. + // External thread: FUSE the helical ridge onto the existing body, or onto a + // standalone rod when there is none. if (have_body && !result.IsNull()) { if (have_ridge) { BRepAlgoAPI_Fuse fuse(result, ridge); @@ -2827,12 +3138,16 @@ static TriangleMesh tessellate_bodies(const std::vector& bodies, void CadDocument::apply_boolean(std::vector& bodies, const CadFeature& f) const { const int nb = int(bodies.size()); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; - const int tool = (f.bool_tool_body >= 0 && f.bool_tool_body < nb) ? f.bool_tool_body : -1; - if (tgt < 0 || tool < 0 || tgt == tool) return; // need two distinct bodies; otherwise no-op + const int tgt = pick_body(f.target_body, nb); + const int tool = f.bool_tool_body == -1 ? -1 : pick_body(f.bool_tool_body, nb); + // Same rule as Cut, Mirror and Transform: an operation that cannot run says why, rather + // than leaving the bodies as they were and reporting success. + if (tgt < 0) throw std::runtime_error("boolean: no target body"); + if (tool < 0) throw std::runtime_error("boolean: no tool body — pick a second body"); + if (tgt == tool) throw std::runtime_error("boolean: the target and the tool are the same body"); const TopoDS_Shape A = bodies[tgt].shape; // target survives const TopoDS_Shape B = bodies[tool].shape; // tool, consumed unless kept - if (A.IsNull() || B.IsNull()) return; + if (A.IsNull() || B.IsNull()) throw std::runtime_error("boolean: a body has no geometry"); TopTools_ListOfShape args, tools; args.Append(A); @@ -2850,7 +3165,7 @@ void CadDocument::apply_boolean(std::vector& bodies, const CadFeature& case BooleanMode::Add: { BRepAlgoAPI_Fuse op; result = run(op); break; } // union case BooleanMode::Cut: { BRepAlgoAPI_Cut op; result = run(op); break; } // target - tool case BooleanMode::Intersect: { BRepAlgoAPI_Common op; result = run(op); break; } // overlap - default: return; // BooleanMode::New is meaningless between two existing bodies + default: throw std::runtime_error("boolean: choose Union, Subtract or Intersect"); // New means nothing between two bodies } if (result.IsNull()) throw std::runtime_error("boolean produced an empty shape"); @@ -2862,7 +3177,7 @@ void CadDocument::apply_cut(std::vector& bodies, const CadFeature& f) c { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("cut: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("cut: no target body"); if (!f.cut_keep_upper && !f.cut_keep_lower) @@ -2870,7 +3185,7 @@ void CadDocument::apply_cut(std::vector& bodies, const CadFeature& f) c SketchPlane cp; if (f.cut_face >= 0) { - const int fb = (f.cut_face_body >= 0 && f.cut_face_body < nb) ? f.cut_face_body : tgt; + const int fb = f.cut_face_body == -1 ? tgt : pick_body(f.cut_face_body, nb); if (bodies[fb].shape.IsNull()) throw std::runtime_error("cut: face body is empty"); TopoDS_Face fc = GeometryEngine::face_by_index(bodies[fb].shape, f.cut_face); if (fc.IsNull()) throw std::runtime_error("cut: face not found"); @@ -2933,7 +3248,7 @@ void CadDocument::apply_mirror(std::vector& bodies, const CadFeature& f { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("mirror: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("mirror: no target body"); const TopoDS_Shape& src = bodies[tgt].shape; @@ -2980,7 +3295,7 @@ void CadDocument::apply_transform(std::vector& bodies, const CadFeature { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("transform: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("transform: no target body"); gp_Trsf rot; @@ -3009,7 +3324,7 @@ void CadDocument::apply_thicken(std::vector& bodies, const CadFeature& { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("thicken: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("thicken: no target body"); TopoDS_Face fc = GeometryEngine::face_by_index(bodies[tgt].shape, f.thicken_face); @@ -3044,7 +3359,7 @@ void CadDocument::apply_thicken_surface(std::vector& bodies, const CadF { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("thicken-surface: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("thicken-surface: no target body"); if (!is_sheet_shape(bodies[tgt].shape)) throw std::runtime_error("thicken-surface: target is not a sheet body"); if (std::abs(f.thicken_thickness) < 1e-9) throw std::runtime_error("thicken-surface: thickness is zero"); @@ -3147,7 +3462,7 @@ void CadDocument::apply_surface_offset(std::vector& bodies, const CadFe { const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("surface-offset: no target body"); - const int tgt = (f.target_body >= 0 && f.target_body < nb) ? f.target_body : nb - 1; + const int tgt = pick_body(f.target_body, nb); if (tgt < 0 || bodies[tgt].shape.IsNull()) throw std::runtime_error("surface-offset: no target body"); if (!is_sheet_shape(bodies[tgt].shape)) throw std::runtime_error("surface-offset: target is not a sheet body"); const double d = f.plane_offset; @@ -3239,8 +3554,7 @@ void CadDocument::apply_project(const std::vector& bodies, CadFeature& f.entities.clear(); const int nb = int(bodies.size()); if (nb == 0) throw std::runtime_error("project: no source body"); - const int src = (f.project_source_body >= 0 && f.project_source_body < nb) - ? f.project_source_body : nb - 1; + const int src = pick_body(f.project_source_body, nb); if (src < 0 || bodies[src].shape.IsNull()) throw std::runtime_error("project: source body is empty"); const TopoDS_Shape& shape = bodies[src].shape; @@ -3300,7 +3614,7 @@ void CadDocument::detect_mate_conflicts() 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) + + first_name + "' (feature " + std::to_string(first_fi + 1) + ") — '" + this_name + "' overrides it; suppress one"}); } else { first_driver[dst] = fi; @@ -3424,6 +3738,8 @@ void CadDocument::apply_mate(std::vector& bodies, const CadFeature& f) 0, 0, -1, 0); } + if (f.mate_kind < 0 || f.mate_kind > 4) + throw std::runtime_error("mate: unknown mate type " + std::to_string(f.mate_kind)); gp_Trsf T; if (f.mate_kind == 0) { // Fastened: T = M_A * Rz(mate_angle) * Tz(mate_offset) * F * M_B^-1 @@ -3562,8 +3878,7 @@ void CadDocument::route_feature(std::vector& bodies, const CadFeature& if (f.type == CadFeatureType::SurfaceOffset) { apply_surface_offset(bodies, f); return; } if (f.type == CadFeatureType::Project) return; // sketch-like: consumed downstream, no body // Resolve the target body: explicit target_body when valid, else the last body. - const int t = (f.target_body >= 0 && f.target_body < int(bodies.size())) - ? f.target_body : int(bodies.size()) - 1; + const int t = pick_body(f.target_body, int(bodies.size())); const TopoDS_Shape context = (t >= 0) ? bodies[t].shape : TopoDS_Shape(); // A New extrude (or the very first solid feature) starts a fresh body; everything else // mutates the target body in place. @@ -3642,8 +3957,9 @@ bool CadDocument::recompute() if (f.type == CadFeatureType::Plane) continue; // datum: no solid, derived on demand if (f.type == CadFeatureType::Axis) continue; // datum axis if (f.type == CadFeatureType::CoordSys) continue; // datum coordinate system - // Past the skips: this feature is one that means to leave a body behind. - any_solid_feature = true; + // Past the skips. Project runs here too but leaves no body of its own. + if (produces_body(f.type)) any_solid_feature = true; + settle_body_refs(f, built, true); if (f.type == CadFeatureType::Project) { apply_project(built, f); } else { route_feature(built, f); } // Record which feature made each body. "Still unset?" is the whole rule, and it is @@ -3693,22 +4009,37 @@ bool CadDocument::recompute() } // recompute() replaces the bodies vector wholesale, which would drop any per-body - // colour override (Color tool). Body indices are stable across a rebuild (bodies are - // appended in feature order), so carry the override forward by index — same indexing - // contract the GUI relies on for per-body visibility/Move. - for (size_t i = 0; i < built.size() && i < bodies.size(); ++i) { + // colour override (Color tool) and the name the user gave a body. They are carried forward + // by the body's IDENTITY — the feature that made it and which of its bodies it is — not by + // its index: a Boolean consuming its tool, a Mirror replacing its source or a Cut splitting + // one body into two all shift every later index, and by index the colour and the name + // moved to a different body. + for (size_t i = 0; i < bodies.size(); ++i) { + if (!bodies[i].has_color && !bodies[i].has_user_name) continue; + const int j = find_body(built, body_id_of(bodies, int(i))); + if (j < 0) continue; if (bodies[i].has_color) { - built[i].has_color = true; - built[i].color = bodies[i].color; + built[j].has_color = true; + built[j].color = bodies[i].color; } - // ...and the name the user gave the body, for the same reason and by the same index - // contract. Without this a rename would live exactly until the next feature was added. if (bodies[i].has_user_name) { - built[i].has_user_name = true; - built[i].user_name = bodies[i].user_name; + built[j].has_user_name = true; + built[j].user_name = bodies[i].user_name; } } bodies = std::move(built); + // Datum references index the FINISHED body list; settle them against it, and close every + // pending re-point now that both kinds have been through settle_body_refs. + try { + for (CadFeature& f : features) { + if (!f.enabled) continue; // a hidden feature keeps its pending re-point for when it returns + settle_body_refs(f, bodies, false); + f.body_refs_pending = false; + } + } catch (const std::exception& e) { + error = e.what(); + return false; + } // The face and edge maps have just been rebuilt, so every global id handed out before this // point now means something else. Bump here rather than in each mutator: this is the single // line where the topology is actually replaced, so it cannot be forgotten by a new feature @@ -3869,6 +4200,22 @@ std::string CadDocument::serialize_recipe() const uint32_t blen = static_cast(bb.size()); ar(blen); ar(cereal::binary_data(bb.data(), bb.size())); + // DOCUMENT EXTRAS, appended after the body names on the same terms (older builds stop + // reading before it): the modeling origin, and the body colours. A colour set with the + // Color tool used to live only until the project was closed. + std::map> colours; + for (uint32_t i = 0; i < bodies.size(); ++i) + if (bodies[i].has_color) + colours[i] = { bodies[i].color.r(), bodies[i].color.g(), bodies[i].color.b(), bodies[i].color.a() }; + std::ostringstream xos; + { + cereal::BinaryOutputArchive xa(xos); + xa(modeling_origin.x(), modeling_origin.y(), modeling_origin.z(), colours); + } + std::string xb = xos.str(); + uint32_t xlen = static_cast(xb.size()); + ar(xlen); + ar(cereal::binary_data(xb.data(), xb.size())); } return oss.str(); } @@ -3950,6 +4297,24 @@ bool CadDocument::deserialize_recipe(const std::string& blob) } catch (...) { named.clear(); } + // Document extras (origin, colours), when the project has them. The origin must be + // in place BEFORE the rebuild: datum planes are resolved against it. + std::map> colours; + try { + uint32_t xlen; + ar(xlen); + std::string xbuf(xlen, '\0'); + if (xlen > 0) + ar(cereal::binary_data(&xbuf[0], xlen)); + std::istringstream xs(xbuf); + cereal::BinaryInputArchive xa(xs); + double ox = 0, oy = 0, oz = 0; + xa(ox, oy, oz, colours); + modeling_origin = Vec3d(ox, oy, oz); + origin_from_recipe = true; + } catch (...) { + colours.clear(); // older project: the caller's origin stands + } // AFTER the rebuild, never before: recompute() replaces the bodies vector wholesale. const bool ok = recompute(); for (const auto& kv : named) @@ -3957,11 +4322,21 @@ bool CadDocument::deserialize_recipe(const std::string& blob) bodies[kv.first].has_user_name = true; bodies[kv.first].user_name = kv.second; } + for (const auto& kv : colours) + if (kv.first < bodies.size()) { + bodies[kv.first].has_color = true; + bodies[kv.first].color = ColorRGBA(kv.second[0], kv.second[1], kv.second[2], kv.second[3]); + } return ok; } if (v == 4) { - // Pre-framing flat path, unchanged: v4 projects keep opening exactly as before. - ar(features); + // Pre-framing flat path: v4 projects keep opening, read with the FROZEN v4 field list + // (CadFeature::load_flat_v4) so fields appended to the framed layout since cannot + // shift every byte after them. + cereal::size_type n = 0; + ar(cereal::make_size_tag(n)); + features.assign(size_t(n), CadFeature{}); + for (CadFeature& f : features) f.load_flat_v4(ar); ar(variables); return recompute(); } diff --git a/src/libslic3r/CAD/CadDocument.hpp b/src/libslic3r/CAD/CadDocument.hpp index f62536384a..6969473b69 100644 --- a/src/libslic3r/CAD/CadDocument.hpp +++ b/src/libslic3r/CAD/CadDocument.hpp @@ -74,7 +74,7 @@ struct CadFeature { // the OCCT shape verbatim — it is adopted as a base body in route_feature (no parametric // recipe). Downstream face/edge features (fillet/chamfer/cut/shell/...) act on it like any // other body. TopoDS_Shape is a cheap handle, so copying it through recompute/checkpoint - // snapshots is cheap. In-session only for now (no BRep serialization yet). + // snapshots is cheap. Saved with the recipe as a BRep string (see save()/load()). TopoDS_Shape imported_solid; // Non-destructive placement transform for imported_regions (Text/SVG), @@ -130,12 +130,21 @@ struct CadFeature { std::string hole_standard; // provenance only, e.g. "M6" / "1/4-20"; not used by geometry // Thread params (helical thread about the plane normal at a positioned point) - double thread_radius{5}; // nominal cylinder radius + double thread_radius{5}; // MAJOR (nominal) radius when thread_major_nominal, + // else the legacy reference radius (see below) double thread_pitch{2}; // axial advance per turn double thread_height{10}; // total axial length - double thread_depth{1}; // radial crest depth of the thread profile - bool thread_internal{false}; // false = external threaded rod (New body); - // true = tapped bore cut into the current body + double thread_depth{1}; // radial depth of the thread profile + bool thread_internal{false}; // false = external: the thread goes on the target body + // (or on a standalone rod when there is none); + // true = tapped bore cut into the target body + // How thread_radius is read. true (every thread made since): it is the nominal MAJOR + // radius, for both kinds — an internal thread bores to R - depth and grooves out to R, an + // external one is a rod of radius R with its groove cut in to R - depth, so an M6 is 6 mm + // across its crests either way. false (older recipes, kept bit-for-bit): the ridge was + // added OUTSIDE R, so an internal thread given the minor diameter came out undersized and + // an external one given the major diameter came out oversized. + bool thread_major_nominal{false}; double thread_x{0}; // axis position on the plane (u/x axis) double thread_y{0}; // axis position on the plane (v/y axis) @@ -174,6 +183,12 @@ struct CadFeature { double pattern_spacing{20}; // linear step (mm) int pattern_dir{0}; // linear direction: 0 = plane X, 1 = plane Y double pattern_angle{360}; // circular total angle (degrees) + // How a circular pattern spreads its copies over pattern_angle. true (every pattern made + // since): a full turn is split into `count` equal steps, anything less is spanned end to end, + // first copy at 0 and last at pattern_angle — the way a pattern along a curve spans its + // curve. false (older recipes, kept as they were built): always angle / count, so 90° with 3 + // copies stopped at 60°. + bool pattern_inclusive{false}; // Pattern along a curve: when pattern_curve_sketch >= 0 this mode takes precedence over // linear/circular. Copies are placed at equal-parameter points along entity @@ -186,6 +201,16 @@ struct CadFeature { // BEFORE geometry runs. Empty (the common case) means the feature uses its literal fields. std::map expr; + // Transient, never serialized. A body is referred to above by its INDEX, and an index is + // only meaningful against the history it was taken in: delete, reorder or disable a + // feature that makes a body and every later index points at a different body. So recompute + // records, for each body field, the identity of the body it resolved to — the feature that + // made it, and which of that feature's bodies it is — and when the history changes the + // fields are re-pointed from those identities (see CadDocument::recompute). + struct BodyId { int src{-1}; int ord{0}; }; + std::vector body_ref_ids; + bool body_refs_pending{false}; + // Datum/reference plane: a derived SketchPlane the document offers as a selectable // sketch plane (no solid). plane_base selects the reference (0=XY,1=XZ,2=YZ, or 3+N // = the Nth earlier datum plane); plane_offset shifts along the base normal; @@ -357,10 +382,56 @@ struct CadFeature { pattern_curve_sketch, pattern_curve_entity, expr, mate_kind, mate_cs_a, mate_cs_b, mate_offset, mate_angle, mate_flip, - coordsys_face_kind, coordsys_face_edges); + coordsys_face_kind, coordsys_face_edges, + thread_major_nominal, pattern_inclusive); } template void load(Archive& ar) { + std::string brep; + ar(type, name, enabled, shape, plane, width, height, radius, + profile, entities, constraints, entity_constraints, imported_regions, + import_offset, import_scale_x, import_scale_y, import_on_face, import_face_body, + sketch_ref, distance, symmetric, mode, extrude_end, distance2, taper_deg, flip, + up_to_face, extrude_src_face, up_to_point, target_body, + dressup_size, face_group, dressup_edge, + hole_diameter, hole_depth, hole_through, hole_x, hole_y, + thread_radius, thread_pitch, thread_height, thread_depth, thread_internal, thread_x, thread_y, + shell_thickness, shell_face, + draft_face, draft_angle, + revolve_angle, revolve_axis, + sweep_path_ref, loft_profile_refs, loft_ruled, + pattern_circular, pattern_count, pattern_spacing, pattern_dir, pattern_angle, + plane_base, plane_offset, plane_angle_tilt, plane_axis, + bool_tool_body, bool_keep_tool, bool_tolerance, bool_target_face, bool_tool_face, + cut_offset, cut_flip, cut_keep_upper, cut_keep_lower, + brep, + plane_type, plane_face_body, plane_face, plane_face2_body, plane_face2, + plane_edge_body, plane_edge, plane_edge2_body, plane_edge2, plane_u_size, plane_v_size, + mirror_keep_original, + axis_type, axis_p1, axis_p2, axis_body, axis_face, axis_edge, axis_plane_a, axis_plane_b, + coordsys_type, coordsys_point, coordsys_body, coordsys_face, coordsys_edge, coordsys_x_hint, + helix_radius, helix_pitch, helix_height, helix_left_handed, helix_taper_deg, + xf_translate, xf_axis, xf_pivot, xf_angle_deg, xf_copy, + thicken_face, thicken_thickness, thicken_flip, + cut_face_body, cut_face, + project_source_body, project_edges, project_face, + delete_faces, + hole_style, hole_cbore_diameter, hole_cbore_depth, + hole_csink_diameter, hole_csink_angle, hole_standard, + rib_sketch_ref, rib_entity, rib_thickness, rib_depth, + pattern_curve_sketch, pattern_curve_entity, + expr, + mate_kind, mate_cs_a, mate_cs_b, mate_offset, mate_angle, mate_flip, + coordsys_face_kind, coordsys_face_edges, + thread_major_nominal, pattern_inclusive); + imported_solid = brep_from_string(brep); + } + // The pre-framing (v4) layout, FROZEN. A v4 recipe is one flat stream with no per-feature + // length, so it can only be read with exactly the field list it was written with; reading it + // through load() above breaks the moment a field is appended there (every field added since + // v5 is appended ONLY above, never here). + template + void load_flat_v4(Archive& ar) { std::string brep; ar(type, name, enabled, shape, plane, width, height, radius, profile, entities, constraints, entity_constraints, imported_regions, @@ -399,6 +470,7 @@ struct CadFeature { coordsys_face_kind, coordsys_face_edges); imported_solid = brep_from_string(brep); } + }; // Serialize a TopoDS_Shape to/from a BRep string for cereal persistence. @@ -436,6 +508,13 @@ public: // Named document variables: name -> expression. Evaluated topologically each recompute(); // an expression may reference other variables. Feature `expr` bindings resolve against these. std::map variables; + // Can a feature expression drive this numeric field? The single allow-list the evaluator + // uses, so a caller can refuse a name before it breaks the next recompute. + static bool is_bindable_field(const std::string& field); + // Does a feature of this type leave a body behind? Sketches, datums, the helix curve and + // Project (which emits sketch entities) do not. The one answer recompute(), the rollback + // rule and the GUI's preview all use. + static bool produces_body(CadFeatureType t); // Multi-body result of the last replay. A "New" extrude appends a body; other ops // mutate a target body. Empty after a failed/empty recompute. std::vector bodies; @@ -455,8 +534,11 @@ public: // 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. + // world 0) — for a NEW document. It is saved with the recipe: sketches bake it into their + // planes while datum planes add it when they are resolved, so a project reopened on another + // printer must keep the origin it was made with or its datums move and its sketches do not. Vec3d modeling_origin{Vec3d::Zero()}; + bool origin_from_recipe{false}; // modeling_origin came from the loaded project // Tessellation quality, matched to Orca's OWN STEP importer (Format/STEP.hpp defaults: // linear 0.003, angular 0.5 rad) so a body modelled here reaches the screen at the same @@ -703,6 +785,9 @@ public: // an Extrude, its sketch_ref are preserved from the original). bool remove_feature(int index); bool move_feature(int index, int delta); + // Show or hide a feature. Transactional like the others, and it keeps body and datum-plane + // references pointing at the same objects, which a bare `enabled` flip does not. + bool set_feature_enabled(int index, bool enabled); bool replace_feature(int index, const CadFeature& edited); // replace_sketch_extrude: a box is two linked features (Sketch + Extrude); // overwrite both slots from one `edited` candidate (sketch params -> diff --git a/src/libslic3r/CAD/GeometryEngine.cpp b/src/libslic3r/CAD/GeometryEngine.cpp index 2b923938c2..03ae1de979 100644 --- a/src/libslic3r/CAD/GeometryEngine.cpp +++ b/src/libslic3r/CAD/GeometryEngine.cpp @@ -275,21 +275,21 @@ FaceGroup GeometryEngine::classify_face(const TopoDS_Face& face, const TopoDS_Sh std::vector GeometryEngine::collect_edges(const TopoDS_Shape& solid, FaceGroup target) { + // Each edge ONCE. An explorer visits a shared edge from both of its faces, so walking one + // handed every edge to the fillet twice; the de-duplicated map is also what edge_by_index + // numbers edges by. std::vector result; + TopTools_IndexedDataMapOfShapeListOfShape edgeFaceMap; + TopExp::MapShapesAndAncestors(solid, TopAbs_EDGE, TopAbs_FACE, edgeFaceMap); if (target == FaceGroup::All) { - for (TopExp_Explorer exp(solid, TopAbs_EDGE); exp.More(); exp.Next()) - result.push_back(TopoDS::Edge(exp.Current())); + for (int i = 1; i <= edgeFaceMap.Extent(); ++i) + result.push_back(TopoDS::Edge(edgeFaceMap.FindKey(i))); return result; } - // Build edge-to-face map once - TopTools_IndexedDataMapOfShapeListOfShape edgeFaceMap; - TopExp::MapShapesAndAncestors(solid, TopAbs_EDGE, TopAbs_FACE, edgeFaceMap); - - for (TopExp_Explorer edgeExp(solid, TopAbs_EDGE); edgeExp.More(); edgeExp.Next()) { - const TopoDS_Edge& edge = TopoDS::Edge(edgeExp.Current()); - if (!edgeFaceMap.Contains(edge)) continue; - const TopTools_ListOfShape& faces = edgeFaceMap.FindFromKey(edge); + for (int ei = 1; ei <= edgeFaceMap.Extent(); ++ei) { + const TopoDS_Edge& edge = TopoDS::Edge(edgeFaceMap.FindKey(ei)); + const TopTools_ListOfShape& faces = edgeFaceMap.FindFromIndex(ei); bool include = false; for (auto it = faces.begin(); it != faces.end(); ++it) { @@ -325,12 +325,14 @@ std::vector GeometryEngine::collect_edges(const TopoDS_Shape& solid // ---- Fillet/Chamfer ---- +// All four dress-up entry points fail the same way — with a reason — instead of two of them +// handing the solid back unchanged, which recompute then reported as a success. TopoDS_Shape GeometryEngine::apply_fillet(const TopoDS_Shape& solid, double radius, FaceGroup faces) { - if (radius <= 0.001) return solid; + if (radius <= 0.001) throw std::runtime_error("the fillet radius must be greater than 0"); std::vector edges = collect_edges(solid, faces); - if (edges.empty()) return solid; + if (edges.empty()) throw std::runtime_error("no edges to fillet in that group"); BRepFilletAPI_MakeFillet fillet(solid); for (const auto& edge : edges) @@ -346,10 +348,10 @@ TopoDS_Shape GeometryEngine::apply_fillet(const TopoDS_Shape& solid, double radi TopoDS_Shape GeometryEngine::apply_chamfer(const TopoDS_Shape& solid, double distance, FaceGroup faces) { - if (distance <= 0.001) return solid; + if (distance <= 0.001) throw std::runtime_error("the chamfer distance must be greater than 0"); std::vector edges = collect_edges(solid, faces); - if (edges.empty()) return solid; + if (edges.empty()) throw std::runtime_error("no edges to chamfer in that group"); BRepFilletAPI_MakeChamfer chamfer(solid); for (const auto& edge : edges) @@ -362,7 +364,7 @@ TopoDS_Shape GeometryEngine::apply_chamfer(const TopoDS_Shape& solid, double dis TopoDS_Shape GeometryEngine::apply_fillet(const TopoDS_Shape& solid, double radius, int edge_id) { - if (radius <= 0.001) return solid; + if (radius <= 0.001) throw std::runtime_error("the fillet radius must be greater than 0"); TopoDS_Edge edge = edge_by_index(solid, edge_id); if (edge.IsNull()) throw std::runtime_error("apply_fillet: invalid edge id"); @@ -377,7 +379,7 @@ TopoDS_Shape GeometryEngine::apply_fillet(const TopoDS_Shape& solid, double radi TopoDS_Shape GeometryEngine::apply_chamfer(const TopoDS_Shape& solid, double distance, int edge_id) { - if (distance <= 0.001) return solid; + if (distance <= 0.001) throw std::runtime_error("the chamfer distance must be greater than 0"); TopoDS_Edge edge = edge_by_index(solid, edge_id); if (edge.IsNull()) throw std::runtime_error("apply_chamfer: invalid edge id"); diff --git a/src/libslic3r/CAD/GeometryEngine.hpp b/src/libslic3r/CAD/GeometryEngine.hpp index f27a0f2f17..bad29ed953 100644 --- a/src/libslic3r/CAD/GeometryEngine.hpp +++ b/src/libslic3r/CAD/GeometryEngine.hpp @@ -36,8 +36,9 @@ struct PrimitiveParams { double dressup_radius{1.0}; // fillet radius double dressup_chamfer_dist{1.0}; // chamfer distance (symmetric) - // Mesh quality - double linear_deflection{0.01}; + // Mesh quality — the Design tab's own density (CadDocument::linear_deflection), so a + // primitive and the same body modelled in the Design tab reach the screen alike. + double linear_deflection{0.003}; double angular_deflection{0.5}; template @@ -121,7 +122,7 @@ public: int edge_id); static TriangleMesh tessellate(const TopoDS_Shape& shape, - double linear_deflection = 0.01, + double linear_deflection = 0.003, double angular_deflection = 0.5); static std::string primitive_name(PrimitiveType type); diff --git a/src/libslic3r/CAD/SketchEngine.cpp b/src/libslic3r/CAD/SketchEngine.cpp index 5cc36ce648..8aa440d707 100644 --- a/src/libslic3r/CAD/SketchEngine.cpp +++ b/src/libslic3r/CAD/SketchEngine.cpp @@ -62,14 +62,6 @@ void set_sketch_auto_close(bool on) { s_auto_close = on; } // ---- SketchPlane ---- -gp_Pln SketchPlane::to_occt() const -{ - gp_Pnt o(origin.x(), origin.y(), origin.z()); - gp_Dir n(normal.x(), normal.y(), normal.z()); - gp_Dir x(x_axis.x(), x_axis.y(), x_axis.z()); - return gp_Pln(gp_Ax3(o, n, x)); -} - SketchPlane SketchPlane::from_face(const TopoDS_Face& face) { SketchPlane sp; @@ -124,26 +116,6 @@ Vec3d SketchPlane::to_world(const Vec2d& pt) const // ---- SketchProfile ---- -bool SketchProfile::is_closed(double tolerance) const -{ - if (points.size() < 3) return false; - return (points.front() - points.back()).norm() < tolerance; -} - -bool SketchProfile::try_close(double tolerance) -{ - if (is_closed(tolerance)) { - closed = true; - return true; - } - if (points.size() < 2) return false; - if ((points.front() - points.back()).norm() < tolerance) { - closed = true; - return true; - } - return false; -} - TopoDS_Wire SketchProfile::to_occt_wire(const SketchPlane& plane) const { if (points.size() < 2) @@ -276,7 +248,9 @@ TopoDS_Shape SketchEngine::make_extrude_regions( // degenerate OCCT edge and make the wire builder throw — sanitising keeps a // single bad glyph from killing the whole extrude. auto clean = [](const std::vector& pts) { - const double eps2 = 1e-12; // ~1e-6 mm + // The sketch's own joint tolerance: points closer than this are one point everywhere + // else in the sketcher, so they must not become a sub-micron edge here either. + const double eps2 = kSketchJoinTol * kSketchJoinTol; std::vector out; out.reserve(pts.size()); for (const Vec2d& p : pts) @@ -304,6 +278,10 @@ TopoDS_Shape SketchEngine::make_extrude_regions( }; gp_Dir dir(plane.normal.x(), plane.normal.y(), plane.normal.z()); + // Faces are built ON the sketch plane, as wires_to_face does: a surface inferred from the + // outer wire need not share the sketch normal, and then the extrude direction and the hole + // classification disagree with it. + const gp_Pln pln(gp_Pnt(plane.origin.x(), plane.origin.y(), plane.origin.z()), dir); // Accumulate each region's solid into a compound rather than boolean-fusing: // glyphs are independent profiles, so a compound avoids every boolean-failure @@ -324,7 +302,10 @@ TopoDS_Shape SketchEngine::make_extrude_regions( // classify outer vs holes by area/containment and set correct wire // orientations. This is winding-independent, so holed glyphs extrude // with a solid body and empty counters regardless of source winding. - BRepBuilderAPI_MakeFace fm(outer); + // Probe on the inferred surface first: naming a plane makes MakeFace accept a wire + // that bounds nothing, and this check is what skips such a contour. + if (!BRepBuilderAPI_MakeFace(outer).IsDone()) continue; + BRepBuilderAPI_MakeFace fm(pln, outer); if (!fm.IsDone()) continue; for (size_t h = 1; h < region.size(); ++h) { TopoDS_Wire hole = contour_wire(region[h]); @@ -567,7 +548,12 @@ std::vector SketchEngine::entities_to_wires(const std::vector gp_Elips { Vec3d c3 = plane.to_world(c.center); gp_Pnt center(c3.x(), c3.y(), c3.z()); - gp_Dir n(plane.normal.x(), plane.normal.y(), plane.normal.z()); + // The frame's normal is x_axis x y_axis, not plane.normal: OCCT takes the ellipse's Y + // as N x X, and the parametric angles were measured in the sketch's own (x, y). The two + // agree on XY and YZ; the XZ base plane stores normal = +Y while x x y = -Y, so there + // every elliptical arc came out mirrored against what the sketch showed. + const Vec3d nz = plane.x_axis.cross(plane.y_axis); + gp_Dir n(nz.x(), nz.y(), nz.z()); Vec2d maj2(std::cos(c.rotation), std::sin(c.rotation)); Vec3d x3 = plane.to_world(c.center + maj2) - c3; gp_Dir xdir(x3.x(), x3.y(), x3.z()); @@ -691,6 +677,9 @@ std::vector SketchEngine::entities_to_wires(const std::vector SketchEngine::mirror_entities( } else { m.p0 = reflect(e.p0); m.p1 = reflect(e.p1); - // Reflection reverses orientation: recompute parametric angles in - // the reflected frame, original end -> new start (CCW sense kept). + // Reflection reverses orientation. Like the Arc branch above, this pass keeps + // each angle with ITS point (start with p0) and lets the sweep run clockwise; + // the reversal pass below then swaps points and angles together, giving a CCW + // arc whose start is p0. Pairing them crosswise here, as this used to, was + // undone by that same swap and produced the complementary arc. auto param = [&](const Vec2d& P) { const Vec2d d = P - m.center; const double cu = std::cos(m.rotation), su = std::sin(m.rotation); @@ -1045,8 +1037,9 @@ std::vector SketchEngine::mirror_entities( const double v = -d.x() * su + d.y() * cu; return std::atan2(v / std::max(e.rminor, 1e-9), u / std::max(e.radius, 1e-9)); }; - m.start_angle = param(m.p1); - m.end_angle = param(m.p0); + m.start_angle = param(m.p0); + m.end_angle = param(m.p1); + while (m.end_angle >= m.start_angle) m.end_angle -= 2.0 * M_PI; } break; } @@ -1106,9 +1099,14 @@ std::vector SketchEngine::mirror_entities( // get their last-to-first seam repaired too, which is what makes the result closed again. namespace { -constexpr double kOffJoinEps = 1e-6; - -bool off_same(const Vec2d& a, const Vec2d& b) { return (a - b).squaredNorm() < kOffJoinEps * kOffJoinEps; } +// Two ends are the same point when the WIRE BUILDER would weld them: a loop the viewport and +// the kernel treat as closed must offset as one closed chain, not as separate open pieces with +// unrepaired seams. The floor keeps exact coincidence meaningful with auto-close switched off. +bool off_same(const Vec2d& a, const Vec2d& b) +{ + const double tol = std::max(sketch_join_tol(), 1e-6); + return (a - b).squaredNorm() <= tol * tol; +} // Does this entity type take part in chaining (i.e. does it have two ends)? bool off_is_open_curve(const SketchEntity& e) @@ -1214,7 +1212,10 @@ bool off_one(const SketchEntity& e, double d, SketchEntity& out) return true; } case SketchEntity::Type::Circle: { - const double r = e.radius + d; + // A circle runs CCW (it is what a 360° CCW arc chain closes into), so the rule below + // applies to it too: +d is the left side, the inside, and the radius SHRINKS. It used to + // grow, so a full circle and the same outline drawn as arcs offset opposite ways. + const double r = e.radius - d; if (r <= 1e-9) return false; out = e; out.radius = r; out.p0 = out.center; return true; @@ -1453,6 +1454,9 @@ std::vector SketchEngine::transform_entities( out.reserve(src.size()); const double ca = std::cos(angle), sa = std::sin(angle); const double rs = std::abs(scale); // radii are unsigned magnitudes + // A negative scale is |scale| plus a half turn about the pivot: points get that from xf() + // below, and angle-valued fields (arc ends, ellipse axis) need the same extra pi. + const double turn = angle + (scale < 0.0 ? M_PI : 0.0); // Affine map: translate pivot to origin, scale, rotate, then translate by `move`. auto xf = [&](const Vec2d& p) -> Vec2d { const Vec2d d = scale * (p - pivot); @@ -1475,8 +1479,8 @@ std::vector SketchEngine::transform_entities( case SketchEntity::Type::Arc: { m.center = xf(e.center); m.radius = e.radius * rs; - m.start_angle = e.start_angle + angle; - m.end_angle = e.end_angle + angle; // rigid sweep, shifted by rotation + m.start_angle = e.start_angle + turn; + m.end_angle = e.end_angle + turn; // rigid sweep, shifted by rotation m.p0 = m.center + m.radius * Vec2d(std::cos(m.start_angle), std::sin(m.start_angle)); m.p1 = m.center + m.radius * Vec2d(std::cos(m.end_angle), std::sin(m.end_angle)); break; @@ -1486,7 +1490,7 @@ std::vector SketchEngine::transform_entities( m.center = xf(e.center); m.radius = e.radius * rs; m.rminor = e.rminor * rs; - m.rotation = e.rotation + angle; // major axis rotates with the body + m.rotation = e.rotation + turn; // major axis rotates with the body if (e.type == SketchEntity::Type::Ellipse) { m.p0 = m.center; } else { @@ -1833,6 +1837,15 @@ static double wrap_2pi(double x) return x; } +// An arc's endpoints are stored twice: as angles and as p0/p1. Everything downstream reads +// p0/p1 (the wire builder welds on them, the solver seeds from them, snapping uses them), so +// any edit of the angles must write them back or the arc's ends silently stay where they were. +static void sync_arc_ends(SketchEntity& e) +{ + e.p0 = e.center + e.radius * Vec2d(std::cos(e.start_angle), std::sin(e.start_angle)); + e.p1 = e.center + e.radius * Vec2d(std::cos(e.end_angle), std::sin(e.end_angle)); +} + bool SketchEngine::trim_entity(SketchEntity& e, const std::vector& others, const Vec2d& pick) { @@ -1867,6 +1880,7 @@ bool SketchEngine::trim_entity(SketchEntity& e, const std::vector& if (uc == -std::numeric_limits::max()) return false; e.end_angle = e.start_angle + uc * sweep; // drop (uc, 1] } + sync_arc_ends(e); return true; } @@ -1898,6 +1912,7 @@ bool SketchEngine::trim_entity(SketchEntity& e, const std::vector& e.type = SketchEntity::Type::Arc; e.start_angle = hi; e.end_angle = lo + 2.0 * M_PI; + sync_arc_ends(e); // p0 was the circle's centre (circle convention); now the arc's start return true; } @@ -1970,6 +1985,7 @@ bool SketchEngine::extend_entity(SketchEntity& e, const std::vector::max()) return false; if (extend_end) e.end_angle += sgn * best; else e.start_angle -= sgn * best; + sync_arc_ends(e); return true; } @@ -2010,7 +2026,9 @@ bool SketchEngine::extend_entity(SketchEntity& e, const std::vector points; bool closed{false}; - bool is_closed(double tolerance = 0.5) const; - bool try_close(double tolerance = 0.5); void clear() { points.clear(); closed = false; } TopoDS_Wire to_occt_wire(const SketchPlane& plane) const; @@ -108,8 +108,8 @@ enum class SketchConstraintType { // inserting anywhere but the end reinterprets every constraint in every saved recipe. EqualRadius, Collinear, - DistanceX, // |dx| between two points, projected onto the sketch X axis - DistanceY, // |dy| between two points, projected onto the sketch Y axis + DistanceX, // signed dx between two points (eb - ea), projected onto the sketch X axis + DistanceY, // signed dy between two points (eb - ea), projected onto the sketch Y axis SymmetricAboutY, // mirror across the sketch's vertical axis (x = 0); axis is implicit SymmetricAboutX // mirror across the sketch's horizontal axis (y = 0); axis is implicit }; @@ -314,7 +314,7 @@ public: // offset together and their seams repaired (miter join), so a closed profile comes back // closed and can still be extruded; per-entity offsetting cannot do that. Sign convention: // +d moves each curve to the LEFT of its direction of travel, which for a CCW closed loop - // is inward. Ellipses and splines are not offset (a parallel of either is not the same + // is inward; a full circle counts as CCW, so +d shrinks it. Ellipses and splines are not offset (a parallel of either is not the same // kind of curve) and are dropped from the result. static std::vector offset_entities( const std::vector& src, double d); diff --git a/src/libslic3r/CAD/SketchInference.cpp b/src/libslic3r/CAD/SketchInference.cpp index cfeb2cd509..4b056600e1 100644 --- a/src/libslic3r/CAD/SketchInference.cpp +++ b/src/libslic3r/CAD/SketchInference.cpp @@ -74,6 +74,18 @@ InferenceSnap infer_point_snap(const std::vector& entities, offer(InferenceSnap::Kind::Midpoint, ei, SketchPointRole::P0, Vec2d(e.center.x() + e.radius * std::cos(am), e.center.y() + e.radius * std::sin(am))); + // Nearest point on the arc itself (PointOnObject candidate), as for a line or a + // circle — the solver now takes a point on an arc's rim. + const Vec2d v = query - e.center; + const double n = v.norm(); + const double sweep = e.end_angle - e.start_angle; + if (n > 1e-9 && e.radius > 1e-9 && std::abs(sweep) > 1e-9) { + double u = (std::atan2(v.y(), v.x()) - e.start_angle) / sweep; + while (u < 0.0) u += 2.0 * M_PI / std::abs(sweep); + if (u > 0.02 && u < 0.98) + offer(InferenceSnap::Kind::OnEdge, ei, SketchPointRole::Center, + e.center + v * (e.radius / n)); + } break; } case SketchEntity::Type::Circle: { @@ -141,6 +153,10 @@ infer_relations(const std::vector& entities, int new_ei, { std::vector out; if (new_ei <= 0 || new_ei >= int(entities.size())) return out; + // Ends count as meeting at the same tolerance the wire builder welds them at; floor keeps + // exact coincidence meaningful when auto-close is switched off. + const double weld = std::max(sketch_join_tol(), 1e-7); + auto joined = [weld](const Vec2d& a, const Vec2d& b) { return (a - b).squaredNorm() <= weld * weld; }; // 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 @@ -170,10 +186,9 @@ infer_relations(const std::vector& entities, int new_ei, 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; + // "Connected" is the wire builder's weld, so what the viewport shows joined is. + const bool connected = joined(n.p0, o.p0) || joined(n.p0, o.p1) || + joined(n.p1, o.p0) || joined(n.p1, o.p1); 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); @@ -202,7 +217,7 @@ infer_relations(const std::vector& entities, int new_ei, 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; + if (!joined(le[k], ce[m])) 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; @@ -210,7 +225,7 @@ infer_relations(const std::vector& entities, int new_ei, } } 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 (std::abs(r.norm() - cv.radius) > weld) continue; if (r.squaredNorm() < 1e-18) continue; tangent = std::abs(unsigned_angle(ldir, r) - M_PI / 2.0) <= ang_tol_rad; } diff --git a/src/libslic3r/CAD/SketchInference.hpp b/src/libslic3r/CAD/SketchInference.hpp index be51f0240e..2b2345bffa 100644 --- a/src/libslic3r/CAD/SketchInference.hpp +++ b/src/libslic3r/CAD/SketchInference.hpp @@ -36,8 +36,12 @@ InferenceSnap infer_point_snap(const std::vector& entities, // within `ang_tol_rad` of an axis, returns Horizontal or Vertical (the constraint to // auto-emit on the committed segment); std::nullopt otherwise. Degenerate (near-zero // length) segments return nullopt. +// One angular tolerance for every "is this relation already true?" inference, so a pair of +// lines that reads as horizontal to one rule cannot read as not-quite-parallel to the other. +inline constexpr double kSketchInferAngleTol = 3.0 * M_PI / 180.0; + std::optional -infer_axis_constraint(const Vec2d& anchor, const Vec2d& tip, double ang_tol_rad = 3.0 * M_PI / 180.0); +infer_axis_constraint(const Vec2d& anchor, const Vec2d& tip, double ang_tol_rad = kSketchInferAngleTol); // 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. @@ -47,7 +51,7 @@ infer_axis_constraint(const Vec2d& anchor, const Vec2d& tip, double ang_tol_rad // 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 ang_tol_rad = kSketchInferAngleTol, double len_tol_frac = 0.01); } // namespace Slic3r diff --git a/src/libslic3r/CAD/SketchSolver.cpp b/src/libslic3r/CAD/SketchSolver.cpp index 952f9de16f..c07e287fb1 100644 --- a/src/libslic3r/CAD/SketchSolver.cpp +++ b/src/libslic3r/CAD/SketchSolver.cpp @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -41,12 +42,15 @@ struct Build { { return E(Slvs_MakePoint2d(++eh, g, wp, P(g, u), P(g, v))); } // Generic constraint (entityC unused by Slvs_MakeConstraint — set it manually below). + // `other` / `other2` pick the END point (point[2]) of entity A / B instead of its start + // (point[1]) for the constraints that read an arc's endpoint (the tangencies). void C(int type, double val, Slvs_hEntity ptA, Slvs_hEntity ptB, - Slvs_hEntity eA, Slvs_hEntity eB, Slvs_hEntity eC = 0, int other = 0) + Slvs_hEntity eA, Slvs_hEntity eB, Slvs_hEntity eC = 0, int other = 0, int other2 = 0) { Slvs_Constraint c = Slvs_MakeConstraint(++ch, G_SK, type, wp, val, ptA, ptB, eA, eB); c.entityC = eC; c.other = other; + c.other2 = other2; cons.push_back(c); } }; @@ -108,9 +112,15 @@ static SketchSolveResult solve_system(std::vector& entities, case SketchEntity::Type::Circle: { s.center = b.pt2d(G_SK, e.center.x(), e.center.y()); s.p0 = s.center; // p0 mirrors centre for circles - s.rparam = b.P(G_SK, e.radius > 1e-9 ? e.radius : 1.0); - Slvs_hEntity dist = b.E(Slvs_MakeDistance(++b.eh, G_SK, b.wp, s.rparam)); - s.prim = b.E(Slvs_MakeCircle(++b.eh, G_SK, b.wp, s.center, b.normal, dist)); + // A zero-radius circle is degenerate everywhere else (the wire builder, offset and + // inference all reject it). It used to be seeded with radius 1 here and written + // back, so any unrelated solve silently turned it into a 1 mm circle. It gets no + // primitive: its centre still solves, anything needing the rim is skipped. + if (e.radius > 1e-9) { + s.rparam = b.P(G_SK, e.radius); + Slvs_hEntity dist = b.E(Slvs_MakeDistance(++b.eh, G_SK, b.wp, s.rparam)); + s.prim = b.E(Slvs_MakeCircle(++b.eh, G_SK, b.wp, s.center, b.normal, dist)); + } break; } case SketchEntity::Type::Arc: @@ -170,6 +180,20 @@ static SketchSolveResult solve_system(std::vector& entities, switch (r) { case Role::P0: return e.p0; case Role::P1: return e.p1; case Role::Center: return e.center; } return e.p0; }; + // Which side of line `li` the point (ei, r) is on, in slvs' sign convention for the + // in-workplane PT_LINE_DISTANCE: with d = point[0] - point[1] and a = point[0], the signed + // distance has the sign of d x (p - a) (pinned by the tests "a point below the line stays + // below it" and "tangent line to circle, line below it"). +1 when on the line. + auto side_of = [&](int ei, Role r, int li) -> double { + Vec2d a, bb; + if (li == kSketchRefAxisX) { a = Vec2d(1, 0); bb = Vec2d(0, 0); } + else if (li == kSketchRefAxisY) { a = Vec2d(0, 1); bb = Vec2d(0, 0); } + else if (valid(li)) { a = entities[li].p0; bb = entities[li].p1; } + else return 1.0; + const Vec2d d = a - bb, ap = coordOf(ei, r) - a; + const double cr = d.x() * ap.y() - d.y() * ap.x(); + return cr < 0.0 ? -1.0 : 1.0; + }; // A fixed reference point at (x,y) — used to pin coordinates (Fix / LockX / LockY). auto fixedRef = [&](double x, double y) -> Slvs_hEntity { return b.pt2d(G_FIXED, x, y); }; @@ -207,7 +231,7 @@ static SketchSolveResult solve_system(std::vector& entities, case CT::Collinear: ref_ok = primOf(c.ea) && primOf(c.eb); break; } - if (!ref_ok) continue; + if (!ref_ok) { out.skipped.push_back(int(&c - constraints.data())); continue; } switch (c.type) { case CT::Coincident: b.C(SLVS_C_POINTS_COINCIDENT, 0, ptOf(c.ea, c.ra), ptOf(c.eb, c.rb), 0, 0); @@ -282,9 +306,36 @@ static SketchSolveResult solve_system(std::vector& entities, case CT::Tangent: { const bool aCurve = valid(c.ea) && entities[c.ea].type != SketchEntity::Type::Line; const bool bCurve = valid(c.eb) && entities[c.eb].type != SketchEntity::Type::Line; - if (aCurve && bCurve) - b.C(SLVS_C_CURVE_CURVE_TANGENT, 0, 0, 0, primOf(c.ea), primOf(c.eb)); - else { + const bool aCircle = aCurve && entities[c.ea].type == SketchEntity::Type::Circle; + const bool bCircle = bCurve && entities[c.eb].type == SketchEntity::Type::Circle; + if (aCurve && bCurve && (aCircle || bCircle)) { + // CURVE_CURVE_TANGENT reads each curve's ENDPOINT, which a full circle does not + // have: slvs asserts and takes the process down (the same trap the circle-line + // case below documents). Two round curves are tangent exactly when their + // centres are r1 + r2 apart (outside each other) or |r1 - r2| apart (one inside + // the other); keep whichever the sketch is closer to now. Radii are captured as + // constants, with the same caveat as the circle-line case. + const SketchEntity& A = entities[c.ea]; + const SketchEntity& B = entities[c.eb]; + const double d = (A.center - B.center).norm(); + const double ext = A.radius + B.radius; + const double in = std::abs(A.radius - B.radius); + b.C(SLVS_C_PT_PT_DISTANCE, std::abs(d - ext) <= std::abs(d - in) ? ext : in, + ptOf(c.ea, Role::Center), ptOf(c.eb, Role::Center), 0, 0); + } else if (aCurve && bCurve) { + // Tangent where the two arcs MEET: pick, for each arc, the endpoint closest to + // the other arc's nearest endpoint. Always binding the start point (the old + // behaviour) made a tangency at an arc's end act on its start instead. + const SketchEntity& A = entities[c.ea]; + const SketchEntity& B = entities[c.eb]; + int oa = 0, ob = 0; double best = 1e300; + for (int i = 0; i < 2; ++i) + for (int j = 0; j < 2; ++j) { + const double dd = ((i ? A.p1 : A.p0) - (j ? B.p1 : B.p0)).squaredNorm(); + if (dd < best) { best = dd; oa = i; ob = j; } + } + b.C(SLVS_C_CURVE_CURVE_TANGENT, 0, 0, 0, primOf(c.ea), primOf(c.eb), 0, oa, ob); + } else { const int ci = aCurve ? c.ea : c.eb; // the curve const int li = aCurve ? c.eb : c.ea; // the line if (valid(ci) && entities[ci].type == SketchEntity::Type::Circle) { @@ -305,24 +356,47 @@ static SketchSolveResult solve_system(std::vector& entities, // not being changed by another constraint in the same solve; if some other // constraint drives the radius, re-solving restores tangency. Tying them // would need an auxiliary point constrained onto both circle and line. - b.C(SLVS_C_PT_LINE_DISTANCE, entities[ci].radius, + b.C(SLVS_C_PT_LINE_DISTANCE, side_of(ci, Role::Center, li) * entities[ci].radius, ptOf(ci, Role::Center), 0, primOf(li), 0); } else { - b.C(SLVS_C_ARC_LINE_TANGENT, 0, 0, 0, primOf(ci), primOf(li)); + // The arc endpoint that touches the line is the one the tangency is at. The + // constraint used to bind the START point always, so a fillet — its start on + // one leg, its end on the other — forced both legs parallel. + int other = 0; + if (valid(ci) && valid(li)) { + const SketchEntity& A = entities[ci]; + const SketchEntity& L = entities[li]; + auto seg_dist = [&](const Vec2d& p) { + const Vec2d d = L.p1 - L.p0; + const double t = d.squaredNorm() > 1e-18 + ? std::clamp((p - L.p0).dot(d) / d.squaredNorm(), 0.0, 1.0) : 0.0; + return (L.p0 + t * d - p).norm(); + }; + other = seg_dist(A.p1) < seg_dist(A.p0) ? 1 : 0; + } + b.C(SLVS_C_ARC_LINE_TANGENT, 0, 0, 0, primOf(ci), primOf(li), 0, other); } } break; } case CT::PointOnLine: + // slvs' in-workplane point-line distance is SIGNED. A negative stored value is an + // explicit side; a positive one (what the UI stores) keeps the side the point is on + // now. Passing |value| forced every point to the positive side, flipping any that + // sat on the other one across the line. if (std::abs(c.value) < 1e-9) b.C(SLVS_C_PT_ON_LINE, 0, ptOf(c.ea, c.ra), 0, primOf(c.eb), 0); else - b.C(SLVS_C_PT_LINE_DISTANCE, std::abs(c.value), ptOf(c.ea, c.ra), 0, primOf(c.eb), 0); + b.C(SLVS_C_PT_LINE_DISTANCE, + c.value < 0.0 ? c.value : side_of(c.ea, c.ra, c.eb) * c.value, + ptOf(c.ea, c.ra), 0, primOf(c.eb), 0); break; case CT::PointOnObject: - // Point (ea,ra) lies on entity edge eb: a circle rim -> PT_ON_CIRCLE, - // otherwise the segment line -> PT_ON_LINE. - if (valid(c.eb) && entities[c.eb].type == SketchEntity::Type::Circle) + // Point (ea,ra) lies on entity edge eb: a circle or arc rim -> PT_ON_CIRCLE (slvs + // takes both), otherwise the segment line -> PT_ON_LINE. An arc used to fall to + // PT_ON_LINE, which is not an equation about an arc at all. + if (valid(c.eb) && (entities[c.eb].type == SketchEntity::Type::Circle || + entities[c.eb].type == SketchEntity::Type::Arc)) b.C(SLVS_C_PT_ON_CIRCLE, 0, ptOf(c.ea, c.ra), 0, primOf(c.eb), 0); else b.C(SLVS_C_PT_ON_LINE, 0, ptOf(c.ea, c.ra), 0, primOf(c.eb), 0); @@ -433,6 +507,29 @@ static SketchSolveResult solve_system(std::vector& entities, e.start_angle = ns; e.end_angle = ns + sweep; e.radius = 0.5 * ((e.p0 - e.center).norm() + (e.p1 - e.center).norm()); + } else if (e.type == SketchEntity::Type::EllipseArc && s.center && e.radius > 1e-9 && e.rminor > 1e-9) { + // The solver moves the centre and the two ends as free points (it has no conic), so + // after a solve they need not agree with the stored angles, and the wire builder then + // finds the arc's ends away from its vertices and drops the whole sketch. Re-derive + // the parametric angles from the solved ends and put the ends back ON the ellipse. + const double cr = std::cos(e.rotation), sr = std::sin(e.rotation); + auto param = [&](const Vec2d& p) { + const Vec2d d = p - e.center; + const double x = d.x() * cr + d.y() * sr, y = -d.x() * sr + d.y() * cr; + return std::atan2(y / e.rminor, x / e.radius); + }; + auto at = [&](double t) { + const double x = e.radius * std::cos(t), y = e.rminor * std::sin(t); + return Vec2d(e.center.x() + x * cr - y * sr, e.center.y() + x * sr + y * cr); + }; + const double old_sweep = e.end_angle - e.start_angle; + double t0 = param(e.p0), t1 = param(e.p1); + if (old_sweep >= 0.0) { while (t1 <= t0) t1 += 2.0 * M_PI; } + else { while (t1 >= t0) t1 -= 2.0 * M_PI; } + e.start_angle = t0; + e.end_angle = t1; + e.p0 = at(t0); + e.p1 = at(t1); } } @@ -460,6 +557,21 @@ static SketchSolveResult solve_system(std::vector& entities, // that fits today keeps its exact current behaviour, including its reported degrees of freedom. // A genuinely over-constrained sketch still fails: the conflict lives inside one component and // that component still rejects it. +// Degrees of freedom an entity has on its own, as the whole-system solve counts them. +static int natural_dof(const SketchEntity& e) +{ + switch (e.type) { + case SketchEntity::Type::Line: return 4; + case SketchEntity::Type::Point: return 2; + case SketchEntity::Type::Circle: return e.radius > 1e-9 ? 3 : 2; + case SketchEntity::Type::Arc: return 5; // centre + two ends, ends equidistant + case SketchEntity::Type::Ellipse: return 2; // only the centre is a solver point + case SketchEntity::Type::EllipseArc: return 6; // centre + two ends + case SketchEntity::Type::BSpline: return 2 * int(e.ctrl.size()); + } + return 0; +} + static SketchSolveResult solve_partitioned(std::vector& entities, const std::vector& constraints, int dragged_ei, Role dragged_role) @@ -478,12 +590,18 @@ static SketchSolveResult solve_partitioned(std::vector& entities, }; for (const auto& c : constraints) { unite(c.ea, c.eb); unite(c.ea, c.ec); } - // Group the constraints by the component they belong to. + // Group the constraints by the component they belong to: that of the first ENTITY they + // reference. ea can be a sketch reference (origin / axis, negative) while eb is the entity, + // and such a constraint was dropped outright. std::map> groups; + std::vector constrained(n, false); for (size_t i = 0; i < constraints.size(); ++i) { - const int a = constraints[i].ea; - if (a < 0 || a >= n) continue; + const SketchEntityConstraintDef& c = constraints[i]; + const int a = (c.ea >= 0 && c.ea < n) ? c.ea : (c.eb >= 0 && c.eb < n) ? c.eb + : (c.ec >= 0 && c.ec < n) ? c.ec : -1; + if (a < 0) continue; groups[find(a)].push_back(int(i)); + for (int e : { c.ea, c.eb, c.ec }) if (e >= 0 && e < n) constrained[e] = true; } SketchSolveResult out; @@ -509,7 +627,8 @@ static SketchSolveResult solve_partitioned(std::vector& entities, subc.reserve(cidx.size()); for (int ci : cidx) { SketchEntityConstraintDef d = constraints[ci]; - auto map1 = [&](int& e) { e = (e >= 0 && local.count(e)) ? local[e] : -1; }; + // Sketch references (origin, axes: negative sentinels) are global and pass through. + auto map1 = [&](int& e) { if (e >= 0) e = local.count(e) ? local[e] : -1; }; map1(d.ea); map1(d.eb); map1(d.ec); subc.push_back(d); } @@ -522,8 +641,14 @@ static SketchSolveResult solve_partitioned(std::vector& entities, if (bi >= 0 && bi < int(cidx.size())) out.bad.push_back(cidx[bi]); } if (r.dof > 0) out.dof += r.dof; + for (int si : r.skipped) + if (si >= 0 && si < int(cidx.size())) out.skipped.push_back(cidx[si]); solved.emplace_back(std::move(ents), std::move(sub)); } + // Entities no constraint touches are in no group but still have their freedoms; the + // whole-system solve counts them, so this path must too or the two report different dof. + for (int i = 0; i < n; ++i) + if (!constrained[i]) out.dof += natural_dof(entities[i]); if (!out.ok) return out; for (auto& [ents, sub] : solved) for (size_t k = 0; k < ents.size(); ++k) entities[ents[k]] = sub[k]; diff --git a/src/libslic3r/CAD/SketchSolver.hpp b/src/libslic3r/CAD/SketchSolver.hpp index ada4474c7a..e15eafe11c 100644 --- a/src/libslic3r/CAD/SketchSolver.hpp +++ b/src/libslic3r/CAD/SketchSolver.hpp @@ -16,6 +16,10 @@ struct SketchSolveResult { int dof{-1}; // remaining degrees of freedom (>0 under-constrained) int result{0}; // raw SLVS_RESULT_* code std::vector bad; // indices (into `constraints`) of conflicting constraints + // Indices of constraints that were NOT applied because an entity they reference has no + // solver representation for the role they need (an ellipse rim, a spline curve, a + // zero-radius circle...). A solve can be ok with some skipped; callers should say so. + std::vector skipped; }; // Solve `constraints` over `entities` in place (writes solved coordinates back into the diff --git a/src/libslic3r/CAD/ThreadStandards.hpp b/src/libslic3r/CAD/ThreadStandards.hpp index e9bfcc314f..c03c2033d8 100644 --- a/src/libslic3r/CAD/ThreadStandards.hpp +++ b/src/libslic3r/CAD/ThreadStandards.hpp @@ -24,6 +24,8 @@ struct ThreadSpec { double thread_depth_mm() const { return 0.6134 * pitch_mm; } // Internal/tapped minor (tap-drill) diameter for the same nominal thread. double minor_diameter_mm() const { return major_diameter_mm - 1.0825 * pitch_mm; } + // Radial depth of the internal (tapped) thread, minor to major: (D - D1) / 2. + double internal_depth_mm() const { return 0.5 * (major_diameter_mm - minor_diameter_mm()); } bool imperial() const { return series == Series::UNC || series == Series::UNF; } }; diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index d69ada09b6..fed30c1e86 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -605,7 +605,21 @@ TEST_CASE("entity constraints: point-on-line positions a centre onto an axis", " cons.push_back({T::Fix, 0, -1, R::P1, R::P1, 0.0}); cons.push_back({T::PointOnLine, 1, 0, R::P0, R::P0, 2.0}); // hold at distance 2 REQUIRE(solve_sketch_entities(ents, cons)); - REQUIRE(std::abs(std::abs(ents[1].p0.y()) - 2.0) < 1e-6); // 2 mm off the axis + REQUIRE(std::abs(ents[1].p0.y() - 2.0) < 1e-6); // 2 mm off, same side + } + + SECTION("a point below the line stays below it") { + std::vector ents = { + {SketchEntity::Type::Line, Vec2d(0,0), Vec2d(10,0)}, + {SketchEntity::Type::Point, Vec2d(4,-9)}, // point BELOW the axis + }; + std::vector cons; + cons.push_back({T::Fix, 0, -1, R::P0, R::P0, 0.0}); + cons.push_back({T::Fix, 0, -1, R::P1, R::P1, 0.0}); + cons.push_back({T::PointOnLine, 1, 0, R::P0, R::P0, 2.0}); + REQUIRE(solve_sketch_entities(ents, cons)); + // |value| used to be forced onto the positive side, flipping the point across the line. + REQUIRE(std::abs(ents[1].p0.y() + 2.0) < 1e-6); } } @@ -674,7 +688,38 @@ TEST_CASE("entity constraints: tangent/midpoint/symmetric/angle", "[CadDocument] REQUIRE(doc.solve_sketch_feature(sk)); const auto& e = doc.features[sk].entities; - REQUIRE(std::abs(std::abs(e[1].p0.y()) - 5.0) < 1e-3); + // The line started ABOVE the circle (y=8) and must stay on that side: tangency keeps + // the side the geometry is on, it does not pick one. + REQUIRE(std::abs(e[1].p0.y() - 5.0) < 1e-3); + } + + SECTION("tangent line to circle, line below it") { + CadDocument doc; + std::vector ents = { + {SketchEntity::Type::Circle, Vec2d(0,0), Vec2d(0,0), Vec2d(0,0), 5.0}, + {SketchEntity::Type::Line, Vec2d(-10,-8), Vec2d(10,-8)}, + }; + int sk = doc.add_sketch_entities(ents, SketchPlane::XY(), "S"); + auto& ec = doc.features[sk].entity_constraints; + ec.push_back({T::Fix, 0,-1, R::Center,R::Center, 0.0,-1,R::P0}); + ec.push_back({T::LockX, 1,-1, R::P0, R::P0, -10.0,-1,R::P0}); + ec.push_back({T::LockX, 1,-1, R::P1, R::P0, 10.0,-1,R::P0}); + ec.push_back({T::Horizontal, 1, 1, R::P0, R::P1, 0.0,-1,R::P0}); + ec.push_back({T::Tangent, 0, 1, R::Center,R::P0, 0.0,-1,R::P0}); + REQUIRE(doc.solve_sketch_feature(sk)); + REQUIRE(std::abs(doc.features[sk].entities[1].p0.y() + 5.0) < 1e-3); + } + + SECTION("tangent circle to circle does not abort and keeps them touching") { + std::vector ents = { + {SketchEntity::Type::Circle, Vec2d(0,0), Vec2d(0,0), Vec2d(0,0), 5.0}, + {SketchEntity::Type::Circle, Vec2d(9,0), Vec2d(9,0), Vec2d(9,0), 3.0}, + }; + std::vector cons; + cons.push_back({T::Fix, 0,-1, R::Center,R::Center, 0.0,-1,R::P0}); + cons.push_back({T::Tangent, 0, 1, R::Center,R::Center, 0.0,-1,R::P0}); + REQUIRE(solve_sketch_entities(ents, cons)); + CHECK(std::abs((ents[1].center - ents[0].center).norm() - 8.0) < 1e-6); // r1 + r2 } SECTION("symmetric across a line") { @@ -2136,7 +2181,7 @@ TEST_CASE("a v4 project still opens", "[CadDocument][recipe]") ifs.close(); REQUIRE_FALSE(blob.empty()); - // Trusted reference read of the flat v4 layout (what the golden test's Layer 1 does). + // Reference read of the flat v4 layout with the frozen v4 field list. std::vector flat; { std::istringstream iss(blob); @@ -2144,19 +2189,40 @@ TEST_CASE("a v4 project still opens", "[CadDocument][recipe]") uint32_t v; ar(v); REQUIRE(v == 4); - ar(flat); + cereal::size_type n = 0; + ar(cereal::make_size_tag(n)); + flat.assign(size_t(n), CadFeature{}); + for (CadFeature& f : flat) f.load_flat_v4(ar); + // The whole stream must be consumed by features + variables: a layout mismatch leaves + // bytes behind (or runs out), which is exactly what reading v4 with the CURRENT field + // list would do the moment a field is appended to it. + std::map vars; + ar(vars); + CHECK(iss.peek() == std::char_traits::eof()); } REQUIRE_FALSE(flat.empty()); CadDocument doc; - doc.deserialize_recipe(blob); - // Not refused at the gate: the only failure allowed is the fixture's own geometry - // (the golden doc is a serialization-coverage tree whose fillet radius is too large), - // which is orthogonal to the v5 framing change and unchanged by it. - REQUIRE(doc.error.find("older version") == std::string::npos); - REQUIRE(doc.error.find("newer version") == std::string::npos); - // The v4 flat path read the same feature tree as the trusted reference. + const bool ok = doc.deserialize_recipe(blob); + // It must OPEN: either cleanly, or failing only on the fixture's own geometry (the golden + // doc is a serialization-coverage tree whose fillet radius is too large). "CAD data could not + // be read" — a misread stream — passed the old version of this test, which only ruled out + // the version-gate messages. + if (!ok) { + INFO("v4 load error: " << doc.error); + CHECK(doc.error.find("could not be read") == std::string::npos); + CHECK(doc.error.find("older version") == std::string::npos); + CHECK(doc.error.find("newer version") == std::string::npos); + } + // The v4 path read the same feature tree as the reference, field for field. REQUIRE(doc.features.size() == flat.size()); + for (size_t i = 0; i < flat.size(); ++i) { + CHECK(doc.features[i].name == flat[i].name); + CHECK(doc.features[i].type == flat[i].type); + CHECK(doc.features[i].distance == flat[i].distance); + CHECK(doc.features[i].mate_kind == flat[i].mate_kind); + CHECK(doc.features[i].coordsys_face_edges == flat[i].coordsys_face_edges); + } } // Do NOT regenerate cad_recipe_v5.bin either. It was written by the build that predates the @@ -2280,10 +2346,13 @@ TEST_CASE("a truncated feature keeps what it could read", "[CadDocument][recipe] off += 4 + f_len.back(); } - // Shorten feature 1 by dropping its final field (4 bytes): rewrite its length prefix - // and erase the tail bytes. The reader then runs out inside fa(f), throws, and keeps - // everything it had already assigned — that is the whole point of the try/catch. - const size_t drop = sizeof(uint32_t); + // Shorten feature 1 so it ends right after coordsys_face_kind: drop coordsys_face_edges + // (4 bytes) and the two flags appended after it (thread_major_nominal, pattern_inclusive: + // 1 byte each). Rewrite its length prefix and erase the tail bytes. The reader then runs + // out inside fa(f), throws, and keeps everything it had already assigned — that is the + // whole point of the try/catch. (Cut on a field boundary: a field cut in half is read as + // whatever half arrived.) + const size_t drop = sizeof(uint32_t) + 2 * sizeof(bool); REQUIRE(f_len[1] > drop); std::string shortened = blob; shortened.erase(f_off[1] + 4 + f_len[1] - drop, drop); @@ -2299,6 +2368,8 @@ TEST_CASE("a truncated feature keeps what it could read", "[CadDocument][recipe] REQUIRE(loaded.features[1].name == doc.features[1].name); REQUIRE(loaded.features[1].coordsys_face_kind == 777); // right before the cut REQUIRE(loaded.features[1].coordsys_face_edges == -1); // defaulted by the cut + REQUIRE_FALSE(loaded.features[1].thread_major_nominal); // ...and so were the later flags + REQUIRE_FALSE(loaded.features[1].pattern_inclusive); REQUIRE(loaded.features[0].name == doc.features[0].name); REQUIRE(loaded.features[2].name == doc.features[2].name); } @@ -4166,8 +4237,12 @@ TEST_CASE("golden recipe v1 still deserialises", "[CadDocument]") cereal::BinaryInputArchive ar(iss); uint32_t v; ar(v); - REQUIRE(v <= CadDocument::ORCA_CAD_RECIPE_VERSION); - ar(features); + REQUIRE(v == 4); + // The fixture is a v4 (flat) blob: read it with the frozen v4 field list. + cereal::size_type n = 0; + ar(cereal::make_size_tag(n)); + features.assign(size_t(n), CadFeature{}); + for (CadFeature& f : features) f.load_flat_v4(ar); } CadDocument expected = make_golden_doc_v1(); @@ -4641,41 +4716,26 @@ TEST_CASE("bridge closes a C profile and extrudes", "[CadDocument][bridge]") // top edge: (10,10) to (-10,10) {SketchEntity::Type::Line, Vec2d(10,10), Vec2d(-10,10)}, }; - // Missing: left edge from (-10,10) to (-10,-10). Build it as a separate line - // entity so the bridge connects two existing lines. + // The C is open on the left: entity 2 (top) ends at (-10,10), entity 0 (bottom) starts at + // (-10,-10). The bridge IS the closing edge. int sk = doc.add_sketch_entities(ents, SketchPlane::XY(), "C"); REQUIRE(sk == 0); - - // Add the closing line as entity 3: (-10,10) to (-10,-10) CadFeature& f = doc.features[sk]; - SketchEntity closing; - closing.type = SketchEntity::Type::Line; - closing.p0 = Vec2d(-10, 10); - closing.p1 = Vec2d(-10, -10); - // The C is entities 0,1,2 (bottom cap, right side, top cap). - // Entity 0 end=1 is (10,-10); entity 2 start=0 is (10,10). That's a U. - // But we need a closed square from C shape. - // Re-think: a C shape open on the left side. - // Entities: 0 = bottom edge (-10,-10)->(10,-10) [end=1 at (10,-10)] - // 1 = right edge (10,-10)->(10,10) [start=0 at (10,-10), end=1 at (10,10)] - // 2 = top edge (10,10)->(-10,10) [start=0 at (10,10), end=1 at (-10,10)] - // The C is open: entity 2's end is at (-10,10) and entity 0's start is at (-10,-10). - // Bridge: entity 2 end=1 (-10,10) -> entity 0 start=0 (-10,-10). - f.entities.push_back(closing); + + int bi = doc.add_bridge(sk, 2/*top edge*/, 1/*end*/, 0/*bottom edge*/, 0/*start*/, "Bridge"); + REQUIRE(bi == 3); REQUIRE(f.entities.size() == 4); - // Now bridge from top end (entity 2 end=1 = (-10,10)) to bottom start (entity 0 end=0 = (-10,-10)) - int bi = doc.add_bridge(sk, 2/*top edge*/, 1/*end*/, 0/*bottom edge*/, 0/*start*/, "Bridge"); - REQUIRE(bi == 4); - REQUIRE(f.entities.size() == 5); - - // Now the entities should form a closed loop -> extrude int ex = doc.add_extrude(sk, 5.0, false, BooleanMode::New, "Extrude"); REQUIRE(ex >= 0); REQUIRE(doc.recompute()); REQUIRE(doc.error.empty()); REQUIRE(doc.display_mesh.facets_count() > 0); - REQUIRE_THAT(double(doc.display_mesh.volume()), WithinRel(20.0 * 20.0 * 5.0, 1e-2)); + // G1 with both edges means the bridge LEAVES the top edge heading -X and ARRIVES on the + // bottom edge heading +X: it bulges out to the left, a cubic whose inner poles sit 20/3 mm + // out, enclosing 3/5 x 20/3 x 20 = 80 mm2 beyond the square. (This test used to expect the + // bare square: the old end pole sent the curve back across x = -10 in an S, a cusp.) + REQUIRE_THAT(double(doc.display_mesh.volume()), WithinRel((20.0 * 20.0 + 80.0) * 5.0, 1e-2)); } TEST_CASE("bridge bad indices throw", "[CadDocument][bridge]") @@ -7383,11 +7443,16 @@ TEST_CASE("Failed sketch solve leaves geometry untouched", "[CadDocument]") auto tang = [&](int ln) { SketchEntityConstraintDef d; d.type = CT::Tangent; d.ea = xi; d.eb = ln; return d; }; - // Rung 1 of the ladder: a tangent on each leg. Over-constrained against the legs' - // own H/V, so it must be rejected -- and must not move a single point. + // A genuinely conflicting batch (the right leg held at two different lengths) must be + // rejected -- and must not move a single point. { std::vector pc = cs; - for (const auto& c : { coin(R::P0, a, R::P0), coin(R::P1, b, R::P1), tang(a), tang(b) }) + SketchEntityConstraintDef d10, d20; // one leg held at two different lengths + d10.type = d20.type = CT::Distance; + d10.ea = d20.ea = d10.eb = d20.eb = 1; + d10.ra = d20.ra = R::P0; d10.rb = d20.rb = R::P1; + d10.value = 100.0; d20.value = 120.0; + for (const auto& c : { coin(R::P0, a, R::P0), coin(R::P1, b, R::P1), d10, d20 }) pc.push_back(c); std::vector e = ents; REQUIRE_FALSE(solve_sketch_entities(e, pc)); @@ -7398,6 +7463,22 @@ TEST_CASE("Failed sketch solve leaves geometry untouched", "[CadDocument]") } } + // Rung 1 of the ladder — a tangent on EACH leg — is a well-posed fillet, not a conflict. It + // used to be rejected because both tangencies were bound to the arc's START point, which + // forced the two legs parallel; each is now bound to the end that touches its leg. + { + std::vector pc = cs; + for (const auto& c : { coin(R::P0, a, R::P0), coin(R::P1, b, R::P1), tang(a), tang(b) }) + pc.push_back(c); + std::vector e = ents; + REQUIRE(solve_sketch_entities(e, pc)); + CHECK(e[xi].radius == Approx(28.205).margin(1e-6)); + // Tangent at both ends: the radius to each end is perpendicular to that end's leg. + const Vec2d da = (e[a].p1 - e[a].p0).normalized(), db = (e[b].p1 - e[b].p0).normalized(); + CHECK(std::abs((e[xi].p0 - e[xi].center).normalized().dot(da)) < 1e-6); + CHECK(std::abs((e[xi].p1 - e[xi].center).normalized().dot(db)) < 1e-6); + } + // Rung 2 (one tangent) solves, and the arc keeps the radius the fillet gave it. { std::vector pc = cs; @@ -8412,3 +8493,202 @@ TEST_CASE("deleting a feature remaps the references of every consumer, not just REQUIRE(doc.features.empty()); } } + +// A sketch past libslvs' MAX_UNKNOWNS is solved component by component. Constraints onto the +// sketch origin / axes reference NEGATIVE sentinels; the partitioned path used to turn them into +// "no entity" and drop the constraint while still reporting success. +TEST_CASE("Partitioned solve keeps constraints onto the origin and the axes", "[CadDocument][sketch]") +{ + using R = SketchPointRole; + using T = SketchConstraintType; + std::vector ents; + std::vector cons; + const int n = 600; // 600 lines x 4 params: well past the 1024-unknown ceiling + for (int i = 0; i < n; ++i) { + SketchEntity e; e.type = SketchEntity::Type::Line; + e.p0 = Vec2d(1.0 + 0.01 * i, 2.0); e.p1 = Vec2d(5.0 + 0.01 * i, 7.0 + i); + ents.push_back(e); + // P0 onto the origin: the sentinel is in eb, the entity in ea... + cons.push_back({T::Coincident, i, kSketchRefOrigin, R::P0, R::P0, 0.0}); + } + // ...and line 1's far end onto the X axis. + SketchEntityConstraintDef on_x; on_x.type = T::PointOnObject; + on_x.ea = 1; on_x.ra = R::P1; on_x.eb = kSketchRefAxisX; + cons.push_back(on_x); + REQUIRE(solve_sketch_entities(ents, cons)); + for (int i = 0; i < n; ++i) + REQUIRE(ents[i].p0.norm() < 1e-6); + CHECK(std::abs(ents[1].p1.y()) < 1e-6); +} + +// ---- references that must follow their target across a change of history ---------------- + +static CadDocument two_boxes_and_a_lift(int& ea, int& eb, int& lift) +{ + CadDocument doc; + SketchPlane pa = SketchPlane::XY(); + SketchPlane pb = SketchPlane::XY(); pb.origin = Vec3d(100, 0, 0); + const int sa = doc.add_sketch(SketchShape::Rectangle, pa, 10, 10, 0, "A"); + ea = doc.add_extrude(sa, 5, false, BooleanMode::New, "EA"); // body 0 + const int sb = doc.add_sketch(SketchShape::Rectangle, pb, 10, 10, 0, "B"); + eb = doc.add_extrude(sb, 5, false, BooleanMode::New, "EB"); // body 1 + lift = doc.add_transform(1, Vec3d(0, 0, 50), Vec3d(0, 0, 1), Vec3d(0, 0, 0), 0, false, "Lift B"); + REQUIRE(doc.recompute()); + return doc; +} + +TEST_CASE("Deleting the feature that made an EARLIER body keeps later features on their body", + "[CadDocument][history]") +{ + int ea, eb, lift; + CadDocument doc = two_boxes_and_a_lift(ea, eb, lift); + REQUIRE(doc.bodies.size() == 2); + // Body A goes away, so B becomes body 0. "Lift B" said body 1: by index it would now name + // nothing (and used to fall back to the last body in silence); by identity it is still B. + REQUIRE(doc.remove_feature(ea)); + REQUIRE(doc.bodies.size() == 1); + CHECK(doc.features[lift - 1].target_body == 0); + const auto bb = doc.display_mesh.bounding_box(); + CHECK(bb.min.z() > 49.0); // B was lifted + CHECK(bb.min.x() > 90.0); // and it is B, not A +} + +TEST_CASE("Deleting the feature that made a body a later feature works on is refused", + "[CadDocument][history]") +{ + int ea, eb, lift; + CadDocument doc = two_boxes_and_a_lift(ea, eb, lift); + const size_t n = doc.features.size(); + CHECK_FALSE(doc.remove_feature(eb)); // "Lift B" would have nothing to lift + CHECK(doc.features.size() == n); // rolled back + CHECK(doc.bodies.size() == 2); + CHECK_FALSE(doc.error.empty()); +} + +TEST_CASE("Hiding a body-making feature keeps later features on their body", "[CadDocument][history]") +{ + int ea, eb, lift; + CadDocument doc = two_boxes_and_a_lift(ea, eb, lift); + REQUIRE(doc.set_feature_enabled(ea, false)); + REQUIRE(doc.bodies.size() == 1); + CHECK(doc.display_mesh.bounding_box().min.z() > 49.0); + // ...and showing it again puts the reference back where it was. + REQUIRE(doc.set_feature_enabled(ea, true)); + REQUIRE(doc.bodies.size() == 2); + CHECK(doc.features[lift].target_body == 1); +} + +TEST_CASE("Datum-plane references follow their plane when an earlier plane is deleted", + "[CadDocument][history]") +{ + CadDocument doc; + const int p1 = doc.add_plane(0, 10, 0, 0, "P1"); // datum 0 (3 + 0) + doc.add_plane(0, 20, 0, 0, "P2"); // datum 1 (3 + 1) + const int p3 = doc.add_plane(3 + 1, 5, 0, 0, "P3"); // built on P2: z = 25 + auto planes = doc.resolve_datum_planes(); + REQUIRE(planes.size() == 3); + REQUIRE(std::abs(planes[2].second.origin.z() - 25.0) < 1e-9); + REQUIRE(doc.remove_feature(p1)); + CHECK(doc.features[p3 - 1].plane_base == 3 + 0); // P2 is datum 0 now + planes = doc.resolve_datum_planes(); + REQUIRE(planes.size() == 2); + CHECK(std::abs(planes[1].second.origin.z() - 25.0) < 1e-9); // still on P2, not on XY +} + +TEST_CASE("clear() starts a document without the previous one's variables", "[CadDocument]") +{ + CadDocument doc; + doc.variables["w"] = "12"; + doc.clear(); + CHECK(doc.variables.empty()); +} + +TEST_CASE("Expressions: trig takes degrees, deg()/rad() convert, pi is the number", "[CadDocument][expr]") +{ + CadDocument doc; + const int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "S"); + const int ex = doc.add_extrude(sk, 5, false, BooleanMode::New, "E"); + doc.features[ex].expr["distance"] = "10 * sin(deg(pi / 2)) + 10 * cos(rad(0) + 90) + 2 * cos(60)"; + REQUIRE(doc.recompute()); + CHECK(std::abs(doc.features[ex].distance - 11.0) < 1e-9); // 10 + 0 + 1 +} + +TEST_CASE("Every field the Design tab offers for an expression can be bound", "[CadDocument][expr]") +{ + // The per-tool pickers in DesignPanel::fields_for_tool. A name the kernel does not know made + // the next recompute throw "unknown parameter" and the whole model stopped rebuilding. + for (const char* f : { "width", "height", "radius", "distance", "distance2", "taper_deg", + "dressup_size", "hole_diameter", "hole_depth", "hole_x", "hole_y", + "thread_diameter", "thread_pitch", "thread_height", "thread_depth", + "thread_x", "thread_y", "shell_thickness", "revolve_angle", + "pattern_count", "pattern_spacing", "pattern_angle", "plane_offset", + "plane_angle_tilt", "draft_angle", "thicken_thickness", "rib_thickness", + "rib_depth", "helix_radius", "helix_pitch", "helix_height", + "helix_taper_deg" }) { + INFO(f); + CHECK(CadDocument::is_bindable_field(f)); + } + CHECK_FALSE(CadDocument::is_bindable_field("no_such_field")); +} + +TEST_CASE("A circular pattern spans its whole angle", "[CadDocument][pattern]") +{ + // A 2 mm cube 10 mm out on +X, three copies over 90°: at 0°, 45° and 90°. The old spacing + // (angle / count) stopped at 60°, so the last copy never reached the +Y axis. + CadDocument doc; + SketchPlane p = SketchPlane::XY(); p.origin = Vec3d(10, 0, 0); + const int sk = doc.add_sketch(SketchShape::Rectangle, p, 2, 2, 0, "S"); + doc.add_extrude(sk, 2, false, BooleanMode::New, "E"); + doc.add_pattern(true, 3, 0, 0, 90.0, -1, "P"); + REQUIRE(doc.recompute()); + CHECK(doc.display_mesh.bounding_box().max.y() > 10.5); +} + +TEST_CASE("Hole standards: inch sizes by either name, with their 82° countersink", "[CadDocument][hole]") +{ + CadDocument doc; + const int a = doc.add_hole_standard("1/4-20", 2, true, 10, 0, 0, SketchPlane::XY(), "H1"); + const int b = doc.add_hole_standard("#10-24 UNC", 2, true, 10, 0, 0, SketchPlane::XY(), "H2"); + CHECK(doc.features[a].hole_standard == "1/4-20 UNC"); + CHECK(doc.features[a].hole_csink_angle == 82.0); + CHECK(doc.features[b].hole_standard == "#10-24 UNC"); + const int m = doc.add_hole_standard("M6", 1, true, 10, 0, 0, SketchPlane::XY(), "H3"); + CHECK(doc.features[m].hole_csink_angle == 90.0); + CHECK(doc.features[m].hole_cbore_diameter == 11.0); +} + +TEST_CASE("Body colours and names survive a save and a load", "[CadDocument][recipe]") +{ + CadDocument doc; + const int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 10, 10, 0, "S"); + doc.add_extrude(sk, 5, false, BooleanMode::New, "E"); + doc.modeling_origin = Vec3d(110, 120, 0); + REQUIRE(doc.recompute()); + doc.bodies[0].has_color = true; + doc.bodies[0].color = ColorRGBA(0.1f, 0.2f, 0.3f, 1.0f); + const std::string blob = doc.serialize_recipe(); + CadDocument back; + back.modeling_origin = Vec3d(1, 2, 3); // a different printer's bed centre + REQUIRE(back.deserialize_recipe(blob)); + REQUIRE(back.bodies.size() == 1); + CHECK(back.bodies[0].has_color); + CHECK(std::abs(back.bodies[0].color.g() - 0.2f) < 1e-6); + CHECK((back.modeling_origin - Vec3d(110, 120, 0)).norm() < 1e-9); // the project's own + CHECK(back.origin_from_recipe); +} + +TEST_CASE("A new thread reads its radius as the nominal (major) radius", "[CadDocument][thread]") +{ + CadDocument doc; + const int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 30, 30, 0, "S"); + doc.add_extrude(sk, 12, false, BooleanMode::New, "E"); + // M6 internal: major radius 3, depth (major - minor) / 2. + const double P = 1.0, depth = 0.5 * 1.0825 * P; + const int t = doc.add_thread(3.0, P, 10.0, depth, true, 0, 0, SketchPlane::XY(), "T"); + CHECK(doc.features[t].thread_major_nominal); + REQUIRE(doc.recompute()); + // A degenerate new thread says why instead of doing nothing. + doc.features[t].thread_depth = 0.9 * P; + CHECK_FALSE(doc.recompute()); + CHECK(doc.error.find("thread") != std::string::npos); +} diff --git a/tests/libslic3r/test_sketchedit.cpp b/tests/libslic3r/test_sketchedit.cpp index 634e743b99..9e97684fc2 100644 --- a/tests/libslic3r/test_sketchedit.cpp +++ b/tests/libslic3r/test_sketchedit.cpp @@ -110,7 +110,10 @@ TEST_CASE("Offset Line by positive d", "[SketchEdit]") REQUIRE_THAT(o.p1.y(), WithinAbs(2.0, 1e-9)); } -TEST_CASE("Offset Circle: expand and collapse", "[SketchEdit]") +// CONTRACT CHANGED: a circle follows the arc's "+d = left of travel" rule and counts as CCW, so +// +d shrinks it and -d grows it. It used to grow on +d, the opposite of the same outline drawn +// as a CCW chain of arcs. +TEST_CASE("Offset Circle: +d shrinks (a circle is CCW), -d grows, too far collapses", "[SketchEdit]") { SketchEntity e; e.type = SketchEntity::Type::Circle; @@ -118,14 +121,36 @@ TEST_CASE("Offset Circle: expand and collapse", "[SketchEdit]") e.p0 = Vec2d(0, 0); e.radius = 5; - auto expanded = SketchEngine::offset_entities({e}, 2.0); - REQUIRE(expanded.size() == 1); - REQUIRE_THAT(expanded[0].radius, WithinAbs(7.0, 1e-9)); + auto inward = SketchEngine::offset_entities({e}, 2.0); + REQUIRE(inward.size() == 1); + REQUIRE_THAT(inward[0].radius, WithinAbs(3.0, 1e-9)); - auto collapsed = SketchEngine::offset_entities({e}, -5.0); + auto outward = SketchEngine::offset_entities({e}, -2.0); + REQUIRE(outward.size() == 1); + REQUIRE_THAT(outward[0].radius, WithinAbs(7.0, 1e-9)); + + auto collapsed = SketchEngine::offset_entities({e}, 5.0); REQUIRE(collapsed.empty()); } +TEST_CASE("Offset: a full circle and the same CCW outline drawn as arcs go the same way", "[SketchEdit]") +{ + SketchEntity c; + c.type = SketchEntity::Type::Circle; c.center = Vec2d(0, 0); c.p0 = c.center; c.radius = 5; + SketchEntity a0, a1; + a0.type = a1.type = SketchEntity::Type::Arc; + a0.center = a1.center = Vec2d(0, 0); + a0.radius = a1.radius = 5; + a0.start_angle = 0.0; a0.end_angle = M_PI; a0.p0 = Vec2d(5, 0); a0.p1 = Vec2d(-5, 0); + a1.start_angle = M_PI; a1.end_angle = 2.0 * M_PI; a1.p0 = Vec2d(-5, 0); a1.p1 = Vec2d(5, 0); + const auto oc = SketchEngine::offset_entities({ c }, 1.0); + const auto oa = SketchEngine::offset_entities({ a0, a1 }, 1.0); + REQUIRE(oc.size() == 1); + REQUIRE(oa.size() == 2); + CHECK_THAT(oc[0].radius, WithinAbs(oa[0].radius, 1e-9)); + CHECK_THAT(oc[0].radius, WithinAbs(oa[1].radius, 1e-9)); +} + // CONTRACT CHANGED: +d used to mean "radius + d" for every arc regardless of its sweep, while // for a line it meant "left of the direction of travel". The two disagreed, so a profile made // of lines AND arcs (any slot outline) offset with its straights going one way and its caps the @@ -375,6 +400,9 @@ TEST_CASE("Trim arc drops the picked (start) side", "[SketchEdit]") REQUIRE_THAT(e.radius, WithinAbs(5.0, 1e-9)); REQUIRE_THAT(e.start_angle, WithinAbs(M_PI / 2.0, 1e-9)); REQUIRE_THAT(e.end_angle, WithinAbs(M_PI, 1e-9)); + // The stored endpoints follow the angles: everything downstream reads p0/p1. + CHECK_THAT(e.p0.x(), WithinAbs(0.0, 1e-9)); CHECK_THAT(e.p0.y(), WithinAbs(5.0, 1e-9)); + CHECK_THAT(e.p1.x(), WithinAbs(-5.0, 1e-9)); CHECK_THAT(e.p1.y(), WithinAbs(0.0, 1e-9)); } TEST_CASE("Trim arc drops the picked (end) side", "[SketchEdit]") @@ -397,6 +425,8 @@ TEST_CASE("Trim arc drops the picked (end) side", "[SketchEdit]") REQUIRE(e.type == SketchEntity::Type::Arc); REQUIRE_THAT(e.start_angle, WithinAbs(0.0, 1e-9)); REQUIRE_THAT(e.end_angle, WithinAbs(M_PI / 2.0, 1e-9)); + CHECK_THAT(e.p0.x(), WithinAbs(5.0, 1e-9)); CHECK_THAT(e.p0.y(), WithinAbs(0.0, 1e-9)); + CHECK_THAT(e.p1.x(), WithinAbs(0.0, 1e-9)); CHECK_THAT(e.p1.y(), WithinAbs(5.0, 1e-9)); } TEST_CASE("Trim circle opens into an arc excluding the pick", "[SketchEdit]") @@ -406,7 +436,7 @@ TEST_CASE("Trim circle opens into an arc excluding the pick", "[SketchEdit]") SketchEntity e; e.type = SketchEntity::Type::Circle; e.center = Vec2d(0, 0); - e.p0 = Vec2d(5, 0); + e.p0 = e.center; // circle convention: p0 mirrors the centre e.radius = 5; SketchEntity cut; @@ -423,6 +453,11 @@ TEST_CASE("Trim circle opens into an arc excluding the pick", "[SketchEdit]") double mid = 0.5 * (e.start_angle + e.end_angle); REQUIRE_THAT(5 * std::cos(mid), WithinAbs(-5.0, 1e-9)); REQUIRE_THAT(5 * std::sin(mid), WithinAbs(0.0, 1e-9)); + // p0 was the circle's centre; as an arc it must be the arc's start, on the rim. + CHECK_THAT((e.p0 - e.center).norm(), WithinAbs(5.0, 1e-9)); + CHECK_THAT((e.p1 - e.center).norm(), WithinAbs(5.0, 1e-9)); + CHECK_THAT(e.p0.x(), WithinAbs(5.0 * std::cos(e.start_angle), 1e-9)); + CHECK_THAT(e.p1.x(), WithinAbs(5.0 * std::cos(e.end_angle), 1e-9)); } TEST_CASE("Extend arc forward (end) to a crossing", "[SketchEdit]") @@ -446,6 +481,7 @@ TEST_CASE("Extend arc forward (end) to a crossing", "[SketchEdit]") REQUIRE(e.type == SketchEntity::Type::Arc); REQUIRE_THAT(e.start_angle, WithinAbs(0.0, 1e-9)); REQUIRE_THAT(e.end_angle, WithinAbs(M_PI, 1e-9)); + CHECK_THAT(e.p1.x(), WithinAbs(-5.0, 1e-9)); CHECK_THAT(e.p1.y(), WithinAbs(0.0, 1e-9)); } TEST_CASE("Extend arc backward (start) to a crossing", "[SketchEdit]") @@ -475,7 +511,7 @@ TEST_CASE("Extend circle returns false (closed)", "[SketchEdit]") SketchEntity e; e.type = SketchEntity::Type::Circle; e.center = Vec2d(0, 0); - e.p0 = Vec2d(5, 0); + e.p0 = e.center; // circle convention: p0 mirrors the centre e.radius = 5; SketchEntity cut; @@ -728,3 +764,66 @@ TEST_CASE("sketch_open_ends names where a chain fails to close", "[SketchEngine] REQUIRE_THAT(got[1].x(), WithinAbs(0.0, 1e-9)); REQUIRE_THAT(got[1].y(), WithinAbs(10.0, 1e-9)); } + +TEST_CASE("Mirror EllipseArc keeps the same arc, not its complement", "[SketchEdit]") +{ + // A quarter of an ellipse a=6 b=3 centred at (10,0), CCW from param 0 to pi/2, mirrored + // across the Y axis: the image is a quarter again (the complement would be three quarters), + // its start angle belongs to its p0, and its middle is still above the X axis. + SketchEntity e; + e.type = SketchEntity::Type::EllipseArc; + e.center = Vec2d(10, 0); e.radius = 6; e.rminor = 3; e.rotation = 0.0; + e.start_angle = 0.0; e.end_angle = M_PI / 2.0; + e.p0 = Vec2d(16, 0); e.p1 = Vec2d(10, 3); + const auto out = SketchEngine::mirror_entities({ e }, Vec2d(0, -1), Vec2d(0, 1)); + REQUIRE(out.size() == 1); + const SketchEntity& m = out[0]; + REQUIRE(m.type == SketchEntity::Type::EllipseArc); + CHECK_THAT(m.end_angle - m.start_angle, WithinAbs(M_PI / 2.0, 1e-9)); + const double cr = std::cos(m.rotation), sr = std::sin(m.rotation); + auto at = [&](double t) { + return Vec2d(m.center.x() + m.radius * std::cos(t) * cr - m.rminor * std::sin(t) * sr, + m.center.y() + m.radius * std::cos(t) * sr + m.rminor * std::sin(t) * cr); + }; + CHECK((at(m.start_angle) - m.p0).norm() < 1e-6); + CHECK((at(m.end_angle) - m.p1).norm() < 1e-6); + CHECK(at(0.5 * (m.start_angle + m.end_angle)).y() > 1.0); +} + +TEST_CASE("Bridge between two collinear lines is straight and stays between them", "[SketchEdit]") +{ + SketchEntity a, b; + a.type = b.type = SketchEntity::Type::Line; + a.p0 = Vec2d(-10, 0); a.p1 = Vec2d(0, 0); // ends at x=0 heading +X + b.p0 = Vec2d(9, 0); b.p1 = Vec2d(19, 0); // starts at x=9 heading +X + const SketchEntity br = SketchEngine::make_bridge(a, 1, b, 0); + REQUIRE(br.ctrl.size() == 4); + // G1 into b: the last inner pole sits BEFORE b's start, on the side the curve arrives from. + CHECK(br.ctrl[1].x() > 0.0); + CHECK(br.ctrl[2].x() < 9.0); + for (const Vec2d& p : br.ctrl) CHECK_THAT(p.y(), WithinAbs(0.0, 1e-9)); +} + +TEST_CASE("Transform with a negative scale keeps an arc on its endpoints", "[SketchEdit]") +{ + SketchEntity e; + e.type = SketchEntity::Type::Arc; + e.center = Vec2d(3, 0); e.radius = 2; e.start_angle = 0.0; e.end_angle = M_PI / 2.0; + e.p0 = Vec2d(5, 0); e.p1 = Vec2d(3, 2); + const auto out = SketchEngine::transform_entities({ e }, Vec2d(0, 0), 0.0, -1.0, Vec2d(0, 0)); + REQUIRE(out.size() == 1); + const SketchEntity& m = out[0]; + // A point reflection through the origin: centre (-3,0), start (-5,0), end (-3,-2). + CHECK((m.center - Vec2d(-3, 0)).norm() < 1e-9); + CHECK((m.p0 - Vec2d(-5, 0)).norm() < 1e-9); + CHECK((m.p1 - Vec2d(-3, -2)).norm() < 1e-9); +} + +TEST_CASE("A zero-radius circle does not build a wire (and does not throw)", "[SketchEdit]") +{ + SketchEntity c; + c.type = SketchEntity::Type::Circle; c.center = Vec2d(0, 0); c.p0 = c.center; c.radius = 0.0; + std::vector w; + REQUIRE_NOTHROW(w = SketchEngine::entities_to_wires({ c }, SketchPlane::XY(), true)); + CHECK(w.empty()); +}