Skip to content

SMDS_3 - add c3t3.set_triangulation() #8851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 12, 2025

Conversation

janetournois
Copy link
Member

@janetournois janetournois commented Apr 18, 2025

When loading a .mesh file to a Triangulation_3, and then a C3t3, as done in this example leads to a C3t3 for which number_of_facets() and number_of_cells() return 0, because the internal counters have not been updated by c3t3.triangulation() = tr.

It is not clear to the user why these numbers are 0, and the function rescan_after_load_of_triangulation() (which fixes the counters) is not documented.

Summary of Changes

Add the use of rescan_after_load_of_triangulation() in an example.

The other option would be to use it automatically in the functions that load a triangulation.

@lrineau @MaelRL do you have an opinion?

Todo : discuss the API

Add and document a function c3t3.set_triangulation(const Tr& tr) that internally calls rescan_after_load_of_triangulation(), and is easier to use

Release Management

  • Affected package(s): SMDS_3
  • Link to compiled documentation (obligatory for small feature) wrong link name to be changed
  • License and copyright ownership: unchanged

@janetournois janetournois changed the title use rescan_after_load_of_triangulation() in an example use c3t3.rescan_after_load_of_triangulation() in an example Apr 18, 2025
@sloriot
Copy link
Member

sloriot commented Apr 24, 2025

Successfully tested in CGAL-6.1-Ic-136

@sloriot
Copy link
Member

sloriot commented Apr 24, 2025

@janetournois Mergeable?

@janetournois
Copy link
Member Author

The function is not documented, so I would say no.
We need to discuss it!

@janetournois janetournois marked this pull request as ready for review April 25, 2025 07:44
@janetournois janetournois added this to the 6.1-beta milestone Apr 25, 2025
@github-actions github-actions bot removed the Tested label Apr 25, 2025
Copy link

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

@janetournois janetournois force-pushed the SMDS_3-fix_example-jtournois branch from 2c36c5f to d4a335f Compare April 25, 2025 08:39
that internally calls rescan_after_load_of_triangulation()
it avoids calling it explicitly or documenting it,
but fixes the issue of updating the internal metadata of C3t3 automatically
@janetournois janetournois force-pushed the SMDS_3-fix_example-jtournois branch from d4a335f to 9c58f6c Compare April 25, 2025 08:40
@janetournois janetournois changed the title use c3t3.rescan_after_load_of_triangulation() in an example SMDS_3 - add c3t3.set_triangulation() Apr 25, 2025
@lrineau
Copy link
Member

lrineau commented Apr 29, 2025

should not we remove the triangulation() returning a reference to non-const?

@janetournois
Copy link
Member Author

should not we remove the triangulation() returning a reference to non-const?

It is used quite a lot in Tetrahedral_remeshing (maybe also in Mesh_3) so I would be against that change.

@sloriot
Copy link
Member

sloriot commented May 12, 2025

Successfully tested in CGAL-6.1-Ic-151

@sloriot sloriot merged commit c6c9f77 into CGAL:master May 12, 2025
9 checks passed
@sloriot sloriot deleted the SMDS_3-fix_example-jtournois branch May 12, 2025 12:33
Comment on lines 334 to 340
/// returns a reference to the triangulation
/// \cgalAdvancedBegin
/// This function should only be used by advanced users: it merely swaps the triangulation without
/// rebuilding critical C3T3 information such as the number of simplexes in complex.
/// On the other hand, this is performed by `set_triangulation()`
/// \cgalAdvancedEnd
Triangulation& triangulation() { return tr_; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand:

it merely swaps the triangulation without...

The function just returns the reference to the non-const triangulation. It does not swap anything. What was the intent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went too fast, it was meant in the context of using the reference to assign a different triangulation, i.e. something like, "If you use this reference to call the change the triangulation, [...]"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants