BOPAlgo_InvalidCurveOnSurface and tolerances.

[blobfish]

I was experimenting with tightening tolerances when I noticed the tightening operation was causing BOPAlgo_InvalidCurveOnSurface errors in BOPAlgo_ArgumentAnalyzer. I see BOPAlgo_InvalidCurveOnSurface errors quite frequently so decided to look into it. Here is my unit test. It should be self explanatory and I will attach my input file in case anybody is curious.

TEST_CASE("000012", "[.000012]")
{
  auto basePath = getTestPath() / "000012/base.brep";
  REQUIRE(std::filesystem::exists(basePath));
  TopoDS_Shape baseShape = ocs::readBRep(basePath.string());
  REQUIRE(!baseShape.IsNull());
 
  //require that the input shape passes bopalgo check.
  BOPAlgo_ArgumentAnalyzer BOPCheck0;
  BOPCheck0.SetRunParallel(true);
  BOPCheck0.CurveOnSurfaceMode() = true;
  BOPCheck0.SetShape1(baseShape);
  BOPCheck0.Perform();
  REQUIRE(!BOPCheck0.HasFaulty());
 
  //tighten all edges and vertex tolerances to precision confusion.
  ShapeFix_ShapeTolerance tighten;
  tighten.SetTolerance(baseShape, Precision::Confusion(), TopAbs_EDGE);
  tighten.SetTolerance(baseShape, Precision::Confusion(), TopAbs_VERTEX);
 
  //call sameparameter to loosen tolerances
  REQUIRE(ShapeFix::SameParameter(baseShape, false));
 
  //now call bopcheck again.
  BOPAlgo_ArgumentAnalyzer BOPCheck1;
  BOPCheck1.SetRunParallel(true);
  BOPCheck1.CurveOnSurfaceMode() = true;
  BOPCheck1.SetShape1(baseShape);
  BOPCheck1.Perform();
  CHECK(!BOPCheck1.HasFaulty()); //this fails as we now have BOPAlgo_InvalidCurveOnSurface errors
}

I have found the one location(BOPAlgo_ArgumentAnalyzer.cxx:902) where BOPAlgo_InvalidCurveOnSurface errors are assigned and it has to do with the current edge tolerance being less than/tighter than what BOPAlgo thinks it should be. So I traced ShapeFix and BopAlgo to determine where/how the edge tolerances are calculated. Here are those backtraces.

backtrace for bopalgo:
  #0  0x00007ffff5ffe440 in GeomLib_CheckCurveOnSurface::Perform(opencascade::handle<Adaptor3d_CurveOnSurface> const&, bool)@plt ()
    from ./development/occt/build/debug/lin64/gcc/libd/libTKTopAlgo.so.7
  #1  0x00007ffff60e3d4a in BRepLib_CheckCurveOnSurface::Compute (this=0x7fffffffc790, theCurveOnSurface=..., isMultiThread=false)
      at ./development/occt/src/BRepLib/BRepLib_CheckCurveOnSurface.cxx:116
  #2  0x00007ffff60e3cd2 in BRepLib_CheckCurveOnSurface::Perform (this=0x7fffffffc790, isMultiThread=false)
      at ./development/occt/src/BRepLib/BRepLib_CheckCurveOnSurface.cxx:95
  #3  0x00007ffff7f26859 in BOPTools_AlgoTools::ComputeTolerance (theFace=..., theEdge=..., theMaxDist=@0x7fffffffc928: 7.5189693618638839e-304,
      theMaxPar=@0x7fffffffc930: 6.9533558072235487e-310) at ./development/occt/src/BOPTools/BOPTools_AlgoTools_1.cxx:1023
  #4  0x00007ffff7e65c4a in BOPAlgo_ArgumentAnalyzer::TestCurveOnSurface (this=0x7fffffffcf30)
      at ./development/occt/src/BOPAlgo/BOPAlgo_ArgumentAnalyzer.cxx:898
  #5  0x00007ffff7e634db in BOPAlgo_ArgumentAnalyzer::Perform (this=0x7fffffffcf30, theRange=...)
      at ./development/occt/src/BOPAlgo/BOPAlgo_ArgumentAnalyzer.cxx:250
  #6  0x00005555555a63cc in ____C_A_T_C_H____T_E_S_T____20 () at ../../main.cpp:476

backtrace for sameparameter:
  #0  0x00007ffff6339900 in BRepLib_ValidateEdge::Process()@plt () from ./development/occt/build/debug/lin64/gcc/libd/libTKShHealing.so.7
  #1  0x00007ffff639ce55 in ShapeAnalysis_Edge::CheckSameParameter (this=0x7fffffffc5d0, edge=..., face=..., maxdev=@0x7fffffffc560: 0, NbControl=23)
      at ./development/occt/src/ShapeAnalysis/ShapeAnalysis_Edge.cxx:809
  #2  0x00007ffff63ecfae in ShapeFix_Edge::FixSameParameter (this=0x555555709730, edge=..., face=..., tolerance=0)
      at ./development/occt/src/ShapeFix/ShapeFix_Edge.cxx:810
  #3  0x00007ffff63d12ca in ShapeFix::SameParameter (shape=..., enforce=false, preci=0, theProgress=..., theMsgReg=...)
      at ./development/occt/src/ShapeFix/ShapeFix.cxx:142
  #4  0x00005555555a6612 in ____C_A_T_C_H____T_E_S_T____20 () at ../../main.cpp:488

What jumped out at me between the two code paths is, ShapeFix::SameParameter ends up using a magic number of 23 control points where the bopalgo path uses control point count equal to the degree of curve or at least 3. I used gdb to verify that same parameter always used 23 control points and bopalgo used one of the following values: 3, 5, 7, 10, 11, which are specific to my test file. Then I decided to put a breakpoint in the bopalgo check where the error is assigned and compare the tolerance values. Here is a list of respective pairs of the bopalgo computed tolerance and the edge current tolerance which was assigned by same parameter.

$101 = {0.0073508010943501723, 0.0072388350053866251}
$102 = {0.016235237310777358, 0.016193603476251869}
$103 = {0.0051260083941153519, 0.0051231979010308761}
$104 = {0.0073508010947810056, 0.0072388350057895736}
$105 = {0.016235237311410657, 0.016193603476860344}
$106 = {0.0051260083935450495, 0.0051231979004558439}
$107 = {6.7988064362276725e-05, 6.7954958097421014e-05}
$108 = {6.798806435464569e-05, 6.7954958087833387e-05}

Those values looks pretty close and, I think, strengthen the theory that the bopalgo error is from the control point count discrepancy described above. I would think that edge tolerance calculation from edge, pcurve and surface would/should be unified in the api. Any thoughts or opinions?

 

[Quaoar]

They had some test files for CAD translators, and they used some magic numbers like 23 to make their tests work. In my opinion, you're right that BOP checks should at least use the same probe points over the curves, otherwise you have two different tolerances computed for a single entity. The thing here is that ShapeFix was done by a data exchange team and BOPs were done by another group of people. You know what I mean.

One idea was to reuse the BOP check in the BRepCheck package, but that would inevitably lead to many files being reported as "broken" (by checkshape) while they are essentially not. Making it another way around (using these 23 points in BOPs) might not work for Booleans though, as the regression testing database for Booleans is larger and BOPs are very sensitive to such changes. Still, it's worth a try, I guess.

 

[blobfish]

IMHO:
It looks like the magic number of sample points was accepted practice in the early days of opencascade and has since fell out of favor for logical algos. I think BRepLib_ValidateEdge is an attempt to bridge between the two. If I understand it correctly, BRepLib_ValidateEdge:rocessApprox() will actually be more accurate than BRepLib_ValidateEdge:rocessExact() for lower order curves. I think this idea was proven by my previous experiment. The algos for determining sample points makes more sense to me. Anything with magic numbers is suspect. So I think the bopalgo check of tolerance is the direction the rest of the library needs to go. Probably the 'fallout' from altering the rest of the library to match probably can't be overstated. The only thing I am sure of is that edge tolerance calculation needs a heavy refactor. 'DRY' has become a cult nowadays, but occ could take a few sips of the 'kool-aid'. Here are some notes from further investigation. Line numbers relative to v7.7.0 beta.

magic numbers for sample points.
  BRepLib_ValidateEdge::processApprox(): uses member myControlPointsNumber(defaults 22) for sample points.
  TopOpeBRepBuild_Tools_1.cxx:306 magic number 23 for sample points.
  ShapeFix_Edge::FixSameParameter calls ShapeAnalysis_Edge::CheckSameParameter with the default magic 23 sample points
  ChFi3d_Builder_0.cxx:2853 has magic number of 45 for sample points. double point count for testing?
  BOPTest_TolerCommands.cxx:208 magic number 23 for sample points. //draw command.

Algo for sample points.
  GeomLib_CheckCurveOnSurface.cxx:388 has note on theory of using sample points on monotonicity intervals.
  BRepLib_CheckCurveOnSurface thin wrap of GeomLib_CheckCurveOnSurface
  BRepTopAdaptor_TopolTool::ComputeSamplePoints
    Computes number of samples from a surface without curves. Used for pcurve generation.
    This looks similar to GeomLib_CheckCurveOnSurface in how sample count is calculated. no magic number.

of interest:
  BRepLib_ValidateEdge:
    wrote in 2021.
    BRepLib_ValidateEdge::process() public method dispatches to private methods below based upon:
      myIsExactMethod defaults to false.
      theSameParameter set in construction.
      if (myIsExactMethod && mySameParameter) processExact();
      else processApprox();
    BRepLib_ValidateEdge::processApprox():
      uses member myControlPointsNumber(defaults 22) for sample points.
      note: myControlPointsNumber can be set externally from public method: SetControlPointsNumber
    BRepLib_ValidateEdge::processExact():
      uses GeomLib_CheckCurveOnSurface
  BOPAlgo_ArgumentAnalyzer::TestCurveOnSurface() ->
    BOPTools_AlgoTools::ComputeTolerance ->
      BRepLib_CheckCurveOnSurface