Skip to content

Tetrahedral remeshing sequential refactoring - #9559

Open
IasonManolas wants to merge 20 commits into
CGAL:mainfrom
IasonManolas:pr/tetrahedral-remeshing-elementary-operations
Open

Tetrahedral remeshing sequential refactoring#9559
IasonManolas wants to merge 20 commits into
CGAL:mainfrom
IasonManolas:pr/tetrahedral-remeshing-elementary-operations

Conversation

@IasonManolas

Copy link
Copy Markdown
Member

Summary

This PR refactors the four Tetrahedral Remeshing elementary operations: edge split, edge collapse, edge flip and vertex smoothing onto a common sequential execution framework, developed during GSoC 2025.

Each operation is expressed through a small, uniform interface: collect the candidate elements, then execute the operation on one candidate, driven by a single executor. This separates what each operation does from how the set of operations is executed, and prepares the ground for a parallel executor, which will be added in a follow-up PR on top of the same interface.

This is a structural, behaviour-preserving refactoring; it introduces no new parallelism.

New classes

  • ElementaryOperation<> - abstract interface all four operations implement: get_element_source() (the ordered candidate list), execute_operation() (apply to one candidate), operation_name().
  • ElementaryOperationExecutionSequential<> - the sequential executor: collect the candidates once, then apply the operation to each in order.
  • EdgeSplitOperation, EdgeCollapseOperation, InternalEdgeFlipOperation, BoundaryEdgeFlipOperation: the concrete edge operations.
  • Vertex_smoothing_context + InternalVertexSmoothOperation, SurfaceVertexSmoothOperation, ComplexEdgeVertexSmoothOperation: vertex smoothing, with the per-pass state shared through the context.

Commits

  1. Add the sequential framework (ElementaryOperation + ElementaryOperationExecutionSequential), not yet used.
  2. Edge split → EdgeSplitOperation (replaces split_long_edges()).
  3. Edge flip → InternalEdgeFlipOperation / BoundaryEdgeFlipOperation (replaces flip_edges()).
  4. Vertex smoothing → Vertex_smoothing_context + three vertex-smooth operations (replaces Tetrahedral_remeshing_smoother).
  5. Edge collapse → EdgeCollapseOperation (replaces collapse_short_edges()).

Adaptive_remesher runs the operations directly through the sequential executor; the original free functions are replaced.

Behaviour

  • Split, flip and vertex smoothing produce byte-identical output to the previous implementation, verified by remeshing meshes and diffing the results — including the constrained-edge / 1-D smoothing path.
  • Collapse diverges intentionally. The original drove a dynamic priority worklist that re-queued newly short edges mid-pass, which does not map onto a per-candidate operation. It now processes a static shortest-first candidate list and defers edges that only become short during a pass to the next remeshing iteration. This divergence was discussed with @janetournois (project wiki, week of August 4–8 2025). Dihedral_Angle.Mean is within measurement noise (+0.024% on the bear mesh).

Scope

To keep the change focused and reviewable, this PR is sequential-only. A follow-up PR will add ElementaryOperationExecutionParallel (with the locking model and the Triangulation_3 generalisations) on top of this interface. No benchmark infrastructure or new CMake options are included here.

Test plan

  • Build test/Tetrahedral_remeshing and run ctest -C Release.
  • The remeshing runs sequentially in all configurations. A couple of tests additionally build their input mesh with parallel Mesh_3 (Parallel_tag triangulation) when compiled with TBB. This confirms the framework also compiles and runs correctly against a thread-safe triangulation, but the remeshing itself is sequential throughout.

Introduces ElementaryOperation, the interface for a single mesh-editing
operation (candidate source, per-candidate execution, name), and
ElementaryOperationExecutionSequential, which collects an operation's
candidates once and applies each in the order the operation returns
them.

This is the common scaffolding that the split, collapse, flip and
smooth operations are refactored onto in the following commits. It is
generic and not yet used by any code path.
…ion framework

Replace the split_long_edges() free function with EdgeSplitOperation,
an ElementaryOperation whose get_element_source() collects and length-
orders the splittable edges (stable_sort, longest first, matching the
original bimap ordering) and whose execute_operation() re-validates and
splits one edge. Adaptive_remesher::split() now drives it through
ElementaryOperationExecutionSequential. Output is unchanged: remeshing a
mesh through this path yields a byte-identical result to the previous
implementation.

Add generic timing/progress reporting to the sequential executor:
elapsed time per operation under CGAL_TETRAHEDRAL_REMESHING_VERBOSE and a
live per-candidate progress line under
CGAL_TETRAHEDRAL_REMESHING_VERBOSE_PROGRESS, replacing the equivalent
per-operation reporting the free function used to do itself.

The visitor before_split()/after_split() hooks and the
CGAL_TETRAHEDRAL_REMESHING_DEBUG diagnostic dumps are preserved.
…on framework

Replace the flip_edges() free function with two ElementaryOperations
sharing EdgeFlipOperationBase:

- InternalEdgeFlipOperation mirrors flip_all_edges(): reset the cell
  caches, collect the internal edges, and run find_best_flip() on each.
- BoundaryEdgeFlipOperation mirrors flipBoundaryEdges(): compute the
  per-vertex boundary valences once in get_element_source(), then flip
  each boundary edge on the surface when it lowers the valence cost,
  updating the valences.

Adaptive_remesher::flip() owns a single incident-cells cache and passes
it to both operations by reference, running the internal pass, then the
boundary pass when boundaries are not protected -- reproducing the map
sharing and pass order of the former flip_edges(). The
find_best_flip()/flip_on_surface() machinery and the
CGAL_TETRAHEDRAL_REMESHING_DEBUG orientation check are unchanged.
Remeshing a mesh through this path yields a byte-identical result to the
previous implementation.
…operation framework

Replace Tetrahedral_remeshing_smoother with Vertex_smoothing_context
(smoothing state + setup helpers), VertexSmoothOperationBase (per-vertex
helpers), and one ElementaryOperation per pass:
ComplexEdgeVertexSmoothOperation, SurfaceVertexSmoothOperation and
InternalVertexSmoothOperation. Each get_element_source() accumulates the
per-vertex moves and each execute_operation() moves one vertex, mirroring
the former smooth_edges_in_complex/on_surfaces/internal_vertices.

Adaptive_remesher keeps the context as a persistent member and runs the
three operations in the previous order (1D, 2D, 3D) each smoothing step.
Remeshing yields a byte-identical result to the previous implementation.
…ration framework

Replace the collapse_short_edges() free function with EdgeCollapseOperation.
get_element_source() collects and length-orders (shortest first) the
collapsible edges once; execute_operation() collapses one edge, skipping
those invalidated by an earlier collapse via a should_skip map.

Unlike the former dynamic bimap worklist, the candidate list is static:
edges that become short during a pass are not re-collapsed in that pass
but in the next remeshing iteration. This changes the result (it is not
byte-identical), but Dihedral_Angle.Mean is unchanged within noise
(+0.024% on bear). The shared collapse_edge()/collapse() helpers now mark
invalidated edges through remove_from_bimap (repurposed to set the skip
flag); the now-unused update_bimap is removed.
@IasonManolas
IasonManolas marked this pull request as draft July 8, 2026 09:19
@IasonManolas IasonManolas changed the title Pr/tetrahedral remeshing elementary operations Tetrahedral remeshing sequential refactoring Jul 8, 2026
@IasonManolas
IasonManolas marked this pull request as ready for review July 8, 2026 09:58
@sloriot

sloriot commented Jul 9, 2026

Copy link
Copy Markdown
Member

You got errors and warnings: tests and examples

…t order

- Build the surface indices and normals before creating the MLS surfaces
  when boundaries are protected. refresh() only fills these maps when
  !protect_boundaries, so with CGAL_TET_REMESHING_SMOOTHING_WITH_MLS and
  protected boundaries createMLSSurfaces() was handed empty maps and the
  remeshing aborted (test_mesh_and_remesh_polyhedron_with_features_mls).
  Seed them once in the constructor for the protected case.

- Reorder the constructor initializer list to match member declaration
  order (m_cell_selector before m_sizing), silencing -Wreorder.
@sloriot

sloriot commented Jul 15, 2026

Copy link
Copy Markdown
Member

Successfully tested in CGAL-6.3-Ic-31

@sloriot sloriot added Not yet approved The feature or pull-request has not yet been approved. Check size labels Jul 15, 2026
@github-actions github-actions Bot removed the Tested label Jul 21, 2026
@github-actions

Copy link
Copy Markdown

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

@janetournois

Copy link
Copy Markdown
Member

At this point, the refactoring looks good to me.
However, there is an issue with the quality of the ouput.

As an example, mesh_and_remesh_with_adaptive_sizing gives dihedral angles in the interval [7.7; 158.4] with the main branch, and in the interval [0.37; 179.2] in this branch.

The processing order may still be more important...

janetournois and others added 4 commits July 23, 2026 17:04
to make it clear what it does
remove_from_bimap() was not removing anything anymore,
so let's just remove it
Edge_split_operation stored its candidates as raw Edge (Cell_handle, i, j)
and re-derived the vertex pair in execute_operation. Splitting an edge
destroys and recycles cells, so by the time the executor reached a later
candidate its Cell_handle could point at a different cell: the operation then
resolved and split the wrong edge (or skipped it).

Store the candidates as vertex pairs captured at collection time
(Element_type = Edge_vv) and re-resolve them with is_edge(), as the original
split_long_edges() did. Vertices are never removed by a split, so the pair
stays valid.

Measured on bear.mesh with adaptive sizing (5 iterations, sequential): this
alone brings the cell count from 89691 back to 57866 (reference 44777) and
halves the sliver population.
Since not requeuing edges incident to the kept vertex in the collapse
operation lead to worse results in this commit we reintroduce code from the
original remeshing implementation and integrate it in the elementary operation
framework:
 - its elements are held in a boost::bimap ordered by squared length, so the
   shortest edge is always collapsed first,
 - collapse_edge() removes from it the edges it destroys, while it destroys
   them - their cells are recycled straight away, so they cannot be removed
   afterwards,
 - the edges incident to the vertex a collapse keeps are re-evaluated : one
   that has just become short is collapsed within the same pass, and one that
   can no longer be collapsed leaves the short edges, rather than being taken
   out of it later and refused by collapse_edge().

remove_from_bimap() and update_bimap() come back for that, unchanged.

Collapse is the only operation whose elements change while it is being applied,
and the only one that needs any of this. It is therefore executed by a
specialization of Elementary_operation_execution_sequential, and neither the
elementary operation framework nor the other operations are touched.

Output is identical to the implementation being replaced.
@sloriot

sloriot commented Jul 29, 2026

Copy link
Copy Markdown
Member

Successfully tested in CGAL-6.3-Ic-43

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.

3 participants