From f2cd2d2664e4b1788e722d25923dc1502ffd193b Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:40:00 +0200 Subject: [PATCH 01/12] fix(Degeneration): capture loop variable by value in spawned task The small edges task of each surface captured the loop-local enable_edges flag by reference and read it after the iteration ended (use after scope). Not covered by a deterministic test: only sanitizers can catch it. Co-Authored-By: Claude Opus 5.5 --- .../criterion/internal/component_meshes_degeneration.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp b/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp index a0ecf0bb..39516c3e 100644 --- a/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp @@ -98,7 +98,7 @@ namespace geode const auto enable_edges = absl::c_contains( surfaces_to_enable, surface.id() ); surface_tasks.emplace_back( - async::spawn( [&enable_edges, &threshold, &surface] { + async::spawn( [enable_edges, &threshold, &surface] { if( enable_edges ) { surface.mesh().enable_edges(); From 923d23534ab0c3b76bd5703d40ba616ff81f7099 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:41:21 +0200 Subject: [PATCH 02/12] fix(Colocation): colocated vertices without unique vertex are reported Vertices without unique vertex all compared equal (NO_ID) and were filtered out as if they were the same unique vertex. The filter now keeps one point per unique vertex plus every unlinked point, with one unique vertex lookup per point instead of one per pair. Co-Authored-By: Claude Opus 5.5 --- .../component_meshes_colocation.cpp | 36 +++--- tests/inspection/CMakeLists.txt | 10 ++ .../test-brep-colocation-degeneration.cpp | 116 ++++++++++++++++++ 3 files changed, 140 insertions(+), 22 deletions(-) create mode 100644 tests/inspection/test-brep-colocation-degeneration.cpp diff --git a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp index 549a758a..6ec3245c 100644 --- a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp @@ -23,6 +23,8 @@ #include +#include + #include #include #include @@ -56,33 +58,23 @@ namespace new_colocated_points_groups; for( const auto& point_group : colocated_points_groups ) { - absl::FixedArray< bool > filtered_colocation( - point_group.size() - 1, true ); - for( const auto first_id : geode::Range{ point_group.size() - 1 } ) + // Keep one point per unique vertex; points without unique vertex + // are always kept + absl::flat_hash_set< geode::index_t > unique_vertices; + const auto first_uv = + model.unique_vertex( { component_id, point_group[0] } ); + if( first_uv != geode::NO_ID ) { - if( !filtered_colocation[first_id] ) - { - continue; - } - const auto first_uv = model.unique_vertex( - { component_id, point_group[first_id] } ); - for( const auto second_id : - geode::Range{ first_id + 1, point_group.size() } ) - { - if( first_uv - == model.unique_vertex( - { component_id, point_group[second_id] } ) ) - { - filtered_colocation[second_id - 1] = false; - } - } + unique_vertices.insert( first_uv ); } std::vector< geode::index_t > colocated_points; - for( const auto pt_id : geode::Indices{ filtered_colocation } ) + for( const auto pt_id : geode::Range{ 1, point_group.size() } ) { - if( filtered_colocation[pt_id] ) + const auto uv = + model.unique_vertex( { component_id, point_group[pt_id] } ); + if( uv == geode::NO_ID || unique_vertices.insert( uv ).second ) { - colocated_points.push_back( point_group[pt_id + 1] ); + colocated_points.push_back( point_group[pt_id] ); } } if( !colocated_points.empty() ) diff --git a/tests/inspection/CMakeLists.txt b/tests/inspection/CMakeLists.txt index f51c1a42..dd0eb3b8 100755 --- a/tests/inspection/CMakeLists.txt +++ b/tests/inspection/CMakeLists.txt @@ -62,6 +62,16 @@ add_geode_test( ${PROJECT_NAME}::inspection ) +add_geode_test( + SOURCE "test-brep-colocation-degeneration.cpp" + DEPENDENCIES + OpenGeode::basic + OpenGeode::geometry + OpenGeode::mesh + OpenGeode::model + ${PROJECT_NAME}::inspection +) + add_geode_test( SOURCE "test-solid-colocation.cpp" DEPENDENCIES diff --git a/tests/inspection/test-brep-colocation-degeneration.cpp b/tests/inspection/test-brep-colocation-degeneration.cpp new file mode 100644 index 00000000..fc73194d --- /dev/null +++ b/tests/inspection/test-brep-colocation-degeneration.cpp @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2019 - 2026 Geode-solutions + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +#include +#include + +#include + +#include +#include + +#include +#include +#include + +#include + +namespace +{ + // Surface with vertices 0 and 1 colocated, unique vertices given per + // vertex (NO_ID to leave the vertex unlinked) + geode::BRep build_colocated_surface( + const std::array< geode::index_t, 4 >& unique_vertices ) + { + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + builder.create_unique_vertices( 4 ); + const auto& surface = brep.surface( builder.add_surface() ); + auto mesh_builder = builder.surface_mesh_builder( surface ); + mesh_builder->create_point( geode::Point3D{ { 0, 0, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 0, 0, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 1, 0, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 0, 1, 0 } } ); + mesh_builder->create_polygon( { 0, 2, 3 } ); + mesh_builder->create_polygon( { 1, 3, 2 } ); + for( const auto v : geode::LRange{ 4 } ) + { + if( unique_vertices[v] != geode::NO_ID ) + { + builder.set_unique_vertex( + { surface.component_id(), v }, unique_vertices[v] ); + } + } + return brep; + } + + geode::index_t nb_colocated_groups( const geode::BRep& brep ) + { + const geode::BRepComponentMeshesColocation inspector{ brep }; + const auto result = inspector.inspect_meshes_point_colocations(); + geode::index_t nb_groups{ 0 }; + for( const auto& issues : result.colocated_points_groups.issues_map() ) + { + nb_groups += issues.second.nb_issues(); + } + return nb_groups; + } +} // namespace + +void check_colocation_without_unique_vertices() +{ + const auto unlinked = build_colocated_surface( + { geode::NO_ID, geode::NO_ID, geode::NO_ID, geode::NO_ID } ); + geode::OpenGeodeInspectorInspectionException::test( + nb_colocated_groups( unlinked ) == 1, + "[Test] Colocated vertices without unique vertex should be reported" ); + + const auto same_unique_vertex = build_colocated_surface( { 0, 0, 1, 2 } ); + geode::OpenGeodeInspectorInspectionException::test( + nb_colocated_groups( same_unique_vertex ) == 0, + "[Test] Colocated vertices with the same unique vertex should not be " + "reported" ); + + const auto different_unique_vertices = + build_colocated_surface( { 0, 3, 1, 2 } ); + geode::OpenGeodeInspectorInspectionException::test( + nb_colocated_groups( different_unique_vertices ) == 1, + "[Test] Colocated vertices with different unique vertices should be " + "reported" ); +} + +int main() +{ + try + { + geode::OpenGeodeInspectorInspectionLibrary::initialize(); + check_colocation_without_unique_vertices(); + + geode::Logger::info( "TEST SUCCESS" ); + return 0; + } + catch( ... ) + { + return geode::geode_lippincott(); + } +} From 5db70c7a1e569d9c85dfe75821910342d919df6f Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:42:53 +0200 Subject: [PATCH 03/12] fix(Degeneration): BRep edges degeneration inspection includes block edges inspect_edges_degeneration(threshold) only checked line and surface edges, while inspect_elements_degeneration also reports block small edges. Co-Authored-By: Claude Opus 5.5 --- .../degeneration/brep_meshes_degeneration.cpp | 43 +++++++++++++++---- .../test-brep-colocation-degeneration.cpp | 28 ++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp index f237b994..6b5792aa 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp @@ -86,19 +86,13 @@ namespace geode } } - void add_solid_small_elements( + void add_blocks_small_edges( InspectionIssuesMap< index_t >& small_edges_map, - InspectionIssuesMap< index_t >& small_polyhedra_map, double threshold ) const { for( const auto& block : model().active_blocks() ) { - const auto& mesh = block.mesh(); - if( !mesh.are_edges_enabled() ) - { - mesh.enable_edges(); - enabled_edges_blocks_.emplace( block.id() ); - } + const auto& mesh = enable_block_edges( block ); const geode::SolidMeshDegeneration3D inspector{ mesh }; auto small_edges = inspector.small_edges( threshold ); small_edges.set_description( absl::StrCat( "Block ", @@ -106,6 +100,17 @@ namespace geode block.id().string(), ") small edges polyhedra" ) ); small_edges_map.add_issues_to_map( block.id(), std::move( small_edges ) ); + } + } + + void add_blocks_small_height_polyhedra( + InspectionIssuesMap< index_t >& small_polyhedra_map, + double threshold ) const + { + for( const auto& block : model().active_blocks() ) + { + const auto& mesh = enable_block_edges( block ); + const geode::SolidMeshDegeneration3D inspector{ mesh }; auto small_polyhedra = inspector.small_height_polyhedra( threshold ); small_polyhedra.set_description( absl::StrCat( "Block ", @@ -116,6 +121,15 @@ namespace geode } } + void add_solid_small_elements( + InspectionIssuesMap< index_t >& small_edges_map, + InspectionIssuesMap< index_t >& small_polyhedra_map, + double threshold ) const + { + add_blocks_small_edges( small_edges_map, threshold ); + add_blocks_small_height_polyhedra( small_polyhedra_map, threshold ); + } + void add_solid_degenerations( InspectionIssuesMap< index_t >& degenerated_edges_map, InspectionIssuesMap< index_t >& degenerated_polyhedra_map ) const @@ -124,6 +138,18 @@ namespace geode degenerated_polyhedra_map, GLOBAL_EPSILON ); } + private: + const SolidMesh3D& enable_block_edges( const Block3D& block ) const + { + const auto& mesh = block.mesh(); + if( !mesh.are_edges_enabled() ) + { + mesh.enable_edges(); + enabled_edges_blocks_.emplace( block.id() ); + } + return mesh; + } + private: mutable absl::flat_hash_set< uuid > enabled_edges_blocks_; }; @@ -154,6 +180,7 @@ namespace geode { BRepMeshesDegenerationInspectionResult result; impl_->add_small_edges( result.degenerated_edges, threshold ); + impl_->add_blocks_small_edges( result.degenerated_edges, threshold ); return result; } diff --git a/tests/inspection/test-brep-colocation-degeneration.cpp b/tests/inspection/test-brep-colocation-degeneration.cpp index fc73194d..aff907a5 100644 --- a/tests/inspection/test-brep-colocation-degeneration.cpp +++ b/tests/inspection/test-brep-colocation-degeneration.cpp @@ -26,14 +26,18 @@ #include +#include #include +#include #include +#include #include #include #include #include +#include namespace { @@ -99,12 +103,36 @@ void check_colocation_without_unique_vertices() "reported" ); } +void check_block_small_edges() +{ + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + const auto& block = brep.block( builder.add_block() ); + auto mesh_builder = builder.block_mesh_builder( block ); + mesh_builder->create_point( geode::Point3D{ { 0, 0, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 1e-3, 0, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 0, 1, 0 } } ); + mesh_builder->create_point( geode::Point3D{ { 0, 0, 1 } } ); + mesh_builder->create_polyhedron( { 0, 1, 2, 3 }, + { { 1, 3, 2 }, { 0, 2, 3 }, { 3, 1, 0 }, { 0, 1, 2 } } ); + + const geode::BRepComponentMeshesDegeneration inspector{ brep }; + const auto result = inspector.inspect_edges_degeneration( 1e-2 ); + const auto& issues_map = result.degenerated_edges.issues_map(); + geode::OpenGeodeInspectorInspectionException::test( + issues_map.contains( block.id() ) + && issues_map.at( block.id() ).nb_issues() == 1, + "[Test] Block small edge should be reported by " + "inspect_edges_degeneration" ); +} + int main() { try { geode::OpenGeodeInspectorInspectionLibrary::initialize(); check_colocation_without_unique_vertices(); + check_block_small_edges(); geode::Logger::info( "TEST SUCCESS" ); return 0; From aff9057957ebe239503d4b2a4dd93747638573df Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:43:54 +0200 Subject: [PATCH 04/12] perf(Degeneration): compare squared edge lengths to the threshold Small edges detection computed a square root per edge. Co-Authored-By: Claude Opus 5.5 --- .../criterion/degeneration/edgedcurve_degeneration.cpp | 7 ++++++- .../inspection/criterion/internal/degeneration_impl.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp index 40c184e9..bbcbdea0 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp @@ -28,6 +28,7 @@ #include #include +#include #include @@ -58,7 +59,11 @@ namespace geode }; for( const auto edge_id : Range{ mesh_.nb_edges() } ) { - if( mesh_.edge_length( edge_id ) <= threshold ) + const auto& edge_vertices = mesh_.edge_vertices( edge_id ); + if( Vector< dimension >{ mesh_.point( edge_vertices[0] ), + mesh_.point( edge_vertices[1] ) } + .length2() + <= threshold * threshold ) { degenerated_edges_index.add_issue( edge_id, absl::StrCat( "degenerated edge ", edge_id, diff --git a/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp b/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp index 1ec06970..264f20d1 100644 --- a/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp +++ b/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp @@ -30,8 +30,8 @@ #include #include -#include #include +#include namespace geode { @@ -103,9 +103,9 @@ namespace geode { const auto edge_vertices = mesh_.edges().edge_vertices( edge_index ); - const auto p1 = mesh_.point( edge_vertices[0] ); - const auto p2 = mesh_.point( edge_vertices[1] ); - return point_point_distance( p1, p2 ) < threshold; + const Vector< MeshType::dim > edge{ mesh_.point( edge_vertices[0] ), + mesh_.point( edge_vertices[1] ) }; + return edge.length2() < threshold * threshold; } template < class MeshType > From a7654f09e8c77131f0137d4e61199795ecd29272 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:46:26 +0200 Subject: [PATCH 05/12] perf(Degeneration): build mesh edges only when small edges are found Small edges detection enabled the SurfaceEdges/SolidEdges of every surface and block (one hash map insertion per element edge) and kept them attached to the meshes, only to test edge lengths. Element edges are now scanned directly; edges are built only when a small edge is found, to report its index, and only kept on the components having issues. cva/brep_from_topo 1.81s -> 1.55s, corbi_tet_mesh 0.37s -> 0.36s (inspect_elements_degeneration, identical results). Co-Authored-By: Claude Opus 5.5 --- .../component_meshes_degeneration.hpp | 3 - .../criterion/internal/degeneration_impl.hpp | 5 +- .../degeneration/brep_meshes_degeneration.cpp | 29 ++--- .../component_meshes_degeneration.cpp | 68 ++++------ .../criterion/internal/degeneration_impl.cpp | 121 +++++++++++++----- .../test-brep-colocation-degeneration.cpp | 10 ++ 6 files changed, 144 insertions(+), 92 deletions(-) diff --git a/include/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.hpp b/include/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.hpp index 31122b3d..4d9c8ab3 100644 --- a/include/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.hpp +++ b/include/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.hpp @@ -72,9 +72,6 @@ namespace geode [[nodiscard]] const Model& model() const; - private: - std::vector< uuid > surfaces_on_which_enable_edges() const; - private: const Model& model_; mutable absl::flat_hash_set< uuid > enabled_edges_surfaces_; diff --git a/include/geode/inspector/inspection/criterion/internal/degeneration_impl.hpp b/include/geode/inspector/inspection/criterion/internal/degeneration_impl.hpp index 25b517fe..403f88d5 100644 --- a/include/geode/inspector/inspection/criterion/internal/degeneration_impl.hpp +++ b/include/geode/inspector/inspection/criterion/internal/degeneration_impl.hpp @@ -48,9 +48,8 @@ namespace geode private: [[nodiscard]] bool edge_is_smaller_than_threshold( - index_t edge_index, double threshold ) const; - - [[nodiscard]] bool edge_is_degenerated( index_t edge_index ) const; + const std::array< index_t, 2 >& edge_vertices, + double threshold ) const; void enable_edges_on_mesh() const; diff --git a/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp index 6b5792aa..a3da63cf 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/brep_meshes_degeneration.cpp @@ -92,9 +92,17 @@ namespace geode { for( const auto& block : model().active_blocks() ) { - const auto& mesh = enable_block_edges( block ); - const geode::SolidMeshDegeneration3D inspector{ mesh }; - auto small_edges = inspector.small_edges( threshold ); + const auto& mesh = block.mesh(); + auto small_edges = [&mesh, threshold] { + const geode::SolidMeshDegeneration3D inspector{ mesh }; + return inspector.small_edges( threshold ); + }(); + // Keep the edges referenced by the issues + if( small_edges.nb_issues() != 0 && !mesh.are_edges_enabled() ) + { + mesh.enable_edges(); + enabled_edges_blocks_.emplace( block.id() ); + } small_edges.set_description( absl::StrCat( "Block ", block.name().value_or( block.id().string() ), " (", block.id().string(), ") small edges polyhedra" ) ); @@ -109,8 +117,7 @@ namespace geode { for( const auto& block : model().active_blocks() ) { - const auto& mesh = enable_block_edges( block ); - const geode::SolidMeshDegeneration3D inspector{ mesh }; + const geode::SolidMeshDegeneration3D inspector{ block.mesh() }; auto small_polyhedra = inspector.small_height_polyhedra( threshold ); small_polyhedra.set_description( absl::StrCat( "Block ", @@ -138,18 +145,6 @@ namespace geode degenerated_polyhedra_map, GLOBAL_EPSILON ); } - private: - const SolidMesh3D& enable_block_edges( const Block3D& block ) const - { - const auto& mesh = block.mesh(); - if( !mesh.are_edges_enabled() ) - { - mesh.enable_edges(); - enabled_edges_blocks_.emplace( block.id() ); - } - return mesh; - } - private: mutable absl::flat_hash_set< uuid > enabled_edges_blocks_; }; diff --git a/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp b/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp index 39516c3e..cef94d39 100644 --- a/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/internal/component_meshes_degeneration.cpp @@ -88,39 +88,45 @@ namespace geode components_small_edges.add_issues_to_map( line_id, std::move( issues ) ); } - std::vector< - async::task< std::pair< uuid, InspectionIssues< index_t > > > > + std::vector< async::task< + std::tuple< uuid, InspectionIssues< index_t >, bool > > > surface_tasks; surface_tasks.reserve( model_.nb_active_surfaces() ); - const auto surfaces_to_enable = surfaces_on_which_enable_edges(); for( const auto& surface : model_.active_surfaces() ) { - const auto enable_edges = - absl::c_contains( surfaces_to_enable, surface.id() ); - surface_tasks.emplace_back( - async::spawn( [enable_edges, &threshold, &surface] { - if( enable_edges ) - { - surface.mesh().enable_edges(); - } + surface_tasks.emplace_back( async::spawn( [&threshold, + &surface] { + const auto& mesh = surface.mesh(); + std::tuple< uuid, InspectionIssues< index_t >, bool > + result; + auto& [surface_id, issues, edges_enabled] = result; + surface_id = surface.id(); + { const geode::SurfaceMeshDegeneration< Model::dim > - inspector{ surface.mesh() }; - std::pair< uuid, InspectionIssues< index_t > > result; - auto& [surface_id, issues] = result; - surface_id = surface.id(); + inspector{ mesh }; issues = inspector.small_edges( threshold ); - issues.set_description( absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), - " (", surface.id().string(), - ") small facet edges" ) ); - return result; - } ) ); + } + // Keep the edges referenced by the issues + if( issues.nb_issues() != 0 && !mesh.are_edges_enabled() ) + { + mesh.enable_edges(); + edges_enabled = true; + } + issues.set_description( absl::StrCat( "Surface ", + surface.name().value_or( surface.id().string() ), " (", + surface.id().string(), ") small facet edges" ) ); + return result; + } ) ); } for( auto& task : async::when_all( surface_tasks.begin(), surface_tasks.end() ) .get() ) { - auto [surface_id, issues] = task.get(); + auto [surface_id, issues, edges_enabled] = task.get(); + if( edges_enabled ) + { + enabled_edges_surfaces_.emplace( surface_id ); + } components_small_edges.add_issues_to_map( surface_id, std::move( issues ) ); } @@ -181,24 +187,6 @@ namespace geode return model_; } - template < typename Model > - std::vector< uuid > ComponentMeshesDegeneration< - Model >::surfaces_on_which_enable_edges() const - { - std::vector< uuid > result; - for( const auto& surface : model_.active_surfaces() ) - { - const auto& mesh = surface.mesh(); - if( !mesh.are_edges_enabled() - && !enabled_edges_surfaces_.contains( surface.id() ) ) - { - result.emplace_back( surface.id() ); - enabled_edges_surfaces_.emplace( surface.id() ); - } - } - return result; - } - template class opengeode_inspector_inspection_api ComponentMeshesDegeneration< Section >; template class opengeode_inspector_inspection_api diff --git a/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp b/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp index 264f20d1..dc6d6cdb 100644 --- a/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp +++ b/src/geode/inspector/inspection/criterion/internal/degeneration_impl.cpp @@ -23,6 +23,9 @@ #include +#include +#include + #include #include @@ -33,6 +36,49 @@ #include #include +namespace +{ + // Calls the action on the vertices of each edge of each element, edges + // shared by several elements are visited several times. Stops when the + // action returns true. + template < geode::index_t dimension, typename Action > + bool for_each_element_edge( + const geode::SurfaceMesh< dimension >& mesh, const Action& action ) + { + for( const auto polygon_id : geode::Range{ mesh.nb_polygons() } ) + { + for( const auto edge_id : + geode::LRange{ mesh.nb_polygon_edges( polygon_id ) } ) + { + if( action( mesh.polygon_edge_vertices( + { polygon_id, edge_id } ) ) ) + { + return true; + } + } + } + return false; + } + + template < typename Action > + bool for_each_element_edge( + const geode::SolidMesh3D& mesh, const Action& action ) + { + for( const auto polyhedron_id : geode::Range{ mesh.nb_polyhedra() } ) + { + for( const auto& edge_vertices : + mesh.polyhedron_edges_vertices( polyhedron_id ) ) + { + if( action( edge_vertices ) ) + { + return true; + } + } + } + return false; + } +} // namespace + namespace geode { namespace internal @@ -55,38 +101,63 @@ namespace geode template < class MeshType > bool DegenerationImpl< MeshType >::is_mesh_degenerated() const { - enable_edges_on_mesh(); - for( const auto edge_index : Range{ mesh_.edges().nb_edges() } ) - { - if( edge_is_degenerated( edge_index ) ) - { - return true; - } - } - return false; + return for_each_element_edge( + mesh_, [this]( const std::array< index_t, 2 >& edge_vertices ) { + return edge_is_smaller_than_threshold( + edge_vertices, GLOBAL_EPSILON ); + } ); } template < class MeshType > InspectionIssues< index_t > DegenerationImpl< MeshType >::small_edges( double threshold ) const { - enable_edges_on_mesh(); InspectionIssues< index_t > degenerated_edges_index{ "degenerated edges" }; - for( const auto edge_index : Range{ mesh_.edges().nb_edges() } ) + // Edges are only built when some small edge is found, to convert + // its vertices into an edge index + absl::flat_hash_set< std::array< index_t, 2 > > small_edges; + for_each_element_edge( + mesh_, [this, threshold, &small_edges]( + std::array< index_t, 2 > edge_vertices ) { + if( edge_is_smaller_than_threshold( + edge_vertices, threshold ) ) + { + if( edge_vertices[0] > edge_vertices[1] ) + { + std::swap( edge_vertices[0], edge_vertices[1] ); + } + small_edges.insert( edge_vertices ); + } + return false; + } ); + if( small_edges.empty() ) + { + return degenerated_edges_index; + } + enable_edges_on_mesh(); + std::vector< index_t > small_edge_indices; + small_edge_indices.reserve( small_edges.size() ); + for( const auto& edge_vertices : small_edges ) { - if( edge_is_smaller_than_threshold( edge_index, threshold ) ) + if( const auto edge_index = + mesh_.edges().edge_from_vertices( edge_vertices ) ) { - const auto edge_vertices = - mesh_.edges().edge_vertices( edge_index ); - degenerated_edges_index.add_issue( edge_index, - absl::StrCat( "degenerated edge between vertices ", - edge_vertices[0], " and ", edge_vertices[1], - ", at position [", - mesh_.point( edge_vertices[0] ).string(), "]" ) ); + small_edge_indices.push_back( edge_index.value() ); } } + absl::c_sort( small_edge_indices ); + for( const auto edge_index : small_edge_indices ) + { + const auto& edge_vertices = + mesh_.edges().edge_vertices( edge_index ); + degenerated_edges_index.add_issue( edge_index, + absl::StrCat( "degenerated edge between vertices ", + edge_vertices[0], " and ", edge_vertices[1], + ", at position [", + mesh_.point( edge_vertices[0] ).string(), "]" ) ); + } return degenerated_edges_index; } @@ -99,10 +170,9 @@ namespace geode template < class MeshType > bool DegenerationImpl< MeshType >::edge_is_smaller_than_threshold( - index_t edge_index, double threshold ) const + const std::array< index_t, 2 >& edge_vertices, + double threshold ) const { - const auto edge_vertices = - mesh_.edges().edge_vertices( edge_index ); const Vector< MeshType::dim > edge{ mesh_.point( edge_vertices[0] ), mesh_.point( edge_vertices[1] ) }; return edge.length2() < threshold * threshold; @@ -118,13 +188,6 @@ namespace geode } } - template < class MeshType > - bool DegenerationImpl< MeshType >::edge_is_degenerated( - index_t edge_index ) const - { - return edge_is_smaller_than_threshold( edge_index, GLOBAL_EPSILON ); - } - template < class MeshType > const MeshType& DegenerationImpl< MeshType >::mesh() const { diff --git a/tests/inspection/test-brep-colocation-degeneration.cpp b/tests/inspection/test-brep-colocation-degeneration.cpp index aff907a5..a1379eba 100644 --- a/tests/inspection/test-brep-colocation-degeneration.cpp +++ b/tests/inspection/test-brep-colocation-degeneration.cpp @@ -21,6 +21,8 @@ * */ +#include + #include #include @@ -28,6 +30,7 @@ #include #include +#include #include #include @@ -124,6 +127,13 @@ void check_block_small_edges() && issues_map.at( block.id() ).nb_issues() == 1, "[Test] Block small edge should be reported by " "inspect_edges_degeneration" ); + // The reported edge index refers to the block mesh edges + const auto edge_id = issues_map.at( block.id() ).issues().front(); + auto edge_vertices = block.mesh().edges().edge_vertices( edge_id ); + absl::c_sort( edge_vertices ); + geode::OpenGeodeInspectorInspectionException::test( + edge_vertices[0] == 0 && edge_vertices[1] == 1, + "[Test] Wrong block small edge" ); } int main() From f1f2e36dbc8f802d5297d0e88f0f35fddedb28a4 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:47:21 +0200 Subject: [PATCH 06/12] perf(Degeneration): compute polygon and polyhedron heights in parallel small_height_polygons/polyhedra computed the minimum height of each element serially. Heights are now computed in parallel, issues are added in element order afterwards. cva/brep_from_topo 1.55s -> 0.38s, corbi_tet_mesh 0.36s -> 0.10s (inspect_elements_degeneration, identical results). Co-Authored-By: Claude Opus 5.5 --- .../degeneration/solid_degeneration.cpp | 21 ++++++++++++------- .../degeneration/surface_degeneration.cpp | 19 ++++++++++++----- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/degeneration/solid_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/solid_degeneration.cpp index fff418bb..388b6f33 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/solid_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/solid_degeneration.cpp @@ -23,6 +23,8 @@ #include +#include + #include #include #include @@ -70,15 +72,20 @@ namespace geode InspectionIssues< index_t > wrong_polyhedra{ "Degenerated Polyhedra." }; - for( const auto polyhedron_id : - Range{ this->mesh().nb_polyhedra() } ) + const auto nb_elements = this->mesh().nb_polyhedra(); + std::vector< char > is_small( nb_elements, false ); + async::parallel_for( async::irange( index_t{ 0 }, nb_elements ), + [this, threshold, &is_small]( index_t element_id ) { + is_small[element_id] = + this->mesh().polyhedron_minimum_height( element_id ) + <= threshold; + } ); + for( const auto element_id : Range{ nb_elements } ) { - if( this->mesh().polyhedron_minimum_height( polyhedron_id ) - <= threshold ) + if( is_small[element_id] ) { - wrong_polyhedra.add_issue( - polyhedron_id, absl::StrCat( "degenerated polyhedra ", - polyhedron_id ) ); + wrong_polyhedra.add_issue( element_id, + absl::StrCat( "degenerated polyhedra ", element_id ) ); } } return wrong_polyhedra; diff --git a/src/geode/inspector/inspection/criterion/degeneration/surface_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/surface_degeneration.cpp index 76d67dc5..5d512b2c 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/surface_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/surface_degeneration.cpp @@ -23,6 +23,8 @@ #include +#include + #include #include #include @@ -69,13 +71,20 @@ namespace geode InspectionIssues< index_t > wrong_polygons{ "Degenerated Polygons." }; - for( const auto polygon_id : Range{ this->mesh().nb_polygons() } ) + const auto nb_elements = this->mesh().nb_polygons(); + std::vector< char > is_small( nb_elements, false ); + async::parallel_for( async::irange( index_t{ 0 }, nb_elements ), + [this, threshold, &is_small]( index_t element_id ) { + is_small[element_id] = + this->mesh().polygon_minimum_height( element_id ) + <= threshold; + } ); + for( const auto element_id : Range{ nb_elements } ) { - if( this->mesh().polygon_minimum_height( polygon_id ) - <= threshold ) + if( is_small[element_id] ) { - wrong_polygons.add_issue( polygon_id, - absl::StrCat( "degenerated polygons ", polygon_id ) ); + wrong_polygons.add_issue( element_id, + absl::StrCat( "degenerated polygons ", element_id ) ); } } return wrong_polygons; From 009143af4de60be0c4275669552f30dcd6cb667f Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:49:07 +0200 Subject: [PATCH 07/12] perf(Colocation): build colocated groups only for colocated points A vector was allocated for every unique point (one allocation per mesh vertex) before dropping the groups with a single point. Points are now counted per unique point and groups only built when needed, in the same order. cva/brep_from_topo 0.114s -> 0.092s (identical results). Co-Authored-By: Claude Opus 5.5 --- .../criterion/internal/colocation_impl.cpp | 55 +++++++++++++------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp index aac834b8..098d7733 100644 --- a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp +++ b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp @@ -35,9 +35,7 @@ namespace { template < geode::index_t dimension, typename Mesh > - typename geode::NNSearch< dimension >::ColocatedInfo - mesh_points_colocated_info( - const Mesh& mesh, double colocation_distance ) + geode::NNSearch< dimension > mesh_points_search( const Mesh& mesh ) { std::vector< geode::Point< dimension > > mesh_points( mesh.nb_vertices() ); @@ -45,8 +43,16 @@ namespace { mesh_points[point_index] = mesh.point( point_index ); } - const geode::NNSearch< dimension > nnsearch{ mesh_points }; - return nnsearch.colocated_index_mapping( colocation_distance ); + return geode::NNSearch< dimension >{ std::move( mesh_points ) }; + } + + template < geode::index_t dimension, typename Mesh > + typename geode::NNSearch< dimension >::ColocatedInfo + mesh_points_colocated_info( + const Mesh& mesh, double colocation_distance ) + { + return mesh_points_search< dimension >( mesh ).colocated_index_mapping( + colocation_distance ); } template < geode::index_t dimension, typename Mesh > @@ -56,22 +62,35 @@ namespace const auto mesh_colocation_info = mesh_points_colocated_info< dimension, Mesh >( mesh, colocation_distance ); + // Groups are only built for unique points having several input + // points, ordered by unique point + const auto& mapping = mesh_colocation_info.colocated_mapping; + std::vector< geode::index_t > nb_points_per_unique( + mesh_colocation_info.nb_unique_points(), 0 ); + for( const auto unique_point : mapping ) + { + nb_points_per_unique[unique_point]++; + } + std::vector< geode::index_t > group_of_unique( + nb_points_per_unique.size(), geode::NO_ID ); + geode::index_t nb_groups{ 0 }; + for( const auto unique_point : geode::Indices{ nb_points_per_unique } ) + { + if( nb_points_per_unique[unique_point] > 1 ) + { + group_of_unique[unique_point] = nb_groups++; + } + } std::vector< std::vector< geode::index_t > > colocated_points_indices( - mesh_colocation_info.nb_unique_points() ); - for( const auto point_index : - geode::Indices{ mesh_colocation_info.colocated_mapping } ) + nb_groups ); + for( const auto point_index : geode::Indices{ mapping } ) { - colocated_points_indices[mesh_colocation_info - .colocated_mapping[point_index]] - .push_back( point_index ); + const auto group = group_of_unique[mapping[point_index]]; + if( group != geode::NO_ID ) + { + colocated_points_indices[group].push_back( point_index ); + } } - const auto colocated_points_groups_end = std::remove_if( - colocated_points_indices.begin(), colocated_points_indices.end(), - []( const std::vector< geode::index_t >& colocated_points_group ) { - return colocated_points_group.size() < 2; - } ); - colocated_points_indices.erase( - colocated_points_groups_end, colocated_points_indices.end() ); geode::InspectionIssues< std::vector< geode::index_t > > groups_of_colocated_points{ "groups of colocated points" }; From cebcc497fcfd836fa882c32bece19a496991d751 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:49:19 +0200 Subject: [PATCH 08/12] perf(Colocation): mesh_has_colocated_points stops at the first colocation The boolean query computed the full colocation mapping. It now looks for a second point within GLOBAL_EPSILON among the two nearest neighbors of each point, in parallel, and stops as soon as one is found. Co-Authored-By: Claude Opus 5.5 --- .../criterion/internal/colocation_impl.cpp | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp index 098d7733..42c9b4c2 100644 --- a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp +++ b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp @@ -23,6 +23,10 @@ #include +#include + +#include + #include #include @@ -30,6 +34,7 @@ #include #include +#include #include namespace @@ -128,10 +133,29 @@ namespace geode bool ColocationImpl< dimension, Mesh >::mesh_has_colocated_points() const { - return mesh_points_colocated_info< dimension, Mesh >( - mesh_, GLOBAL_EPSILON ) - .nb_colocated_points() - > 0; + const auto nnsearch = mesh_points_search< dimension >( mesh_ ); + std::atomic< bool > has_colocated_points{ false }; + async::parallel_for( + async::irange( index_t{ 0 }, nnsearch.nb_points() ), + [&nnsearch, &has_colocated_points]( index_t point_index ) { + if( has_colocated_points ) + { + return; + } + const auto& point = nnsearch.point( point_index ); + for( const auto neighbor : nnsearch.neighbors( point, 2 ) ) + { + if( neighbor != point_index + && point_point_distance( + point, nnsearch.point( neighbor ) ) + <= GLOBAL_EPSILON ) + { + has_colocated_points = true; + return; + } + } + } ); + return has_colocated_points; } template < index_t dimension, typename Mesh > From 3a0bfab1a5fc8ade5956760e9d1c6d90b8d8b280 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:50:42 +0200 Subject: [PATCH 09/12] perf(Colocation): build component labels once, only when issues exist The line, surface and block colocation loops were three copies of the same code, each formatting the component name and uuid twice per issue. They share one helper which builds the label once, only for components having colocated vertices. Messages are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../component_meshes_colocation.cpp | 162 +++++++----------- .../test-brep-colocation-degeneration.cpp | 15 ++ 2 files changed, 73 insertions(+), 104 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp index 6ec3245c..b8b45125 100644 --- a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp @@ -87,87 +87,74 @@ namespace return new_colocated_points_groups; } - template < typename Model > - void add_model_components_colocated_points_groups_base( const Model& model, - geode::InspectionIssuesMap< std::vector< geode::index_t > >& - components_colocated_points ) + template < typename Inspector, typename Model, typename Component > + std::optional< geode::InspectionIssues< std::vector< geode::index_t > > > + component_colocated_points_groups( + const Model& model, const Component& component ) { - for( const auto& line : model.active_lines() ) + const auto& mesh = component.mesh(); + const Inspector inspector{ mesh }; + const auto colocated_pts = + filter_colocated_points_with_same_unique_vertex< Model >( model, + component.component_id(), + inspector.colocated_points_groups().issues() ); + if( colocated_pts.empty() ) { - const geode::EdgedCurveColocation< Model::dim > inspector{ - line.mesh() - }; - auto colocated_pts = - filter_colocated_points_with_same_unique_vertex< Model >( model, - line.component_id(), - inspector.colocated_points_groups().issues() ); - if( !colocated_pts.empty() ) + return std::nullopt; + } + const auto& id = component.id(); + const auto label = absl::StrCat( component.component_type().get(), " ", + component.name() ? component.name().value() : id.string(), " (", + id.string(), ")" ); + std::optional< + geode::InspectionIssues< std::vector< geode::index_t > > > + issues{ std::in_place, + absl::StrCat( label, " colocated vertices" ) }; + for( const auto& colocated_points_group : colocated_pts ) + { + std::string point_group_string; + for( const auto point_index : colocated_points_group ) { - geode::InspectionIssues< std::vector< geode::index_t > > - line_issues{ absl::StrCat( "Line ", - line.name().value_or( line.id().string() ), " (", - line.id().string(), ") colocated vertices" ) }; - const auto& line_mesh = line.mesh(); - for( const auto& colocated_points_group : colocated_pts ) - { - std::string point_group_string; - for( const auto point_index : colocated_points_group ) - { - absl::StrAppend( - &point_group_string, " ", point_index ); - } - line_issues.add_issue( colocated_points_group, - absl::StrCat( "Line ", - line.name().value_or( line.id().string() ), " (", - line.id().string(), ") has vertices ", - point_group_string, " colocated at position [", - line_mesh.point( colocated_points_group[0] ) - .string(), - "]" ) ); - } - components_colocated_points.add_issues_to_map( - line.id(), std::move( line_issues ) ); + absl::StrAppend( &point_group_string, " ", point_index ); } + issues->add_issue( colocated_points_group, + absl::StrCat( label, " has vertices ", point_group_string, + " colocated at position [", + mesh.point( colocated_points_group[0] ).string(), "]" ) ); } - for( const auto& surface : model.active_surfaces() ) + return issues; + } + + template < typename Inspector, typename Model, typename Range > + void add_components_colocated_points_groups( const Model& model, + const Range& components, + geode::InspectionIssuesMap< std::vector< geode::index_t > >& + components_colocated_points ) + { + for( const auto& component : components ) { - const geode::SurfaceMeshColocation< Model::dim > inspector{ - surface.mesh() - }; - auto colocated_pts = - filter_colocated_points_with_same_unique_vertex< Model >( model, - surface.component_id(), - inspector.colocated_points_groups().issues() ); - if( !colocated_pts.empty() ) + if( auto issues = component_colocated_points_groups< Inspector >( + model, component ) ) { - geode::InspectionIssues< std::vector< geode::index_t > > - surface_issues{ absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), " (", - surface.id().string(), ") colocated vertices" ) }; - const auto& surface_mesh = surface.mesh(); - for( const auto& colocated_points_group : colocated_pts ) - { - std::string point_group_string; - for( const auto point_index : colocated_points_group ) - { - absl::StrAppend( - &point_group_string, " ", point_index ); - } - surface_issues.add_issue( colocated_points_group, - absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), - " (", surface.id().string(), ") has vertices ", - point_group_string, " colocated at position [", - surface_mesh.point( colocated_points_group[0] ) - .string(), - "]" ) ); - } components_colocated_points.add_issues_to_map( - surface.id(), std::move( surface_issues ) ); + component.id(), std::move( issues.value() ) ); } } } + template < typename Model > + void add_model_components_colocated_points_groups_base( const Model& model, + geode::InspectionIssuesMap< std::vector< geode::index_t > >& + components_colocated_points ) + { + add_components_colocated_points_groups< + geode::EdgedCurveColocation< Model::dim > >( + model, model.active_lines(), components_colocated_points ); + add_components_colocated_points_groups< + geode::SurfaceMeshColocation< Model::dim > >( + model, model.active_surfaces(), components_colocated_points ); + } + void add_model_components_colocated_points_groups( const geode::Section& model, geode::InspectionIssuesMap< std::vector< geode::index_t > >& @@ -183,41 +170,8 @@ namespace { add_model_components_colocated_points_groups_base< geode::BRep >( model, components_colocated_points ); - for( const auto& block : model.active_blocks() ) - { - const geode::SolidMeshColocation3D inspector{ block.mesh() }; - auto colocated_pts = - filter_colocated_points_with_same_unique_vertex< geode::BRep >( - model, block.component_id(), - inspector.colocated_points_groups().issues() ); - if( !colocated_pts.empty() ) - { - geode::InspectionIssues< std::vector< geode::index_t > > - block_issues{ absl::StrCat( "Block ", - block.name().value_or( block.id().string() ), " (", - block.id().string(), ") colocated vertices" ) }; - const auto& block_mesh = block.mesh(); - for( const auto& colocated_points_group : colocated_pts ) - { - std::string point_group_string; - for( const auto point_index : colocated_points_group ) - { - absl::StrAppend( - &point_group_string, " ", point_index ); - } - block_issues.add_issue( colocated_points_group, - absl::StrCat( "Block ", - block.name().value_or( block.id().string() ), " (", - block.id().string(), ") has vertices ", - point_group_string, " colocated at position [", - block_mesh.point( colocated_points_group[0] ) - .string(), - "]" ) ); - } - components_colocated_points.add_issues_to_map( - block.id(), std::move( block_issues ) ); - } - } + add_components_colocated_points_groups< geode::SolidMeshColocation3D >( + model, model.active_blocks(), components_colocated_points ); } } // namespace diff --git a/tests/inspection/test-brep-colocation-degeneration.cpp b/tests/inspection/test-brep-colocation-degeneration.cpp index a1379eba..a0d56c3d 100644 --- a/tests/inspection/test-brep-colocation-degeneration.cpp +++ b/tests/inspection/test-brep-colocation-degeneration.cpp @@ -22,6 +22,7 @@ */ #include +#include #include #include @@ -91,6 +92,20 @@ void check_colocation_without_unique_vertices() geode::OpenGeodeInspectorInspectionException::test( nb_colocated_groups( unlinked ) == 1, "[Test] Colocated vertices without unique vertex should be reported" ); + const geode::BRepComponentMeshesColocation inspector{ unlinked }; + const auto message = inspector.inspect_meshes_point_colocations().string(); + std::string surface_id; + for( const auto& surface : unlinked.surfaces() ) + { + surface_id = surface.id().string(); + } + geode::OpenGeodeInspectorInspectionException::test( + absl::StrContains( message, absl::StrCat( "Surface ", surface_id, " (", + surface_id, ") colocated vertices" ) ) + && absl::StrContains( message, + absl::StrCat( "Surface ", surface_id, " (", surface_id, + ") has vertices 1 0 colocated at position [0 0 0]" ) ), + "[Test] Wrong colocation message: ", message ); const auto same_unique_vertex = build_colocated_surface( { 0, 0, 1, 2 } ); geode::OpenGeodeInspectorInspectionException::test( From e115f9e37170cd454944827eeb01fe6a3194f878 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:51:21 +0200 Subject: [PATCH 10/12] perf(Colocation): inspect component meshes colocation in parallel Each line, surface and block built its NNSearch serially. Components are now inspected in parallel tasks; issues are added in the same order. corbi_tet_mesh 0.037s -> 0.013s, fimiston 0.073s -> 0.036s (identical results). Co-Authored-By: Claude Opus 5.5 --- .../component_meshes_colocation.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp index b8b45125..5fed3d2e 100644 --- a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp @@ -25,6 +25,8 @@ #include +#include + #include #include #include @@ -131,13 +133,24 @@ namespace geode::InspectionIssuesMap< std::vector< geode::index_t > >& components_colocated_points ) { + using Issues = std::optional< + geode::InspectionIssues< std::vector< geode::index_t > > >; + std::vector< async::task< std::pair< geode::uuid, Issues > > > tasks; for( const auto& component : components ) { - if( auto issues = component_colocated_points_groups< Inspector >( - model, component ) ) + tasks.emplace_back( async::spawn( [&model, &component] { + return std::pair{ component.id(), + component_colocated_points_groups< Inspector >( + model, component ) }; + } ) ); + } + for( auto& task : async::when_all( tasks ).get() ) + { + auto [id, issues] = task.get(); + if( issues ) { components_colocated_points.add_issues_to_map( - component.id(), std::move( issues.value() ) ); + id, std::move( issues.value() ) ); } } } From 3429d15789ad54aeae11335885a401ac9bdf07f2 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:52:51 +0200 Subject: [PATCH 11/12] perf(Colocation): lighter and lazy unique vertices colocation - component lookups: model.component(id) tried every component type before a second typed lookup; the cmv type is now dispatched once - the active unique vertices PointSet is built directly instead of being created for all unique vertices then shrunk with delete_vertices, and the GenericMapping is replaced by index vectors - points and per unique vertex checks are computed in parallel - the work is done on first use instead of in the constructor, which BRepMeshesInspector always calls fimiston 0.19s -> 0.085s, cva/brep_from_topo 0.21s -> 0.087s (inspect_unique_vertices, identical results). Co-Authored-By: Claude Opus 5.5 --- .../colocation/unique_vertices_colocation.cpp | 285 +++++++++--------- 1 file changed, 135 insertions(+), 150 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp index 2f775746..0b2930e9 100644 --- a/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp @@ -23,6 +23,8 @@ #include +#include + #include #include @@ -47,122 +49,85 @@ namespace { - template < typename Model > - bool model_cmv_is_colocated_on_point_base( const Model& model, - const geode::ComponentMeshVertex& cmv, - const geode::Point< Model::dim >& point ) + // Point of a component mesh vertex, std::nullopt if its component is + // inactive + template < geode::index_t dimension, typename Component > + std::optional< geode::Point< dimension > > active_component_point( + const Component& component, geode::index_t vertex ) { - if( cmv.component_id.type - == geode::Line< Model::dim >::component_type_static() ) + if( !component.is_active() ) { - return point.inexact_equal( - model.line( cmv.component_id.id ).mesh().point( cmv.vertex ) ); + return std::nullopt; } - else if( cmv.component_id.type - == geode::Surface< Model::dim >::component_type_static() ) - { - return point.inexact_equal( model.surface( cmv.component_id.id ) - .mesh() - .point( cmv.vertex ) ); - } - return point.inexact_equal( - model.corner( cmv.component_id.id ).mesh().point( cmv.vertex ) ); + return component.mesh().point( vertex ); } - bool model_cmv_is_colocated_on_point( const geode::Section& model, - const geode::ComponentMeshVertex& cmv, - const geode::Point2D& point ) - { - return model_cmv_is_colocated_on_point_base< geode::Section >( - model, cmv, point ); - } - - bool model_cmv_is_colocated_on_point( const geode::BRep& model, - const geode::ComponentMeshVertex& cmv, - const geode::Point3D& point ) + template < typename Model > + std::optional< geode::Point< Model::dim > > active_cmv_point_base( + const Model& model, const geode::ComponentMeshVertex& cmv ) { - if( cmv.component_id.type == geode::Block3D::component_type_static() ) + const auto& type = cmv.component_id.type; + const auto& id = cmv.component_id.id; + if( type == geode::Line< Model::dim >::component_type_static() ) { - return point.inexact_equal( - model.block( cmv.component_id.id ).mesh().point( cmv.vertex ) ); + return active_component_point< Model::dim >( + model.line( id ), cmv.vertex ); } - return model_cmv_is_colocated_on_point_base< geode::BRep >( - model, cmv, point ); + if( type == geode::Surface< Model::dim >::component_type_static() ) + { + return active_component_point< Model::dim >( + model.surface( id ), cmv.vertex ); + } + return active_component_point< Model::dim >( + model.corner( id ), cmv.vertex ); } - template < typename Model > - bool model_cmvs_are_colocated_on_point( const Model& model, - absl::Span< const geode::ComponentMeshVertex > cmvs, - const geode::Point< Model::dim >& point ) + std::optional< geode::Point2D > active_cmv_point( + const geode::Section& model, const geode::ComponentMeshVertex& cmv ) { - for( const auto& cmv : cmvs ) - { - if( !model.component( cmv.component_id.id ).is_active() ) - { - continue; - } - if( !model_cmv_is_colocated_on_point( model, cmv, point ) ) - { - return false; - } - } - return true; + return active_cmv_point_base( model, cmv ); } - template < typename Model > - geode::Point< Model::dim > model_cmv_point( - const Model& model, const geode::ComponentMeshVertex& cmv ) + std::optional< geode::Point3D > active_cmv_point( + const geode::BRep& model, const geode::ComponentMeshVertex& cmv ) { - if( cmv.component_id.type - == geode::Line< Model::dim >::component_type_static() ) - { - return model.line( cmv.component_id.id ).mesh().point( cmv.vertex ); - } - if( cmv.component_id.type - == geode::Surface< Model::dim >::component_type_static() ) + if( cmv.component_id.type == geode::Block3D::component_type_static() ) { - return model.surface( cmv.component_id.id ) - .mesh() - .point( cmv.vertex ); + return active_component_point< 3 >( + model.block( cmv.component_id.id ), cmv.vertex ); } - return model.corner( cmv.component_id.id ).mesh().point( cmv.vertex ); + return active_cmv_point_base( model, cmv ); } - std::optional< geode::Point2D > model_unique_vertex_point( - const geode::Section& model, + template < typename Model > + std::optional< geode::Point< Model::dim > > model_unique_vertex_point( + const Model& model, absl::Span< const geode::ComponentMeshVertex > cmvs ) { for( const auto& cmv : cmvs ) { - if( !model.component( cmv.component_id.id ).is_active() ) + if( auto point = active_cmv_point( model, cmv ) ) { - continue; + return point; } - return model_cmv_point< geode::Section >( model, cmv ); } return std::nullopt; } - std::optional< geode::Point3D > model_unique_vertex_point( - const geode::BRep& model, - absl::Span< const geode::ComponentMeshVertex > cmvs ) + template < typename Model > + bool model_cmvs_are_colocated_on_point( const Model& model, + absl::Span< const geode::ComponentMeshVertex > cmvs, + const geode::Point< Model::dim >& point ) { for( const auto& cmv : cmvs ) { - if( !model.component( cmv.component_id.id ).is_active() ) - { - continue; - } - if( cmv.component_id.type - == geode::Block3D::component_type_static() ) + const auto cmv_point = active_cmv_point( model, cmv ); + if( cmv_point && !point.inexact_equal( cmv_point.value() ) ) { - return model.block( cmv.component_id.id ) - .mesh() - .point( cmv.vertex ); + return false; } - return model_cmv_point< geode::BRep >( model, cmv ); } - return std::nullopt; + return true; } } // namespace @@ -189,118 +154,138 @@ namespace geode class UniqueVerticesColocation< Model >::Impl { public: - Impl( const Model& model ) - : model_( model ), - active_uv_pointset_{ PointSet< Model::dim >::create() } - { - auto builder = - PointSetBuilder< Model::dim >::create( *active_uv_pointset_ ); - builder->create_vertices( model.nb_unique_vertices() ); - std::vector< bool > not_assigned( - model.nb_unique_vertices(), true ); - for( const auto unique_vertex_id : - Range{ model.nb_unique_vertices() } ) - { - if( auto point = model_unique_vertex_point( model, - model.component_mesh_vertices( unique_vertex_id ) ) ) - { - builder->set_point( unique_vertex_id, point.value() ); - not_assigned[unique_vertex_id] = false; - } - } - const auto old2new = builder->delete_vertices( not_assigned ); - for( const auto old_index : Indices{ old2new } ) - { - uv_to_active_uv_.map( old_index, old2new[old_index] ); - } - } + Impl( const Model& model ) : model_( model ) {} bool model_has_unique_vertices_linked_to_different_points() const { - for( const auto unique_vertex_id : - Range{ model_.nb_unique_vertices() } ) - { - const auto active_uv_id = - uv_to_active_uv_.in2out( unique_vertex_id ).at( 0 ); - if( active_uv_id == NO_ID ) - { - continue; - } - if( !model_cmvs_are_colocated_on_point( model_, - model_.component_mesh_vertices( unique_vertex_id ), - active_uv_pointset_->point( active_uv_id ) ) ) - { - return true; - } - } - return false; + return !unique_vertices_linked_to_different_points().empty(); } bool model_has_colocated_unique_vertices() const { const PointSetColocation< Model::dim > pointset_inspector{ - *active_uv_pointset_ + active_uv_pointset() }; - const auto has_colocation = - pointset_inspector.mesh_has_colocated_points(); - return has_colocation; + return pointset_inspector.mesh_has_colocated_points(); } void add_unique_vertices_linked_to_different_points( InspectionIssues< index_t >& vertices_issues ) const { for( const auto unique_vertex_id : - Range{ model_.nb_unique_vertices() } ) + unique_vertices_linked_to_different_points() ) { - const auto active_uv_id = - uv_to_active_uv_.in2out( unique_vertex_id ).at( 0 ); - if( active_uv_id == NO_ID ) - { - continue; - } - if( !model_cmvs_are_colocated_on_point( model_, - model_.component_mesh_vertices( unique_vertex_id ), - active_uv_pointset_->point( active_uv_id ) ) ) - { - vertices_issues.add_issue( unique_vertex_id, - absl::StrCat( "unique vertex ", unique_vertex_id, - " is linked to several mesh vertices on different " - "positions" ) ); - } + vertices_issues.add_issue( unique_vertex_id, + absl::StrCat( "unique vertex ", unique_vertex_id, + " is linked to several mesh vertices on different " + "positions" ) ); } } void add_colocated_unique_vertices_groups( InspectionIssues< std::vector< index_t > >& vertices_issues ) const { + const auto& pointset = active_uv_pointset(); const PointSetColocation< Model::dim > pointset_inspector{ - *active_uv_pointset_ + pointset }; const auto colocated_pts_groups = pointset_inspector.colocated_points_groups(); for( const auto& point_group : colocated_pts_groups.issues() ) { std::vector< index_t > fixed_point_group; + fixed_point_group.reserve( point_group.size() ); std::string point_group_string; for( const auto active_uv_index : point_group ) { const auto model_uv_index = - uv_to_active_uv_.out2in( active_uv_index ).at( 0 ); + active_uv_to_uv_[active_uv_index]; fixed_point_group.push_back( model_uv_index ); absl::StrAppend( &point_group_string, " ", model_uv_index ); } vertices_issues.add_issue( fixed_point_group, absl::StrCat( "unique vertices ", point_group_string, " are colocated at the position [", - active_uv_pointset_->point( point_group[0] ).string(), - "]" ) ); + pointset.point( point_group[0] ).string(), "]" ) ); + } + } + + private: + // Computed once, on first use + const PointSet< Model::dim >& active_uv_pointset() const + { + absl::call_once( once_, [this] { + initialize(); + } ); + return *active_uv_pointset_; + } + + void initialize() const + { + const auto nb_unique_vertices = model_.nb_unique_vertices(); + std::vector< std::optional< Point< Model::dim > > > points( + nb_unique_vertices ); + async::parallel_for( + async::irange( index_t{ 0 }, nb_unique_vertices ), + [this, &points]( index_t unique_vertex_id ) { + points[unique_vertex_id] = model_unique_vertex_point( + model_, + model_.component_mesh_vertices( unique_vertex_id ) ); + } ); + active_uv_pointset_ = PointSet< Model::dim >::create(); + auto builder = + PointSetBuilder< Model::dim >::create( *active_uv_pointset_ ); + uv_to_active_uv_.resize( nb_unique_vertices, NO_ID ); + for( const auto unique_vertex_id : Range{ nb_unique_vertices } ) + { + if( const auto& point = points[unique_vertex_id] ) + { + uv_to_active_uv_[unique_vertex_id] = + builder->create_point( point.value() ); + active_uv_to_uv_.push_back( unique_vertex_id ); + } + } + } + + std::vector< index_t > + unique_vertices_linked_to_different_points() const + { + const auto& pointset = active_uv_pointset(); + const auto nb_unique_vertices = model_.nb_unique_vertices(); + std::vector< char > linked_to_different_points( + nb_unique_vertices, false ); + async::parallel_for( + async::irange( index_t{ 0 }, nb_unique_vertices ), + [this, &pointset, &linked_to_different_points]( + index_t unique_vertex_id ) { + const auto active_uv_id = + uv_to_active_uv_[unique_vertex_id]; + if( active_uv_id == NO_ID ) + { + return; + } + linked_to_different_points[unique_vertex_id] = + !model_cmvs_are_colocated_on_point( model_, + model_.component_mesh_vertices( unique_vertex_id ), + pointset.point( active_uv_id ) ); + } ); + std::vector< index_t > result; + for( const auto unique_vertex_id : Range{ nb_unique_vertices } ) + { + if( linked_to_different_points[unique_vertex_id] ) + { + result.push_back( unique_vertex_id ); + } } + return result; } private: const Model& model_; - std::unique_ptr< PointSet< Model::dim > > active_uv_pointset_; - geode::GenericMapping< index_t > uv_to_active_uv_; + mutable absl::once_flag once_; + mutable std::unique_ptr< PointSet< Model::dim > > active_uv_pointset_; + mutable std::vector< index_t > uv_to_active_uv_; + mutable std::vector< index_t > active_uv_to_uv_; }; template < typename Model > From 3d6658cfcfa08960fd35affde6f8d50f5467f259 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Fri, 16 Oct 2026 08:43:24 +0200 Subject: [PATCH 12/12] fix(Inspector): address review on colocation and degeneration Remove the double space in colocated vertices messages and use EdgedCurve::edge_length for readability. Co-Authored-By: Claude Opus 5.5 --- .../criterion/colocation/component_meshes_colocation.cpp | 2 +- .../criterion/colocation/unique_vertices_colocation.cpp | 2 +- .../criterion/degeneration/edgedcurve_degeneration.cpp | 7 +------ .../inspection/criterion/internal/colocation_impl.cpp | 2 +- tests/inspection/test-brep-colocation-degeneration.cpp | 2 +- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp index 5fed3d2e..6e818ff3 100644 --- a/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/component_meshes_colocation.cpp @@ -120,7 +120,7 @@ namespace absl::StrAppend( &point_group_string, " ", point_index ); } issues->add_issue( colocated_points_group, - absl::StrCat( label, " has vertices ", point_group_string, + absl::StrCat( label, " has vertices", point_group_string, " colocated at position [", mesh.point( colocated_points_group[0] ).string(), "]" ) ); } diff --git a/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp b/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp index 0b2930e9..4e748d26 100644 --- a/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp +++ b/src/geode/inspector/inspection/criterion/colocation/unique_vertices_colocation.cpp @@ -204,7 +204,7 @@ namespace geode absl::StrAppend( &point_group_string, " ", model_uv_index ); } vertices_issues.add_issue( fixed_point_group, - absl::StrCat( "unique vertices ", point_group_string, + absl::StrCat( "unique vertices", point_group_string, " are colocated at the position [", pointset.point( point_group[0] ).string(), "]" ) ); } diff --git a/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp b/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp index bbcbdea0..40c184e9 100644 --- a/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp +++ b/src/geode/inspector/inspection/criterion/degeneration/edgedcurve_degeneration.cpp @@ -28,7 +28,6 @@ #include #include -#include #include @@ -59,11 +58,7 @@ namespace geode }; for( const auto edge_id : Range{ mesh_.nb_edges() } ) { - const auto& edge_vertices = mesh_.edge_vertices( edge_id ); - if( Vector< dimension >{ mesh_.point( edge_vertices[0] ), - mesh_.point( edge_vertices[1] ) } - .length2() - <= threshold * threshold ) + if( mesh_.edge_length( edge_id ) <= threshold ) { degenerated_edges_index.add_issue( edge_id, absl::StrCat( "degenerated edge ", edge_id, diff --git a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp index 42c9b4c2..beac75a1 100644 --- a/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp +++ b/src/geode/inspector/inspection/criterion/internal/colocation_impl.cpp @@ -107,7 +107,7 @@ namespace absl::StrAppend( &point_group_string, " ", point_index ); } groups_of_colocated_points.add_issue( colocated_points_group, - absl::StrCat( "vertices ", point_group_string, + absl::StrCat( "vertices", point_group_string, " are colocated at the position [", mesh_colocation_info .unique_points[mesh_colocation_info diff --git a/tests/inspection/test-brep-colocation-degeneration.cpp b/tests/inspection/test-brep-colocation-degeneration.cpp index a0d56c3d..33c51a8e 100644 --- a/tests/inspection/test-brep-colocation-degeneration.cpp +++ b/tests/inspection/test-brep-colocation-degeneration.cpp @@ -104,7 +104,7 @@ void check_colocation_without_unique_vertices() surface_id, ") colocated vertices" ) ) && absl::StrContains( message, absl::StrCat( "Surface ", surface_id, " (", surface_id, - ") has vertices 1 0 colocated at position [0 0 0]" ) ), + ") has vertices 1 0 colocated at position [0 0 0]" ) ), "[Test] Wrong colocation message: ", message ); const auto same_unique_vertex = build_colocated_surface( { 0, 0, 1, 2 } );