ShapeFix_Wire doesn't actually fix the wire after calling Perform()

I am trying to understand how the shape healing functions work but I am having a hard time following the documentation.

I am working on a bigger STEP file but I have extracted a single face to analyse it.

I got a simple STEP file with a single face and the wire analysis says that it self intersects.

I declare a shape healing tool ShapeFix_Wire and call Perform, however the resulting wire is still self-intersecting.

I have attached the file tmp.stp and a quick test.cpp file. Am I doing something wrong? I tried with different precision values and it changes nothing. Any help or tip is appreciated.

/* Copy paste this into a .sh file to build to compile and run
#!/bin/sh -x

OCCT_LIB_DIR="/usr/lib/x86_64-linux-gnu"
OCCT_LIBS="$(ls ${OCCT_LIB_DIR}/libT*.so*)"
OCCT_INCLUDE="-I/usr/include/opencascade"

g++ -std=c++20 -O3 test.cpp ${OCCT_INCLUDE} ${OCCT_LIBS} -Wno-deprecated-enum-enum-conversion
./a.out
*/

#include <iomanip>
#include <iostream>

#include "STEPCAFControl_Reader.hxx"
#include "ShapeAnalysis_Wire.hxx"
#include "ShapeFix_Wire.hxx"
#include "TDF_LabelSequence.hxx"
#include "TDocStd_Document.hxx"
#include "TopAbs_ShapeEnum.hxx"
#include "TopExp_Explorer.hxx"
#include "TopoDS.hxx"
#include "TopoDS_Face.hxx"
#include "TopoDS_Iterator.hxx"
#include "TopoDS_Shape.hxx"
#include "XCAFDoc_DocumentTool.hxx"
#include "XCAFDoc_ShapeTool.hxx"

int main() {
  // Load the sample step file into a document and get the OCCT representation
  Handle(TDocStd_Document) doc = new TDocStd_Document{"BinXCAF"};
  STEPCAFControl_Reader reader;
  reader.ReadFile("tmp.stp");
  reader.Transfer(doc);

  // Grab the tool to manipulate the shapes
  Handle(XCAFDoc_ShapeTool) shape_tool =
      XCAFDoc_DocumentTool::ShapeTool(doc->Main());

  // Grab the labels of root shapes (only one shell in this file)
  TDF_LabelSequence labels;
  shape_tool->GetFreeShapes(labels);

  // Get the shell
  TopoDS_Shape shape = shape_tool->GetShape(labels.Value(1));
  TopoDS_Iterator it{shape};
  // Get the only face in the shell
  TopoDS_Face face = TopoDS::Face(it.Value());
  // Get the only wire in this face
  TopExp_Explorer exp{face, TopAbs_WIRE};
  // Cast to an actual wire from a generic shape
  TopoDS_Wire wire = TopoDS::Wire(exp.Value());

  ShapeAnalysis_Wire wire_analyser{wire, face, 1e-7};

  // This returns true as there are self intersections
  if (wire_analyser.CheckSelfIntersection()) {
    std::cout
        << "This wire is self intersecting. Fix using OCCT Shape Healing\n";

    // Build a fixer object with a certain precision
    ShapeFix_Wire wire_fixer{wire, face, 1e-7};
    wire_fixer.ClosedWireMode() = true;

    // Why does this return false? How to investigate further the cause?
    std::cout << "Is it fixed? " << std::boolalpha
              << wire_fixer.FixSelfIntersection() << '\n';

    // Build a new analyser for the new "fixed" wire and face
    ShapeAnalysis_Wire fixed_wire_analyser{wire_fixer.Wire(), wire_fixer.Face(),
                                           1e-7};
    // Even the fixed wire is still self intersecting
    if (fixed_wire_analyser.CheckSelfIntersection())
      std::cout << "Still not fixed\n";
  }
}
Attachments: 
lucmobz's picture

Also I am not understanding if the variable `wire` (which feels like a reference variable to the underlying geometry) is automatically updated or if `ShapeFix_Wire::Wire()` method produces a deep copy that has to be then used to replace manually the initial wire?

Dmitrii Pasukhin's picture

Hello,

The wire_fixer.FixSelfIntersection() function is a crucial part of the healing process. To ensure it functions properly, it is necessary to prepare all the data beforehand. It is recommended to call the "Perform" method for best results.

Additionally, the shape healing process is applied after the StepRead stage automatically.

Deep copy to avoid a lot of problems. Because the  topology is a very delicate part.

Best regards, Dmitrii.

lucmobz's picture

Thanks a lot for the answer.

What I meant with my comment is: do wire and wire_fixer.Wire() (after calling perform) point to the same underlying geometry or does a Perform call create a deep copy of wire that is then return with a call to ShapeFix_Wire::Wire()? You are saying it performs a deep copy right and that I have to use ShapeBuilder_ReShape to swap it into my initial shape?

I will certainly investigate more this automatic shape healing process that comes with the reading phase.

Dmitrii Pasukhin's picture

As far as I know, a copy is actually made there. But there are times when the original is modified.

We use XSAlgo::AlgoContainer::ProcessShape and then XSAlgo::AlgoContainer::MergeTransferInfo to merge transfer history.

Fixer has the own history(look like a ReShape) and you are free to use it directly to update each shape in your model.

There an example from DRAW command, whist setting up fixer to fix needed parts and save new shape into new name.

  theCommands.Add ("fixshape",
"res shape [preci [maxpreci]] [{switches}]\n"
"  [-maxtaila <degrees>] [-maxtailw <width>]",
		   __FILE__,fixshape,g);

//=======================================================================
//function : fixshape
//purpose  : 
//=======================================================================

static Standard_Integer fixshape (Draw_Interpretor& di, Standard_Integer argc, const char** argv)
{
  Handle(ShapeExtend_MsgRegistrator) msg = new ShapeExtend_MsgRegistrator;
  Handle(ShapeFix_Shape) sfs = new ShapeFix_Shape;
  sfs->SetMsgRegistrator ( msg );
  
  Standard_CString res = 0;
  Standard_Integer par = 0, mess=0;
  for ( Standard_Integer i=1; i < argc; i++ )
  {
    if (strlen(argv[i]) == 2 &&
      (argv[i][0] == '-' || argv[i][0] == '+' || argv[i][0] == '*'))
    {
      Standard_Integer val = ( argv[i][0] == '-' ? 0 : argv[i][0] == '+' ? 1 : -1 );
      switch ( argv[i][1] ) {
      case 'l': sfs->FixWireTool()->FixLackingMode()          = val; break;
      case 'o': sfs->FixFaceTool()->FixOrientationMode()      = val; break;
      case 'h': sfs->FixWireTool()->FixShiftedMode()          = val; break;
      case 'm': sfs->FixFaceTool()->FixMissingSeamMode()      = val; break;
      case 'd': sfs->FixWireTool()->FixDegeneratedMode()      = val; break;
      case 's': sfs->FixWireTool()->FixSmallMode()            = val; break;
      case 'i': sfs->FixWireTool()->FixSelfIntersectionMode() = val; break;
      case 'n': sfs->FixWireTool()->FixNotchedEdgesMode()     = val; break;
      case '?': mess                                          = val; break;
      }
      continue;
    }
    else if (!strcmp(argv[i], "-maxtaila"))
    {
      if (++i >= argc)
      {
        break;
      }

      sfs->FixWireTool()->SetMaxTailAngle(Draw::Atof(argv[i]) * (M_PI / 180));
    }
    else if (!strcmp(argv[i], "-maxtailw"))
    {
      if (++i >= argc)
      {
        break;
      }

      sfs->FixWireTool()->SetMaxTailWidth(Draw::Atof(argv[i]));
      sfs->FixWireTool()->FixTailMode() = 1;
    }
    else
    {
      switch ( par ) {
      case 0: res = argv[i];                       break;
      case 1: {
        TopoDS_Shape initShape =  DBRep::Get(argv[i]);
        if(initShape.IsNull()) continue;
        sfs->Init ( initShape );
      } break;
      case 2: sfs->SetPrecision   (Draw::Atof(argv[i])); break;
      case 3: sfs->SetMaxTolerance(Draw::Atof(argv[i])); break;
      }
    }
    par++;
  }

  if ( par <2 ) {
    di << "Use: " << argv[0] << " result shape [tolerance [max_tolerance]] [switches]\n"
      "[-maxtaila <degrees>] [-maxtailw <width>]\n";
    di << "Switches allow to tune parameters of ShapeFix\n"; 
    di << "The following syntax is used: <symbol><parameter>\n"; 
    di << "- symbol may be - to set parameter off, + to set on or * to set default\n"; 
    di << "- parameters are identified by letters:\n"; 
    di << "  l - FixLackingMode\n"; 
    di << "  o - FixOrientationMode\n"; 
    di << "  h - FixShiftedMode\n"; 
    di << "  m - FixMissingSeamMode\n"; 
    di << "  d - FixDegeneratedMode\n"; 
    di << "  s - FixSmallMode\n"; 
    di << "  i - FixSelfIntersectionMode\n"; 
    di << "  n - FixNotchedEdgesMode\n"; 
    di << "For enhanced message output, use switch '+?'\n"; 
    return 1;
  }

  Handle(Draw_ProgressIndicator) aProgress = new Draw_ProgressIndicator (di, 1);
  sfs->Perform (aProgress->Start());
  DBRep::Set (res,sfs->Shape());

  if ( mess ) 
  {
    TColStd_DataMapOfAsciiStringInteger aMapOfNumberOfFixes;
    Standard_SStream aSStream;
    TopoDS_Compound aCompound;
    BRep_Builder aBuilder;
    aBuilder.MakeCompound (aCompound);
    const ShapeExtend_DataMapOfShapeListOfMsg &map = msg->MapShape();
    // Counting the number of each type of fixes. If the switch '*?' store all modified shapes in compound.
    for ( ShapeExtend_DataMapIteratorOfDataMapOfShapeListOfMsg it ( map ); it.More(); it.Next() )
    {
      for ( Message_ListIteratorOfListOfMsg iter ( it.Value() ); iter.More(); iter.Next() )
      {
        if ( aMapOfNumberOfFixes.IsBound ( iter.Value().Value() ) )
        {
          aMapOfNumberOfFixes ( iter.Value().Value() )++;
        }
        else
        {
          aMapOfNumberOfFixes.Bind ( iter.Value().Value(), 1 );
        }
      }
      if ( mess < 0 )
      {
        aBuilder.Add ( aCompound, it.Key() );
      }
    }
    
    aSStream << " Fix" << std::setw (58) << "Count\n";
    aSStream << " ------------------------------------------------------------\n";
    for ( TColStd_DataMapIteratorOfDataMapOfAsciiStringInteger anIter ( aMapOfNumberOfFixes ); anIter.More(); anIter.Next() )
    {
      aSStream << " " << anIter.Key() << std::setw ( 60 - anIter.Key().Length() ) << anIter.Value() << "\n";
    }
    aSStream << " ------------------------------------------------------------\n";
    di << aSStream;

    if ( mess < 0 )
    {
      char buff[256];
      Sprintf ( buff, "%s_%s", res, "m" );
      di << " Modified shapes saved in compound: " << buff;
      DBRep::Set (buff, aCompound);
    }
  }
  
  return 0; // Done
}

Best regards, Dmitrii.