From 065315d7a4be9aa2f0bb9e17bad7fbaf6fbd7f7a Mon Sep 17 00:00:00 2001 From: WandererFan Date: Tue, 10 Mar 2026 23:57:59 -0400 Subject: [PATCH] TechDraw: fix crash in area dim for face with nested voids (#28097) --- src/Mod/TechDraw/App/DrawViewDimension.cpp | 8 +- src/Mod/TechDraw/App/Geometry.cpp | 87 +++++++++++++++++----- src/Mod/TechDraw/App/Geometry.h | 4 +- src/Mod/TechDraw/Gui/CommandCreateDims.cpp | 8 +- 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/src/Mod/TechDraw/App/DrawViewDimension.cpp b/src/Mod/TechDraw/App/DrawViewDimension.cpp index d1a01e4621..bf2d5e6230 100644 --- a/src/Mod/TechDraw/App/DrawViewDimension.cpp +++ b/src/Mod/TechDraw/App/DrawViewDimension.cpp @@ -1353,6 +1353,10 @@ areaPoint DrawViewDimension::getAreaParameters(ReferenceVector references) areaPoint pts; App::DocumentObject* refObject = references.front().getObject(); + if (!refObject) { + throw Base::RuntimeError("Area dimension has no reference object"); + } + if (refObject->isDerivedFrom() && !references[0].getSubName().empty()) { // this is a 2d object (a DVP + subelements) TechDraw::FacePtr face = getViewPart()->getFace(references[0].getSubName()); @@ -1363,8 +1367,8 @@ areaPoint DrawViewDimension::getAreaParameters(ReferenceVector references) } auto dvp = static_cast(refObject); - auto filteredFaces = GeometryUtils::findHolesInFace(dvp, references.front().getSubName()); - auto perforatedFace = GeometryUtils::makePerforatedFace(face, filteredFaces); + std::vector holesInFace = GeometryUtils::findHolesInFace(dvp, references.front().getSubName()); + TopoDS_Face perforatedFace = GeometryUtils::makePerforatedFace(face, holesInFace); // these areas are scaled because the source geometry is scaled, but it makes no sense to // report a scaled area. diff --git a/src/Mod/TechDraw/App/Geometry.cpp b/src/Mod/TechDraw/App/Geometry.cpp index fb6d4896b7..2e61c922b2 100644 --- a/src/Mod/TechDraw/App/Geometry.cpp +++ b/src/Mod/TechDraw/App/Geometry.cpp @@ -1777,7 +1777,8 @@ TopoDS_Face GeometryUtils::makePerforatedFace(FacePtr bigCheese, const std::vec } -//! find faces within the bounds of the input face +//! Find faces within the bounds of the input face. For area dimensions, we only want the first +//! "level" (term?) of holes. std::vector GeometryUtils::findHolesInFace(const DrawViewPart* dvp, const std::string& bigCheeseSubRef) { if (!dvp || bigCheeseSubRef.empty()) { @@ -1793,10 +1794,11 @@ std::vector GeometryUtils::findHolesInFace(const DrawViewPart* dvp, con // tarfu throw Base::RuntimeError("GU::findHolesInFace - no holes to find!!"); } - - auto bigCheeseFace = facesAll.at(bigCheeseIndex); - auto bigCheeseOCCFace = bigCheeseFace->toOccFace(); - auto bigCheeseArea = bigCheeseFace->getArea(); + + if (facesAll.at(bigCheeseIndex)->wires.empty()) { + return {}; + } + TopoDS_Wire bigCheeseWire = facesAll.at(bigCheeseIndex)->wires.front()->toOccWire(); int iFace{0}; for (auto& face : facesAll) { @@ -1804,24 +1806,75 @@ std::vector GeometryUtils::findHolesInFace(const DrawViewPart* dvp, con iFace++; continue; } - if (face->getArea() > bigCheeseArea) { - iFace++; - continue; - } - auto faceCenter = Base::convertTo(face->getCenter()); - auto faceCenterVertex = BRepBuilderAPI_MakeVertex(faceCenter); - auto distance = DU::simpleMinDist(faceCenterVertex, bigCheeseOCCFace); - if (distance > EWTOLERANCE) { - // hole center not within outer contour. not the best test but cheese maker handles it - // for us? - // FaceMakerCheese does not support partial overlaps and just ignores them? + TopoDS_Wire faceWire = face->wires.front()->toOccWire(); + if (!Part::FaceMakerCheese::isInside(bigCheeseWire, faceWire)) { iFace++; continue; } + holes.push_back(face); iFace++; } - return holes; + return removeNestedHoles(holes); +} + +//! Remove level 2+ faces. Expects holes to be sorted by size? +std::vector GeometryUtils::removeNestedHoles(const std::vector& holes) +{ + if (holes.empty()) { + return {}; + } + + std::vector unNestedFaces; + if (holes.size() == 1) { + // no nesting present + unNestedFaces.push_back(holes.front()); + return unNestedFaces; + } + + std::vector nestedFaceIndices = findNestedFaceIndices(holes); + + std::reverse(nestedFaceIndices.begin(), nestedFaceIndices.end()); + + int ihole{0}; + for (auto& hole : holes) { + if (std::find(nestedFaceIndices.begin(), nestedFaceIndices.end(), ihole) == nestedFaceIndices.end()) { + unNestedFaces.push_back(hole); + } + ihole++; + } + return unNestedFaces; +} + + +//! returns (unique) indices of holes contained within another hole. +std::vector GeometryUtils::findNestedFaceIndices(const std::vector& holes) +{ + int iouter{0}; + std::vector nestedFaceIndices; + for (auto& outer : holes) { + TopoDS_Wire outerWire = outer->wires.front()->toOccWire(); + int iinner{0}; + for (auto& inner : holes) { + if (iouter == iinner) { + iinner++; + continue; + } + TopoDS_Wire innerWire = inner->wires.front()->toOccWire(); + if (Part::FaceMakerCheese::isInside(outerWire, innerWire)) { + nestedFaceIndices.push_back(iinner); + } + iinner++; + } + iouter++; + } + + std::sort(nestedFaceIndices.begin(), nestedFaceIndices.end()); + auto last = std::unique(nestedFaceIndices.begin(), nestedFaceIndices.end()); + if (last != nestedFaceIndices.end()) { + nestedFaceIndices.erase(last, nestedFaceIndices.end()); + } + return nestedFaceIndices; } diff --git a/src/Mod/TechDraw/App/Geometry.h b/src/Mod/TechDraw/App/Geometry.h index d1e293f894..6efaee8c2f 100644 --- a/src/Mod/TechDraw/App/Geometry.h +++ b/src/Mod/TechDraw/App/Geometry.h @@ -470,6 +470,8 @@ class TechDrawExport GeometryUtils Base::Vector3d D, double tolerance); + static std::vector removeNestedHoles(const std::vector& holes); + static std::vector findNestedFaceIndices(const std::vector& holes); }; -} //end namespace TechDraw \ No newline at end of file +} //end namespace TechDraw diff --git a/src/Mod/TechDraw/Gui/CommandCreateDims.cpp b/src/Mod/TechDraw/Gui/CommandCreateDims.cpp index e09b5e9ab0..01df60faa1 100644 --- a/src/Mod/TechDraw/Gui/CommandCreateDims.cpp +++ b/src/Mod/TechDraw/Gui/CommandCreateDims.cpp @@ -154,6 +154,7 @@ public: bool hasFaces() const { return s_fcs > 0; } bool has1Face() const { return s_pts == 0 && s_lns == 0 && s_cir == 0 && s_ell == 0 && s_spl == 0 && s_fcs == 1; } + bool hasFacesOnly() const { return s_pts == 0 && s_lns == 0 && s_cir == 0 && s_ell == 0 && s_spl == 0 && s_fcs > 0; } bool has1Point() const { return s_pts == 1 && s_lns == 0 && s_cir == 0 && s_ell == 0 && s_spl == 0 && s_fcs == 0; } bool has2Points() const { return s_pts == 2 && s_lns == 0 && s_cir == 0 && s_ell == 0 && s_spl == 0 && s_fcs == 0; } @@ -709,7 +710,12 @@ protected: bool selAllowed = false; GeomSelectionSizes selection(selPoints.size(), selLine.size(), selCircleArc.size(), selEllipseArc.size(), selSplineAndCo.size(), selFaces.size()); - if (selection.hasFaces()) { + + // if we drop a dimension text on a face, interpreting selection with hasFaces() will cause + // us to try to create an area dim instead of positioning the text. Since we do not have a + // dimension type that involves faces + some other geometry, we must use hasFacesOnly() to + // determine if we are creating an area. + if (selection.hasFacesOnly()) { makeCts_Faces(selAllowed); if (!selection.has1Face()) { Base::Console().warning("Multiple faces are selected. Using first.\n");