View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0025013CommunityOCCT:Shape Healingpublic2014-06-16 20:012015-09-18 18:30
Reporterdrazmyslovich Assigned Tobugmaster  
PrioritynormalSeveritytweak 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2010 
Product Version6.6.0 
Target Version6.9.0Fixed in Version6.9.0 
Summary0025013: ShapeFix_Wire tweaks for better results
DescriptionI 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).
Steps To ReproduceNot required.
Additional information
and documentation updates		I'll be able later to provide a face, which is affected by these modifications.
TagsNo tags attached.
Test case numberbugs heal(017) bug25013_1, bug25013_2

Attached Files

  • 25013face.stp (8,620 bytes)
  • 25013faces.stp (117,263 bytes)
  • 25013_step_1_B6.png (114,594 bytes)

Relationships

related to 0026671 closedkgv Infinite loop in ShapeFix_Wire::FixSelfIntersection() 

Activities

drazmyslovich

2014-06-16 20:04

developer   ~0029791

The tweaks are committed. Please, review them

drazmyslovich

2014-06-17 12:22

developer  

25013face.stp (8,620 bytes)

gka

2014-06-17 14:57

developer   ~0029797

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.

drazmyslovich

2014-06-17 15:05

developer   ~0029798

The attached face corresponds to this fix

gka

2014-06-18 18:21

developer   ~0029821

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

drazmyslovich

2014-06-23 13:18

developer   ~0029856

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!

drazmyslovich

2014-06-27 15:34

developer   ~0029902

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.

drazmyslovich

2014-06-27 19:09

developer  

25013faces.stp (117,263 bytes)

drazmyslovich

2014-06-27 19:16

developer   ~0029916

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.

gka

2014-06-30 10:41

developer   ~0029919

Branch CR25013_2 is redy to be tested

mkv

2014-06-30 13:49

tester   ~0029921

Dear gka,
could you please rebase branch CR25013_2, there are conflict files.

gka

2014-07-01 15:13

developer   ~0029935

Dear ika.

Could you please to rebase branch CR25013_2 on current master

ika

2014-07-01 17:07

developer   ~0029937

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.

gka

2014-07-02 16:18

developer   ~0029943

Branch CR25013_3 is ready to be tested

mkv

2014-07-03 12:13

tester   ~0029950

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.

gka

2014-07-08 11:00

developer   ~0029995

Dear ika.

Could you please analyze differences obtained after tests.

ika

2014-07-15 15:46

developer   ~0030168

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.

ika

2014-07-15 15:50

developer  

25013_step_1_B6.png (114,594 bytes)

git

2014-08-06 12:51

administrator   ~0030603

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

drazmyslovich

2014-08-06 12:57

developer   ~0030604

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

git

2014-09-02 19:11

administrator   ~0031286

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

ika

2014-09-02 19:13

developer   ~0031287

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).

ika

2014-09-02 19:16

developer   ~0031288

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?

gka

2014-09-03 12:23

developer   ~0031301

Branch CR25013_5 is ready to be tested

mkv

2014-09-04 16:56

tester   ~0031384

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.

git

2014-09-04 16:56

administrator   ~0031385

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

git

2015-03-02 17:33

administrator   ~0038013

Branch CR25013_5 has been updated forcibly by ika.

SHA-1: f2b7f9b69b73ddf25ea00157cdd99784fc9ddf1c

ika

2015-03-02 17:43

developer   ~0038014

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?

gka

2015-03-02 17:53

developer   ~0038015

Branch CR25013_5 is ready to be tested

mkv

2015-03-04 14:07

tester   ~0038076

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.

mkv

2015-03-04 14:08

tester   ~0038077

Dear ika,
could you please review test cases
bugs heal(017) bug25013_1, bug25013_2

ika

2015-03-04 15:21

developer   ~0038081

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.

git

2015-03-05 12:43

administrator   ~0038133

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

mkv

2015-03-05 12:44

tester   ~0038134

Branch CR25013_5 is TESTED.

git

2015-03-18 13:32

administrator   ~0038493

Branch CR25013 has been deleted by inv.

SHA-1: 9fe67960de074dddd7e92d3d4bfc7830acd3119f

git

2015-03-18 13:32

administrator   ~0038494

Branch CR25013_2 has been deleted by inv.

SHA-1: 88151ffe90a42106bd5f405fc39c6e059868b1c3

git

2015-03-18 13:32

administrator   ~0038495

Branch CR25013_3 has been deleted by inv.

SHA-1: 0315817b8af5f309af5a4fa9fe859dcbec57eb03

git

2015-03-18 13:32

administrator   ~0038496

Branch CR25013_4 has been deleted by inv.

SHA-1: 01c1b38b353d2b32c0d39ff2542fdfc7e39cec71

git

2015-03-18 13:32

administrator   ~0038497

Branch CR25013_5 has been deleted by inv.

SHA-1: cbe0416ca12de9513556116a6082a1a9d4bd5675

Related Changesets

occt: master 3163e9fd

2015-03-05 11:52:06

ika


Committer: bugmaster Details Diff		 0025013: ShapeFix_Wire tweaks for better results

Recalculate of tolerance before edge cutting

Test case for issue CR25013

add try to increase tolerance before splitting in singularity during PCurve adding.

Correction of test cases for issue CR25013		 Affected Issues
0025013
mod - src/ShapeFix/ShapeFix_Wire.cxx Diff File
add - tests/bugs/heal/bug25013_1 Diff File
add - tests/bugs/heal/bug25013_2 Diff File
mod - tests/de/iges_1/J3 Diff File
mod - tests/de/iges_1/O5 Diff File
mod - tests/de/iges_2/A1 Diff File
mod - tests/de/iges_2/A4 Diff File
mod - tests/de/iges_2/A7 Diff File
mod - tests/de/iges_2/A8 Diff File
mod - tests/de/iges_2/B1 Diff File
mod - tests/de/iges_2/D9 Diff File
mod - tests/de/iges_2/E3 Diff File
mod - tests/de/iges_2/G5 Diff File
mod - tests/de/iges_2/G7 Diff File
mod - tests/de/iges_2/H9 Diff File
mod - tests/de/iges_2/I9 Diff File
mod - tests/de/iges_3/A4 Diff File
mod - tests/de/step_1/J6 Diff File
mod - tests/de/step_3/B9 Diff File
mod - tests/de/step_3/E6 Diff File

Issue History

Date Modified Username Field Change
2014-06-16 20:01 drazmyslovich New Issue
2014-06-16 20:01 drazmyslovich Assigned To => drazmyslovich
2014-06-16 20:04 drazmyslovich Note Added: 0029791
2014-06-16 20:04 drazmyslovich Status new => resolved
2014-06-17 12:22 drazmyslovich File Added: 25013face.stp
2014-06-17 14:57 gka Note Added: 0029797
2014-06-17 15:05 drazmyslovich Note Added: 0029798
2014-06-18 18:21 gka Note Added: 0029821
2014-06-23 10:17 gka Note Edited: 0029821
2014-06-23 13:18 drazmyslovich Note Added: 0029856
2014-06-23 13:33 drazmyslovich Status resolved => assigned
2014-06-27 15:34 drazmyslovich Note Added: 0029902
2014-06-27 19:09 drazmyslovich File Added: 25013faces.stp
2014-06-27 19:16 drazmyslovich Note Added: 0029916
2014-06-27 19:16 drazmyslovich Assigned To drazmyslovich => gka
2014-06-27 19:16 drazmyslovich Status assigned => resolved
2014-06-30 10:41 gka Note Added: 0029919
2014-06-30 10:41 gka Status resolved => reviewed
2014-06-30 11:22 mkv Assigned To gka => mkv
2014-06-30 13:49 mkv Note Added: 0029921
2014-06-30 13:49 mkv Assigned To mkv => gka
2014-06-30 13:49 mkv Status reviewed => feedback
2014-07-01 15:13 gka Assigned To gka => ika
2014-07-01 15:13 gka Note Added: 0029935
2014-07-01 15:14 gka Status feedback => assigned
2014-07-01 17:07 ika Note Added: 0029937
2014-07-01 17:07 ika Assigned To ika => gka
2014-07-01 17:07 ika Status assigned => resolved
2014-07-02 16:18 gka Note Added: 0029943
2014-07-02 16:18 gka Status resolved => reviewed
2014-07-02 16:21 mkv Assigned To gka => mkv
2014-07-03 12:13 mkv Note Added: 0029950
2014-07-03 12:14 mkv Assigned To mkv => gka
2014-07-03 12:14 mkv Status reviewed => assigned
2014-07-08 10:58 gka Assigned To gka => ika
2014-07-08 11:00 gka Note Added: 0029995
2014-07-15 15:46 ika Note Added: 0030168
2014-07-15 15:46 ika Assigned To ika => drazmyslovich
2014-07-15 15:46 ika Status assigned => feedback
2014-07-15 15:50 ika File Added: 25013_step_1_B6.png
2014-08-06 12:51 git Note Added: 0030603
2014-08-06 12:57 drazmyslovich Note Added: 0030604
2014-08-06 12:58 drazmyslovich Assigned To drazmyslovich => ika
2014-08-06 12:58 drazmyslovich Status feedback => resolved
2014-09-02 19:11 git Note Added: 0031286
2014-09-02 19:13 ika Note Added: 0031287
2014-09-02 19:13 ika Status resolved => assigned
2014-09-02 19:16 ika Note Added: 0031288
2014-09-02 19:16 ika Assigned To ika => gka
2014-09-02 19:16 ika Status assigned => resolved
2014-09-03 12:23 gka Note Added: 0031301
2014-09-03 12:23 gka Status resolved => reviewed
2014-09-03 16:02 mkv Assigned To gka => mkv
2014-09-04 16:56 mkv Note Added: 0031384
2014-09-04 16:56 git Note Added: 0031385
2014-09-04 16:57 mkv Test case number => bugs heal(017) bug25013_1, bug25013_2
2014-09-04 16:57 mkv Assigned To mkv => ika
2014-09-04 16:57 mkv Status reviewed => assigned
2014-09-11 17:48 abv Target Version => 7.1.0
2015-03-02 17:33 git Note Added: 0038013
2015-03-02 17:43 ika Note Added: 0038014
2015-03-02 17:43 ika Assigned To ika => gka
2015-03-02 17:43 ika Status assigned => resolved
2015-03-02 17:43 ika Steps to Reproduce Updated
2015-03-02 17:53 gka Note Added: 0038015
2015-03-02 17:53 gka Assigned To gka => bugmaster
2015-03-02 17:53 gka Status resolved => reviewed
2015-03-02 18:33 mkv Assigned To bugmaster => mkv
2015-03-04 14:07 mkv Note Added: 0038076
2015-03-04 14:08 mkv Note Added: 0038077
2015-03-04 14:08 mkv Assigned To mkv => ika
2015-03-04 14:08 mkv Status reviewed => assigned
2015-03-04 15:21 ika Note Added: 0038081
2015-03-04 15:21 ika Assigned To ika => mkv
2015-03-04 15:21 ika Status assigned => feedback
2015-03-05 12:43 git Note Added: 0038133
2015-03-05 12:44 mkv Note Added: 0038134
2015-03-05 12:44 mkv Assigned To mkv => bugmaster
2015-03-05 12:44 mkv Status feedback => tested
2015-03-06 15:13 bugmaster Changeset attached => occt master 3163e9fd
2015-03-06 15:13 bugmaster Status tested => verified
2015-03-06 15:13 bugmaster Resolution open => fixed
2015-03-10 12:59 bugmaster Target Version 7.1.0 => 6.9.0
2015-03-18 13:32 git Note Added: 0038493
2015-03-18 13:32 git Note Added: 0038494
2015-03-18 13:32 git Note Added: 0038495
2015-03-18 13:32 git Note Added: 0038496
2015-03-18 13:32 git Note Added: 0038497
2015-05-14 15:29 aiv Status verified => closed
2015-05-14 15:32 aiv Fixed in Version => 6.9.0
2015-09-18 18:30 Roman Lygin Relationship added related to 0026671