View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0031926 | Community | OCCT:Shape Healing | public | 2020-11-13 17:36 | 2023-03-19 22:33 |
| Reporter | Oliver R | Assigned To | oan | ||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Platform | Linux | OS | Ubuntu | ||
| Product Version | 7.4.0 | ||||
| Target Version | 7.7.0 | Fixed in Version | 7.7.0 | ||
| Summary | 0031926: Shape Healing - ShapeAnalysis::OuterWire() considers next iteration element always to be a wire causing skipping of primal one | ||||
| Description | Cutting a sewn tetrahedron made up from faces from a sphere will hang in BRepMesh_IncrementalMesh. | ||||
| Steps To Reproduce | pload MODELING VISUALIZATION axo vertex v11 0 1 0; vertex v12 1 0 0; vertex v13 0 0 0 edge e11 v11 v12; edge e12 v12 v13; edge e13 v13 v11 wire w1 e11 e12 e13 mkplane f1 w1 vertex v21 0 0 2; vertex v22 1 0 0; vertex v23 0 0 0 edge e21 v21 v22; edge e22 v22 v23; edge e23 v23 v21 wire w2 e21 e22 e23 mkplane f2 w2 vertex v31 0 0 2; vertex v32 0 1 0; vertex v33 1 0 0 edge e31 v31 v32; edge e32 v32 v33; edge e33 v33 v31 wire w3 e31 e32 e33 mkplane f3 w3 vertex v41 0 0 2; vertex v42 0 0 0; vertex v43 0 1 0 edge e41 v41 v42; edge e42 v42 v43; edge e43 v43 v41 wire w4 e41 e42 e43 mkplane f4 w4 psphere s1 1 sewing sh2 f1 f2 f3 f4 ssolid sh2 s2 bcut c s1 s2 #save c c.brep incmesh c 1 vinit View1 vdisplay -dispMode 1 c vfit Compile the attached file with g++ -O0 -g -I ~/ooc/build/include/opencascade -L ~/ooc/build/lin64/gcc/lib demo_simple_10.cpp -lTKBin -lTKBinL -lTKBinTObj -lTKBinXCAF -lTKBO -lTKBool -lTKBRep -lTKCAF -lTKCDF -lTKDCAF -lTKDraw -lTKernel -lTKFeat -lTKFillet -lTKG2d -lTKG3d -lTKGeomAlgo -lTKGeomBase -lTKHLR -lTKIGES -lTKLCAF -lTKMath -lTKMesh -lTKMeshVS -lTKOffset -lTKOpenGl -lTKPrim -lTKQADraw -lTKRWMesh -lTKService -lTKShHealing -lTKStd -lTKStdL -lTKSTEP209 -lTKSTEP -lTKSTEPAttr -lTKSTEPBase -lTKSTL -lTKTObj -lTKTObjDRAW -lTKTopAlgo -lTKTopTest -lTKV3d -lTKVCAF -lTKViewerTest -lTKVRML -lTKXCAF -lTKXDEDRAW -lTKXDEIGES -lTKXDESTEP -lTKXMesh -lTKXml -lTKXmlL -lTKXmlTObj -lTKXmlXCAF -lTKXSBase -lTKXSDRAW | ||||
| Tags | No tags attached. | ||||
| Test case number | |||||
|
|
demo_simple_10.cpp (2,499 bytes) |
|
|
The problem is reproducible on OCCT 7.4.0, but not on 7.5.0 - so it seems the bug has been fixed by some patch. |
|
|
Boolean operation produces a broken Solid:Draw[26]> bcut c s1 s2 Warning: Unable to orient the shape correctly On OCCT 7.5.0 such Solid doesn't cause BRepMesh hanging, but still should be checked if Boolean operation works as expected. |
|
|
Branch CR31926 has been created by oan. SHA-1: 1b37f39f9782361fd7fccf831e5a6d3fb6d92930 Detailed log of new commits: Author: oan Date: Wed Aug 31 17:40:33 2022 +0300 0031926: Shape Healing - ShapeAnalysis::OuterWire() considers next iteration element always to be a wire causing skipping of primal one ShapeAnalysis::OuterWire(): fixed missed logic when TopoDS_Iterator notifies about more objects to iterate, but there are only vertices and no additional wires at all. |
|
|
Branch CR31926 has been updated forcibly by oan. SHA-1: 6656c79b800270c6c6bc6239f8ffd345678c77df |
|
|
Branch CR31926 has been updated forcibly by oan. SHA-1: 3a73df38df7397efded153dae59c2d246bd5a058 |
|
|
Patch is ready for review. Test reports are available at: http://jenkins-test-occt/view/master-CR31926-OAN/view/COMPARE/ To integrate: OCCT: CR31926 PRODUCTS: None |
|
|
Using explorer instead of iterator is unjustified here. Explorer is a heavy object involving stack of iterators and additional memory allocations. It is needed to provide correct logic with using iterator.- TopoDS_Iterator anIt (F, Standard_False); + TopExp_Explorer anIt (F, TopAbs_WIRE); Declare aWire as a reference instead of a value:
const TopoDS_Shape aWire = anIt.Value();
It is better to remove "REQUIRED" statement instead of commenting it out: -puts "REQUIRED ALL: Meshing statuses: Failure" +#puts "REQUIRED ALL: Meshing statuses: Failure" |
|
|
Branch CR31926 has been updated forcibly by oan. SHA-1: 086192792fa177e03d3acc6946dfb509a61b32db |
|
|
Despite of being lightweight, TopoDS_Iterator requires a lot of an additional code to prevent the described issue which partially duplicates functionality of TopExp_Explorer, whereas TopExp_Explorer itself provides required functionality by default without any additional charge, moreover, if you check, it is widely used along the remaining code of ShapeAnalysis. Why we need it at all then if there is just a better, and quicker, and simpler TopoDS_Iterator - we should either use it or remove it from OCCT, right? So, indeed, I urge to use TopExp_Explorer to solve the problem here instead of inventing a wheel one more time and introduce new custom hidden problem like it was with 0031144 which was expected to fix exactly the same problem, but with different order of shapes (vertex-wire, instead of wire-vertex in here). Overhead seems to be quite small, but code will be as clear and robust as possible. >>> Declare aWire as a reference instead of a value: The entire procedure will fail with suggested approach due to a detail with implementation of the iterator (both TopoDS_Iterator and TopExp_Explorer) which returns reference to its field instead of reference to the source shape which becomes invalid right at the next line when ShapeAnalysis::OuterWire() calls Next(). >>> It is better to remove "REQUIRED" statement instead of commenting it out: Agree. Done. |
|
|
Here we discuss only the method OuterWire. It is used as a separate method not dealing with other methods of the same class. So, it is not worth mentioning other methods here. Explorer is very useful tool. We use it when we need to explore the shape more than one level in depth. But when we need to get access only to direct children it is preferable to use iterator. In particular, here I do not see much complexity to solve the problem using iterator (wheel is not needed to invent here). I believe that it can be done without making the code unclear. About declaring aWire as a reference, you are right, and it is my bad that I did not think about that side effect. |
|
|
Branch CR31926_1 has been created by emv. SHA-1: 07a8adf531ade729f817f7a32c3c998f0257f3c8 Detailed log of new commits: Author: Eugeny Maltchikov Date: Tue Sep 13 10:43:37 2022 +0300 # Avoid using ShapeAnalysis::OuterBound for computing area Author: oan Date: Wed Aug 31 17:40:33 2022 +0300 0031926: Shape Healing - ShapeAnalysis::OuterWire() considers next iteration element always to be a wire causing skipping of primal one ShapeAnalysis::OuterWire(): fixed missed logic when TopoDS_Iterator notifies about more objects to iterate, but there are only vertices and no additional wires at all. |
|
|
Branch CR31926_1 has been updated forcibly by emv. SHA-1: f232fa228ec867a2a75c9fdc6dae2ca8fa329ecb |
|
|
Oleg, could you please review the changes in CR31926_1? http://jenkins-test-08.nnov.opencascade.com/view/CR31926_1-master-emv/view/COMPARE/ |
|
|
Please have a look. |
|
|
To integrate: occt - CR31926_1 products - none |
|
|
Branch CR31926 has been deleted by mnt. SHA-1: 086192792fa177e03d3acc6946dfb509a61b32db |
|
|
Branch CR31926_1 has been deleted by mnt. SHA-1: c8689bbb8de4d2986673155eb4b69a9f1ddc9346 |
|
occt: master 59223e11 2022-08-31 17:40:33 Committer: |
0031926: Shape Healing - ShapeAnalysis::OuterWire() considers next iteration element always to be a wire causing skipping of primal one ShapeAnalysis::OuterWire(): fixed missed logic when TopoDS_Iterator notifies about more objects to iterate, but there are only vertices and no additional wires at all. |
Affected Issues 0031926 |
|
| mod - src/ShapeAnalysis/ShapeAnalysis.cxx | Diff File | ||
| mod - src/ShapeAnalysis/ShapeAnalysis.hxx | Diff File | ||
| mod - tests/bugs/mesh/bug31144 | Diff File | ||
| add - tests/bugs/mesh/bug31926 | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-11-13 17:36 | Oliver R | New Issue | |
| 2020-11-13 17:36 | Oliver R | Assigned To | => oan |
| 2020-11-13 17:36 | Oliver R | File Added: demo_simple_10.cpp | |
| 2020-11-13 18:18 | kgv | Summary | Hang => Mesh - Hang |
| 2020-11-13 22:25 | kgv | Note Added: 0096770 | |
| 2020-11-13 22:25 | kgv | Target Version | => 7.5.0 |
| 2020-11-13 22:49 | kgv | Steps to Reproduce Updated | |
| 2020-11-13 22:53 | kgv | Note Added: 0096771 | |
| 2020-11-13 22:54 | kgv | Note Edited: 0096771 | |
| 2020-11-13 22:55 | kgv | Target Version | 7.5.0 => 7.6.0 |
| 2021-11-01 18:13 |
|
Target Version | 7.6.0 => 7.7.0 |
| 2022-08-31 17:33 | oan | Relationship added | related to 0031144 |
| 2022-08-31 17:38 | oan | Summary | Mesh - Hang => Shape Healing - ShapeAnalysis::OuterWire() considers next iteration element always to be a wire causing skipping of primal one |
| 2022-08-31 17:42 | oan | Category | OCCT:Mesh => OCCT:Shape Healing |
| 2022-09-02 15:49 | git | Note Added: 0110575 | |
| 2022-09-02 16:14 | git | Note Added: 0110576 | |
| 2022-09-02 18:54 | git | Note Added: 0110582 | |
| 2022-09-05 12:00 | oan | Assigned To | oan => gka |
| 2022-09-05 12:00 | oan | Status | new => resolved |
| 2022-09-05 12:00 | oan | Note Added: 0110632 | |
| 2022-09-05 12:00 | oan | Assigned To | gka => msv |
| 2022-09-07 12:32 |
|
Note Added: 0110809 | |
| 2022-09-07 12:33 |
|
Assigned To | msv => oan |
| 2022-09-07 12:33 |
|
Status | resolved => assigned |
| 2022-09-07 12:34 |
|
Note Edited: 0110809 | |
| 2022-09-07 12:34 |
|
Note Edited: 0110809 | |
| 2022-09-07 17:55 | git | Note Added: 0110831 | |
| 2022-09-07 18:02 | oan | Note Added: 0110832 | |
| 2022-09-07 18:03 | oan | Assigned To | oan => msv |
| 2022-09-07 18:03 | oan | Status | assigned => feedback |
| 2022-09-07 18:03 | oan | Note Edited: 0110832 | |
| 2022-09-08 10:54 |
|
Assigned To | msv => oan |
| 2022-09-08 10:54 |
|
Status | feedback => assigned |
| 2022-09-08 10:54 |
|
Note Added: 0110845 | |
| 2022-09-12 12:27 |
|
Note Edited: 0110845 | |
| 2022-09-12 12:47 |
|
Assigned To | oan => emv |
| 2022-09-13 11:20 | git | Note Added: 0111000 | |
| 2022-09-13 12:33 | git | Note Added: 0111004 | |
| 2022-09-22 07:23 |
|
Assigned To | emv => oan |
| 2022-09-22 07:23 |
|
Status | assigned => resolved |
| 2022-09-22 07:23 |
|
Note Added: 0111180 | |
| 2022-10-24 11:09 | oan | Assigned To | oan => msv |
| 2022-10-24 11:15 | oan | Note Added: 0111727 | |
| 2022-10-24 11:52 |
|
Assigned To | msv => bugmaster |
| 2022-10-24 11:52 |
|
Status | resolved => reviewed |
| 2022-10-24 11:52 |
|
Note Added: 0111728 | |
| 2022-11-02 13:39 |
|
Status | reviewed => tested |
| 2022-11-02 13:46 |
|
Changeset attached | => occt master 59223e11 |
| 2022-11-02 13:46 | oan | Assigned To | bugmaster => oan |
| 2022-11-02 13:46 | oan | Status | tested => verified |
| 2022-11-02 13:46 | oan | Resolution | open => fixed |
| 2022-11-02 13:52 | git | Note Added: 0111860 | |
| 2022-11-02 13:52 | git | Note Added: 0111861 | |
| 2023-03-19 22:33 | vglukhik | Status | verified => closed |
| 2023-03-19 22:33 | vglukhik | Fixed in Version | => 7.7.0 |
