View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0030422Open CASCADEOCCT:Modeling Algorithmspublic2018-12-22 23:372021-09-18 09:53
Reporteragv Assigned Tosmoskvin  
PrioritynormalSeveritymajor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2015 
Product Version7.3.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0030422: Modeling Data - Random behaviour of BRepAdaptor_CompCurve
DescriptionWith 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.
Steps To Reproducetest 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;
}
TagsNo tags attached.
Test case numberbugs/moddata_3/bug30422

Attached Files

  • sect.brep (2,622 bytes)
  • edges.png (8,258 bytes)

Activities

agv

2018-12-22 23:37

developer  

sect.brep (2,622 bytes)

ifv

2021-08-18 10:42

developer  

edges.png (8,258 bytes)

ifv

2021-08-18 10:58

developer   ~0103271

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.

msv

2021-08-18 22:33

developer   ~0103293

Igor, does it mean that BRepTools::Read each time produces different shape? If so, then we need to add a bug for this behavior.

msv

2021-08-18 22:35

developer   ~0103294

I suspect somewhere a map with shape as a key is iterated, and this is a cause of instability.

ifv

2021-08-19 09:18

developer   ~0103297

Last edited: 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.

agv

2021-08-19 09:36

developer   ~0103298

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?

ifv

2021-08-19 10:00

developer   ~0103299

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.

msv

2021-08-19 12:24

developer   ~0103300

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.

git

2021-08-19 15:07

administrator   ~0103307

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

ifv

2021-08-19 15:47

developer   ~0103308

Analysis

ifv

2021-08-19 15:49

developer   ~0103309

Solution implementation

git

2021-09-09 14:05

administrator   ~0103937

Branch CR30422 has been updated forcibly by ifv.

SHA-1: 1035121ab77c689ac8bd39140b686df00aead3de

ifv

2021-09-10 10:25

developer   ~0103969

Debugging

ifv

2021-09-10 10:26

developer   ~0103970

Testing

git

2021-09-13 10:42

administrator   ~0104054

Branch CR30422 has been updated forcibly by ifv.

SHA-1: 23ccba55ed3086ee9748a51a8f7b60f818ed98e5

git

2021-09-13 13:53

administrator   ~0104061

Branch CR30422 has been updated forcibly by ifv.

SHA-1: 35b54d64be2dc90ec264aa0497801d630d020eac

git

2021-09-13 14:05

administrator   ~0104062

Branch CR30422 has been updated forcibly by ifv.

SHA-1: c948a1ea4bb39d7991babdaa7100fa4b0e99ee2e

ifv

2021-09-13 16:14

developer   ~0104064

Branch CR30422 is ready for review

Branches for integrations
OCCT - CR30422
Products - not

msv

2021-09-13 17:06

developer   ~0104065

For integration:
occt - CR30422
products - none

git

2021-09-14 09:14

administrator   ~0104071

Branch CR30422 has been updated forcibly by ifv.

SHA-1: 4b3cc1e13bd0816c6979fd36565d8dd343b21797

git

2021-09-14 09:15

administrator   ~0104072

Branch CR30422 has been updated forcibly by ifv.

SHA-1: dbd1f368c4957dcfd752f2d64b542573ad35988a

git

2021-09-14 09:16

administrator   ~0104073

Branch CR30422 has been updated forcibly by ifv.

SHA-1: 515a1cf913a07559d47b39ab2fe35f448ac9e0ef

bugmaster

2021-09-18 09:37

administrator   ~0104188

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

git

2021-09-18 09:53

administrator   ~0104189

Branch CR30422 has been deleted by mnt.

SHA-1: 515a1cf913a07559d47b39ab2fe35f448ac9e0ef

Related Changesets

occt: master e2421df5

2021-08-19 12:05:53

ifv


Committer: smoskvin Details Diff		 0030422: Random behaviour of BRepAdaptor_CompCurve

BRepTools/BRepTools_WireExplorer.cxx - replace MapOfShape vmap by IndexedMapOfShape
to avoid random behavior of algorithm

bugs/moddata_3/bug30422 - new test case added		 Affected Issues
0030422
mod - src/BRepTools/BRepTools_WireExplorer.cxx Diff File
add - tests/bugs/moddata_3/bug30422 Diff File

Issue History

Date Modified Username Field Change
2018-12-22 23:37 agv New Issue
2018-12-22 23:37 agv Assigned To => msv
2018-12-22 23:37 agv File Added: sect.brep
2018-12-24 11:06 msv Target Version Unscheduled => 7.4.0
2019-08-12 17:45 msv Target Version 7.4.0 => 7.5.0
2020-09-14 22:54 msv Target Version 7.5.0 => 7.6.0
2021-08-17 12:40 szy Assigned To msv => ifv
2021-08-17 12:40 szy Status new => assigned
2021-08-18 10:42 ifv File Added: edges.png
2021-08-18 10:58 ifv Note Added: 0103271
2021-08-18 10:58 ifv Assigned To ifv => msv
2021-08-18 10:58 ifv Status assigned => resolved
2021-08-18 22:33 msv Note Added: 0103293
2021-08-18 22:35 msv Note Added: 0103294
2021-08-18 22:35 msv Assigned To msv => ifv
2021-08-18 22:35 msv Status resolved => feedback
2021-08-19 09:18 ifv Note Added: 0103297
2021-08-19 09:18 ifv Assigned To ifv => msv
2021-08-19 09:18 ifv Status feedback => resolved
2021-08-19 09:24 ifv Note Edited: 0103297
2021-08-19 09:36 agv Note Added: 0103298
2021-08-19 10:00 ifv Note Added: 0103299
2021-08-19 10:01 ifv Note Edited: 0103297
2021-08-19 12:24 msv Note Added: 0103300
2021-08-19 12:25 msv Assigned To msv => ifv
2021-08-19 12:25 msv Status resolved => assigned
2021-08-19 15:07 git Note Added: 0103307
2021-08-19 15:47 ifv Note Added: 0103308
2021-08-19 15:49 ifv Note Added: 0103309
2021-09-09 14:05 git Note Added: 0103937
2021-09-09 18:00 ifv Summary Random behaviour of BRepAdaptor_CompCurve => Modeling Data - Random behaviour of BRepAdaptor_CompCurve
2021-09-10 10:25 ifv Note Added: 0103969
2021-09-10 10:26 ifv Note Added: 0103970
2021-09-13 10:42 git Note Added: 0104054
2021-09-13 13:53 git Note Added: 0104061
2021-09-13 14:05 git Note Added: 0104062
2021-09-13 16:04 ifv Test case number => bugs/moddata_3/bug30422
2021-09-13 16:04 ifv Steps to Reproduce Updated
2021-09-13 16:14 ifv Note Added: 0104064
2021-09-13 16:14 ifv Assigned To ifv => msv
2021-09-13 16:14 ifv Status assigned => resolved
2021-09-13 17:06 msv Note Added: 0104065
2021-09-13 17:06 msv Assigned To msv => bugmaster
2021-09-13 17:06 msv Status resolved => reviewed
2021-09-14 09:14 git Note Added: 0104071
2021-09-14 09:15 git Note Added: 0104072
2021-09-14 09:16 git Note Added: 0104073
2021-09-18 09:37 bugmaster Status reviewed => tested
2021-09-18 09:37 bugmaster Note Added: 0104188
2021-09-18 09:43 smoskvin Changeset attached => occt master e2421df5
2021-09-18 09:43 smoskvin Assigned To bugmaster => smoskvin
2021-09-18 09:43 smoskvin Status tested => verified
2021-09-18 09:43 smoskvin Resolution open => fixed
2021-09-18 09:53 git Note Added: 0104189