View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0032261CommunityOCCT:Meshpublic2021-03-29 14:542021-08-23 22:59
Reporterdrazmyslovich Assigned Tobugmaster  
PrioritynormalSeveritytrivial 
Status closedResolutionfixed 
Product Version7.6.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0032261: Mesh - some trivial improvements for mesher
DescriptionHi,

in course of investigating diverse meshing problems with different models I have produced several trivial code improvements in the context of meshing algorithm, which I would like to share here.

The improvements are the following:
1. BRepMesh_Delaun::isVertexInsidePolygon aTotalAng can be equal to -2 * M_PI, which still means 'inside'
2. BRepMesh_Delaun::decomposeSimplePolygon in case of anUsedLinkId == 3 the function always splits the polygon to one polygon with 2 links and the rest, which leads to additional memory allocations, while at the next iteration the first polygon is directly dropped
3. BRepMesh_FaceChecker::perform BndBox2dTreeSelector works in the parametric space, while the tolerance is provided in 3D space units
4. BRepMesh_GeomTool::IntSegSeg misses one case, when the result is Glued
5. BRepMesh_ModelHealer::amplifyEdges use a single instance of EdgeAmplifier
6. CSLib_Class2d even thought the implementation of NCollection_Handle::get is quite trivial, I saw it several times in the profiler taking some 5% of meshing time (maybe this was just due to debug mode and some address sanitizing - I am not sure)
7. GCPnts_TangentialDeflection::PerformCurve consider near points not only by parameter value, but also by 3D location

Please, let me know, if I should split the improvements into different commits/tickets.

Regards,
Dima
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberNot required

Relationships

related to 0032265 closedbugmaster Community Modeling Algorithms - some trivial code improvements in CSLib_Class2d and GCPnts_TangentialDeflection 
related to 0032395 assignedoan Open CASCADE Mesh - possible mistake in BRepMesh_Delaun::isVertexInsidePolygon 

Activities

git

2021-03-29 14:55

administrator   ~0099817

Branch CR0032261 has been created by drazmyslovich.

SHA-1: dd519df32b999a83842cb293331069529d6bf959


Detailed log of new commits:

Author: Dzmitry Razmyslovich
Date: Mon Mar 29 13:54:42 2021 +0200

    0032261: Some trivial improvements for mesher

oan

2021-03-29 16:07

developer   ~0099819

Hello Dima,

Yes, it would be nice if you split it to separate issues (branches):
- issues related to particular use case in BRepMesh, in Mesh category;
- CSLib_Class2d and GCPnts_TangentialDeflection should be registered in context of Modeling Algorithms.

This will help us to run tests for each particular issue, measure results and elaborate solutions if some of the patches would be the cause of unexpected regressions.

This issue might be used as the reference one to accumulate status of all subsequent tasks.

TIA,
Oleg.

oan

2021-03-29 16:39

developer   ~0099821

By the way, could you please keep OCCT coding convention regarding names of variables, e.g. in CSLib_Class2d, BRepMesh_ModelHealer, etc.

kgv

2021-03-29 16:59

developer   ~0099822 

+    auto & pnts2dX = *MyPnts2dX;

Tip: usage of "auto" is discouraged in OCCT source code.

git

2021-03-30 08:19

administrator   ~0099838

Branch CR0032261 has been updated forcibly by drazmyslovich.

SHA-1: 5bee4a8d3c1783622d3a799263f9dc88ddc6236e

drazmyslovich

2021-03-30 08:31

developer   ~0099840

Hello, Oleg,

I separated the changes for CSLib_Class2d and GCPnts_TangentialDeflection into 0032265. And I checked all variable names used in the code I committed.

Please, let me know if I can somehow further improve the code.
Thank you.

Regards,
Dima

oan

2021-04-01 19:27

developer   ~0099969

Is it possible to provide any use cases these fixes are intended for?

drazmyslovich

2021-04-06 11:24

developer   ~0100121

Only for the 3rd point BRepMesh_FaceChecker::perform I can try and produce some use cases.

oan

2021-04-06 11:55

developer   ~0100127

OK, it would be great anyway!

git

2021-04-13 09:00

administrator   ~0100270

Branch CR0032261 has been updated forcibly by drazmyslovich.

SHA-1: 94d1b1f412ed2d1f9c61b30c7af04d34a6035e0b

git

2021-04-13 09:01

administrator   ~0100271

Branch CR0032261 has been updated forcibly by drazmyslovich.

SHA-1: ac5da7d3654939ef797ad0e93bb9b6e61dffd86d

drazmyslovich

2021-04-13 09:05

developer   ~0100272

Hello, Oleg,

unfortunately, after testing the change in BRepMesh_FaceChecker::perform within the whole our models database, I am not able to provide any reliable case, where this change has an influence.

Therefore, I dropped this change from the commit set. Moreover, I have separated all mentioned changes into several commits, so that you can easily check them isolated and drop some if necessary.

I am looking forward for your feedback.

Regards,
Dima

drazmyslovich

2021-04-13 09:06

developer   ~0100273

So, the current branch contains the following changes:
1. BRepMesh_Delaun::isVertexInsidePolygon aTotalAng can be equal to -2 * M_PI, which still means 'inside'
2. BRepMesh_Delaun::decomposeSimplePolygon in case of anUsedLinkId == 3 the function always splits the polygon to one polygon with 2 links and the rest, which leads to additional memory allocations, while at the next iteration the first polygon is directly dropped
3. BRepMesh_GeomTool::IntSegSeg misses one case, when the result is Glued
4. BRepMesh_ModelHealer::amplifyEdges use a single instance of EdgeAmplifier

oan

2021-04-13 14:35

developer   ~0100278

The entire set of the suggested changes leads to several regressions on OCCT's test database:

http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/master-CR0032261-OAN/view/COMPARE/

I will try to apply changes one by one to identify the most stable combination.

git

2021-04-14 20:28

administrator   ~0100301

Branch CR32261 has been created by oan.

SHA-1: 8447810a4210e0c3811eec75a8c5a480a66fe48b


Detailed log of new commits:

Author: Dzmitry Razmyslovich
Date: Wed Apr 14 20:28:29 2021 +0300

    0032261: Mesh - some trivial improvements for mesher
    
    BRepMesh_Delaun::isVertexInsidePolygon aTotalAng can be equal to -2 * M_PI, which still means 'inside'

Author: Dzmitry Razmyslovich
Date: Wed Apr 14 20:27:51 2021 +0300

    0032261: Mesh - some trivial improvements for mesher
    
    BRepMesh_ModelHealer::amplifyEdges use a single instance of EdgeAmplifier

git

2021-04-16 16:00

administrator   ~0100344

Branch CR32261 has been updated forcibly by oan.

SHA-1: 5a6fee3fafce390a663d7cc5591235f567fa2400

git

2021-05-24 12:22

administrator   ~0101319

Branch CR32261 has been deleted by oan.

SHA-1: 5a6fee3fafce390a663d7cc5591235f567fa2400

git

2021-05-24 12:23

administrator   ~0101320

Branch CR0032261 has been updated forcibly by oan.

SHA-1: 4e5bfc43d3d5b76c3f0bfecbfeb2692f9084016b

git

2021-05-24 12:23

administrator   ~0101321

Branch CR32261_1 has been created by oan.

SHA-1: 47582bc2ce13227d8f3371f95d420af41b29f013


No new revisions were added by this update.

git

2021-05-24 12:24

administrator   ~0101322

Branch CR32261_2 has been created by oan.

SHA-1: f6af526a137526f9248a079d19912ca66a7c5a2c


No new revisions were added by this update.

git

2021-05-24 12:24

administrator   ~0101323

Branch CR32261_3 has been created by oan.

SHA-1: 621f7238feaef12e20205025d40c1aa1de06502e


No new revisions were added by this update.

oan

2021-05-24 18:06

developer   ~0101334

Sorry for the delay.

The first three commits in branch CR0032261 seem safe and fine and even provide some improvements upon known invalid shapes (e.g. "mesh standard_shading U7"), see test reports:

CR32261_1: http://jenkins-test-occt/view/master-CR32261_1-OAN/view/COMPARE/
CR32261_2: http://jenkins-test-occt/view/master-CR32261_2-OAN/view/COMPARE/
CR32261_3: http://jenkins-test-occt/view/master-CR32261_3-OAN/view/COMPARE/

However, the latest modification in the branch leads to regression in the form of async edges in case of "mesh advanced_shading A1", degrades the results of previous commits on "mesh standard_shading U7"), and also leads to exception on "bugs xde bug24759", but the exact reason should be investigated closely due to possibility of some side effects:

http://jenkins-test-occt/view/master-CR0032261-OAN/view/COMPARE/

I suggest to integrate first three commits and postpone the last one until the detail investigation.

Mikhail, please check the results and plan further actions, either allocate some person to finalize the remaining issue or postpone it.

msv

2021-05-25 10:18

developer   ~0101345

For the last one, I have created a new bug 0032395. I suspect that the code is wrong there.

git

2021-05-25 17:30

administrator   ~0101357

Branch CR32261 has been created by oan.

SHA-1: a129698384ece01255ec70594b140490544485e3


Detailed log of new commits:

Author: oan
Date: Tue May 25 17:30:49 2021 +0300

    0032261: Mesh - some trivial improvements for mesher
    
    BRepMesh_Delaun::decomposeSimplePolygon(): minor change to improve code readability;
    Correct test case

git

2021-05-25 19:30

administrator   ~0101364

Branch CR32261 has been updated forcibly by oan.

SHA-1: 433940cb7a3d6e5679492da681f6de1c8d255f94

oan

2021-05-26 11:45

developer   ~0101382

Test cases have been corrected.

Updated results are available at:
http://jenkins-test-occt/view/master-CR32261-OAN/view/COMPARE/

git

2021-05-26 16:48

administrator   ~0101396

Branch CR32261_1 has been deleted by oan.

SHA-1: 47582bc2ce13227d8f3371f95d420af41b29f013

git

2021-05-26 16:48

administrator   ~0101397

Branch CR32261_2 has been deleted by oan.

SHA-1: f6af526a137526f9248a079d19912ca66a7c5a2c

git

2021-05-26 16:48

administrator   ~0101398

Branch CR32261_3 has been deleted by oan.

SHA-1: 621f7238feaef12e20205025d40c1aa1de06502e

git

2021-05-26 17:03

administrator   ~0101399

Branch CR32261 has been updated forcibly by oan.

SHA-1: ccbd254cadf118d8014904d93f82b2691a725bfc

msv

2021-05-26 18:14

developer   ~0101406

Please raise CR32261 (3 commits, leave them separate) into occt.

bugmaster

2021-05-29 12:02

administrator   ~0101472

Combination -
OCCT branch : IR-2021-05-28
master SHA - 2315a044240803013da63dd2f5209c739ab03727
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-05-28 SHA - 2131ac830b9e3c707d427f2aac9c50dcfa5c74db
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: 17574.5700000004 / 17867.41000000037 [-1.64%]
Products
Total CPU difference: 11533.700000000124 / 11535.680000000108 [-0.02%]
Windows-64-VC14:
OCCT
Total CPU difference: 19393.5625 / 19367.75 [+0.13%]
Products
Total CPU difference: 12891.8125 / 12920.9375 [-0.23%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-08-23 22:59

administrator   ~0103351

Branch CR0032261 has been deleted by kgv.

SHA-1: 4e5bfc43d3d5b76c3f0bfecbfeb2692f9084016b

git

2021-08-23 22:59

administrator   ~0103358

Branch CR32261 has been deleted by kgv.

SHA-1: ccbd254cadf118d8014904d93f82b2691a725bfc

Related Changesets

occt: master ccb3502a

2021-03-29 11:54:42

drazmyslovich


Committer: bugmaster Details Diff		 0032261: Mesh - some trivial improvements for mesher

BRepMesh_GeomTool::IntSegSeg misses one case, when the result is Glued		 Affected Issues
0032261
mod - src/BRepMesh/BRepMesh_GeomTool.cxx Diff File

occt: master 3326f923

2021-04-13 05:58:53

drazmyslovich


Committer: bugmaster Details Diff		 0032261: Mesh - some trivial improvements for mesher

BRepMesh_ModelHealer::amplifyEdges use a single instance of EdgeAmplifier		 Affected Issues
0032261
mod - src/BRepMesh/BRepMesh_ModelHealer.cxx Diff File

occt: master 793d165a

2021-05-26 14:03:42

drazmyslovich


Committer: bugmaster Details Diff		 0032261: Mesh - some trivial improvements for mesher

BRepMesh_Delaun::decomposeSimplePolygon(): minor change to improve code readability;
Correct test case		 Affected Issues
0032261
mod - src/BRepMesh/BRepMesh_Delaun.cxx Diff File
mod - tests/bugs/moddata_2/bug428 Diff File
mod - tests/hlr/poly_hlr/bug27979_1 Diff File
mod - tests/hlr/poly_hlr/bug27979_2 Diff File
mod - tests/hlr/poly_hlr/bug27979_4 Diff File
mod - tests/hlr/poly_hlr/bug27979_6 Diff File
mod - tests/hlr/poly_hlr/C16 Diff File
mod - tests/hlr/poly_hlr/C5 Diff File

Issue History

Date Modified Username Field Change
2021-03-29 14:54 drazmyslovich New Issue
2021-03-29 14:54 drazmyslovich Assigned To => drazmyslovich
2021-03-29 14:55 git Note Added: 0099817
2021-03-29 14:58 drazmyslovich Assigned To drazmyslovich => oan
2021-03-29 14:58 drazmyslovich Status new => resolved
2021-03-29 14:58 drazmyslovich Steps to Reproduce Updated
2021-03-29 16:07 oan Note Added: 0099819
2021-03-29 16:39 oan Note Added: 0099821
2021-03-29 16:59 kgv Note Added: 0099822
2021-03-29 16:59 kgv Summary Some trivial improvements for mesher => Mesh - some trivial improvements for mesher
2021-03-30 08:19 git Note Added: 0099838
2021-03-30 08:28 drazmyslovich Relationship added related to 0032265
2021-03-30 08:31 drazmyslovich Note Added: 0099840
2021-04-01 19:27 oan Note Added: 0099969
2021-04-06 11:24 drazmyslovich Note Added: 0100121
2021-04-06 11:24 drazmyslovich Assigned To oan => drazmyslovich
2021-04-06 11:24 drazmyslovich Status resolved => assigned
2021-04-06 11:55 oan Note Added: 0100127
2021-04-13 09:00 git Note Added: 0100270
2021-04-13 09:01 git Note Added: 0100271
2021-04-13 09:02 drazmyslovich Assigned To drazmyslovich => oan
2021-04-13 09:05 drazmyslovich Note Added: 0100272
2021-04-13 09:06 drazmyslovich Note Added: 0100273
2021-04-13 14:35 oan Note Added: 0100278
2021-04-14 20:28 git Note Added: 0100301
2021-04-16 16:00 git Note Added: 0100344
2021-05-24 12:22 git Note Added: 0101319
2021-05-24 12:23 git Note Added: 0101320
2021-05-24 12:23 git Note Added: 0101321
2021-05-24 12:24 git Note Added: 0101322
2021-05-24 12:24 git Note Added: 0101323
2021-05-24 18:06 oan Assigned To oan => msv
2021-05-24 18:06 oan Note Added: 0101334
2021-05-25 10:17 msv Relationship added related to 0032395
2021-05-25 10:18 msv Note Added: 0101345
2021-05-25 17:30 git Note Added: 0101357
2021-05-25 19:01 msv Assigned To msv => oan
2021-05-25 19:30 git Note Added: 0101364
2021-05-26 11:45 oan Note Added: 0101382
2021-05-26 11:46 oan Assigned To oan => msv
2021-05-26 11:46 oan Status assigned => resolved
2021-05-26 16:48 git Note Added: 0101396
2021-05-26 16:48 git Note Added: 0101397
2021-05-26 16:48 git Note Added: 0101398
2021-05-26 17:03 git Note Added: 0101399
2021-05-26 18:14 msv Note Added: 0101406
2021-05-26 18:14 msv Assigned To msv => bugmaster
2021-05-26 18:14 msv Status resolved => reviewed
2021-05-29 12:02 bugmaster Note Added: 0101472
2021-05-29 12:02 bugmaster Status reviewed => tested
2021-05-29 12:06 bugmaster Test case number => Not required
2021-05-29 12:12 bugmaster Changeset attached => occt master 793d165a
2021-05-29 12:12 bugmaster Changeset attached => occt master 3326f923
2021-05-29 12:12 bugmaster Changeset attached => occt master ccb3502a
2021-05-29 12:12 bugmaster Status tested => verified
2021-05-29 12:12 bugmaster Resolution open => fixed
2021-08-23 22:59 git Note Added: 0103351
2021-08-23 22:59 git Note Added: 0103358