TechDraw: fix crash in area dim for face with nested voids (#28097)

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<DrawViewPart>() && !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<DrawViewPart*>(refObject);
 
-        auto filteredFaces  = GeometryUtils::findHolesInFace(dvp, references.front().getSubName());
-        auto perforatedFace = GeometryUtils::makePerforatedFace(face, filteredFaces);
+        std::vector<FacePtr> 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.

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<FacePtr> GeometryUtils::findHolesInFace(const DrawViewPart* dvp, const std::string& bigCheeseSubRef)
 {
     if (!dvp || bigCheeseSubRef.empty()) {
@@ -1793,10 +1794,11 @@ std::vector<FacePtr> 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<FacePtr> GeometryUtils::findHolesInFace(const DrawViewPart* dvp, con
             iFace++;
             continue;
         }
-        if (face->getArea() > bigCheeseArea) {
-            iFace++;
-            continue;
-        }
-        auto faceCenter = Base::convertTo<gp_Pnt>(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<FacePtr> GeometryUtils::removeNestedHoles(const std::vector<FacePtr>& holes)
+{
+    if (holes.empty()) {
+        return {};
+    }
+
+    std::vector<FacePtr> unNestedFaces;
+    if (holes.size() == 1) {
+        // no nesting present
+        unNestedFaces.push_back(holes.front());
+        return unNestedFaces;
+    }
+
+    std::vector<int> 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<int> GeometryUtils::findNestedFaceIndices(const std::vector<FacePtr>& holes)
+{
+    int iouter{0};
+    std::vector<int> 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;
 }
 

src/Mod/TechDraw/App/Geometry.h

@@ -470,6 +470,8 @@ class TechDrawExport GeometryUtils
                                       Base::Vector3d D,
                                       double tolerance);
 
+        static std::vector<FacePtr> removeNestedHoles(const std::vector<FacePtr>& holes);
+        static std::vector<int> findNestedFaceIndices(const std::vector<FacePtr>& holes);
 };
 
-} //end namespace TechDraw
+} //end namespace TechDraw

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");