Collapse star reuse - #9590
Open
IasonManolas wants to merge 9 commits into
Open
Conversation
…scan surface_patch_index(v) walks v's whole incident-facet star with no early exit. can_be_collapsed() calls it for both endpoints of every non-boundary edge, and collapse_short_edges()'s initial scan over all finite edges reaches the same vertex once per incident edge, so its star gets walked as many times as it has edges. The scan does not modify the mesh, so the first answer for a vertex stays valid for the rest of it: a cache scoped to the scan replaces the repeated walks with a hash lookup after the first. The cache is opt-in via a pointer defaulting to null, since can_be_collapsed() is also called after individual collapses (when the cache would be stale) to re-evaluate newly-incident edges. Byte-identical: same values, same order, so the same edges enter the bimap in the same sequence.
…ee attempts collapse_edge() calls is_valid_collapse(edge, type, new_pos, c3t3), which walks the star of v0 and/or v1 into a heap vector, runs an orientation predicate per cell, and then runs a ring test over the cells around the edge. On failure with TO_MIDPOINT the whole thing repeats for TO_V0 and again for TO_V1. Further down, the angle and manifold tests build cells_to_insert from those same two stars a fourth time. The ring test depends on neither the collapse type nor the new position, so it decides all three attempts at once: it is hoisted ahead of the orientation cascade, since it is also the cheap one (one cell circulation, no allocation, no orientation predicate). Each star is now collected once, lazily, into a small_vector and handed to every attempt and to cells_to_insert. cells_to_insert is still filled one handle at a time in the original walk order, so its iteration order is unchanged. Byte-identical: same predicates, same order of evaluation, only the redundant walks are removed.
… walk incident_subdomains(v) collected the whole incident-cell star into a heap vector and then read nothing from it but subdomain_index(). Emitting straight to the output iterator as the star is walked removes one heap allocation per call on the hottest star-walk site in the profile (nb_incident_subdomains <- topology_test <- collapse_edge). Identical emission order, so byte-identical.
…ound a vertex nb_incident_subdomains(v, c3t3) > 1 asks for a count when it only needs "is there a second index". The count walks every cell around v to build a set of distinct subdomain indices; the question only needs the walk to reach a second index, which on a vertex lying on a surface happens after a handful of cells rather than the whole star. has_several_incident_subdomains() reuses the TDS's own conflict-mark traversal - the same one TDS_3::incident_cells_3() performs - and stops as soon as a second index appears. Same boolean, so byte-identical by construction. The marks are shared state, so the function must not be called from inside another marking traversal; the collapse guard chain that calls it is not.
janetournois
reviewed
Aug 6, 2026
…nal/collapse_short_edges.h using Geom_traits::Point_3 instead of Point Co-authored-by: Jane Tournois <janetournois@users.noreply.github.com>
…nal/collapse_short_edges.h removed redundant point() Co-authored-by: Jane Tournois <janetournois@users.noreply.github.com>
…nal/collapse_short_edges.h Co-authored-by: Jane Tournois <janetournois@users.noreply.github.com>
…nal/collapse_short_edges.h Co-authored-by: Jane Tournois <janetournois@users.noreply.github.com>
This file contains hidden or 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
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
Profiling the collapse pass found four independent costs, all coming from re-walking or
re-collecting the same handful of cells and vertices around a candidate collapse edge. Four
commits:
can_be_collapsed()callssurface_patch_index(v)for both endpoints of every non-boundary edge, and it walks avertex's whole incident-facet star with no early exit.
collapse_short_edges()'s initialscan over all finite edges reaches the same vertex once per incident edge, so its star gets
walked as many times as it has edges. The scan does not modify the mesh, so a cache scoped to
the scan replaces the repeats with a hash lookup after the first. The cache is opt-in via a
pointer defaulting to null, since
can_be_collapsed()is also called after individualcollapses (when the cache would be stale) to re-evaluate newly-incident edges.
collapse_edge()callsis_valid_collapse(edge, type, new_pos, c3t3), which walks the star ofv0and/orv1intoa heap vector, runs an orientation predicate per cell, and then runs a ring test over the
cells around the edge. On failure with
TO_MIDPOINTthe whole thing repeats forTO_V0andagain for
TO_V1. Further down, the manifold test andCollapseTriangulationbuildcells_to_insertfrom those same two stars a fourth time. The ring test depends on neitherthe collapse type nor the new position, so it is hoisted ahead of the orientation cascade (it
is also the cheap one: one cell circulation, no allocation, no orientation predicate), and
each star is now collected once, lazily, into a
small_vectorand handed to every attempt andto
cells_to_insert.incident_subdomains(v)collectedthe whole incident-cell star into a heap vector and then read nothing from it but
subdomain_index(). Emitting straight to the output iterator as the star is walked removesone heap allocation per call on the hottest star-walk site in the profile
(
nb_incident_subdomains<-topology_test<-collapse_edge).every cell around
vto build a set of distinct subdomain indices, andtopology_test's onlyuse of it is the
> 1comparison. A traversal that stops at the second distinct index -reusing the TDS's own conflict-mark flags for the walk, the same traversal
TDS_3::incident_cells_3()performs - answers the same question after a handful of cellsinstead of the whole star.
Testing
Verdict equivalence. All four changes are pure reorderings, caches, or streaming
equivalents over read-only predicates; none change which collapse is ultimately selected or in
what order. Full test suite (
ctest, all executables intest/Tetrahedral_remeshing) passeswith all four commits applied.
Byte-identical output, all four commits applied to
cgal/main:Performance.
perf stat -e instructions, Linux, Release, sequential, against plaincgal/main.cgal/maininstrRelease Management