Design: fix hole placement (top-face default) + invisible internal thread

Hole: with no face explicitly picked, the tool fell back to the XY datum at
z=0 (the model's underside), so placing a hole from a top view read parallax-
shifted. Default to the solid's top face (top_face_index_of) so the footprint
sits on the surface being viewed; the XY/XZ/YZ dropdown still overrides.

Internal thread: the bore was re-cut at the nominal radius, which coincides
with an existing hole's wall — the coincident faces fouled the groove boolean
so it removed ~nothing (invisible thread). Cut the bore at the minor diameter
(radius - depth) instead: strictly inside any existing wall, leaving it clean
for the groove; on solid stock it forms the tap-drill.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVzKmX6Y1aEteit1HTXG4Q
This commit is contained in:
Tommaso Bianchi
2026-07-02 20:18:41 +02:00
co-authored by Claude Opus 4.8
parent 2a9a433127
commit f8175fc9a4
2 changed files with 37 additions and 5 deletions
+7 -1
View File
@@ -1406,7 +1406,13 @@ void CadDocument::apply_feature(TopoDS_Shape& result, bool& have_body,
// 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.
TopoDS_Shape bore = BRepPrimAPI_MakeCylinder(ax2, f.thread_radius,
// 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.
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();
try {
BRepAlgoAPI_Cut cut_bore(result, bore);
+30 -4
View File
@@ -140,6 +140,25 @@ static SketchPlane face_plane_inward(const TopoDS_Face& face)
return p;
}
// Highest upward-facing planar face of a solid — the surface the user is looking down on.
// Hole placement defaults here (instead of the z=0 datum) so the footprint sits on the top
// face at the right depth, not on the model's underside where a top-view drag reads parallax-
// shifted. Returns -1 if the shape has no clearly-upward face.
static int top_face_index_of(const TopoDS_Shape& shape)
{
int best = -1; double bestz = -1e30;
const int n = GeometryEngine::face_count(shape);
for (int i = 0; i < n; ++i) {
const TopoDS_Face f = GeometryEngine::face_by_index(shape, i);
if (f.IsNull()) continue;
const Vec3d nrm = GeometryEngine::face_normal_world(f);
if (nrm.z() < 0.5) continue; // only faces pointing substantially up
const Vec3d c = GeometryEngine::face_centroid_world(f);
if (c.z() > bestz) { bestz = c.z(); best = i; }
}
return best;
}
DesignPanel::DesignPanel(wxWindow* parent)
: wxPanel(parent, wxID_ANY)
{
@@ -534,14 +553,21 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_hole_on_face = false;
m_hole_face_body = -1;
m_hole_has_bounds = false;
if (m_sel_solid_face >= 0 && m_sel_solid_body >= 0
&& m_sel_solid_body < int(m_doc.bodies.size())) {
// Use the explicitly-picked face; otherwise default to the top face of the
// selected (or first) body so the hole lands on the surface being viewed, not
// the z=0 datum under the model. The XY/XZ/YZ dropdown still overrides.
int hb = m_sel_solid_body, hf = m_sel_solid_face;
if (hf < 0 && !m_doc.bodies.empty()) {
hb = (hb >= 0 && hb < int(m_doc.bodies.size())) ? hb : 0;
hf = top_face_index_of(m_doc.bodies[hb].shape);
}
if (hf >= 0 && hb >= 0 && hb < int(m_doc.bodies.size())) {
const TopoDS_Face face = GeometryEngine::face_by_index(
m_doc.bodies[m_sel_solid_body].shape, m_sel_solid_face);
m_doc.bodies[hb].shape, hf);
if (!face.IsNull()) {
m_hole_face_plane = face_plane_inward(face);
m_hole_on_face = true;
m_hole_face_body = m_sel_solid_body;
m_hole_face_body = hb;
// Face (u,v) extents so the hole dims read from the sides (#2 Part B).
m_hole_has_bounds = GeometryEngine::face_plane_bounds(
face, m_hole_face_plane.origin, m_hole_face_plane.x_axis,