Skip to content
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

Mesh_3 - add surface_only() named parameter #8781

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

janetournois
Copy link
Member

@janetournois janetournois commented Mar 11, 2025

Summary of Changes

Add the option surface_only() to make_mesh_3(), to completely skip the "refine_cells" part of Mesh_3, and cancel perturbation and exudation.
@soesau and I noticed that scanning cells, even in the context of "Surface mesher", takes a lot of useless time.

@soesau you can use this branch for your benchmark

@lrineau do you think surface_only should be a member of Mesher_3 or a parameter of Mesher_3::refine_mesh() (as done here)?

Release Management

to completely skip the "refine_cells" part of Mesh_3
@janetournois janetournois self-assigned this Mar 11, 2025
@janetournois janetournois added this to the 6.1-beta milestone Mar 11, 2025
@sloriot
Copy link
Member

sloriot commented Mar 11, 2025

no diff in the PMP header?

@afabri
Copy link
Member

afabri commented Mar 11, 2025

We could also have a function named make_surface_mesh()

@janetournois
Copy link
Member Author

janetournois commented Mar 11, 2025

no diff in the PMP header?

I think it's not needed here. It's a parameter function like features() or mesh_3_dump()

@sloriot
Copy link
Member

sloriot commented Mar 12, 2025

no diff in the PMP header?

I think it's not needed here. It's a parameter function like features() or mesh_3_dump()

OK I thought that you would need to pass the option in the call to make_mesh_3() here.

@janetournois

This comment was marked as outdated.

This comment was marked as outdated.

@janetournois
Copy link
Member Author

/build:v0

Copy link

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8781/v0/Manual/index.html

@janetournois janetournois requested a review from lrineau March 13, 2025 11:56
@janetournois janetournois added Not yet approved The feature or pull-request has not yet been approved. Small feature and removed TODO labels Mar 13, 2025
@janetournois janetournois added the pre-approved For pre-approved small features. After 15 days the feature will be accepted. label Mar 17, 2025
Comment on lines 253 to 255
* When this option is enabled, the ouput mesh has no complex cells, only complex facets, edges and vertices.
* Full-3D optimization steps such as mesh perturbation and mesh exudation are automatically disabled.
*
Copy link
Member

Choose a reason for hiding this comment

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

"complex cells/facets": sounds strange. The correct wording, used elsewhere is "cells in the 3D complex"

Comment on lines 54 to 58
facet_distance(0.001));

// Mesh generation
C3t3 c3t3 = CGAL::make_mesh_3<C3t3>(domain, criteria, params::no_perturb().no_exude());
C3t3 c3t3 = CGAL::make_mesh_3<C3t3>(domain, criteria, params::surface_only());

Copy link
Member

Choose a reason for hiding this comment

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

Now that there is params::surface_only(), this example and its documentation in the user manual will change drastically: there is no need to use a trick with the constructor of Polyhedral_mesh_domain_with_features_3 from a sequence of polyhedra with only one polyhedron.

Comment on lines +485 to 491
if(surface_only)
{
for(auto cit = r_tr.finite_cells_begin(); cit != r_tr.finite_cells_end(); ++cit)
r_c3t3_.remove_from_complex(cit);
}
else if(!forced_stop())
{
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is a way to use that Boolean surface_only at compile time. Compiling Mesh_3 with parameters::surface_only() should be faster than without, and the resulting code should similar to surface_mesher. Maybe even the type of Vertex and Cell could be simpler with surface_only().

Copy link
Member

Choose a reason for hiding this comment

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

You can make surface_only a bool template parameter to the class Mesh_3_options instead of a data member, but I think it is probably out of the scope of this PR and a dedicated issue should be opened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not yet approved The feature or pull-request has not yet been approved. Pkg::Mesh_3 pre-approved For pre-approved small features. After 15 days the feature will be accepted. Small feature Under Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants