MantisBT - Community
View Issue Details
0025013Community[OCCT] OCCT:Shape Healingpublic2014-06-16 20:012015-09-18 18:30
drazmyslovich 
bugmaster 
normaltweak 
closedfixed 
WindowsVC++ 201064 bit
[OCCT] 6.6.0 
[OCCT] 6.9.0[OCCT] 6.9.0 
bugs heal(017) bug25013_1, bug25013_2
0025013: ShapeFix_Wire tweaks for better results
I would like to propose the following tweaks for ShapeFix_Wire, which lead to better results according to my tests database:
1. The new generated P curves going over singularities can be fixed by modifying the tolerance on vertices, if the distance from the vertex to the singularity doesn't exceed 1.1 times the current tolerance.
2. If some intersecting edges were fixed without topology modifications, it's reasonable to revisit the fixed pair of edges and make sure, that they aren't intersecting anymore with no matter to myTopoMode flag.
3. While fixing the intersecting neighbor edges, it's better always to try to modify the tolerance first before performing any cuts. And if a cut is necessary, do it even if myTopoMode is false.
4. While bending P curves, it's reasonable to perform TryNewPCurve inside of try..catch block, since it can throw the exceptions, which indicate, that a try wasn't successful.
5. Some code style changes are also proposed (tabs are exchanged with spaces).
Not required.
I'll be able later to provide a face, which is affected by these modifications.
No tags attached.
related to 0026671closed kgv Infinite loop in ShapeFix_Wire::FixSelfIntersection() 
? 25013face.stp (8,620) 2014-06-17 12:22
https://tracker.dev.opencascade.org/
? 25013faces.stp (117,263) 2014-06-27 19:09
https://tracker.dev.opencascade.org/
png 25013_step_1_B6.png (114,594) 2014-07-15 15:50
https://tracker.dev.opencascade.org/
Issue History
2014-06-16 20:01drazmyslovichNew Issue
2014-06-16 20:01drazmyslovichAssigned To => drazmyslovich
2014-06-16 20:04drazmyslovichNote Added: 0029791
2014-06-16 20:04drazmyslovichStatusnew => resolved
2014-06-17 12:22drazmyslovichFile Added: 25013face.stp
2014-06-17 14:57gkaNote Added: 0029797
2014-06-17 15:05drazmyslovichNote Added: 0029798
2014-06-18 18:21gkaNote Added: 0029821
2014-06-23 10:17gkaNote Edited: 0029821bug_revision_view_page.php?bugnote_id=29821#r7648
2014-06-23 13:18drazmyslovichNote Added: 0029856
2014-06-23 13:33drazmyslovichStatusresolved => assigned
2014-06-27 15:34drazmyslovichNote Added: 0029902
2014-06-27 19:09drazmyslovichFile Added: 25013faces.stp
2014-06-27 19:16drazmyslovichNote Added: 0029916
2014-06-27 19:16drazmyslovichAssigned Todrazmyslovich => gka
2014-06-27 19:16drazmyslovichStatusassigned => resolved
2014-06-30 10:41gkaNote Added: 0029919
2014-06-30 10:41gkaStatusresolved => reviewed
2014-06-30 11:22mkvAssigned Togka => mkv
2014-06-30 13:49mkvNote Added: 0029921
2014-06-30 13:49mkvAssigned Tomkv => gka
2014-06-30 13:49mkvStatusreviewed => feedback
2014-07-01 15:13gkaAssigned Togka => ika
2014-07-01 15:13gkaNote Added: 0029935
2014-07-01 15:14gkaStatusfeedback => assigned
2014-07-01 17:07ikaNote Added: 0029937
2014-07-01 17:07ikaAssigned Toika => gka
2014-07-01 17:07ikaStatusassigned => resolved
2014-07-02 16:18gkaNote Added: 0029943
2014-07-02 16:18gkaStatusresolved => reviewed
2014-07-02 16:21mkvAssigned Togka => mkv
2014-07-03 12:13mkvNote Added: 0029950
2014-07-03 12:14mkvAssigned Tomkv => gka
2014-07-03 12:14mkvStatusreviewed => assigned
2014-07-08 10:58gkaAssigned Togka => ika
2014-07-08 11:00gkaNote Added: 0029995
2014-07-15 15:46ikaNote Added: 0030168
2014-07-15 15:46ikaAssigned Toika => drazmyslovich
2014-07-15 15:46ikaStatusassigned => feedback
2014-07-15 15:50ikaFile Added: 25013_step_1_B6.png
2014-08-06 12:51gitNote Added: 0030603
2014-08-06 12:57drazmyslovichNote Added: 0030604
2014-08-06 12:58drazmyslovichAssigned Todrazmyslovich => ika
2014-08-06 12:58drazmyslovichStatusfeedback => resolved
2014-09-02 19:11gitNote Added: 0031286
2014-09-02 19:13ikaNote Added: 0031287
2014-09-02 19:13ikaStatusresolved => assigned
2014-09-02 19:16ikaNote Added: 0031288
2014-09-02 19:16ikaAssigned Toika => gka
2014-09-02 19:16ikaStatusassigned => resolved
2014-09-03 12:23gkaNote Added: 0031301
2014-09-03 12:23gkaStatusresolved => reviewed
2014-09-03 16:02mkvAssigned Togka => mkv
2014-09-04 16:56mkvNote Added: 0031384
2014-09-04 16:56gitNote Added: 0031385
2014-09-04 16:57mkvTest case number => bugs heal(017) bug25013_1, bug25013_2
2014-09-04 16:57mkvAssigned Tomkv => ika
2014-09-04 16:57mkvStatusreviewed => assigned
2014-09-11 17:48abvTarget Version => 7.1.0
2015-03-02 17:33gitNote Added: 0038013
2015-03-02 17:43ikaNote Added: 0038014
2015-03-02 17:43ikaAssigned Toika => gka
2015-03-02 17:43ikaStatusassigned => resolved
2015-03-02 17:43ikaSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=9500#r9500
2015-03-02 17:53gkaNote Added: 0038015
2015-03-02 17:53gkaAssigned Togka => bugmaster
2015-03-02 17:53gkaStatusresolved => reviewed
2015-03-02 18:33mkvAssigned Tobugmaster => mkv
2015-03-04 14:07mkvNote Added: 0038076
2015-03-04 14:08mkvNote Added: 0038077
2015-03-04 14:08mkvAssigned Tomkv => ika
2015-03-04 14:08mkvStatusreviewed => assigned
2015-03-04 15:21ikaNote Added: 0038081
2015-03-04 15:21ikaAssigned Toika => mkv
2015-03-04 15:21ikaStatusassigned => feedback
2015-03-05 12:43gitNote Added: 0038133
2015-03-05 12:44mkvNote Added: 0038134
2015-03-05 12:44mkvAssigned Tomkv => bugmaster
2015-03-05 12:44mkvStatusfeedback => tested
2015-03-06 15:13bugmasterChangeset attached => occt master 3163e9fd
2015-03-06 15:13bugmasterStatustested => verified
2015-03-06 15:13bugmasterResolutionopen => fixed
2015-03-10 12:59bugmasterTarget Version7.1.0 => 6.9.0
2015-03-18 13:32gitNote Added: 0038493
2015-03-18 13:32gitNote Added: 0038494
2015-03-18 13:32gitNote Added: 0038495
2015-03-18 13:32gitNote Added: 0038496
2015-03-18 13:32gitNote Added: 0038497
2015-05-14 15:29aivStatusverified => closed
2015-05-14 15:32aivFixed in Version => 6.9.0
2015-09-18 18:30Roman LyginRelationship addedrelated to 0026671

Notes
(0029791)
drazmyslovich   
2014-06-16 20:04   
The tweaks are committed. Please, review them
(0029797)
gka   
2014-06-17 14:57   
It seems that to me that check that new generated P curves going over singularities can be fixed by modifying the tolerance on vertices made following way:

        Standard_Real dist1 = v1.Distance(s);
        Standard_Real dist2 = v2.Distance(s);
        if (dist1 * 4. < dist2 && tol1 * 1.1 > dist1) // the split is within tolerance
        {
          B.UpdateVertex( V1, s, Max(tol1, dist1) );
          a = seq(1);
          isToleranceFixed = Standard_True;
        }

        if (dist2 * 4. < dist1 && tol2 * 1.1 > dist2) // the split is within tolerance
        {
          B.UpdateVertex( V2, s, Max(tol2, dist2) );
          b = seq(1);
          isToleranceFixed = Standard_True;
        }
is not completely clear.
It seems that split of edge in point of singularity or cut pcurve in the point of singularity is more preferable .
Could you please provide to us example for this case.
(0029798)
drazmyslovich   
2014-06-17 15:05   
The attached face corresponds to this fix
(0029821)
gka   
2014-06-18 18:21   
(edited on: 2014-06-23 10:17)
Dear Dmitry.
I checked this code on attached face and actually this part of code works for this example successfully. But I would like to note that in this code vertices and edges belonging initial shape are modified it is not good practice. In order to avoid it it is necessary to create new edge with new vertex and then replace old edge and vertex on new ones in Context(). Please see on code used for split:
TopoDS_Edge edge = sbe.CopyReplaceVertices ( E, V1, V );
Context()->Replace ( E,edge);
Besides seems that as criteria to increase tolerance can be used for example value of tolerance written in file.

(0029856)
drazmyslovich   
2014-06-23 13:18   
Dear G. (sorry, I don't know your name, only the first letter),
I'll try to search in my models database for the corrupted face, which can be fixed with this peace of code and also I'll promote the proposed change with Context.
Thanks!
(0029902)
drazmyslovich   
2014-06-27 15:34   
Dear G.,
I've checked once again the models. The attached face was fixing with the provided modification of tolerances at 6.6.0 base, but the current code base includes some changes, which fix the curves before ShapeFix... Actually, I would like to know, what code fixes it, do you have any idea?
But I found another model, which fails at the current code base and can be fixed with the tolerances adjustment. I will extract the face and attach later.
By the way, I found out, that this tolerances adjustment fix doesn't work really well for the current code base, so I'll through it away and commit the rest of modifications in the new branch.
(0029916)
drazmyslovich   
2014-06-27 19:16   
The modifications are committed to branch CR25013_2. Additionally I've attached several faces, which represent an issue, when the committed tweaks can fix the corrupt faces.
The modification related to changing the tolerance for fixing P curves going over singularities is obsolete, I can't provide any model, which represents the problem.
(0029919)
gka   
2014-06-30 10:41   
Branch CR25013_2 is redy to be tested
(0029921)
mkv   
2014-06-30 13:49   
Dear gka,
could you please rebase branch CR25013_2, there are conflict files.
(0029935)
gka   
2014-07-01 15:13   
Dear ika.

Could you please to rebase branch CR25013_2 on current master
(0029937)
ika   
2014-07-01 17:07   
Dear GKA,

New branch CR25013_3 with modified ShapeFix_Wire.cxx (from CR25013_2) was created on current master because of too many strange conflicts with other files, which weren't modified in this issue.

Please review.
(0029943)
gka   
2014-07-02 16:18   
Branch CR25013_3 is ready to be tested
(0029950)
mkv   
2014-07-03 12:13   
Dear BugMaster,

Branch CR25013_3 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 0315817b8af5f309af5a4fa9fe859dcbec57eb03

Number of compiler warnings:

occt component :
Linux: 16 (16 on master)
Windows: 0 (0 on master)
MacOS: 200 (200 on master)

products component :
Linux: 11 (11 on master)
Windows: 2 (2 on master)

Regressions/Differences:
http://occt-tests/CR25013-3-master-occt/Debian60-64/summary.html [^]
http://occt-tests/CR25013-3-master-occt/Windows-32-VC9/summary.html [^]

Testing cases:


Testing on Linux:
Total MEMORY difference: 327867032 / 328248344
Total CPU difference: 46716.26999999995 / 45073.6100000001

Testing on Windows:
Total MEMORY difference: 360652180 / 361141392
Total CPU difference: 33465.0 / 35845.953125

There are no differences in images found by testdiff.
(0029995)
gka   
2014-07-08 11:00   
Dear ika.

Could you please analyze differences obtained after tests.
(0030168)
ika   
2014-07-15 15:46   
Dear Dmitry,

There are a problem with myTopoMode flag. Some edges are shared, so its cutting or removing will affect the adjacent faces. myTopoMode protected against this situation, but now a lot of regressions appear:

step_1 B6, V8, X3, Y5
step_2 A6, H1, S9, V6
step_3 C9
step_4 A1, A4, E1, H3, H4, I1
step_5 A5, B2

Picture with invalid face from step_1 B6 for example is attached.
(0030603)
git   
2014-08-06 12:51   
Branch CR25013_4 has been created by drazmyslovich.

SHA-1: 01c1b38b353d2b32c0d39ff2542fdfc7e39cec71


This branch includes the following new commits:

       new 40841e8 0025013: ShapeFix_Wire tweaks for better results rebase CR25013_2 onto current master
       new 01c1b38 0025013: Rebase on the current master and reenable myTopoFlag check before cutting the intersecting edges


Detailed log of new commits:

commit 01c1b38b353d2b32c0d39ff2542fdfc7e39cec71
Author: razmyslovich
Date: Wed Aug 6 10:51:04 2014 +0200

    0025013: Rebase on the current master and reenable myTopoFlag check before cutting the intersecting edges

commit 40841e8a201b1297e45f677bd2fea51c45a55c45
Author: ika
Date: Tue Jul 1 17:01:06 2014 +0400

    0025013: ShapeFix_Wire tweaks for better results
    rebase CR25013_2 onto current master
(0030604)
drazmyslovich   
2014-08-06 12:57   
Dear Irina,

Thank you very much for doing this long lasting tests and analyzing each case. I've reenabled the usage of myTopoMode flag in case of cutting the intersecting edges. For sure I've tested the new version of my tweaks on my models database - without myTopoMode flag the effect isn't that reasonable, just 2-3 faces are meshed properly in comparison to the original code.
Anyway, could you please review and test the latest branch CR25013_4.
Thank you very much in advance!

Regards,
Dima
(0031286)
git   
2014-09-02 19:11   
Branch CR25013_5 has been created by ika.

SHA-1: cb23b4f3f30c43da170c6d580f970830d262bec6


Detailed log of new commits:

Author: ika
Date: Tue Sep 2 19:10:59 2014 +0400

    0025013: ShapeFix_Wire tweaks for better results
    Recalculate of tolerance before edge cutting

Author: ika
Date: Tue Jul 1 17:01:06 2014 +0400

    0025013: ShapeFix_Wire tweaks for better results
    rebase CR25013_2 onto current master
(0031287)
ika   
2014-09-02 19:13   
Dear Dmitry,
There are some regressions on our test base related to your last fix. The cause is in tolerance.
On master tolerance which is used to cut edges (if myTopoMode == 1) is calculated like a distance from common vertex to intersection point with some corrections:
newtol = 1.0001 * ( pnt.Distance ( pint ) + rad );
If myTopoMode == 0, only tolerance of edges and vertices will increased, but the tolerance is calculated by other formula:
Standard_Real te1 = rad + ComputeLocalDeviation (E1, pint, pnt,
        param1, ( isForward1 ? b1 : a1 ), Face() );
Standard_Real te2 = rad + ComputeLocalDeviation (E2, pint, pnt,
        ( isForward2 ? a2 : b2 ), param2, Face() );
Standard_Real maxte = Max ( te1, te2 );
...
newtol = 1.000001 * maxte;

And after your last fix edge may be cut (case 1) with tolerance from case 2. This behavior leads to some mistakes, so I tried to add a small correction in your code(recalculate of tolerance before edge cutting).
(0031288)
ika   
2014-09-02 19:16   
Dear GKA,

There were some troubles with rebasing of CR25013_4, so Dmitry's last fix (reenable the usage of myTopoMode flag) and my corrections (recalculate of tolerance before edge cutting) were combined in one commit and pushed to CR25013_5.
Could you please review it?
(0031301)
gka   
2014-09-03 12:23   
Branch CR25013_5 is ready to be tested
(0031384)
mkv   
2014-09-04 16:56   
Dear BugMaster,
Branch CR25013_5 from occt git-repository (and master from products git-repository) was compiled on Linux and Windows platforms and tested.
SHA-1: cb23b4f3f30c43da170c6d580f970830d262bec6

Number of compiler warnings:

occt component :
Linux: 15 (15 on master)
Windows: 0 (0 on master)

products component :
Linux: 11 (11 on master)
Windows: 1 (1 on master)

Regressions/Differences:
http://occt-tests/CR25013-5-master-occt/Debian60-64/summary.html [^]
http://occt-tests/CR25013-5-master-occt/Windows-32-VC10/summary.html [^]
de iges_1(001) J3, O5, P5
de iges_2(002) A1, A4, A8, B1, C2, D9, E3, G5, G7, H9, I9
de iges_3(003) A4
de step_3(006) B9, E6

Testing cases:
http://occt-tests/CR25013-5-master-occt/Debian60-64/bugs/heal/bug25013_1.html [^]
http://occt-tests/CR25013-5-master-occt/Windows-32-VC10/bugs/heal/bug25013_1.html [^]
bugs heal(017) bug25013_1: OK

http://occt-tests/CR25013-5-master-occt/Debian60-64/bugs/heal/bug25013_2.html [^]
http://occt-tests/CR25013-5-master-occt/Windows-32-VC10/bugs/heal/bug25013_2.html [^]
bugs heal(017) bug25013_2: FAILED

Testing on Linux:
occt component :
Total MEMORY difference: 350401012 / 350335460
Total CPU difference: 44415.320000000094 / 43945.12999999999
products component :
Total MEMORY difference: 109150472 / 109158944
Total CPU difference: 19412.009999999947 / 17204.64000000002

Testing on Windows:
occt component :
Total MEMORY difference: 238818000 / 239096080
Total CPU difference: 40484.984375 / 32371.46875
products component :
Total MEMORY difference: 66441644 / 66443580
Total CPU difference: 13869.8125 / 10981.609375

There are no differences in images found by testdiff.
(0031385)
git   
2014-09-04 16:56   
Branch CR25013_5 has been updated by mkv.

SHA-1: 76772371f5f81ec8eba456052ac7cb4f30364c78


Detailed log of new commits:

Author: mkv
Date: Thu Sep 4 16:56:12 2014 +0400

    Test case for issue CR25013

(0038013)
git   
2015-03-02 17:33   
Branch CR25013_5 has been updated forcibly by ika.

SHA-1: f2b7f9b69b73ddf25ea00157cdd99784fc9ddf1c
(0038014)
ika   
2015-03-02 17:43   
Dear GKA,

failed test case was analyzed and try to increase tolerance before splitting in singularity during PCurve adding was added.

Could you please review branch CR25013_5?
(0038015)
gka   
2015-03-02 17:53   
Branch CR25013_5 is ready to be tested
(0038076)
mkv   
2015-03-04 14:07   
Dear BugMaster,
Branch CR25013_5 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: f2b7f9b69b73ddf25ea00157cdd99784fc9ddf1c

Number of compiler warnings:

occt component :
Linux: 18 (18 on master)
Windows: 2 (2 on master)

products component :
Linux: 11 (11 on master)
Windows: 4 (4 on master)

Regressions/Differences:
http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Debian60-64/summary.html [^]
http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Windows-64-VC10/summary.html [^]
de iges_1(001) J3, O5
de iges_2(002) A1, A4, A7, A8, B1, D9, E3, G5, G7, H9, I9
de iges_3(003) A4
de step_1(004) J6
de step_3(006) B9, E6

Testing cases:
http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Debian60-64/bugs/heal/bug25013_1.html [^]
http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Windows-64-VC10/bugs/heal/bug25013_1.html [^]
bugs heal(017) bug25013_1: OK

http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Debian60-64/bugs/heal/bug25013_2.html [^]
http://occt-tests.nnov.opencascade.com/CR25013-5-master-occt-64/Windows-64-VC10/bugs/heal/bug25013_2.html [^]
bugs heal(017) bug25013_2: OK

Testing on Linux:
occt component :
Total MEMORY difference: 90274150 / 90178258
Total CPU difference: 56633.63000000024 / 58436.729999999996
products component :
Total MEMORY difference: 23636914 / 23641693
Total CPU difference: 15911.799999999988 / 16686.65999999998

Testing on Windows:
occt component :
Total MEMORY difference: 56604392 / 56613641
Total CPU difference: 36209.765625 / 38285.671875
products component :
Total MEMORY difference: 16197321 / 16203200
Total CPU difference: 12482.578125 / 9533.171875

There are no differences in images found by testdiff.
(0038077)
mkv   
2015-03-04 14:08   
Dear ika,
could you please review test cases
bugs heal(017) bug25013_1, bug25013_2
(0038081)
ika   
2015-03-04 15:21   
Dear MKV,
failed test cases are not regressions, but a new correct behavior, so could you please update corresponding scripts?

Test cases bugs heal(017) bug25013_1, bug25013_2 are OK.
(0038133)
git   
2015-03-05 12:43   
Branch CR25013_5 has been updated by mkv.

SHA-1: cbe0416ca12de9513556116a6082a1a9d4bd5675


Detailed log of new commits:

Author: mkv
Date: Thu Mar 5 12:43:12 2015 +0300

    Correction of test cases for issue CR25013

(0038134)
mkv   
2015-03-05 12:44   
Branch CR25013_5 is TESTED.
(0038493)
git   
2015-03-18 13:32   
Branch CR25013 has been deleted by inv.

SHA-1: 9fe67960de074dddd7e92d3d4bfc7830acd3119f
(0038494)
git   
2015-03-18 13:32   
Branch CR25013_2 has been deleted by inv.

SHA-1: 88151ffe90a42106bd5f405fc39c6e059868b1c3
(0038495)
git   
2015-03-18 13:32   
Branch CR25013_3 has been deleted by inv.

SHA-1: 0315817b8af5f309af5a4fa9fe859dcbec57eb03
(0038496)
git   
2015-03-18 13:32   
Branch CR25013_4 has been deleted by inv.

SHA-1: 01c1b38b353d2b32c0d39ff2542fdfc7e39cec71
(0038497)
git   
2015-03-18 13:32   
Branch CR25013_5 has been deleted by inv.

SHA-1: cbe0416ca12de9513556116a6082a1a9d4bd5675