CAD: extrude a sketch region with its holes, and stop crashing at startup

A rectangle with a circle inside it, drawn in ONE sketch, could not be extruded
to a plate with a bore from the GUI. Five defects were in the way. Each was
found by driving the app on a headless rig and measuring the result — the code
reads correctly at every one of these points, which is why they survived.

1. Wire orientation (kernel). SketchEngine::wires_to_face added every hole as
   wires[i].Reversed(), which is only right when the sketch happens to wind both
   loops the same way. A circle drawn clockwise inside a counter-clockwise
   rectangle came out matching the outer boundary, OCCT swept it as a SECOND
   contour, and the prism was the plate with its bore filled and the disc's
   volume counted twice. Measured: bbox 67.17 x 219.67 x 10 with volume
   152088 mm3 against a solid box of 147542 — a body larger than its own
   bounding box, which is the signature. Holes are now added as-is and
   ShapeFix_Face::FixOrientation() classifies them; that is winding-independent
   and is the idiom make_extrude_regions already used for imported glyphs, which
   is why holed TEXT always extruded correctly while a holed SKETCH never did.
   After the fix: 142996 mm3, implied bore radius 12.03 mm against the circle
   drawn.

2. The live-sketch click threw the picked region away. region_at() served only
   as a yes/no gate and on_face_selected() carried no argument, so Extrude fell
   back to whichever loop the resolver found first — clicking the material of a
   plate-with-a-hole extruded the disc. The region is now carried through, and
   DesignPanel also hands it to the tool with set_loop_pick(), AFTER open_tool()
   because that re-derives selection state, since extrude_uses_loop() reads
   selected_loop_entities() and that lives on the tool.

3. region_at() had no hole awareness and no innermost preference: it returned
   the first polygon containing the point. It now skips a region when the point
   lies inside one of that region's holes, and picks the smallest containing
   loop, so a click in the bore selects the disc and a click on the material
   selects the plate.

4. Startup segfault. DesignCanvas::request_repaint probed the GL backend via
   OpenGLManager::get_gl_info().get_renderer() before the canvas had initialised
   GL — glGetString with no context current and, before init_opengl(), no loaded
   function pointers. Anything that asked for a repaint while the panel was
   still being built landed there, with no window and nothing in the log. It now
   bails at the top on !is_initialized() and asks for a Refresh instead. Note
   the crash was in the PROBE, not in render(), which already guards itself.

5. A holed sketch on a plane whose normal points -Z came out as the full box PLUS
   a disc — 220274 mm3 where 163726 was due (192000 + 28274). wires_to_face took
   a SketchPlane parameter it never used and let OCCT infer a surface from the
   outer wire; when the inferred normal disagreed with the sketch's, the hole
   classification produced no hole. Every face is now built on the sketch's own
   gp_Pln.

Also in this change, from the same rig session:

- A right-click that only clears the sketch selection no longer reports itself
  as consumed, so it stops suppressing the offer menu. With any geometry in a
  live sketch there was no menu route left to add a second entity.
- Escape no longer discards a live sketch that holds drawn geometry; it says so
  and keeps the work (live_sketch_has_work()).
- The holed-region fill is an even-odd scanline instead of a keyhole bridge, so
  no corridor triangle leaks from the bore to the nearest corner.
- Cyan is reserved for the selection: an unselected region no longer wears a
  shade one step off the selected one.
- The origin planes follow the mode, so pressing Sketch on a document that
  already has a body offers them again instead of naming a plane you cannot see.

Tests: three [holes] cases over add_extrude_entities asserting the plate-with-bore
volume, solid and face counts on both a +Z and a -Z sketch plane, and the
by-name refusal of two disjoint regions. Full [CadDocument] suite green.
This commit is contained in:
Tommaso Bianchi
2026-08-13 23:43:36 +02:00
parent e27a44e6ef
commit 5889f6640f
9 changed files with 418 additions and 32 deletions
+28 -5
View File
@@ -21,6 +21,7 @@
#include <gp_Circ.hxx>
#include <gp_Elips.hxx>
#include <gp_Ax2.hxx>
#include <gp_Pln.hxx>
#include <BRepPrimAPI_MakePrism.hxx>
#include <BRepOffsetAPI_MakeOffset.hxx>
#include <BRepOffsetAPI_ThruSections.hxx>
@@ -727,10 +728,21 @@ TopoDS_Wire SketchEngine::entities_to_wire(const std::vector<SketchEntity>& enti
}
TopoDS_Face SketchEngine::wires_to_face(const std::vector<TopoDS_Wire>& wires,
const SketchPlane& /*plane*/)
const SketchPlane& plane)
{
if (wires.empty()) throw std::runtime_error("sketch has no closed loop");
// The ASSEMBLED face below is built on the SKETCH's own plane rather than on a surface OCCT
// infers from the outer wire. The inferred plane has no reason to share the sketch's normal,
// and when they disagree the hole classification produces no hole: a plate sketched on a
// plane whose normal points -Z came out as the full box PLUS a disc (measured 220274 mm3
// where 163726 was due — 192000 box + 28274 disc). `plane` was a parameter this function
// never used. Only the assembly is named: the single-wire and per-wire-area builds keep the
// inferred surface, because naming a plane also makes MakeFace accept a wire that does not
// bound a face, and that failure is the check an open stray line is caught by.
const gp_Pln pln(gp_Pnt(plane.origin.x(), plane.origin.y(), plane.origin.z()),
gp_Dir(plane.normal.x(), plane.normal.y(), plane.normal.z()));
if (wires.size() == 1) {
BRepBuilderAPI_MakeFace fm(wires[0]);
if (!fm.IsDone()) throw std::runtime_error("sketch loop does not bound a face");
@@ -759,7 +771,7 @@ TopoDS_Face SketchEngine::wires_to_face(const std::vector<TopoDS_Wire>& wires,
// Note: NOT MakeFace(faces[outer], wires[outer]) — that constructor copies the outer face
// (including its existing boundary wire) and then adds the wire again, doubling the outer
// boundary. The wire-only constructor starts clean and the reversed holes follow.
BRepBuilderAPI_MakeFace fm(wires[outer]);
BRepBuilderAPI_MakeFace fm(pln, wires[outer]);
for (size_t i = 0; i < wires.size(); ++i) {
if (i == outer) continue;
// Containment is checked, not assumed: a vertex of the inner wire must lie strictly
@@ -775,11 +787,22 @@ TopoDS_Face SketchEngine::wires_to_face(const std::vector<TopoDS_Wire>& wires,
BRepClass_FaceClassifier fc(faces[outer], p, 1e-7);
if (fc.State() != TopAbs_IN)
throw std::runtime_error("sketch has two disjoint regions; put each in its own sketch");
// A reversed wire tells OCCT this loop is a hole, not a second boundary.
fm.Add(TopoDS::Wire(wires[i].Reversed()));
// Add the hole loop AS-IS and let ShapeFix_Face sort the orientations out below.
// Reversing it here only works when the sketch happened to wind both loops the same
// way: a circle drawn clockwise inside a counter-clockwise rectangle comes out matching
// the outer boundary, OCCT sweeps it as a second contour, and the prism is the plate
// with the bore FILLED and the disc's volume counted twice. Measured on the rig:
// bbox 67.17 x 219.67 x 10 (the whole plate) with volume 152088 mm3 against a solid-box
// 147520 — a body larger than its own bounding box, which is the signature of it.
fm.Add(wires[i]);
}
if (!fm.IsDone()) throw std::runtime_error("sketch loop does not bound a face");
return fm.Face();
// Winding-independent classification of outer vs holes — the same idiom make_extrude_regions
// already uses for imported glyphs, which is why holed TEXT extruded correctly all along
// while a holed SKETCH did not.
ShapeFix_Face sff(fm.Face());
sff.FixOrientation();
return sff.Face();
}
std::vector<SketchEntity> SketchEngine::mirror_entities(