[finished] project: Making Part Extrude taking care of inner structure

[uwestoehr]

This thread is to make Part Extrude handle inner sketch wires.

Yes, the current Part Extrude implementation fails to handle inner wires properly,

Many use cases for this. I am thinking casting design where we always want to ensure draft angles.

So let's act. I hope with a collaborative effort, we can achieve this.
here is what I did so far: https://github.com/FreeCAD/FreeCAD/pull/5367

The approach is to take the method PD Loft uses to take care of inner wires. However I am stuck and after 3 hours I have to give up for now and I hope that you can join me. I hope a fresh look from outside might reveal my mistake.

[TheMarkster]

I made some more modifications to the PR, just for debugging purposes. Here is the diff:

Code: Select all

--- src/Mod/Part/App/FeatureExtrusion.cpp
+++ src/Mod/Part/App/FeatureExtrusion.cpp
@@ -28,9 +28,12 @@
 # include <gp_Trsf.hxx>
 # include <BRepAdaptor_Surface.hxx>
 # include <BRepAdaptor_Curve.hxx>
+# include <BRepClass3d_SolidClassifier.hxx>
 # include <BRepOffsetAPI_MakeOffset.hxx>
 # include <BRepBuilderAPI_Copy.hxx>
+# include <BRepBuilderAPI_MakeSolid.hxx>
 # include <BRepBuilderAPI_MakeWire.hxx>
+# include <BRepBuilderAPI_Sewing.hxx>
 # include <BRepOffsetAPI_ThruSections.hxx>
 # include <BRepPrimAPI_MakePrism.hxx>
 # include <Precision.hxx>
@@ -45,6 +48,8 @@
 #endif
 
 #include "FeatureExtrusion.h"
+#include <App/Application.h>
+#include <App/Document.h>
 #include <Base/Tools.h>
 #include <Base/Exception.h>
 #include "Part2DObject.h"
@@ -612,16 +617,28 @@ void Extrusion::makeDraft(const ExtrusionParameters& params, const TopoDS_Shape&
             TopoDS_Shape front = shape;
             front.Move(invObjLoc);
             sewer.Add(front);
+            Part::Feature *front_feature = static_cast<Part::Feature*> (App::GetApplication().getActiveDocument()->addObject("Part::Feature","front"));
+            front_feature->Shape.setValue(front);
             std::vector<TopoDS_Wire> backwires;
-            for (auto& wires : extrusionSections)
+            for (auto& wires : extrusionSections){
                 backwires.push_back(TopoDS::Wire(wires.back()));
+                Part::Feature *wire_feature = static_cast<Part::Feature*> (App::GetApplication().getActiveDocument()->addObject("Part::Feature","wire"));
+                wire_feature->Shape.setValue(TopoDS::Wire(wires.back()));
+            }
             TopoDS_Shape back = Part::FaceMakerCheese::makeFace(backwires);
+            Part::Feature *back_feature = static_cast<Part::Feature*> (App::GetApplication().getActiveDocument()->addObject("Part::Feature","back"));
+            back_feature->Shape.setValue(back);
             sewer.Add(back);
-            for (TopoDS_Shape& s : shells)
+            for (TopoDS_Shape& s : shells){
                 sewer.Add(s);
+                Part::Feature *shell_feature = static_cast<Part::Feature*> (App::GetApplication().getActiveDocument()->addObject("Part::Feature","shell"));
+                shell_feature->Shape.setValue(s);
+            }
 
             sewer.Perform();
 
+
+
             // build the solid
             BRepBuilderAPI_MakeSolid mkSolid;
             mkSolid.Add(TopoDS::Shell(sewer.SewedShape())); // <- Building the extrude fails here

These lines are equivalent to in python:

Code: Select all

Part.show(shape,shapename)

Snip macro screenshot-169812.png (44.84 KiB) Viewed 3632 times

Except for the "wire" objects, these are the objects added to the sewer object. The wires are used to build the "back" face. These wires look fine, but the "back" face is not valid. It has self-intersections and is really just a compound of faces made from the various wires. The "front" shape is not a face at all. The shells are all solids and check out okay.

[uwestoehr]

Except for the "wire" objects, these are the objects added to the sewer object. The wires are used to build the "back" face. These wires look fine, but the "back" face is not valid.

Many thanks! This helped me to find one issue: one must perform the offset operation on the unmoved shape. Then it can be moved. This fixes the wires and also the back face.

Now the question is why the shells fail...

[chrisb]

The approach is to take the method PD Loft uses to take care of inner wires.

It is probably better to follow PartDesign Draft than Loft. The attachment shows: While Draft creates Planes, does Loft create BSpline surfaces. This has in the past often lead to problems.

[chrisb]

Draft works well on outer and inner faces simultanously. All faces are planes, no BSplineSurfaces.

SnipScreenshot-d38424.png (18.43 KiB) Viewed 3553 times

[uwestoehr]

Many thanks! This helped me to find one issue: one must perform the offset operation on the unmoved shape. Then it can be moved. This fixes the wires and also the back face.

Now the question is why the shells fail...

I could fix this. Here is a branch in which I have it implemented for PartDesign Pad/Pocket:
https://github.com/donovaly/FreeCAD/tre ... cket-inner

Here in action:

FreeCAD_t8FW0Oq4A5.gif (281.4 KiB) Viewed 3506 times

FreeCAD_ayLAYjydJ9.gif (227.44 KiB) Viewed 3506 times

As I feared, allowing inner wires opens Pandora's box. On negative angles The OCC Kernel sometimes crashes, no matter how smalle the negative angle is. The crashes depend strongly on the inner wire position and size.

So for now I think when there are inner wires, we cannot support negative angles

I'll update the PR for Part accordingly.

[uwestoehr]

I'll update the PR for Part accordingly.

I updated the PR to the method that was successful for PartDesign.

This still fails bit there is only one issue we need to resolve:
When you have a sketch with inner wires, the location of the inner wires is wrong. On making the offset, the location is correctly transformed the bug is that the initial location of the shape when making the offset is actually not the location of the shape in the document.

I fail to find out why this happens not how to restore the correct location.

However also for PD the same issue appears. When one for example makes a pad using the face of another pad as face, the inner wires also loose their location.

Has anybody an idea?

[davidosterberg]

We must take a step back an evaluate our options before merging anything. We should consider RTs implementation and also chrisbs suggestion.

It is a real pity if negative angle cannot be allowed.
Also the BSpline issue deserves consideration. Is it possible to click on a bspline face and make a sketch? If not, we would miss out on the most intuitive workflow (yes I know it is not good practice).

[chrisb]

I can make a test later if this works with the Draft approach. I'm pretty sure it will.

[chrisb]

Here is the example using Draft. The faces created from the circle are no B-SplinesSurfaces either.