Improvement: Move CAD management to t8_cad_handle with cmesh ownership#2093
Improvement: Move CAD management to t8_cad_handle with cmesh ownership#2093LenaRadmer wants to merge 7 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2093 +/- ##
==========================================
+ Coverage 77.97% 78.25% +0.27%
==========================================
Files 113 114 +1
Lines 19106 19050 -56
==========================================
+ Hits 14898 14907 +9
+ Misses 4208 4143 -65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/t8_cad/t8_cad_handle.cxx
Outdated
|
|
||
| t8_cad::t8_cad (const TopoDS_Shape cad_shape) | ||
| void | ||
| t8_cad_handle::load_cad_from_shape (const TopoDS_Shape &cad_shape) |
There was a problem hiding this comment.
| t8_cad_handle::load_cad_from_shape (const TopoDS_Shape &cad_shape) | |
| t8_cad_handle::load (const TopoDS_Shape &cad_shape) |
src/t8_cad/t8_cad_handle.cxx
Outdated
|
|
||
| t8_cad::t8_cad (std::string fileprefix) | ||
| void | ||
| t8_cad_handle::load_cad_from_file (const std::string fileprefix) |
There was a problem hiding this comment.
| t8_cad_handle::load_cad_from_file (const std::string fileprefix) | |
| t8_cad_handle::load (const std::string fileprefix) |
src/t8_cad/t8_cad_handle.cxx
Outdated
|
|
||
| t8_cad::t8_cad (std::string fileprefix) | ||
| void | ||
| t8_cad_handle::load (const std::string fileprefix) |
There was a problem hiding this comment.
String view will accept anything, while a std::string only std::strings
| t8_cad_handle::load (const std::string fileprefix) | |
| t8_cad_handle::load (const std::string_view fileprefix) |
src/t8_cad/t8_cad_handle.cxx
Outdated
|
|
||
| t8_cad::t8_cad (const TopoDS_Shape cad_shape) | ||
| void | ||
| t8_cad_handle::load (const TopoDS_Shape &cad_shape) |
There was a problem hiding this comment.
Since we have to save the cad shape anyway, we have to own it at some point. So we either make a copy of it or the user gives it to us. But by taking a reference, the user cannot give it to us directly via std::move or as an rvalue. And if we do not use a reference, we do not need const.
| t8_cad_handle::load (const TopoDS_Shape &cad_shape) | |
| t8_cad_handle::load (TopoDS_Shape cad_shape) |
src/t8_cad/t8_cad_handle.cxx
Outdated
| TopExp::MapShapesAndUniqueAncestors (cad_shape, TopAbs_VERTEX, TopAbs_EDGE, cad_shape_vertex2edge_map); | ||
| TopExp::MapShapesAndUniqueAncestors (cad_shape, TopAbs_EDGE, TopAbs_FACE, cad_shape_edge2face_map); | ||
| TopExp::MapShapesAndUniqueAncestors (cad_shape, TopAbs_VERTEX, TopAbs_FACE, cad_shape_vertex2face_map); | ||
| map (cad_shape); |
There was a problem hiding this comment.
we do not need this anymore and the next function can take it without making a copy.
| map (cad_shape); | |
| map (std::move(cad_shape)); |
src/t8_cad/t8_cad_handle.cxx
Outdated
| if (cad_shape.IsNull ()) { | ||
| SC_ABORTF ("Shape is null. \n"); | ||
| } | ||
| map (cad_shape); |
There was a problem hiding this comment.
| map (cad_shape); | |
| map (std::move(cad_shape)); |
src/t8_cad/t8_cad_handle.cxx
Outdated
| } | ||
|
|
||
| void | ||
| t8_cad_handle::map (const TopoDS_Shape &cad_shape_in) |
There was a problem hiding this comment.
same as above
| t8_cad_handle::map (const TopoDS_Shape &cad_shape_in) | |
| t8_cad_handle::map (TopoDS_Shape cad_shape_in) |
src/t8_cad/t8_cad_handle.cxx
Outdated
| } | ||
|
|
||
| t8_cad::t8_cad () | ||
| t8_cad_handle::t8_cad_handle (std::string fileprefix) |
There was a problem hiding this comment.
| t8_cad_handle::t8_cad_handle (std::string fileprefix) | |
| t8_cad_handle::t8_cad_handle (const std::string_view fileprefix) |
src/t8_cad/t8_cad_handle.cxx
Outdated
| t8_cad_handle::load (fileprefix); | ||
| } | ||
|
|
||
| t8_cad_handle::t8_cad_handle (const TopoDS_Shape cad_shape_in) |
There was a problem hiding this comment.
| t8_cad_handle::t8_cad_handle (const TopoDS_Shape cad_shape_in) | |
| t8_cad_handle::t8_cad_handle (TopoDS_Shape cad_shape_in) |
src/t8_cad/t8_cad_handle.cxx
Outdated
| t8_refcount_init (&rc); | ||
| t8_debugf ("Constructed the cad_handle from shape.\n"); | ||
|
|
||
| t8_cad_handle::load (cad_shape_in); |
There was a problem hiding this comment.
| t8_cad_handle::load (cad_shape_in); | |
| load (std::move(cad_shape_in)); |
src/t8_cad/t8_cad_handle.cxx
Outdated
| cad_shape_vertex2edge_map.Clear (); | ||
| cad_shape_edge2face_map.Clear (); | ||
|
|
||
| cad_shape = cad_shape_in; |
There was a problem hiding this comment.
| cad_shape = cad_shape_in; | |
| cad_shape = std::move(cad_shape_in); |
Describe your changes here:
Changing t8_cad to t8_cad_handle for a more explicit CAD shape management. The cmesh now owns the CAD handle and uses a reference counter to manage it.
Also, the t8_geom prefix was removed for everything related to CAD shape management for cleanup. The mesh reader and geometry implementations were updated to support this new handle system.
All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.All these boxes must be checked by the REVIEWERS before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
script/find_all_source_files.scpto check the indentation of these files.License
doc/(or already has one).