Fixes for vertex/cell classes of SMDS_3 and Tetrahedral_remeshing #7610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Give it a cell base instead of re-implementing everything.
- Restore its correct API (as in CGAL 5.5): two templates, a traits, and a cell base. - It is not a model of SimplicialMeshCellBase_3, but of RemeshingCellBase_3 - It should not hardcode an inheritance to Simplicial_cell_base_3, but take it as a template paramter's default value. - Use FT instead of hardcoding 'double'
It needs only to document a traits and a vertex base, not expose index types as these are relevant to the Simplicial_vertex_base_3 class, which is not the systematic base. It is a model of RemeshingVertexBase_3, not MeshVertexBase_3 The base needs to be a model of SimplicialVertexBase_3, not TriangulationCellBase_3 Use the proper Rebind mechanism like the other vertex/cell classes
MaelRL
commented
Jul 20, 2023
If you have the following construct:
class V : public Vb;
class V_base
{
struct Rebind --> V;
}
then you cannot rebind twice. More vicious, if Vb
can rebind twice multiple times (e.g. it's a T3 Vb),
then it'll silently drop V in the stack, and rebind
only up to the rebound Vb!
Rebinding multiple times happens for example in
Triangulation_hierarchy_3 (Delaunay_triangulation_3
with Fast_locate).
|
Note that the issue fixed in 3ad1825 probably affects Mesh_3 vertex/cell bases classes. It is quite unlikely that we would rebind multiple times these vertex/cell base classes, but that ought to be fixed too. |
4 tasks
Formerly known as Simplicial_mesh_cell_base_3, this cell base is more efficient in memory, but cannot inherit a base.
This comment was marked as outdated.
This comment was marked as outdated.
sloriot
added
Tests failing
Batch_1
albert-github
suggested changes
Sep 8, 2023
Tetrahedral_remeshing/include/CGAL/Tetrahedral_remeshing/Remeshing_cell_base_3.h
Outdated
Show resolved
Hide resolved
|
Successfully tested in CGAL-6.0-Ic-61 |
sloriot
added
Tested
and removed
Ready to be tested
Under Testing
Batch_1
|
This pull-request was previously marked with the label |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of Changes
The main point of this PR is to restore the API of
Remeshing_cell_base_3to the initial state (since CGAL 5.1), which was changed with the introduction of SMDS_3 in CGAL 5.6 (#5693).In CGAL 5.5, the class
Remeshing_cell_base_3is templated by a geom traits and a cell base.In CGAL 5.6, the class became templated by only two index types, removing the possibility to derive from custom (TDS) base cell types.
Also:
TODO:
Release Management
SMDS_3,Tetrahedral_remeshing