[PD] fix pad uptoface and uptoshape (FreeCAD#16030)

* [PD] fix Pad UpToFace and UpToShape * specify struct pointers for Win * Rename variables for MSVC compatibility - windows.h defines 'near' and 'far' as macros * Add unit test --------- Co-authored-by: Chris Hennes <[email protected]>
mosfet80 · Sep 19, 2024 · 8b9f5bd · 8b9f5bd
1 parent f82bfd5
commit 8b9f5bd
Show file tree

Hide file tree

Showing 8 changed files with 156 additions and 25 deletions.
diff --git a/src/Mod/Part/App/Tools.cpp b/src/Mod/Part/App/Tools.cpp
@@ -28,6 +28,7 @@
 # include <BRepAdaptor_Surface.hxx>
 # include <BRepBuilderAPI_MakeEdge.hxx>
 # include <BRepBuilderAPI_MakeFace.hxx>
+# include <BRepIntCurveSurface_Inter.hxx>
 # include <BRepLProp_SLProps.hxx>
 # include <BRepMesh_IncrementalMesh.hxx>
 # include <CSLib.hxx>
@@ -744,3 +745,37 @@ TopLoc_Location Part::Tools::fromPlacement(const Base::Placement& plm)
     trf.SetTransformation(gp_Quaternion(q1, q2, q3, q4), gp_Vec(t.x, t.y, t.z));
     return {trf};
 }
+
+bool Part::Tools::isConcave(const TopoDS_Face &face, const gp_Pnt &pointOfVue, const gp_Dir &direction){
+    bool result = false;
+
+    Handle(Geom_Surface) surf = BRep_Tool::Surface(face);
+    GeomAdaptor_Surface adapt(surf);
+    if(adapt.GetType() == GeomAbs_Plane){
+        return false;
+    }
+
+            // create a line through the point of vue
+    gp_Lin line;
+    line.SetLocation(pointOfVue);
+    line.SetDirection(direction);
+
+            // Find intersection of line with the face
+    BRepIntCurveSurface_Inter mkSection;
+    mkSection.Init(face, line, Precision::Confusion());
+
+    result = mkSection.Transition() == IntCurveSurface_In;
+
+            // compute normals at the intersection
+    gp_Pnt iPnt;
+    gp_Vec dU, dV;
+    surf->D1(mkSection.U(), mkSection.V(), iPnt, dU, dV);
+
+            // check normals orientation
+    gp_Dir dirdU(dU);
+    result = (dirdU.Angle(direction) - M_PI_2) <= Precision::Confusion();
+    gp_Dir dirdV(dV);
+    result = result || ((dirdV.Angle(direction) - M_PI_2) <= Precision::Confusion());
+
+    return result;
+}
diff --git a/src/Mod/Part/App/Tools.h b/src/Mod/Part/App/Tools.h
@@ -225,6 +225,16 @@ class PartExport Tools
      * \return TopLoc_Location
      */
     static TopLoc_Location fromPlacement(const Base::Placement&);
+
+    /*!
+     * \brief isConcave
+     * \param face
+     * \param pointOfVue
+     * \param direction
+     * \return true if the face is concave when shown from pointOfVue and looking into direction
+     * and false otherwise, plane case included.
+     */
+    static bool isConcave(const TopoDS_Face &face, const gp_Pnt &pointOfVue, const gp_Dir &direction);
 };
 
 } //namespace Part

diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp
@@ -104,6 +104,8 @@
 #include <ShapeAnalysis_FreeBoundsProperties.hxx>
 #include <BRepFeat_MakeRevol.hxx>
 
+#include "Tools.h"
+
 FC_LOG_LEVEL_INIT("TopoShape", true, true)  // NOLINT
 
 #if OCC_VERSION_HEX >= 0x070600
@@ -4231,6 +4233,12 @@ TopoShape& TopoShape::makeElementPrismUntil(const TopoShape& _base,
 
     BRepFeat_MakePrism PrismMaker;
 
+    // don't remove limits of concave face
+    Base::Vector3d vCog;
+    profile.getCenterOfGravity(vCog);
+    gp_Pnt pCog(vCog.x, vCog.y, vCog.z);
+    checkLimits = ! Part::Tools::isConcave(TopoDS::Face(__uptoface.getShape()), pCog , direction);
+
     TopoShape _uptoface(__uptoface);
     if (checkLimits && _uptoface.shapeType(true) == TopAbs_FACE
         && !BRep_Tool::NaturalRestriction(TopoDS::Face(_uptoface.getShape()))) {
@@ -4252,7 +4260,8 @@ TopoShape& TopoShape::makeElementPrismUntil(const TopoShape& _base,
 
     // Check whether the face has limits or not. Unlimited faces have no wire
     // Note: Datum planes are always unlimited
-    if (checkLimits && _uptoface.shapeType(true) == TopAbs_FACE && uptoface.hasSubShape(TopAbs_WIRE)) {
+    if (checkLimits && uptoface.shapeType(true) == TopAbs_FACE
+        && uptoface.hasSubShape(TopAbs_WIRE)) {
         TopoDS_Face face = TopoDS::Face(uptoface.getShape());
         bool remove_limits = false;
         // Remove the limits of the upToFace so that the extrusion works even if profile is larger
@@ -4292,10 +4301,7 @@ TopoShape& TopoShape::makeElementPrismUntil(const TopoShape& _base,
             // use the placement of the adapter, not of the upToFace
             loc = TopLoc_Location(adapt.Trsf());
             BRepBuilderAPI_MakeFace mkFace(adapt.Surface().Surface(), Precision::Confusion());
-            if (!mkFace.IsDone()) {
-                remove_limits = false;
-            }
-            else {
+            if (mkFace.IsDone()) {
                 uptoface.setShape(located(mkFace.Shape(), loc), false);
             }
         }

diff --git a/src/Mod/PartDesign/App/FeatureExtrude.cpp b/src/Mod/PartDesign/App/FeatureExtrude.cpp
@@ -40,6 +40,7 @@
 #include <Base/Tools.h>
 #include <Mod/Part/App/ExtrusionHelper.h>
 #include "Mod/Part/App/TopoShapeOpCode.h"
+#include <Mod/Part/App/PartFeature.h>
 
 #include "FeatureExtrude.h"
 
@@ -131,6 +132,41 @@ bool FeatureExtrude::hasTaperedAngle() const
            fabs(TaperAngle2.getValue()) > Base::toRadians(Precision::Angular());
 }
 
+TopoShape FeatureExtrude::makeShellFromUpToShape(TopoShape shape, TopoShape sketchshape, gp_Dir dir){
+
+    // Find nearest/furthest face
+    std::vector<Part::cutTopoShapeFaces> cfaces =
+        Part::findAllFacesCutBy(shape, sketchshape, dir);
+    if (cfaces.empty()) {
+        dir = -dir;
+        cfaces = Part::findAllFacesCutBy(shape, sketchshape, dir);
+    }
+    struct Part::cutTopoShapeFaces *nearFace;
+    struct Part::cutTopoShapeFaces *farFace;
+    nearFace = farFace = &cfaces.front();
+    for (auto &face : cfaces) {
+        if (face.distsq > farFace->distsq) {
+            farFace = &face;
+        }
+        else if (face.distsq < nearFace->distsq) {
+            nearFace = &face;
+        }
+    }
+
+    if (nearFace != farFace) {
+        std::vector<TopoShape> faceList;
+        for (auto &face : shape.getSubTopoShapes(TopAbs_FACE)) {
+            if (! (face == farFace->face)){
+                // don't use the last face so the shell is open
+                // and OCC works better
+                faceList.push_back(face);
+            }
+        }
+        return shape.makeElementCompound(faceList);
+    }
+    return shape;
+}
+
 // TODO: Toponaming April 2024 Deprecated in favor of TopoShape method.  Remove when possible.
 void FeatureExtrude::generatePrism(TopoDS_Shape& prism,
                                    const TopoDS_Shape& sketchshape,
@@ -579,12 +615,10 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt
                 faceCount = 1;
             }
             else if (method == "UpToShape") {
-                try {
-                    faceCount = getUpToShapeFromLinkSubList(upToShape, UpToShape);
-                    upToShape.move(invObjLoc);
-                }
-                catch (Base::ValueError&){
-                    //no shape selected use the base
+                faceCount = getUpToShapeFromLinkSubList(upToShape, UpToShape);
+                upToShape.move(invObjLoc);
+                if (faceCount == 0){
+                    // No shape selected, use the base
                     upToShape = base;
                     faceCount = 0;
                 }
@@ -594,8 +628,12 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt
                 getUpToFace(upToShape, base, supportface, sketchshape, method, dir);
                 addOffsetToFace(upToShape, dir, Offset.getValue());
             }
-            else if (fabs(Offset.getValue()) > Precision::Confusion()){
-                return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Extrude: Can only offset one face"));
+            else{
+                if (fabs(Offset.getValue()) > Precision::Confusion()){
+                    return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP("Exception", "Extrude: Can only offset one face"));
+                }
+                // open the shell by removing the furthest face
+                upToShape = makeShellFromUpToShape(upToShape, sketchshape, dir);
             }
 
             if (!supportface.hasSubShape(TopAbs_WIRE)) {
@@ -645,13 +683,22 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt
                 this->Shape.setValue(getSolid(prism));
                 return App::DocumentObject::StdReturn;
             }
-            prism.makeElementPrismUntil(base,
-                                        sketchshape,
-                                        supportface,
-                                        upToShape,
-                                        dir,
-                                        TopoShape::PrismMode::None,
-                                        true /*CheckUpToFaceLimits.getValue()*/);
+            try {
+                prism.makeElementPrismUntil(base,
+                                            sketchshape,
+                                            supportface,
+                                            upToShape,
+                                            dir,
+                                            TopoShape::PrismMode::None,
+                                            true /*CheckUpToFaceLimits.getValue()*/);
+            }
+            catch (Base::Exception& e) {
+                if (method == "UpToShape" && faceCount > 1){
+                    return new App::DocumentObjectExecReturn(QT_TRANSLATE_NOOP(
+                        "Exception",
+                        "Unable to reach the selected shape, please select faces"));
+                }
+            }
         }
         else {
             Part::ExtrusionParameters params;

diff --git a/src/Mod/PartDesign/App/FeatureExtrude.h b/src/Mod/PartDesign/App/FeatureExtrude.h
@@ -71,6 +71,7 @@ class PartDesignExport FeatureExtrude : public ProfileBased
     Base::Vector3d computeDirection(const Base::Vector3d& sketchVector, bool inverse);
     bool hasTaperedAngle() const;
 
+
     /// Options for buildExtrusion()
     enum class ExtrudeOption
     {
@@ -84,6 +85,13 @@ class PartDesignExport FeatureExtrude : public ProfileBased
 
     App::DocumentObjectExecReturn* buildExtrusion(ExtrudeOptions options);
 
+    /**
+     * generate an open shell from a given shape
+     * by removing the farthest face from the sketchshape in the direction
+     * if farthest is nearest (circular) then return the initial shape
+     */
+    TopoShape makeShellFromUpToShape(TopoShape shape, TopoShape sketchshape, gp_Dir dir);
+
     /**
       * Generates an extrusion of the input sketchshape and stores it in the given \a prism
       */

diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp
@@ -708,12 +708,10 @@ int ProfileBased::getUpToShapeFromLinkSubList(TopoShape& upToShape, const App::P
         }
     }
     if (ret == 0){
-        throw Base::ValueError("SketchBased: No face selected");
+        return 0;
     }
-
-    upToShape = faceList[0];
-
     if (ret == 1){
+        upToShape = faceList[0];
         return 1;
     }
 

diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.h b/src/Mod/PartDesign/App/FeatureSketchBased.h
@@ -168,7 +168,7 @@ class PartDesignExport ProfileBased : public PartDesign::FeatureAddSub
 
     /// Create a shape with shapes and faces from a given LinkSubList
     /// return 0 if almost one full shape is selected else the face count
-    int getUpToShapeFromLinkSubList(TopoShape& upToShape, const App::PropertyLinkSubList& refShape); // TODO static
+    static int getUpToShapeFromLinkSubList(TopoShape& upToShape, const App::PropertyLinkSubList& refShape);
 
     /// Find a valid face to extrude up to
     static void getUpToFace(TopoShape& upToFace,

diff --git a/src/Mod/PartDesign/PartDesignTests/TestPad.py b/src/Mod/PartDesign/PartDesignTests/TestPad.py
@@ -184,6 +184,33 @@ def testPadTwoDimensionsCase(self):
         self.Doc.recompute()
         self.assertAlmostEqual(self.Pad1.Shape.Volume, 4.0)
 
+    def testPadToConcaveCase(self):
+        self.Body = self.Doc.addObject('PartDesign::Body','Body')
+        # Make a half revolution
+        self.RevolutionSketch = self.Doc.addObject('Sketcher::SketchObject', 'SketchPad')
+        self.Body.addObject(self.RevolutionSketch)
+        TestSketcherApp.CreateRectangleSketch(self.RevolutionSketch, (9, 0), (10, 5))
+        self.Doc.recompute()
+        self.Revolution = self.Doc.addObject("PartDesign::Revolution", "Revolution")
+        self.Body.addObject(self.Revolution)
+        self.Revolution.Profile = self.RevolutionSketch
+        self.Revolution.ReferenceAxis = (self.RevolutionSketch, ['V_Axis'])
+        self.Revolution.Angle = 180
+        self.Doc.recompute()
+        # Make a sketch and pad to first
+        self.PadSketch = self.Doc.addObject('Sketcher::SketchObject', 'SketchPad')
+        self.Body.addObject(self.PadSketch)
+        self.Doc.recompute()
+        TestSketcherApp.CreateRectangleSketch(self.PadSketch, (0, 0), (1, 1))
+        self.Doc.recompute()
+        self.Pad = self.Doc.addObject("PartDesign::Pad", "Pad")
+        self.Body.addObject(self.Pad)
+        self.Pad.Profile = self.PadSketch
+        self.Pad.Type = 2
+        self.Pad.Reversed = True
+        self.Doc.recompute()
+        self.assertAlmostEqual(self.Pad.Shape.Volume, 2208.0963, places=4)
+
     def tearDown(self):
         #closing doc
         FreeCAD.closeDocument("PartDesignTestPad")