MantisBT - Community
View Issue Details
0025113Community[OCCT] OCCT:Meshpublic2014-07-30 18:322020-09-20 11:17
oan 
bugmaster 
normalminor 
verifiedfixed 
 
[OCCT] 7.5.0 
perf/mesh/bug25113_1,bug25113_2
0025113: Mesh - Progress indication and user break functionality for BRepMesh component
User drazmyslovich reported the issue 0025044 that contains a set of modifications for BRepMesh. One of them relates to progress indication functionality that also gives possibility to break execution process by user request.
test perf mesh bug25113_1
test perf mesh bug25113_2
No tags attached.
related to 0025748verified abv Open CASCADE Foundation Classes - Parallel version of progress indicator 
child of 0025044closed bugmaster Community BRepMesh tweaks 
Issue History
2014-07-30 18:32oanNew Issue
2014-07-30 18:32oanAssigned To => oan
2014-07-30 18:33oanRelationship addedchild of 0025044
2014-08-01 17:15gitNote Added: 0030533
2014-08-01 17:20oanNote Added: 0030537
2014-08-01 17:20oanAssigned Tooan => abv
2014-08-01 17:20oanStatusnew => resolved
2014-08-01 19:00oanNote Edited: 0030537bug_revision_view_page.php?bugnote_id=30537#r7784
2014-08-01 19:03gitNote Added: 0030540
2014-08-04 12:14abvNote Added: 0030554
2014-08-04 12:14abvAssigned Toabv => oan
2014-08-04 12:14abvStatusresolved => assigned
2014-08-05 19:29abvTarget Version => 6.8.0
2014-09-26 17:07gitNote Added: 0032259
2014-10-16 11:10oanTarget Version6.8.0 => 7.1.0
2014-12-04 18:50gitNote Added: 0035030
2014-12-04 18:52oanNote Added: 0035031
2014-12-04 18:52oanAssigned Tooan => abv
2014-12-04 18:52oanStatusassigned => resolved
2014-12-04 18:52oanSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=8743#r8743
2015-01-23 19:27abvRelationship addedrelated to 0025748
2016-10-25 17:39oanTarget Version7.1.0 => 7.2.0
2017-01-30 12:29TimoNote Added: 0063186
2017-07-20 12:43oanTarget Version7.2.0 => 7.3.0
2018-02-25 21:09abvTarget Version7.3.0 => 7.4.0
2018-06-18 18:19abvAssigned Toabv => msv
2018-06-18 19:38oanNote Added: 0076825
2019-08-13 10:56msvNote Added: 0086217
2019-08-13 10:56msvAssigned Tomsv => oan
2019-08-13 10:56msvStatusresolved => assigned
2019-08-13 10:56msvTarget Version7.4.0 => 7.5.0
2019-09-04 12:09kgvSummaryProgress indication and user break functionality for BRepMesh component => Mesh - Progress indication and user break functionality for BRepMesh component
2020-07-27 11:52msvAssigned Tooan => akaftasev
2020-07-27 11:54msvNote Added: 0093370
2020-08-26 14:07gitNote Added: 0093640
2020-08-26 18:02akaftasevAssigned Toakaftasev => msv
2020-08-26 18:02akaftasevStatusassigned => resolved
2020-08-27 11:41msvNote Added: 0093659
2020-08-27 11:42msvAssigned Tomsv => akaftasev
2020-08-27 11:42msvStatusresolved => assigned
2020-08-27 11:43msvNote Added: 0093660
2020-08-27 14:27gitNote Added: 0093667
2020-08-31 13:32gitNote Added: 0093760
2020-09-03 13:04gitNote Added: 0093972
2020-09-04 17:25gitNote Added: 0094020
2020-09-07 07:40gitNote Added: 0094175
2020-09-07 09:38gitNote Added: 0094177
2020-09-10 10:57gitNote Added: 0094358
2020-09-10 16:42akaftasevAssigned Toakaftasev => msv
2020-09-10 16:42akaftasevStatusassigned => resolved
2020-09-10 16:42akaftasevSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23389#r23389
2020-09-10 16:53msvNote Added: 0094388
2020-09-10 17:16gitNote Added: 0094390
2020-09-10 17:17akaftasevNote Added: 0094391
2020-09-10 17:17akaftasevNote Edited: 0094391bug_revision_view_page.php?bugnote_id=94391#r23391
2020-09-10 17:19msvNote Added: 0094392
2020-09-10 17:28msvNote Added: 0094393
2020-09-10 17:32msvNote Added: 0094394
2020-09-10 17:37msvAssigned Tomsv => akaftasev
2020-09-10 17:37msvStatusresolved => assigned
2020-09-11 15:25gitNote Added: 0094464
2020-09-12 15:19akaftasevAssigned Toakaftasev => msv
2020-09-12 15:19akaftasevStatusassigned => resolved
2020-09-14 15:51gitNote Added: 0094742
2020-09-14 15:54msvNote Added: 0094744
2020-09-14 15:54msvAssigned Tomsv => akaftasev
2020-09-14 15:54msvStatusresolved => assigned
2020-09-14 20:27akaftasevAssigned Toakaftasev => msv
2020-09-14 20:27akaftasevStatusassigned => resolved
2020-09-14 21:19msvNote Added: 0094777
2020-09-14 21:20msvNote Added: 0094778
2020-09-14 21:20msvAssigned Tomsv => bugmaster
2020-09-14 21:20msvStatusresolved => reviewed
2020-09-16 14:37kgvNote Added: 0094912
2020-09-16 14:38kgvAssigned Tobugmaster => akaftasev
2020-09-16 14:38kgvStatusreviewed => feedback
2020-09-16 16:37gitNote Added: 0094928
2020-09-16 17:15akaftasevNote Added: 0094932
2020-09-16 17:16akaftasevAssigned Toakaftasev => kgv
2020-09-16 17:16akaftasevStatusfeedback => resolved
2020-09-16 17:48kgvNote Added: 0094934
2020-09-16 17:49kgvAssigned Tokgv => akaftasev
2020-09-16 17:49kgvStatusresolved => assigned
2020-09-16 17:50kgvNote Edited: 0094934bug_revision_view_page.php?bugnote_id=94934#r23504
2020-09-16 20:02gitNote Added: 0094944
2020-09-16 20:02akaftasevAssigned Toakaftasev => kgv
2020-09-16 20:02akaftasevStatusassigned => resolved
2020-09-16 20:55msvNote Added: 0094946
2020-09-16 20:56msvNote Edited: 0094946bug_revision_view_page.php?bugnote_id=94946#r23507
2020-09-16 21:18kgvAssigned Tokgv => akaftasev
2020-09-16 21:18kgvStatusresolved => assigned
2020-09-17 09:19gitNote Added: 0094954
2020-09-17 09:19akaftasevAssigned Toakaftasev => kgv
2020-09-17 09:19akaftasevStatusassigned => resolved
2020-09-17 10:38gitNote Added: 0094955
2020-09-17 10:39gitNote Added: 0094956
2020-09-17 10:43kgvNote Added: 0094957
2020-09-17 10:43kgvAssigned Tokgv => bugmaster
2020-09-17 10:43kgvStatusresolved => reviewed
2020-09-18 15:59msvNote Added: 0095036
2020-09-19 15:53bugmasterTest case number => perf/mesh/bug25113_1,bug25113_2
2020-09-19 17:24bugmasterNote Added: 0095091
2020-09-19 17:24bugmasterStatusreviewed => tested
2020-09-20 10:55bugmasterChangeset attached => occt master ce97cd97
2020-09-20 10:55bugmasterStatustested => verified
2020-09-20 10:55bugmasterResolutionopen => fixed
2020-09-20 11:13gitNote Added: 0095106
2020-09-20 11:13gitNote Added: 0095107
2020-09-20 11:14gitNote Added: 0095137
2020-09-20 11:14gitNote Added: 0095138
2020-09-20 11:14gitNote Added: 0095144
2020-09-20 11:14gitNote Added: 0095146

Notes
(0030533)
git   
2014-08-01 17:15   
Branch CR25113 has been created by oan.

SHA-1: 73464900d9d43ba1167df844117a496f4e9160da
(0030537)
oan   
2014-08-01 17:20   
(edited on: 2014-08-01 19:00)
Branch CR25113 is ready to be reviewed.
Corresponding branch has also be pushed to products repository.

User break/progress indication functionality has been integrated to BRepMesh using code base applied by drazmyslovich.

Draw command incmesh has also been changed in order to provide necessary options for mesh algorithm (angular tolerance, relative flag and new option timeout intended to test user break functionality).

(0030540)
git   
2014-08-01 19:03   
Branch CR25113 has been updated by oan.

SHA-1: c6632be5b2b11f28a3ce302fc1c6e9fb4a86c53c


      from 7346490 0025113: Progress indication and user break functionality for BRepMesh component
       new c6632be small correction of grammar


Detailed log of new commits:

commit c6632be5b2b11f28a3ce302fc1c6e9fb4a86c53c
Author: oan
Date: Fri Aug 1 19:03:47 2014 +0400

    small correction of grammar

(0030554)
abv   
2014-08-04 12:14   
Oleg, I have a number of remarks:

- There are a lot of formatting changes (e.g. in BRepMesh_FastDiscret::Add()), I cannot be sure to identify all functional changes

- In BRepMesh_FastDiscretFace::AddInShape, try {} block lacks OCC_CATCH_SIGNALS in the beginning (it was the same before, but worth fixing anyway)

- Using exceptions to implement user break is rather controversial approach, it may be both dangerous if some resources are not released (or e.g. shape is not consistently updated) and inconvenient for debugging. I suggest you to consider more plain approach the progress indicator was designed for: just implement graceful exit from each function if UserBreak is signaled. Apart of safer exit, it may allow you to preserve model consistency keep at least some computed data.

- I suggest you to try the prototype multi-threaded version of the progress indicator, to be able to use it more naturally in the parallel code of BRepMesh. Note that in this case you would deal with Sentry objects rather than progress indicator handle, passing them in function arguments (i.e. not constructors).

At the end, would not it be logical to apply this change when you have your current effort on refactoring of BRepMesh completed?
(0032259)
git   
2014-09-26 17:07   
Branch CR25113_1 has been created by oan.

SHA-1: fd3897bfa8781a52a35d8a4365cd843cea1c7aed


Detailed log of new commits:

Author: oan
Date: Fri Sep 26 17:05:20 2014 +0400

    0025113: Progress indication and user break functionality for BRepMesh component
(0035030)
git   
2014-12-04 18:50   
Branch CR25113_2 has been created by oan.

SHA-1: 803b0b43c9ad4f234a30fa5952252f07488d0eab


Detailed log of new commits:

Author: oan
Date: Thu Dec 4 18:53:04 2014 +0300

    0025113: Progress indication and user break functionality for BRepMesh component
    
    Draft implementation of multi-thread progress indicator
(0035031)
oan   
2014-12-04 18:52   
Branch CR25113_2 is ready to be reviewed.
(0063186)
Timo   
2017-01-30 12:29   
Support of progress indicator or timeout would be highly appreciated (see https://dev.opencascade.org/index.php?q=node/1185 [^]).
(0076825)
oan   
2018-06-18 19:38   
Current patch is not actual and will be implemented after integration of 0026106 feature waiting for review some time...

Parallel progress indicator is supposed for implementation within separate 0025748 issue and, IMHO, base version using mutexes should be intergrated first.
(0086217)
msv   
2019-08-13 10:56   
Dear Oleg, please take care of the issue after integration of patch for 0025748.
(0093370)
msv   
2020-07-27 11:54   
Andrey, please complete the patch.
(0093640)
git   
2020-08-26 14:07   
Branch CR25113 has been updated forcibly by akaftasev.

SHA-1: fba8a22b7a663cb17c43c78cf30f407c6cc973c0
(0093659)
msv   
2020-08-27 11:41   
src/BRepMesh/BRepMesh_ConstrainedBaseMeshAlgo.hxx
- 55: wrap long line

src/BRepMesh/BRepMesh_DelaunayDeflectionControlMeshAlgo.hxx
- 50, 69, 103: wrap long line
- 102: I think the cycle on triangles split is quick and you need no updating progress in it.

src/BRepMesh/BRepMesh_DelaunayNodeInsertionMeshAlgo.hxx
- 88: wrap long line
- 127: The method AddVertices can run long, so it is needed to pass the range there to check user break inside the method.

src/BRepMesh/BRepMesh_FaceDiscret.cxx
- 53: there is no corresponding delete, so memory leak will occur. Also, progress scope is not thread safe to be used concurrently. Consider creating a separate range for each parallel task in the main thread.

src/BRepMesh/BRepMesh_IncrementalMesh.hxx
- 60: wrap long line

src/IMeshTools/IMeshTools_MeshBuilder.cxx
- 86: why status is not set here?
(0093660)
msv   
2020-08-27 11:43   
Combine to one commit and put the commit message corresponding to workflow rules.
(0093667)
git   
2020-08-27 14:27   
Branch CR25113_3 has been created by akaftasev.

SHA-1: 972d1cedcd82b40c65d9b37eb5cc62189615ec2a


Detailed log of new commits:

Author: emv
Date: Fri Jul 10 14:19:31 2020 +0300

    add indicator to BRepMesh
(0093760)
git   
2020-08-31 13:32   
Branch CR25113_3 has been updated forcibly by akaftasev.

SHA-1: 13c9f37dda5b04c716fa075c1d1c0aa8ceb536f3
(0093972)
git   
2020-09-03 13:04   
Branch CR25113_4 has been created by akaftasev.

SHA-1: 999b684183a3dcd3e382533168686fd21fb2f948


Detailed log of new commits:

Author: akaftasev
Date: Thu Sep 3 13:07:06 2020 +0300

    fix

Author: emv
Date: Fri Jul 10 14:19:31 2020 +0300

    add indicator to BRepMesh
(0094020)
git   
2020-09-04 17:25   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: a66ce11108f48670edafa30169100863be162907
(0094175)
git   
2020-09-07 07:40   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: cda9e4bcca00ededd19936713aa0bb8d56753f46
(0094177)
git   
2020-09-07 09:38   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: ec55044acfc9035424bf85cc5237e8c80644287b
(0094358)
git   
2020-09-10 10:57   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 09c1232f08e676b92bf0575858282b52a4244e15
(0094388)
msv   
2020-09-10 16:53   
Andrey, please revert irrelevant changes in src/XSControl/XSControl_TransferWriter.cxx
(0094390)
git   
2020-09-10 17:16   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: ab826e83da1724f25f983979e785185e680ef3e3
(0094391)
akaftasev   
2020-09-10 17:17   
Results of testing on Jenkins:
http://jenkins-test-12.nnov.opencascade.com/view/CR25113-master-akaftasev/view/COMPARE/ [^]

(0094392)
msv   
2020-09-10 17:19   
src/BRepMesh/BRepMesh_FaceDiscret.hxx
- remove commented code of class ScopeFace.
- 101: wrap line
- 105-107: revert space changes.

src/BRepMesh/BRepMesh_FaceDiscret.cxx
- 80: wrap line

    Message_ProgressScope aPS(theRange, NULL, 1);
    if (!aPS.More())
    {
      aDFace->SetStatus(IMeshData_UserBreak);
      return;
    }
    aMeshingAlgo->Perform(aDFace, myParameters, aPS.Next());

No need to create aPS here.
(0094393)
msv   
2020-09-10 17:28   
tests/perf/mesh/bug25113_2
incmesh is called with the same options as IN bug25113_1, no parallel.
(0094394)
msv   
2020-09-10 17:32   
In products, put the commit message corresponding to workflow rules.
(0094464)
git   
2020-09-11 15:25   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 48d867eab71923f13d2ed5f22ef6897d22259d1a
(0094742)
git   
2020-09-14 15:51   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 92ce9e92cea44a6d9ccb925d8451c196c4872313
(0094744)
msv   
2020-09-14 15:54   
Rebase on master and re-test.
(0094777)
msv   
2020-09-14 21:19   
Test results http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR25113-master-akaftasev/view/COMPARE/ [^]
(0094778)
msv   
2020-09-14 21:20   
For integration:
occt - CR25113_4
products - CR25113_4
(0094912)
kgv   
2020-09-16 14:37   
+    if (!aPS.More())
+    {
+      return;
+    }
 
     if (this->getParameters().ControlSurfaceDeflection &&
-        this->getStructure()->ElementsOfDomain().Extent() > 0)
+        this->getStructure()->ElementsOfDomain().Extent() > 0 &&
+        aPS.More())
+    {
+      optimizeMesh(theMesher, aPS.Next());
+    }
+    else
     {
-      optimizeMesh(theMesher);
+      aPS.Next();
     }

aPS.More() in if() statement looks confusing and redundant - as it is preceded by dedicated aPS.More() check.

-  virtual void postProcessMesh(BRepMesh_Delaun& theMesher) Standard_OVERRIDE
+  virtual void postProcessMesh (BRepMesh_Delaun& theMesher, 
+                                const Message_ProgressRange& theRange = Message_ProgressRange()) 
Standard_OVERRIDE
...
   //! Inserts nodes into mesh.
   Standard_Boolean insertNodes(
     const Handle(IMeshData::ListOfPnt2d)& theNodes,
-    BRepMesh_Delaun&                      theMesher)
+    BRepMesh_Delaun&                      theMesher,
+    const Message_ProgressRange&          theRange = Message_ProgressRange())
...
   Standard_EXPORT virtual Standard_Boolean performInternal (
     const Handle (IMeshData_Model)& theModel,
-    const IMeshTools_Parameters&    theParameters) Standard_OVERRIDE;
+    const IMeshTools_Parameters&    theParameters,
+    const Message_ProgressRange&    theRange = Message_ProgressRange()) Standard_OVERRIDE;

Is there any reason defining a default argument value for internal methods, which are supposed to be always called with non-default parameter?

-  OSD_Parallel::For(0, myModel->FacesNb(), *this, !(myParameters.InParallel && myModel->FacesNb() 
> 1));
...
+  std::vector<FaceScope> aSF;
+  OSD_Parallel::ForEach(aSF.begin(), aSF.end(), *this, !(myParameters.InParallel && myModel->FacesNb() 
> 1));

What is the purpose replacing a straight-forward array loop (optimally mapped to parallelization tools) with iterator loop (requiring proxy iterators and additional overhead)?

aSF is a vector - it can be iterated by index range as before.
Just need to define another structure with operator() instead of *this.
(0094928)
git   
2020-09-16 16:37   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 51fdaeae21593905e2113e102e5840dc04c0cf18
(0094932)
akaftasev   
2020-09-16 17:15   
OSD_Parallel::For() was raplaced to OSD_Parallel::ForEach() cause method process, which is used further is the private method of BRepMesh_FaceDiscret class and we can not use it in operator() in structure
(0094934)
kgv   
2020-09-16 17:48   
(edited on: 2020-09-16 17:50)
> OSD_Parallel::For() was raplaced to OSD_Parallel::ForEach()
> cause method process, which is used further
> is the private method of BRepMesh_FaceDiscret class
> and we can not use it in operator() in structure
This is just a design issue to fix - no reason to workaround the own internal classes hierarchy limitations.

   Standard_EXPORT virtual Standard_Boolean performInternal (
     const Handle (IMeshData_Model)& theModel,
-    const IMeshTools_Parameters&    theParameters) Standard_OVERRIDE;
+    const IMeshTools_Parameters&    theParameters,
+    const Message_ProgressRange&    theRange = Message_ProgressRange()) Standard_OVERRIDE;

There are still internal methods with confusing defaults.
Are these necessary?

(0094944)
git   
2020-09-16 20:02   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 1af280f493b6232c8945decd04d9a66583446ca6
(0094946)
msv   
2020-09-16 20:55   
(edited on: 2020-09-16 20:56)
+  void process(const Standard_Integer theFaceIndex, 
+               const Message_ProgressRange& theRange = Message_ProgressRange()) const;

No need to make default value for argument of private method.

(0094954)
git   
2020-09-17 09:19   
Branch CR25113_4 has been updated forcibly by akaftasev.

SHA-1: 246137ff9121eb3ca154b5064745422a56e56c80
(0094955)
git   
2020-09-17 10:38   
Branch CR25113_4 has been updated by kgv.

SHA-1: 5030cb2c04ba9bdc07668cd7e17a96b36989b635


Detailed log of new commits:

Author: kgv
Date: Thu Sep 17 10:40:34 2020 +0300

    # remarks

(0094956)
git   
2020-09-17 10:39   
Branch CR25113_5 has been created by kgv.

SHA-1: 1fb54e99ab6ad507363ce73eb26a3282e2de772b


Detailed log of new commits:

Author: emv
Date: Fri Jul 10 14:19:31 2020 +0300

    0025113: Mesh - Progress indication and user break functionality for BRepMesh component
    
    Added Progress Indicator to BRep_Mesh
(0094957)
kgv   
2020-09-17 10:43   
Dear bugmaster,

please take updated patch in OCCT branch CR25113_5 into IR.
(0095036)
msv   
2020-09-18 15:59   
For integration:
occt - CR25113_5
products - CR25113_4
(0095091)
bugmaster   
2020-09-19 17:24   
Combination -
OCCT branch : IR-2020-09-18
master SHA - b0b766826118f74b9857a932b8cec8c52a25c492
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-09-18 SHA - a6486d839da1ba1383ef6cc1a1a446a172f494c7
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: 17528.97000000011 / 17378.390000000145 [+0.87%]
Products
Total CPU difference: 12127.32000000009 / 12079.540000000095 [+0.40%]
Windows-64-VC14:
OCCT
Total CPU difference: 18862.703125 / 18898.921875 [-0.19%]
Products
Total CPU difference: 13314.828125 / 13329.21875 [-0.11%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0095106)
git   
2020-09-20 11:13   
Branch CR25113_5 has been deleted by inv.

SHA-1: 1fb54e99ab6ad507363ce73eb26a3282e2de772b
(0095107)
git   
2020-09-20 11:13   
Branch CR25113_4 has been deleted by inv.

SHA-1: 5030cb2c04ba9bdc07668cd7e17a96b36989b635
(0095137)
git   
2020-09-20 11:14   
Branch CR25113_3 has been deleted by inv.

SHA-1: 13c9f37dda5b04c716fa075c1d1c0aa8ceb536f3
(0095138)
git   
2020-09-20 11:14   
Branch CR25113 has been deleted by inv.

SHA-1: fba8a22b7a663cb17c43c78cf30f407c6cc973c0
(0095144)
git   
2020-09-20 11:14   
Branch CR25113_2 has been deleted by inv.

SHA-1: 803b0b43c9ad4f234a30fa5952252f07488d0eab
(0095146)
git   
2020-09-20 11:14   
Branch CR25113_1 has been deleted by inv.

SHA-1: fd3897bfa8781a52a35d8a4365cd843cea1c7aed