Invalid Geometry generated by Ellipsoid Primitive

[wsteffe]

The Ellipsoid created using (in example) the Ellipsoid Primitive of PD WB doesn't pass the geometry check.

The Ellipsoid surface is composed of a single face which contains a single Edge (a seam Edge).
Such an edge can form a degenerated closed wire using it in both directions. But this can not be a good representation of the ellipsoid boundary.
The degenerate wire divides the Ellipsoid surface in two parts one having zero area and the other one covering the whole surface. Usually the internal part of a face is defined by the wire orientation with respect to the surface normal. But such a degenerate wire doesn't have a meaningful orientation. In fact the wire representation doesn't change when, in example, all the edges are reversed. It still contains the same single edge used in both orientations. A good representation of the ellipsoid must contain more than one edge.

My suggestion is to split the boundary in two faces each one of them containing two edges: a full ellipse and a meridian seam edge from the equator to the pole. This can be done using using ShapeUpgrade_ShapeDivideClosed (as explained at https://dev.opencascade.org/doc/overview/html/occt_user_guides__shape_healing.html#occt_shg_4_2_1),
or (probably) also changing the initial ellipsoid definition.

[realthunder]

Part ellipsoid feature is defined here, and PartDesign is defined here. The code is identical. It makes a sphere and then use non-uniform scaling to transform it to an ellipsoid. Looks like OCC can't handle this correctly.

[wsteffe]

Hello RT, I made a few experiment as you may see in the here annexed (patched) src/Mod/PartDesign/App/FeaturePrimitive.cpp".

The two lines 479 and 480 split the ball boundary in two halves. You may comment them out if you prefer the original structure with a single closed face.

The following lines of the function Ellipsoid::execute() makes several checks of BRep data. When a data is wrong it is fixed. Shell and Solid are not checked, just fixed. If the split part is kept there are only two kind of data (among those considered) that need to be fixed: missing of two 3D edge curves and something wrong with the shell. I do not know what but you may see that something has been done looking at the integer variable result which is set to 2 at line 528 (just after the application of ShapeFix_Shell).

This fix solves the problems I had with the boolean but the Geometry Check of Part WB still gives a continuity error for an Edge and the two faces. It would be useful to look inside the section of code (I do not know where it is) that generates the error and to see which kind of OCC check is called there. Perhaps we could understand what kind of other fix we need to apply.

FeaturePrimitive.cpp.zip

[wsteffe]

I have done other tests using the here annexed "src/Mod/PartDesign/App/FeaturePrimitive.cpp"
FeaturePrimitive.cpp.zip

I have separated the healing part in the following function which is define starting at line 435:

void healSolid(TopoDS_Solid &theSolid, double aPrecision, int &NwiresFixed, int &NedgesFixed, int &NshellsFixed, int &NsolidsFixed)
NwiresFixed, NedgesFixed, NshellsFixed, NsolidsFixed are the number of element that were found wrong and needed a fix.
I have also added (starting from line 575) a few geometrical checks made with BRepCheck_Analyzer as done in src/Mod/Part/Gui/TaskCheckGeometry.cpp.

Both checks say that the ball is good and no fixes are needed.

The Geometry Check from Part WB gives error just because there are degenerate edges which do not need to have 3D curves. If you look inside OCC function:
FixShapeFix_Edge::AddCurve3d (const TopoDS_Edge& edge)
you will see that it doesn't do anything and return false when the edge is degenerate.
But the OCC function ShapeAnalysis_Edge::.HasCurve3d() still reports that the 3d Curve is missing and BRepCheck_Analyzer when executed on this individual edge (out of the context) reports an invalid shape.

But the point is that BRepCheck_Analyzer should not be applied recursively on all the subsapes (as done in src/Mod/Part/Gui/TaskCheckGeometry.cpp and also in my test). In fact in the file BRepCheck_Analyzer.hxx it is clearly said that

Standard_EXPORT Standard_Boolean IsValid (const TopoDS_Shape& S) const;
//! Returns true if no defect is
//! detected on the shape S or any of its subshapes.

So in conclusion I would say that the function healSolid (called at line 573) and the geometrical checks are not needed and should not be merged.

But please, I would like to merge just the two following lines (numbers 546 and 547 in the annexed file) and the related include of ShapeUpgrade_ShapeDivideClosed.hxx

 ShapeUpgrade_ShapeDivideClosed SDC(theSolid);
 SDC.Perform(); theSolid=TopoDS::Solid(SDC.Result());

Because I have seen that the boolean reported in the (now closed) issue 278 doesn't fail any more when the ellipsoid surface is divided in two halves.

[wsteffe]

Please notice that the edge check at line 595 of my annexed file doesn't rise an exception just because I have explicitly excluded degenerate edges:

if(!BRep_Tool::Degenerated (anEdge)){
   BRepCheck_Analyzer ana(anEdge, Standard_True);
    if (!ana.IsValid())
       Standard_Failure::Raise("Non valid edge")
}

Same thing should be done also in src/Mod/Part/Gui/TaskCheckGeometry.cpp to avoid false errors.

[wsteffe]

Looking more carefully inside src/Mod/Part/Gui/TaskCheckGeometry.cpp I have seen that the error reported by Part/Check Geometry on the Ellispoid was generated by BOPAlgo_ArgumentAnalyzer (and not by BRepCheck_Analyzer as said in my previous post). BOPAlgo_ArgumentAnalyzer is used when "Run BOP Check" is activated in the settings and only if nothing wrong is found with BRepCheck_Analyzer.

But I am still thinking that the Ellipsoid geometry is good and that it is a false error. This geometry is indeed very simple. After being split in two halves it consists of a couple of meridian edges (which have replaced the seam edge) and a couple of polar degenerate edges in each one of the two face.
I have checked all these edges, their connection in the related wires, the orientation of the wires in the faces, the orientation of faces in the shell. All is OK and I really do not understand what kind of problem could have been detected by BOPAlgo_ArgumentAnalyzer that was missed by BRepCheck_Analyzer.
The message printed by Part/Check Geometry about a discontinuity of Edges and Faces (Invalid BOPAlgo_GeomAbs_C0) can not be true. In fact I can not see any discontinuity in any part of the geometry.
So my conclusion is that the Ellipsoid Geometry is Valid and that Part/Check Geometry is wrong.