MantisBT - Open CASCADE
View Issue Details
0030422Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2018-12-22 23:372021-09-18 09:53
agv 
smoskvin 
normalmajor 
verifiedfixed 
WindowsVC++ 201532 bit
[OCCT] 7.3.0 
[OCCT] 7.6.0* 
bugs/moddata_3/bug30422
0030422: Modeling Data - Random behaviour of BRepAdaptor_CompCurve
With a particular wire, the class BRepAdaptor_CompCurve produces random results. My output is:

FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 3.0000, middle:( -28.048 49.225 79.887)
FP = 0.0000; LP = 1.0000, middle:( -27.613 147.673 8.412)

Reproduced in 7.3.0 / vc14 / 32bit
Similar result in OCCT 7.1 and 7.2 and also with VisualStudio 2010.
test case bugs/moddata_3/bug30422

original steps to reproduce:

int main()
{
  TopoDS_Shape aShape;
  BRep_Builder aBld;
  for (int i = 0; i < 20; i++) {
    BRepTools::Read(aShape, "sect.brep", aBld);
    const BRepAdaptor_CompCurve aCurve(TopoDS::Wire(aShape));
    const Standard_Real aParam[2] = {
      aCurve.FirstParameter(), aCurve.LastParameter() };
    gp_Pnt aPnt;
    aCurve.D0(0.5 *(aParam[0]+aParam[1]), aPnt);
    printf("FP = %7.4f; LP = %7.4f, middle:(%8.3f %8.3f %8.3f)\n",
           aParam[0], aParam[1], aPnt.X(), aPnt.Y(), aPnt.Z());
  }
  return 0;
}
No tags attached.
? sect.brep (2,622) 2018-12-22 23:37
https://tracker.dev.opencascade.org/
png edges.png (8,258) 2021-08-18 10:42
https://tracker.dev.opencascade.org/
Issue History
2018-12-22 23:37agvNew Issue
2018-12-22 23:37agvAssigned To => msv
2018-12-22 23:37agvFile Added: sect.brep
2018-12-24 11:06msvTarget VersionUnscheduled => 7.4.0
2019-08-12 17:45msvTarget Version7.4.0 => 7.5.0
2020-09-14 22:54msvTarget Version7.5.0 => 7.6.0*
2021-08-17 12:40szyAssigned Tomsv => ifv
2021-08-17 12:40szyStatusnew => assigned
2021-08-18 10:42ifvFile Added: edges.png
2021-08-18 10:58ifvNote Added: 0103271
2021-08-18 10:58ifvAssigned Toifv => msv
2021-08-18 10:58ifvStatusassigned => resolved
2021-08-18 22:33msvNote Added: 0103293
2021-08-18 22:35msvNote Added: 0103294
2021-08-18 22:35msvAssigned Tomsv => ifv
2021-08-18 22:35msvStatusresolved => feedback
2021-08-19 09:18ifvNote Added: 0103297
2021-08-19 09:18ifvAssigned Toifv => msv
2021-08-19 09:18ifvStatusfeedback => resolved
2021-08-19 09:24ifvNote Edited: 0103297bug_revision_view_page.php?bugnote_id=103297#r25634
2021-08-19 09:36agvNote Added: 0103298
2021-08-19 10:00ifvNote Added: 0103299
2021-08-19 10:01ifvNote Edited: 0103297bug_revision_view_page.php?bugnote_id=103297#r25635
2021-08-19 12:24msvNote Added: 0103300
2021-08-19 12:25msvAssigned Tomsv => ifv
2021-08-19 12:25msvStatusresolved => assigned
2021-08-19 15:07gitNote Added: 0103307
2021-08-19 15:47ifvNote Added: 0103308
2021-08-19 15:49ifvNote Added: 0103309
2021-09-09 14:05gitNote Added: 0103937
2021-09-09 18:00ifvSummaryRandom behaviour of BRepAdaptor_CompCurve => Modeling Data - Random behaviour of BRepAdaptor_CompCurve
2021-09-10 10:25ifvNote Added: 0103969
2021-09-10 10:26ifvNote Added: 0103970
2021-09-13 10:42gitNote Added: 0104054
2021-09-13 13:53gitNote Added: 0104061
2021-09-13 14:05gitNote Added: 0104062
2021-09-13 16:04ifvTest case number => bugs/moddata_3/bug30422
2021-09-13 16:04ifvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=25729#r25729
2021-09-13 16:14ifvNote Added: 0104064
2021-09-13 16:14ifvAssigned Toifv => msv
2021-09-13 16:14ifvStatusassigned => resolved
2021-09-13 17:06msvNote Added: 0104065
2021-09-13 17:06msvAssigned Tomsv => bugmaster
2021-09-13 17:06msvStatusresolved => reviewed
2021-09-14 09:14gitNote Added: 0104071
2021-09-14 09:15gitNote Added: 0104072
2021-09-14 09:16gitNote Added: 0104073
2021-09-18 09:37bugmasterStatusreviewed => tested
2021-09-18 09:37bugmasterNote Added: 0104188
2021-09-18 09:43smoskvinChangeset attached => occt master e2421df5
2021-09-18 09:43smoskvinAssigned Tobugmaster => smoskvin
2021-09-18 09:43smoskvinStatustested => verified
2021-09-18 09:43smoskvinResolutionopen => fixed
2021-09-18 09:53gitNote Added: 0104189

Notes
(0103271)
ifv   
2021-08-18 10:58   
It is not a bug.
Input wire sect.brep is wrong: all edges are FORWARD, but one edge has opposite direction of curve, see edges.png. Algorithm BRepAdaptor_CompCurve builds sequence of co-directional curves starting with "first" edge. "First" edge (it can be left or right one) is set randomly while reading file by BRepTools::Read(aShape, "sect.brep", aBld). Depending of "first" edge CompCurve can contain 3 or 1 curves.

Before using this wire in BRepAdaptor_CompCurve it is necessary to fix problem by Shape healing tools, in Draw command fixshape can be used.
(0103293)
msv   
2021-08-18 22:33   
Igor, does it mean that BRepTools::Read each time produces different shape? If so, then we need to add a bug for this behavior.
(0103294)
msv   
2021-08-18 22:35   
I suspect somewhere a map with shape as a key is iterated, and this is a cause of instability.
(0103297)
ifv   
2021-08-19 09:18   
(edited on: 2021-08-19 10:01)
Dear Mikhail, my previous explanation, of course, is not quite full and correct.
Algorithm BRepAdaptor_CompCurve uses BRepTools_WireExplorer, which defines first edge and sequence of edges. First edge is edge, which starts from first vertex. First vertex is one of two boundary vertices of wire and must have orientation FORWARD, last vertex must have orientation REVERSED. WireExplorer stores wire boundary vertices in simple MapOfShape and takes from this map only vertex with orientation FORWARD. Since wire is wrong, both boundary vertices have orientation FORWARD and algorithm takes first vertex in map, using map iterator. Position vertex in map is defined by its address, which is set while shape is loaded by BRepTools::Read(aShape, "sect.brep", aBld).
If shape loading were before for (...), results were the same for each iteration, but of course, were wrong.
It is possible to use IndexedMapOfShape instead of MapOfShape in WireExplorer, but it does not solve agv problem. I think he would like to have correct result - CompCurve must contain 4 curves, but not 3 or 1.
It is posssible to create bug for WireExplorer, but in my opinion:
For valid wires this modification is useless;
For invalid wires results any case were wrong and were not to respect to customer expectations.

(0103298)
agv   
2021-08-19 09:36   
There is one question: how to detect this kind of error? The command "checkshape" tells that the wire is correct. Also, would it be possible that sometime someone creates on purpose such wire with two forward vertices on its ends, for very specific algorithmic needs?
(0103299)
ifv   
2021-08-19 10:00   
Really checkshape (BRepCheck_Analyser) does not check orientation of 3d wire without face, because, as you mentioned, everyone can create any wire for his specific algorithmic purposes. In particular algorithms, if there is possibility of appearance wire with bad edge orientation, it is possible to compare total number of edges with help of TopoDS_Iterator and BRepTools_WireExplorer. Both values must be the same for valid wires. There is class ShapeAnalysis_Wire, which can check some wire problems, including edge orientation.
(0103300)
msv   
2021-08-19 12:24   
I think random behavior is the problem and should be fixed. So, using Indexed map instead of simple map should fix this problem. In this case the vertex that is first in the brep data will be given first regardless of its memory address.
This fix can be done in scope of this bug.
(0103307)
git   
2021-08-19 15:07   
Branch CR30422 has been created by ifv.

SHA-1: d1d2299f42f5966056080be321bbf8cf479fd124


Detailed log of new commits:

Author: ifv
Date: Thu Aug 19 15:05:53 2021 +0300

    0030422: Random behaviour of BRepAdaptor_CompCurve
    
    BRepTools/BRepTools_WireExplorer.cxx - replace MapOfShape vmap by IndexedMapOfShape
    to avoid random behavior of algorithm
(0103308)
ifv   
2021-08-19 15:47   
Analysis
(0103309)
ifv   
2021-08-19 15:49   
Solution implementation
(0103937)
git   
2021-09-09 14:05   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: 1035121ab77c689ac8bd39140b686df00aead3de
(0103969)
ifv   
2021-09-10 10:25   
Debugging
(0103970)
ifv   
2021-09-10 10:26   
Testing
(0104054)
git   
2021-09-13 10:42   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: 23ccba55ed3086ee9748a51a8f7b60f818ed98e5
(0104061)
git   
2021-09-13 13:53   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: 35b54d64be2dc90ec264aa0497801d630d020eac
(0104062)
git   
2021-09-13 14:05   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: c948a1ea4bb39d7991babdaa7100fa4b0e99ee2e
(0104064)
ifv   
2021-09-13 16:14   
Branch CR30422 is ready for review

Branches for integrations
OCCT - CR30422
Products - not
(0104065)
msv   
2021-09-13 17:06   
For integration:
occt - CR30422
products - none
(0104071)
git   
2021-09-14 09:14   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: 4b3cc1e13bd0816c6979fd36565d8dd343b21797
(0104072)
git   
2021-09-14 09:15   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: dbd1f368c4957dcfd752f2d64b542573ad35988a
(0104073)
git   
2021-09-14 09:16   
Branch CR30422 has been updated forcibly by ifv.

SHA-1: 515a1cf913a07559d47b39ab2fe35f448ac9e0ef
(0104188)
bugmaster   
2021-09-18 09:37   
Combination -
OCCT branch : IR-2021-09-17
master SHA - 812ee2c9bec89902de2ff85201cb314e0de894cc
49e51745631c52b6c452c65adae4d6dfa21a1b1e
Products branch : IR-2021-09-17 SHA - 1127e31e32f90ff63544b0516092694f1a36932f
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17390.540000000547 / 17358.370000000414 [+0.19%]
Products
Total CPU difference: 11364.740000000118 / 11411.36000000011 [-0.41%]
Windows-64-VC14:
OCCT
Total CPU difference: 19321.5625 / 19327.15625 [-0.03%]
Products
Total CPU difference: 12746.171875 / 12747.78125 [-0.01%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0104189)
git   
2021-09-18 09:53   
Branch CR30422 has been deleted by mnt.

SHA-1: 515a1cf913a07559d47b39ab2fe35f448ac9e0ef