Face.build() failing to determine the correct face

[marcocecchiscmgroup]

swept.zip

If you take a partial torus like this one attached, the STRATEGY_SAT face reconstruction via surface.generalFuse()/eliminateOuterFaces() fails because the opencascade fuse algorithm will split the whole torus in the two complementary halves plus the internal faces/ellipses (as expected, being a fusion algorithm). The fact is that every single such entity is 'seam' (in the isSeam() sense) to the given coedges.

Conversely, STEP conversion is just fine, in that is 1-1 with the ACIS surface/coedges representation.

[jmplonka]

This is exactly why I did the workaround with the STEP importer.
Any help on how to do it 'correct' is highly appreciated.
Gracie mille
Jens

[marcocecchiscmgroup]

In native OpenCascade you would probably end up handling many variations of BRepBuilderAPI_MakeFace::BRepBuilderAPI_MakeFace(). I know this is not an easy job and, more important, that would mean reinventing the wheel: the STEP import source code is freely available from OCCT and, as far as I can see, is far from trivial.

So I see two opposite ways to go:

1. reimplement using Part/Python the C++ code around here: https://github.com/Open-Cascade-SAS/OCCT/tree/63fa56bc83f2f366068fd2e67fce3ea4ff0c5ee3/src/StepToTopoDS

2. Your workaround to translate to STEP as the lingua franca is good and will probably save the day, once done extensively. So I vote for the STEP workaround plus removing the core dependencies on Part from Acis.py (and strongly suggest totally removing it from Acis2Step.py).

Acis.py) Part is used to build models for FreeCAD. This is easy to get rid of. Let's say we have produced in some way a STEP representation, we then just save it to file and import it: that means using Part only for the FreeCAD plugin functionality and would be one line of code.

Acis2Step.py) Optionally, but strongly suggested: see the ongoing discussion on #69 . EDIT: for both Acis.py and Acis2Step.py for what concerns the parametric splines construction. But, as said, that would be optional. Keeping Part as it is used now is just fine and very convenient.

[marcocecchiscmgroup]

This is exactly why I did the workaround with the STEP importer.

According to the latest changes, however, Acis2Step.py now builds the face by means of surface.generalFuse(edges, tolerance).

def _createSurfaceSpline(acisFace):
[...]
	shape = acisFace.build()

This was probably done with the good intention of reducing the dependencies from Part in Acis2Step (partly), so complying with #65, but, as you know, the overall functionality is jeopardized. At least before it worked.
I suggest forking a new repo for the standalone project, as we are experiencing too many uncoordinated changes here,

[HolographicPrince]

convertModel() in importerSAT does the step file import trick for FreeCAD already, so that the very useful STEP file generation that everybody longs for is a side-effect in this case.
I agree with @marcocecchiscmgroup that the core issue seems to be the face generation algorithm, which cannot be gotten away with in all available strategies for now, it seems.

[marcocecchiscmgroup]

As far as I can tell, before the latest changes the general fusion algorithm, even if used also in STRATEGY_STEP, was inifluential (and this is proven by the fact that it worked). Maybe the code was not that clear, but:

def _createSurface(acisFace):
	faces = []
	shape = acisFace.build() # will Return Face!
	if (shape):
		for face in shape.Faces:
			f = _createSurfaceFaceShape(acisFace, face.Surface)
			if (f):
				faces.append(f)

depended on the face.Surface (the same even if the chosen face is wrong) and acisFace, which were both good to our goal.

NOTE: I am not that sure that this behaviour has been preserved, just asking for confirmation.

[jmplonka]

@marcocecchiscmgroup : This behaviour has been preserved - confirmed.
The point is, I want to reduce all the XYZ.build(...) calls from the flow, when using STRATEGY_STEP to get rid of the Part dependency. Normally the spline surfaces comes along the NU(R)BS values that are converted to STEP. Only for parametric spline-surfaces like Coils this is not the case and this Surfaces (as mentioned in the readme) will be skipped or tried to be converted NURSB using Part.

[marcocecchiscmgroup]

Okay.

Only for parametric spline-surfaces like Coils this is not the case and this Surfaces (as mentioned in the readme) will be skipped or tried to be converted NURSB using Part.

About this point, the discussion can migrate on #69 , where the Helix construction is being dealt with. Coils can be the next, if the solution will be viable.

About fixing STRATEGY_SAT, which I am still interested in given that at the moment is not production quality, unfortunately, I am debugging the OCCT source code and will report right back.

[marcocecchiscmgroup]

Debugging was easied by this simple model:
revolv.zip

Going top bottom in the call stack (just the relevant instructions):

Handle(TransferBRep_ShapeBinder) STEPControl_ActorRead::TransferEntity(...) {
[...]
  else if (start->IsKind(STANDARD_TYPE(StepShape_ShellBasedSurfaceModel))) {
    myShapeBuilder.Init(GetCasted(StepShape_ShellBasedSurfaceModel, start), TP, myNMTool); // ---> into
    found = Standard_True;
  } 
}

StepToTopoDS_Builder::Init(...) {
[...]
for (Standard_Integer i = 1; i <= Nb && PS.More(); i++, PS.Next()) {
    aShell = aSBSM->SbsmBoundaryValue(i);
    aOpenShell = aShell.OpenShell();
    aClosedShell = aShell.ClosedShell();
    if (!aOpenShell.IsNull()) {
      myTranShell.Init(aOpenShell, myTool, NMTool); // ---> into
      if (myTranShell.IsDone()) {
        Shl = TopoDS::Shell(myTranShell.Value());
        Shl.Closed(Standard_False);
        B.Add(S, Shl);
      }
      else {
        TP->AddWarning
          (aOpenShell, " OpenShell from ShellBasedSurfaceModel not mapped to TopoDS");
      }
    }
}

void StepToTopoDS_TranslateShell::Init(...) {
BRep_Builder B;
TopoDS_Shell Sh;
 StepToTopoDS_TranslateFace myTranFace;
      StepFace = CFS->CfsFacesValue(i);  //acisFace equivalent
      Handle(StepShape_FaceSurface) theFS =
        Handle(StepShape_FaceSurface)::DownCast(StepFace);
 myTranFace.Init(theFS, aTool, NMTool); // --> into, the big deal
}

void StepToTopoDS_TranslateFace::Init(...) {
[...]
  TopoDS_Face   F;
  BRep_Builder B;
  B.MakeFace ( F, GeomSurf, Precision::Confusion() ); // f is initially empty
 // iterating over the bounds
 for (Standard_Integer i = 1; i <= NbBnd; i ++) {
[...]
      else if (Loop->IsKind(STANDARD_TYPE(StepShape_EdgeLoop))) {
        TopoDS_Wire   W;
        myTranEdgeLoop.Init(FaceBound, F, GeomSurf, StepSurf, sameSense, aTool, NMTool); // ---> into, ANOTHER BIG DEAL
        W = TopoDS::Wire(myTranEdgeLoop.Value()); // just recalling what was built with Init() above
        B.Add(F, W);        
[...]
 }

void StepToTopoDS_TranslateEdgeLoop::Init(...) {
 BRep_Builder B;
  B.MakeWire(W);
  for (j=1; j<=NbEdge; j++) {
        isLikeSeam = StepToTopoDS_GeometricTool::IsLikeSeam(SurfCurve, StepSurf, EC, EL);
        isSeam = StepToTopoDS_GeometricTool::IsSeamCurve(SurfCurve, StepSurf, EC, EL);
[...]
        if (hasPcurve && (isSeam || ThereIsLikeSeam)) {
        }
very complex logic with lots of subcases
[...]
  }
}

As I suspected by initially flicking through the code, this whole business is quite difficult to replicate in python and I wonder if it's worth at all. If it's not, it makes little sense keeping the STRATEGY_SAT in place, I am afraid.

[marcocecchiscmgroup]

If it's not, it makes little sense keeping the STRATEGY_SAT in place, I am afraid.

On the other hand, implementing once and for all a full-fledged native Part import would enable a more accurate model building (offset curves, revolution solids and the like) without having to translate to NU(R)BS, with a loss of precision/information.

Of course the whole thing must work, and it might not be an easy job.

What's your opinion?

[jmplonka]

Part Import based on Features was my main topic to achieve, because of all the parts I have constructed with Inventor. Not much of the features I was using are missing right now.
But as I'm not the only one who is using the converter and as some of the Features are hardly to implement, I started working on SAT import. Facing the problem were I wasn't able to rebuild the model in FC.
So I tried to convert the complete stuff to Step to see, if this is more successful.
Et voila - here we are in our discussion.

[marcocecchiscmgroup]

Hello Jens,

I noticed an error in the STEP translation. Aside from the intrinsic limits in the SAT conversion, that we already discussed at length, the STEP translated version misses the two circular (elliptical) faces that limit the toroidal surface.

Do you want me to open a separate issue?

Marco

[jmplonka]

Hello Marco,
thank you so much for your input. Could you please open a new ticket and link an example?
Regards
Jens

[marcocecchiscmgroup]

After more careful checking, I cannot exclude that the input dxf that I was referring to is ill formed. I need to be sure before wasting your time. I will open a new ticket in case.

Apologizes for the noise.

Marco