From ae57007807ba2e4efbca1482ee406db3638cb19d Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 17:45:21 +0200 Subject: [PATCH 01/12] fix(Topology): non-bijective cmv link issue reused the inexistant message Unique vertices non-bijectively linked to a component mesh vertex were reported as "linked to inexistant mesh vertex". Report the unique vertex the mesh vertex is actually linked to instead. Co-Authored-By: Claude Opus 5.5 --- .../inspection/topology/brep_topology.cpp | 19 ++-- .../inspection/topology/section_topology.cpp | 19 ++-- tests/inspection/CMakeLists.txt | 20 ++++ tests/inspection/test-brep-topology.cpp | 92 +++++++++++++++++++ tests/inspection/test-section-topology.cpp | 92 +++++++++++++++++++ 5 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 tests/inspection/test-brep-topology.cpp create mode 100644 tests/inspection/test-section-topology.cpp diff --git a/src/geode/inspector/inspection/topology/brep_topology.cpp b/src/geode/inspector/inspection/topology/brep_topology.cpp index 065126fb..6d594c2a 100644 --- a/src/geode/inspector/inspection/topology/brep_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_topology.cpp @@ -127,13 +127,20 @@ namespace geode cmv.string(), "]." ) ); continue; } - if( brep_.component( cmv.component_id.id ).is_active() - && brep_.unique_vertex( cmv ) != uv_id ) + if( !brep_.component( cmv.component_id.id ).is_active() ) { - linked_to_nonbijective_result.add_issue( - uv_id, absl::StrCat( "unique vertex ", uv_id, - " is linked to inexistant mesh vertex [", - cmv.string(), "]." ) ); + continue; + } + if( const auto linked_unique_vertex = + brep_.unique_vertex( cmv ); + linked_unique_vertex != uv_id ) + { + linked_to_nonbijective_result.add_issue( uv_id, + absl::StrCat( "unique vertex ", uv_id, + " is linked to mesh vertex [", cmv.string(), + "], but this mesh vertex is linked to " + "unique vertex ", + linked_unique_vertex, "." ) ); } } } diff --git a/src/geode/inspector/inspection/topology/section_topology.cpp b/src/geode/inspector/inspection/topology/section_topology.cpp index 8d3459f3..1e68e09b 100644 --- a/src/geode/inspector/inspection/topology/section_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_topology.cpp @@ -124,13 +124,20 @@ namespace geode cmv.string(), "]." ) ); continue; } - if( section_.component( cmv.component_id.id ).is_active() - && section_.unique_vertex( cmv ) != uv_id ) + if( !section_.component( cmv.component_id.id ).is_active() ) { - linked_to_nonbijective_result.add_issue( - uv_id, absl::StrCat( "unique vertex ", uv_id, - " is linked to inexistant mesh vertex [", - cmv.string(), "]." ) ); + continue; + } + if( const auto linked_unique_vertex = + section_.unique_vertex( cmv ); + linked_unique_vertex != uv_id ) + { + linked_to_nonbijective_result.add_issue( uv_id, + absl::StrCat( "unique vertex ", uv_id, + " is linked to mesh vertex [", cmv.string(), + "], but this mesh vertex is linked to " + "unique vertex ", + linked_unique_vertex, "." ) ); } } } diff --git a/tests/inspection/CMakeLists.txt b/tests/inspection/CMakeLists.txt index f51c1a42..b4d790e3 100755 --- a/tests/inspection/CMakeLists.txt +++ b/tests/inspection/CMakeLists.txt @@ -163,6 +163,16 @@ add_geode_test( ESSENTIAL ) +add_geode_test( + SOURCE "test-brep-topology.cpp" + DEPENDENCIES + OpenGeode::basic + OpenGeode::geometry + OpenGeode::mesh + OpenGeode::model + ${PROJECT_NAME}::inspection +) + add_geode_test( SOURCE "test-section.cpp" DEPENDENCIES @@ -172,4 +182,14 @@ add_geode_test( OpenGeode::model OpenGeode-GeosciencesIO::model ${PROJECT_NAME}::inspection +) + +add_geode_test( + SOURCE "test-section-topology.cpp" + DEPENDENCIES + OpenGeode::basic + OpenGeode::geometry + OpenGeode::mesh + OpenGeode::model + ${PROJECT_NAME}::inspection ) \ No newline at end of file diff --git a/tests/inspection/test-brep-topology.cpp b/tests/inspection/test-brep-topology.cpp new file mode 100644 index 00000000..4fba9934 --- /dev/null +++ b/tests/inspection/test-brep-topology.cpp @@ -0,0 +1,92 @@ +/* + * 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 +#include +#include + +#include + +namespace +{ + bool has_message( const geode::InspectionIssues< geode::index_t >& issues, + std::string_view text ) + { + return absl::StrContains( issues.string(), text ); + } + + void test_non_bijective_link_message() + { + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + const auto& corner = brep.corner( builder.add_corner() ); + builder.corner_mesh_builder( corner )->create_point( + geode::Point3D{ { 0, 0, 0 } } ); + builder.create_unique_vertices( 2 ); + builder.set_unique_vertex( { corner.component_id(), 0 }, 0 ); + // Corrupt the link: the corner vertex now points to unique vertex 1 + corner.mesh() + .vertex_attribute_manager() + .find_attribute< geode::VariableAttribute, geode::index_t >( + brep.unique_vertex_attribute_id() ) + ->set_value( 0, 1 ); + const geode::BRepTopologyInspector inspector{ brep }; + const auto result = inspector.inspect_brep_topology(); + const auto& non_bijective = + result.unique_vertices_nonbijectively_linked_to_cmv; + geode::OpenGeodeInspectorInspectionException::test( + non_bijective.nb_issues() == 1, + "[non bijective] Wrong number of non bijective issues" ); + geode::OpenGeodeInspectorInspectionException::test( + !has_message( non_bijective, "inexistant" ) + && has_message( non_bijective, "linked to unique vertex 1" ), + "[non bijective] Wrong message: ", non_bijective.string() ); + } +} // namespace + +int main() +{ + try + { + geode::OpenGeodeInspectorInspectionLibrary::initialize(); + test_non_bijective_link_message(); + geode::Logger::info( "TEST SUCCESS" ); + return 0; + } + catch( ... ) + { + return geode::geode_lippincott(); + } +} diff --git a/tests/inspection/test-section-topology.cpp b/tests/inspection/test-section-topology.cpp new file mode 100644 index 00000000..4893bf6d --- /dev/null +++ b/tests/inspection/test-section-topology.cpp @@ -0,0 +1,92 @@ +/* + * 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 +#include +#include + +#include + +namespace +{ + bool has_message( const geode::InspectionIssues< geode::index_t >& issues, + std::string_view text ) + { + return absl::StrContains( issues.string(), text ); + } + + void test_non_bijective_link_message() + { + geode::Section section; + geode::SectionBuilder builder{ section }; + const auto& corner = section.corner( builder.add_corner() ); + builder.corner_mesh_builder( corner )->create_point( + geode::Point2D{ { 0, 0 } } ); + builder.create_unique_vertices( 2 ); + builder.set_unique_vertex( { corner.component_id(), 0 }, 0 ); + // Corrupt the link: the corner vertex now points to unique vertex 1 + corner.mesh() + .vertex_attribute_manager() + .find_attribute< geode::VariableAttribute, geode::index_t >( + section.unique_vertex_attribute_id() ) + ->set_value( 0, 1 ); + const geode::SectionTopologyInspector inspector{ section }; + const auto result = inspector.inspect_section_topology(); + const auto& non_bijective = + result.unique_vertices_nonbijectively_linked_to_cmv; + geode::OpenGeodeInspectorInspectionException::test( + non_bijective.nb_issues() == 1, + "[non bijective] Wrong number of non bijective issues" ); + geode::OpenGeodeInspectorInspectionException::test( + !has_message( non_bijective, "inexistant" ) + && has_message( non_bijective, "linked to unique vertex 1" ), + "[non bijective] Wrong message: ", non_bijective.string() ); + } +} // namespace + +int main() +{ + try + { + geode::OpenGeodeInspectorInspectionLibrary::initialize(); + test_non_bijective_link_message(); + geode::Logger::info( "TEST SUCCESS" ); + return 0; + } + catch( ... ) + { + return geode::geode_lippincott(); + } +} From c5f3b09d1edb06f203459625ffe1b975bf24d656 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:23:04 +0200 Subject: [PATCH 02/12] fix(Topology): a failing check no longer empties a whole result category inspect_brep/section_topology caught any OpenGeodeException thrown by the corners, lines, surfaces or blocks inspection and silently left that whole result empty, so the model could be reported valid. A unique vertex linked to a missing or unregistered component (which the per vertex checks look up without has_*() guards, and unique_vertex() throws std::out_of_range for) triggered it. Each per unique vertex check now skips such a vertex (it is reported as linked to an inexistant mesh vertex), the validity check treats it as invalid, and other exceptions are no longer swallowed. Such links cannot be built through the public API (removing a component cleans them), only from corrupted files, so this is not covered by a test. Co-Authored-By: Claude Opus 5.5 --- .../topology/brep_blocks_topology.cpp | 77 ++++++++------- .../topology/brep_corners_topology.cpp | 53 +++++----- .../topology/brep_lines_topology.cpp | 63 +++++++----- .../topology/brep_surfaces_topology.cpp | 53 +++++----- .../inspection/topology/brep_topology.cpp | 96 ++++++++++++------- .../topology/section_corners_topology.cpp | 53 +++++----- .../topology/section_lines_topology.cpp | 66 +++++++------ .../topology/section_surfaces_topology.cpp | 36 ++++--- .../inspection/topology/section_topology.cpp | 88 +++++++++++------ 9 files changed, 351 insertions(+), 234 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index cc7d21f5..31e1cab8 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -961,43 +961,52 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - if( const auto problem_message = - unique_vertex_is_part_of_two_blocks_and_no_boundary_surface( - unique_vertex_id ) ) + try { - result - .unique_vertices_part_of_two_blocks_and_no_boundary_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - unique_vertex_block_cmvs_count_is_incorrect( - unique_vertex_id ) ) - { - result.unique_vertices_with_incorrect_block_cmvs_count - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_invalid_single_surface( - unique_vertex_id, not_boundary_surfaces ) ) - { - result.unique_vertices_linked_to_a_single_and_invalid_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_surface_with_wrong_relationships_to_block( - unique_vertex_id, not_boundary_surfaces, - dangling_surfaces ) ) - { - result - .unique_vertices_linked_to_surface_with_wrong_relationship_to_blocks - .add_issue( unique_vertex_id, problem_message.value() ); + if( const auto problem_message = + unique_vertex_is_part_of_two_blocks_and_no_boundary_surface( + unique_vertex_id ) ) + { + result + .unique_vertices_part_of_two_blocks_and_no_boundary_surface + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + unique_vertex_block_cmvs_count_is_incorrect( + unique_vertex_id ) ) + { + result.unique_vertices_with_incorrect_block_cmvs_count + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + vertex_is_part_of_invalid_single_surface( + unique_vertex_id, not_boundary_surfaces ) ) + { + result + .unique_vertices_linked_to_a_single_and_invalid_surface + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + vertex_is_part_of_surface_with_wrong_relationships_to_block( + unique_vertex_id, not_boundary_surfaces, + dangling_surfaces ) ) + { + result + .unique_vertices_linked_to_surface_with_wrong_relationship_to_blocks + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + vertex_is_part_of_invalid_multiple_surfaces( + unique_vertex_id ) ) + { + result.unique_vertex_linked_to_multiple_invalid_surfaces + .add_issue( unique_vertex_id, problem_message.value() ); + } } - if( const auto problem_message = - vertex_is_part_of_invalid_multiple_surfaces( - unique_vertex_id ) ) + catch( const std::exception& ) { - result.unique_vertex_linked_to_multiple_invalid_surfaces - .add_issue( unique_vertex_id, problem_message.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } for( const auto& block : brep_.active_blocks() ) diff --git a/src/geode/inspector/inspection/topology/brep_corners_topology.cpp b/src/geode/inspector/inspection/topology/brep_corners_topology.cpp index 0b848f7b..189cdbb4 100644 --- a/src/geode/inspector/inspection/topology/brep_corners_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_corners_topology.cpp @@ -335,31 +335,40 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - if( const auto problem_message = - unique_vertex_has_multiple_corners( unique_vertex_id ) ) + try { - result.unique_vertices_linked_to_multiple_corners.add_issue( - unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_multiply_embedded( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiply_embedded_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_not_internal_nor_boundary( unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_not_internal_nor_boundary_corner - .add_issue( unique_vertex_id, problem_message.value() ); + if( const auto problem_message = + unique_vertex_has_multiple_corners( unique_vertex_id ) ) + { + result.unique_vertices_linked_to_multiple_corners.add_issue( + unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_is_multiply_embedded( unique_vertex_id ) ) + { + result.unique_vertices_linked_to_multiply_embedded_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_is_not_internal_nor_boundary( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_not_internal_nor_boundary_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_is_part_of_line_but_not_boundary( + unique_vertex_id ) ) + { + result.unique_vertices_linked_to_not_boundary_line_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } } - if( const auto problem_message = - corner_is_part_of_line_but_not_boundary( - unique_vertex_id ) ) + catch( const std::exception& ) { - result.unique_vertices_linked_to_not_boundary_line_corner - .add_issue( unique_vertex_id, problem_message.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp index fac18113..934aacf3 100644 --- a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp @@ -610,35 +610,46 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_line( - unique_vertex_id ) ) + try { - result.unique_vertices_linked_to_a_line_with_invalid_embeddings - .add_issue( - unique_vertex_id, invalid_internal_topology.value() ); - } - if( const auto problem_message = - vertex_is_part_of_line_with_wrong_relationships_to_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_line_with_wrong_relationship_to_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_invalid_single_line( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_a_single_and_invalid_line - .add_issue( unique_vertex_id, problem_message.value() ); + if( const auto invalid_internal_topology = + vertex_is_part_of_invalid_embedded_line( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_line_with_invalid_embeddings + .add_issue( unique_vertex_id, + invalid_internal_topology.value() ); + } + if( const auto problem_message = + vertex_is_part_of_line_with_wrong_relationships_to_surface( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_line_with_wrong_relationship_to_surface + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + vertex_is_part_of_invalid_single_line( + unique_vertex_id ) ) + { + result.unique_vertices_linked_to_a_single_and_invalid_line + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto lines_but_is_not_corner = + vertex_has_lines_but_is_not_a_corner( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner + .add_issue( + unique_vertex_id, lines_but_is_not_corner.value() ); + } } - if( const auto lines_but_is_not_corner = - vertex_has_lines_but_is_not_a_corner( unique_vertex_id ) ) + catch( const std::exception& ) { - result - .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner - .add_issue( - unique_vertex_id, lines_but_is_not_corner.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp index c56ac006..54e675f7 100644 --- a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp @@ -466,31 +466,40 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_surface( - unique_vertex_id ) ) + try { - result - .unique_vertices_linked_to_a_surface_with_invalid_embbedings - .add_issue( - unique_vertex_id, invalid_internal_topology.value() ); - } - if( const auto invalid_multiple_surfaces = - vertex_is_part_of_invalid_multiple_surfaces( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_several_and_invalid_surfaces - .add_issue( - unique_vertex_id, invalid_multiple_surfaces.value() ); + if( const auto invalid_internal_topology = + vertex_is_part_of_invalid_embedded_surface( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_surface_with_invalid_embbedings + .add_issue( unique_vertex_id, + invalid_internal_topology.value() ); + } + if( const auto invalid_multiple_surfaces = + vertex_is_part_of_invalid_multiple_surfaces( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_several_and_invalid_surfaces + .add_issue( unique_vertex_id, + invalid_multiple_surfaces.value() ); + } + if( const auto line_and_not_on_surface_border = + vertex_is_part_of_line_and_not_on_surface_border( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border + .add_issue( unique_vertex_id, + line_and_not_on_surface_border.value() ); + } } - if( const auto line_and_not_on_surface_border = - vertex_is_part_of_line_and_not_on_surface_border( - unique_vertex_id ) ) + catch( const std::exception& ) { - result - .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border - .add_issue( unique_vertex_id, - line_and_not_on_surface_border.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/brep_topology.cpp b/src/geode/inspector/inspection/topology/brep_topology.cpp index 6d594c2a..52041ed7 100644 --- a/src/geode/inspector/inspection/topology/brep_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_topology.cpp @@ -69,6 +69,21 @@ namespace geode public: Impl( const BRep& brep ) : brep_( brep ) {} + // Unique vertex linked to the component mesh vertex, std::nullopt if + // the component is not registered in the unique vertices + std::optional< index_t > linked_unique_vertex_of( + const ComponentMeshVertex& cmv ) const + { + try + { + return brep_.unique_vertex( cmv ); + } + catch( const std::out_of_range& ) + { + return std::nullopt; + } + } + bool brep_unique_vertices_are_bijectively_linked_to_an_existing_component_vertex() const @@ -88,7 +103,7 @@ namespace geode return false; } if( brep_.component( cmv.component_id.id ).is_active() - && brep_.unique_vertex( cmv ) != uv_id ) + && linked_unique_vertex_of( cmv ) != uv_id ) { return false; } @@ -131,16 +146,24 @@ namespace geode { continue; } - if( const auto linked_unique_vertex = - brep_.unique_vertex( cmv ); - linked_unique_vertex != uv_id ) + const auto linked_unique_vertex = + linked_unique_vertex_of( cmv ); + if( !linked_unique_vertex ) + { + linked_to_inexistant_result.add_issue( + uv_id, absl::StrCat( "unique vertex ", uv_id, + " is linked to inexistant mesh vertex [", + cmv.string(), "]." ) ); + continue; + } + if( linked_unique_vertex.value() != uv_id ) { linked_to_nonbijective_result.add_issue( uv_id, absl::StrCat( "unique vertex ", uv_id, " is linked to mesh vertex [", cmv.string(), "], but this mesh vertex is linked to " "unique vertex ", - linked_unique_vertex, "." ) ); + linked_unique_vertex.value(), "." ) ); } } } @@ -165,15 +188,23 @@ namespace geode for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - if( !brep_topology_inspector.brep_corner_topology_is_valid( - unique_vertex_id ) - || !brep_topology_inspector.brep_lines_topology_is_valid( - unique_vertex_id ) - || !brep_topology_inspector.brep_surfaces_topology_is_valid( - unique_vertex_id ) - || !brep_topology_inspector.brep_blocks_topology_is_valid( - unique_vertex_id ) ) + try + { + if( !brep_topology_inspector.brep_corner_topology_is_valid( + unique_vertex_id ) + || !brep_topology_inspector + .brep_lines_topology_is_valid( unique_vertex_id ) + || !brep_topology_inspector + .brep_surfaces_topology_is_valid( unique_vertex_id ) + || !brep_topology_inspector + .brep_blocks_topology_is_valid( unique_vertex_id ) ) + { + return false; + } + } + catch( const std::exception& ) { + // Wrong links to components return false; } } @@ -184,29 +215,22 @@ namespace geode const BRepTopologyInspector& brep_topology_inspector ) const { BRepTopologyInspectionResult result; - try - { - async::parallel_invoke( - [&result, &brep_topology_inspector] { - result.corners = - brep_topology_inspector.inspect_corners_topology(); - }, - [&result, &brep_topology_inspector] { - result.lines = - brep_topology_inspector.inspect_lines_topology(); - }, - [&result, &brep_topology_inspector] { - result.surfaces = - brep_topology_inspector.inspect_surfaces_topology(); - }, - [&result, &brep_topology_inspector] { - result.blocks = - brep_topology_inspector.inspect_blocks(); - } ); - } - catch( OpenGeodeException& ) - { - } + async::parallel_invoke( + [&result, &brep_topology_inspector] { + result.corners = + brep_topology_inspector.inspect_corners_topology(); + }, + [&result, &brep_topology_inspector] { + result.lines = + brep_topology_inspector.inspect_lines_topology(); + }, + [&result, &brep_topology_inspector] { + result.surfaces = + brep_topology_inspector.inspect_surfaces_topology(); + }, + [&result, &brep_topology_inspector] { + result.blocks = brep_topology_inspector.inspect_blocks(); + } ); add_unique_vertices_with_wrong_cmv_link( result ); return result; } diff --git a/src/geode/inspector/inspection/topology/section_corners_topology.cpp b/src/geode/inspector/inspection/topology/section_corners_topology.cpp index fad7faa6..159fa171 100644 --- a/src/geode/inspector/inspection/topology/section_corners_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_corners_topology.cpp @@ -303,31 +303,40 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - if( const auto problem_message = - unique_vertex_has_multiple_corners( unique_vertex_id ) ) + try { - result.unique_vertices_linked_to_multiple_corners.add_issue( - unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_has_multiple_embeddings( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiple_internals_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_not_internal_nor_boundary( unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_not_internal_nor_boundary_corner - .add_issue( unique_vertex_id, problem_message.value() ); + if( const auto problem_message = + unique_vertex_has_multiple_corners( unique_vertex_id ) ) + { + result.unique_vertices_linked_to_multiple_corners.add_issue( + unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_has_multiple_embeddings( unique_vertex_id ) ) + { + result.unique_vertices_linked_to_multiple_internals_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_is_not_internal_nor_boundary( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_not_internal_nor_boundary_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } + if( const auto problem_message = + corner_is_part_of_line_but_not_boundary( + unique_vertex_id ) ) + { + result.unique_vertices_linked_to_not_boundary_line_corner + .add_issue( unique_vertex_id, problem_message.value() ); + } } - if( const auto problem_message = - corner_is_part_of_line_but_not_boundary( - unique_vertex_id ) ) + catch( const std::exception& ) { - result.unique_vertices_linked_to_not_boundary_line_corner - .add_issue( unique_vertex_id, problem_message.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/section_lines_topology.cpp b/src/geode/inspector/inspection/topology/section_lines_topology.cpp index f10755d3..a93f2ac2 100644 --- a/src/geode/inspector/inspection/topology/section_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_lines_topology.cpp @@ -368,36 +368,48 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - if( const auto boundary_nor_internal_line = - vertex_is_part_of_line_with_wrong_relationships_to_surface( - unique_vertex_id ) ) + try { - result - .unique_vertices_linked_to_line_with_wrong_relationship_to_surface - .add_issue( - unique_vertex_id, boundary_nor_internal_line.value() ); - } - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_line( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_a_line_with_invalid_embeddings - .add_issue( - unique_vertex_id, invalid_internal_topology.value() ); - } - if( const auto invalid_unique_line = - vertex_is_part_of_invalid_single_line( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_a_single_and_invalid_line - .add_issue( unique_vertex_id, invalid_unique_line.value() ); + if( const auto boundary_nor_internal_line = + vertex_is_part_of_line_with_wrong_relationships_to_surface( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_line_with_wrong_relationship_to_surface + .add_issue( unique_vertex_id, + boundary_nor_internal_line.value() ); + } + if( const auto invalid_internal_topology = + vertex_is_part_of_invalid_embedded_line( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_line_with_invalid_embeddings + .add_issue( unique_vertex_id, + invalid_internal_topology.value() ); + } + if( const auto invalid_unique_line = + vertex_is_part_of_invalid_single_line( + unique_vertex_id ) ) + { + result.unique_vertices_linked_to_a_single_and_invalid_line + .add_issue( + unique_vertex_id, invalid_unique_line.value() ); + } + if( const auto lines_but_is_not_corner = + vertex_has_lines_but_is_not_a_corner( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner + .add_issue( + unique_vertex_id, lines_but_is_not_corner.value() ); + } } - if( const auto lines_but_is_not_corner = - vertex_has_lines_but_is_not_a_corner( unique_vertex_id ) ) + catch( const std::exception& ) { - result - .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner - .add_issue( - unique_vertex_id, lines_but_is_not_corner.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp index dca5af47..f379d081 100644 --- a/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp @@ -238,23 +238,31 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_surface( - unique_vertex_id ) ) + try { - result - .unique_vertices_linked_to_a_surface_with_invalid_embbedings - .add_issue( - unique_vertex_id, invalid_internal_topology.value() ); + if( const auto invalid_internal_topology = + vertex_is_part_of_invalid_embedded_surface( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_surface_with_invalid_embbedings + .add_issue( unique_vertex_id, + invalid_internal_topology.value() ); + } + if( const auto line_and_not_on_surface_border = + vertex_is_part_of_line_and_not_on_surface_border( + unique_vertex_id ) ) + { + result + .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border + .add_issue( unique_vertex_id, + line_and_not_on_surface_border.value() ); + } } - if( const auto line_and_not_on_surface_border = - vertex_is_part_of_line_and_not_on_surface_border( - unique_vertex_id ) ) + catch( const std::exception& ) { - result - .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border - .add_issue( unique_vertex_id, - line_and_not_on_surface_border.value() ); + // Wrong links to components are reported by the unique + // vertices inspection } } return result; diff --git a/src/geode/inspector/inspection/topology/section_topology.cpp b/src/geode/inspector/inspection/topology/section_topology.cpp index 1e68e09b..acbbc62a 100644 --- a/src/geode/inspector/inspection/topology/section_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_topology.cpp @@ -66,6 +66,21 @@ namespace geode public: Impl( const Section& section ) : section_( section ) {} + // Unique vertex linked to the component mesh vertex, std::nullopt if + // the component is not registered in the unique vertices + std::optional< index_t > linked_unique_vertex_of( + const ComponentMeshVertex& cmv ) const + { + try + { + return section_.unique_vertex( cmv ); + } + catch( const std::out_of_range& ) + { + return std::nullopt; + } + } + bool section_unique_vertices_are_bijectively_linked_to_an_existing_component_vertex() const @@ -85,7 +100,7 @@ namespace geode return false; } if( section_.component( cmv.component_id.id ).is_active() - && section_.unique_vertex( cmv ) != uv_id ) + && linked_unique_vertex_of( cmv ) != uv_id ) { return false; } @@ -128,16 +143,24 @@ namespace geode { continue; } - if( const auto linked_unique_vertex = - section_.unique_vertex( cmv ); - linked_unique_vertex != uv_id ) + const auto linked_unique_vertex = + linked_unique_vertex_of( cmv ); + if( !linked_unique_vertex ) + { + linked_to_inexistant_result.add_issue( + uv_id, absl::StrCat( "unique vertex ", uv_id, + " is linked to inexistant mesh vertex [", + cmv.string(), "]." ) ); + continue; + } + if( linked_unique_vertex.value() != uv_id ) { linked_to_nonbijective_result.add_issue( uv_id, absl::StrCat( "unique vertex ", uv_id, " is linked to mesh vertex [", cmv.string(), "], but this mesh vertex is linked to " "unique vertex ", - linked_unique_vertex, "." ) ); + linked_unique_vertex.value(), "." ) ); } } } @@ -162,14 +185,23 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - if( !section_topology_inspector - .section_corner_topology_is_valid( unique_vertex_id ) - || !section_topology_inspector - .section_lines_topology_is_valid( unique_vertex_id ) - || !section_topology_inspector - .section_vertex_surfaces_topology_is_valid( - unique_vertex_id ) ) + try + { + if( !section_topology_inspector + .section_corner_topology_is_valid( + unique_vertex_id ) + || !section_topology_inspector + .section_lines_topology_is_valid( unique_vertex_id ) + || !section_topology_inspector + .section_vertex_surfaces_topology_is_valid( + unique_vertex_id ) ) + { + return false; + } + } + catch( const std::exception& ) { + // Wrong links to components return false; } } @@ -180,25 +212,19 @@ namespace geode const SectionTopologyInspector& section_topology_inspector ) const { SectionTopologyInspectionResult result; - try - { - async::parallel_invoke( - [&result, §ion_topology_inspector] { - result.corners = section_topology_inspector - .inspect_corners_topology(); - }, - [&result, §ion_topology_inspector] { - result.lines = - section_topology_inspector.inspect_lines_topology(); - }, - [&result, §ion_topology_inspector] { - result.surfaces = - section_topology_inspector.inspect_surfaces(); - } ); - } - catch( OpenGeodeException& ) - { - } + async::parallel_invoke( + [&result, §ion_topology_inspector] { + result.corners = + section_topology_inspector.inspect_corners_topology(); + }, + [&result, §ion_topology_inspector] { + result.lines = + section_topology_inspector.inspect_lines_topology(); + }, + [&result, §ion_topology_inspector] { + result.surfaces = + section_topology_inspector.inspect_surfaces(); + } ); add_unique_vertices_with_wrong_cmv_link( result ); return result; } From 306dd22970202749a597c66d76e90f27e64cb142 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:24:56 +0200 Subject: [PATCH 03/12] fix(Topology): lines embedded in a block get a valid message The invalid embedding messages called brep.surface() on the embedding even when it was a Block, which threw while building the message: the issue was lost (and, before the previous fix, the whole lines result). The message now uses the embedding component type and name. Co-Authored-By: Claude Opus 5.5 --- .../topology/brep_lines_topology.cpp | 31 +++++++----- tests/inspection/test-brep-topology.cpp | 47 +++++++++++++++++++ 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp index 934aacf3..240e2647 100644 --- a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp @@ -39,6 +39,18 @@ #include +namespace +{ + std::string embedding_label( + const geode::BRep& brep, const geode::ComponentID& embedding ) + { + const auto name = brep.component( embedding.id ).name(); + const auto id = embedding.id.string(); + return absl::StrCat( embedding.type.get(), " ", + name ? name.value() : id, " (", id, ")" ); + } +} // namespace + namespace geode { index_t BRepLinesTopologyInspectionResult::nb_issues() const @@ -185,12 +197,8 @@ namespace geode " is part of line ", line.name().value_or( line_id.string() ), " (", line_id.string(), - "), which is both boundary and embedded in " - "Surface ", - brep_.surface( embedding.id ) - .name() - .value_or( embedding.id.string() ), - " (", embedding.id.string() + ")." ); + "), which is both boundary and embedded in ", + embedding_label( brep_, embedding ), "." ); } if( embedding.type == Block3D::component_type_static() && !internal::brep_blocks_are_meshed( brep_ ) ) @@ -206,13 +214,10 @@ namespace geode return absl::StrCat( "unique vertex ", unique_vertex_index, " is part of Line ", line.name().value_or( line_id.string() ), " (", - line_id.string(), "), which is embedded in Surface ", - brep_.surface( embedding.id ) - .name() - .value_or( embedding.id.string() ), - " (", embedding.id.string(), - "), but the unique vertex is not linked to the " - "Surface mesh vertices." ); + line_id.string(), "), which is embedded in ", + embedding_label( brep_, embedding ), + ", but the unique vertex is not linked to the ", + embedding.type.get(), " mesh vertices." ); } } } diff --git a/tests/inspection/test-brep-topology.cpp b/tests/inspection/test-brep-topology.cpp index 4fba9934..215c6084 100644 --- a/tests/inspection/test-brep-topology.cpp +++ b/tests/inspection/test-brep-topology.cpp @@ -30,10 +30,14 @@ #include +#include #include +#include #include +#include #include +#include #include #include @@ -74,6 +78,48 @@ namespace && has_message( non_bijective, "linked to unique vertex 1" ), "[non bijective] Wrong message: ", non_bijective.string() ); } + + void test_line_embedded_in_block_message() + { + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + const auto& block = brep.block( builder.add_block() ); + auto block_builder = builder.block_mesh_builder( block ); + block_builder->create_point( geode::Point3D{ { 0, 0, 0 } } ); + block_builder->create_point( geode::Point3D{ { 4, 0, 0 } } ); + block_builder->create_point( geode::Point3D{ { 0, 4, 0 } } ); + block_builder->create_point( geode::Point3D{ { 0, 0, 4 } } ); + block_builder->create_polyhedron( { 0, 1, 2, 3 }, + { { 1, 3, 2 }, { 0, 2, 3 }, { 3, 1, 0 }, { 0, 1, 2 } } ); + const auto& line = brep.line( builder.add_line() ); + auto line_builder = builder.line_mesh_builder( line ); + line_builder->create_point( geode::Point3D{ { 1, 1, 1 } } ); + line_builder->create_point( geode::Point3D{ { 1, 1, 2 } } ); + line_builder->create_edge( 0, 1 ); + builder.add_line_block_internal_relationship( line, block ); + builder.create_unique_vertices( 6 ); + for( const auto v : geode::LRange{ 4 } ) + { + builder.set_unique_vertex( { block.component_id(), v }, v ); + } + // The line vertices are not linked to block mesh vertices + builder.set_unique_vertex( { line.component_id(), 0 }, 4 ); + builder.set_unique_vertex( { line.component_id(), 1 }, 5 ); + const geode::BRepTopologyInspector inspector{ brep }; + const auto result = inspector.inspect_brep_topology(); + const auto& issues = + result.lines + .unique_vertices_linked_to_a_line_with_invalid_embeddings; + geode::OpenGeodeInspectorInspectionException::test( + issues.nb_issues() == 2, + "[line in block] Wrong number of invalid embedding issues: ", + issues.string() ); + geode::OpenGeodeInspectorInspectionException::test( + has_message( issues, "which is embedded in Block" ) + && has_message( + issues, "not linked to the Block mesh vertices" ), + "[line in block] Wrong message: ", issues.string() ); + } } // namespace int main() @@ -82,6 +128,7 @@ int main() { geode::OpenGeodeInspectorInspectionLibrary::initialize(); test_non_bijective_link_message(); + test_line_embedded_in_block_message(); geode::Logger::info( "TEST SUCCESS" ); return 0; } From 597045ce5007eb91328c6b4320a4563e1549ad36 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:25:57 +0200 Subject: [PATCH 04/12] fix(Topology): no undefined behavior on model boundaries without surfaces closed_linked_surfaces called front()/pop() on an empty queue when the model boundaries had no items (undefined behavior). An empty list is now handled and empty model boundaries have nothing to check. The new test documents the expected result; without the fix the undefined behavior does not crash here. Co-Authored-By: Claude Opus 5.5 --- .../topology/brep_blocks_topology.cpp | 9 ++++++++ tests/inspection/test-brep-topology.cpp | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 31e1cab8..2d2b24ed 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -297,6 +297,10 @@ namespace const geode::BRep& brep, std::queue< geode::uuid >& to_process ) { std::vector< geode::uuid > processed; + if( to_process.empty() ) + { + return processed; + } const auto first_surface_uuid = to_process.front(); to_process.pop(); processed.emplace_back( first_surface_uuid ); @@ -504,6 +508,11 @@ namespace bool brep_boundaries_are_closed( const geode::BRep& brep ) { auto to_process = queue_with_model_boundaries( brep ); + if( to_process.empty() ) + { + // No boundary surface to check + return true; + } auto compute_closed_linked_surfaces = closed_linked_surfaces( brep, to_process ); return to_process.empty(); diff --git a/tests/inspection/test-brep-topology.cpp b/tests/inspection/test-brep-topology.cpp index 215c6084..eca8245b 100644 --- a/tests/inspection/test-brep-topology.cpp +++ b/tests/inspection/test-brep-topology.cpp @@ -120,6 +120,26 @@ namespace issues, "not linked to the Block mesh vertices" ), "[line in block] Wrong message: ", issues.string() ); } + + void test_empty_model_boundary() + { + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + builder.add_model_boundary(); + const auto& corner = brep.corner( builder.add_corner() ); + builder.corner_mesh_builder( corner )->create_point( + geode::Point3D{ { 0, 0, 0 } } ); + builder.create_unique_vertices( 1 ); + builder.set_unique_vertex( { corner.component_id(), 0 }, 0 ); + const geode::BRepTopologyInspector inspector{ brep }; + const auto result = inspector.inspect_brep_topology(); + geode::OpenGeodeInspectorInspectionException::test( + result.blocks.model_boundaries_dont_form_a_closed_surface + .nb_issues() + == 0, + "[empty model boundary] Empty model boundary should not be " + "reported" ); + } } // namespace int main() @@ -129,6 +149,7 @@ int main() geode::OpenGeodeInspectorInspectionLibrary::initialize(); test_non_bijective_link_message(); test_line_embedded_in_block_message(); + test_empty_model_boundary(); geode::Logger::info( "TEST SUCCESS" ); return 0; } From 1fab2fafe777fa20ec9591fb349e161dcc59ee40 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:26:34 +0200 Subject: [PATCH 05/12] fix(Topology): ambiguous ray tracing no longer loses the blocks inspection BRepRayTracing::is_point_inside_block throws on ambiguous rays; the exception escaped find_dangling_surfaces and emptied the whole blocks topology result. A few polygons of the surface are now tried, a surface which cannot be located is not reported as dangling, and only active blocks are considered. No deterministic ambiguous configuration is available for a test; results on all test models are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../topology/brep_blocks_topology.cpp | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 2d2b24ed..c772ea6b 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -255,19 +255,35 @@ namespace { continue; } - const auto polygon_barycenter = - surface_mesh.polygon_barycenter( 0 ); - bool is_dangling{ true }; - for( const auto& block : brep.blocks() ) + // Ray tracing may fail on ambiguous rays: try a few polygons, a + // surface which cannot be located is not reported as dangling + constexpr geode::index_t MAX_TRIALS{ 5 }; + std::optional< bool > is_dangling; + for( const auto polygon_id : geode::Range{ + std::min( surface_mesh.nb_polygons(), MAX_TRIALS ) } ) { - if( ray_tracing.is_point_inside_block( - polygon_barycenter, block ) ) + try { - is_dangling = false; + const auto polygon_barycenter = + surface_mesh.polygon_barycenter( polygon_id ); + is_dangling = true; + for( const auto& block : brep.active_blocks() ) + { + if( ray_tracing.is_point_inside_block( + polygon_barycenter, block ) ) + { + is_dangling = false; + break; + } + } break; } + catch( const geode::OpenGeodeException& ) + { + is_dangling.reset(); + } } - if( is_dangling ) + if( is_dangling.value_or( false ) ) { dangling_surfaces.push_back( surface_id ); } From bd830df6ade1af86168a351104d140e74e5ff7fb Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:26:56 +0200 Subject: [PATCH 06/12] fix(Topology): unlinked vertices are not merged when fusing block boundaries fuse_brep_surfaces_from_list merged vertices by unique vertex: every vertex without unique vertex (NO_ID) was fused into a single point, which broke the enclosing surface used to check block boundaries closure. Unlinked vertices now keep their own point. Not covered by a dedicated test: it requires a block with several boundary shells and unlinked boundary vertices. Co-Authored-By: Claude Opus 5.5 --- .../inspection/topology/brep_blocks_topology.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index c772ea6b..381d6e94 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -439,6 +439,14 @@ namespace { const auto unique_vertex = brep.unique_vertex( { surface.component_id(), polygon_vertex } ); + if( unique_vertex == geode::NO_ID ) + { + // Unlinked vertices cannot be merged with others + polygon_vertex = + polygonal_surface_builder->create_point( + surface_mesh.point( polygon_vertex ) ); + continue; + } if( !unique_vertices_to_polygonal_surface_vertices .has_mapping_input( unique_vertex ) ) { From fa2a3050cc506517c6bbeefb000b8bafbff0c493 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:27:50 +0200 Subject: [PATCH 07/12] fix(Topology): expected number of block cmvs no longer wraps around The predicted number of block component mesh vertices was computed with unsigned arithmetic: an invalid topology giving a negative prediction was reported as expecting about 4294967295 block cmvs. It is now computed signed. Results on all test models are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../topology/internal/expected_nb_cmvs.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp index 61eb4616..867d22d7 100644 --- a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp +++ b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp @@ -163,11 +163,13 @@ namespace } nb_line_boundary_to_several_internal_surfaces_cmvs++; } - geode::index_t predicted_nb_block_cmvs{ 1 + nb_internal_surface_cmvs }; + // Signed: an invalid topology can give a negative prediction + std::int64_t predicted_nb_block_cmvs{ 1 + nb_internal_surface_cmvs }; if( nb_boundary_surface_cmvs > 0 ) { predicted_nb_block_cmvs += - nb_boundary_surface_cmvs - 1 - nb_line_on_boundary_cmvs; + static_cast< std::int64_t >( nb_boundary_surface_cmvs ) - 1 + - nb_line_on_boundary_cmvs; } const geode::index_t nb_line_cmvs_to_remove{ nb_line_internal_to_internal_surface_cmvs + nb_free_line_cmvs @@ -179,6 +181,11 @@ namespace predicted_nb_block_cmvs += unique_vertex_cmvs.corner_cmvs.size(); predicted_nb_block_cmvs += nb_additional_corners_count / 2; } + const auto nb_block_cmvs_signed = + static_cast< std::int64_t >( nb_block_cmvs ); + const auto predicted_nb_block_cmvs_index = + static_cast< geode::index_t >( + std::max( predicted_nb_block_cmvs, std::int64_t{ 0 } ) ); if( nb_lines_on_several_boundaries % 2 == 1 ) { /// On one side of a topological non-manifold => 2 cases possible, @@ -193,14 +200,14 @@ namespace .string(), "] is on a topological non-manifold, which makes the correct " "number of block cmvs unsure." ) ); - if( nb_block_cmvs + if( nb_block_cmvs_signed == predicted_nb_block_cmvs + nb_lines_on_several_boundaries ) { return { nb_block_cmvs, std::nullopt }; } } - return { predicted_nb_block_cmvs, - nb_block_cmvs == predicted_nb_block_cmvs + return { predicted_nb_block_cmvs_index, + nb_block_cmvs_signed == predicted_nb_block_cmvs ? std::nullopt : std::make_optional( absl::StrCat( "unique vertex ", unique_vertex_id, " at position [", From b8aea3231dfb5032b4dddc64bdc4df2359d1ea5a Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:28:18 +0200 Subject: [PATCH 08/12] perf(Topology): no warning logged per unique vertex on a topological non-manifold expected_block_cmvs_and_error logged a formatted warning (point to string) for every unique vertex on one side of a topological non-manifold, from the parallel inspection tasks and again during validity checks. The inspection result already reports the vertices whose count is wrong. Co-Authored-By: Claude Opus 5.5 --- .../inspection/topology/internal/expected_nb_cmvs.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp index 867d22d7..dc26f874 100644 --- a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp +++ b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp @@ -191,15 +191,6 @@ namespace /// On one side of a topological non-manifold => 2 cases possible, /// depending on wether the topological non-manifold is on the /// interior or exterior of the block - geode::Logger::warning( absl::StrCat( - "[expected_block_cmvs_and_error] Unique vertex ", - unique_vertex_id, " at position [", - brep.block( unique_vertex_cmvs.block_cmvs[0].component_id.id ) - .mesh() - .point( unique_vertex_cmvs.block_cmvs[0].vertex ) - .string(), - "] is on a topological non-manifold, which makes the correct " - "number of block cmvs unsure." ) ); if( nb_block_cmvs_signed == predicted_nb_block_cmvs + nb_lines_on_several_boundaries ) { From b19afcc750b7c2f3c4978431c4387247fc8b2505 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:28:46 +0200 Subject: [PATCH 09/12] perf(Topology): do not copy the surfaces inspector in a per vertex lambda Co-Authored-By: Claude Opus 5.5 --- .../inspector/inspection/topology/brep_surfaces_topology.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp index 54e675f7..25032918 100644 --- a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp @@ -214,7 +214,7 @@ namespace geode const auto surface_uuids = internal::components_uuids( brep_, unique_vertex_index, Surface3D::component_type_static() ); if( absl::c_count_if( surface_uuids, - [*this]( const uuid& surface_id ) { + [this]( const uuid& surface_id ) { return brep_.surface( surface_id ).is_active(); } ) < 2 ) From 1173e62bdfaa20b13268079a9b0888f6c0ed3045 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 19:31:12 +0200 Subject: [PATCH 10/12] perf(Topology): only compute block facets around surface polygons For each polygon of each surface, component_mesh_polygons computed the line, surface and block polygons around it (the surface part going through polygons_around_vertex of every surface sharing the vertices), plus one unique_vertex lookup per polygon vertex, while only the block facets are checked. Only the block facets are now computed, surface unique vertices are fetched once, and the surface/block relationships (is_active, is_boundary, is_internal) are computed once per surface instead of once per polygon. inspect_brep_topology: fimiston 0.46s -> 0.15s, cva/brep_from_topo 0.37s -> 0.30s, corbi_tet_mesh 0.25s -> 0.18s (identical results). Co-Authored-By: Claude Opus 5.5 --- .../topology/brep_surfaces_topology.cpp | 164 +++++++++++++----- 1 file changed, 118 insertions(+), 46 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp index 25032918..72940520 100644 --- a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp @@ -26,6 +26,7 @@ #include #include +#include #include @@ -43,6 +44,101 @@ #include +namespace +{ + enum struct SURFACE_BLOCK_RELATION : char + { + inactive, + boundary, + internal, + none + }; + using SurfaceBlocksRelations = + absl::flat_hash_map< geode::uuid, SURFACE_BLOCK_RELATION >; + + SURFACE_BLOCK_RELATION surface_block_relation( const geode::BRep& brep, + const geode::Surface3D& surface, + const geode::Block3D& block, + SurfaceBlocksRelations& relations ) + { + const auto [it, inserted] = + relations.try_emplace( block.id(), SURFACE_BLOCK_RELATION::none ); + if( inserted ) + { + if( !block.is_active() ) + { + it->second = SURFACE_BLOCK_RELATION::inactive; + } + else if( brep.is_boundary( surface, block ) ) + { + it->second = SURFACE_BLOCK_RELATION::boundary; + } + else if( brep.is_internal( surface, block ) ) + { + it->second = SURFACE_BLOCK_RELATION::internal; + } + } + return it->second; + } + + std::optional< std::string > polygon_has_wrong_component_facets_around( + const geode::BRep& brep, + const geode::Surface3D& surface, + geode::index_t facet_index, + const geode::PolygonVertices& polygon_unique_vertices, + SurfaceBlocksRelations& relations ) + { + // Only the Block facets around the polygon are checked + const auto block_polygons = + geode::detail::block_component_mesh_polygons( + brep, polygon_unique_vertices ); + for( const auto& [block_id, block_facets] : block_polygons ) + { + const auto& block = brep.block( block_id ); + const auto relation = + surface_block_relation( brep, surface, block, relations ); + if( relation == SURFACE_BLOCK_RELATION::inactive ) + { + continue; + } + if( relation == SURFACE_BLOCK_RELATION::boundary ) + { + if( block_facets.size() != 1 ) + { + return absl::StrCat( "Surface ", + surface.name().value_or( surface.id().string() ), " (", + surface.id().string(), ") is boundary of block ", + block.name().value_or( block.id().string() ), " (", + block_id.string(), "), but has ", block_facets.size(), + " facets of this Block around it, it should be 1." ); + } + continue; + } + if( relation == SURFACE_BLOCK_RELATION::internal ) + { + if( block_facets.size() != 2 ) + { + return absl::StrCat( "Surface ", + surface.name().value_or( surface.id().string() ), " (", + surface.id().string(), ") is internal to block ", + block.name().value_or( block.id().string() ), " (", + block_id.string(), "), but has ", block_facets.size(), + " facets of this Block around it, it should be 2." ); + } + continue; + } + return absl::StrCat( "Surface ", + surface.name().value_or( surface.id().string() ), " (", + surface.id().string(), ") has facet with id ", facet_index, + " in common with Block", + block.name().value_or( block.id().string() ), " (", + block_id.string(), + "), but is neither boundary of nor internal to it." ); + } + return std::nullopt; + } +} // namespace + namespace geode { index_t BRepSurfacesTopologyInspectionResult::nb_issues() const @@ -364,49 +460,10 @@ namespace geode BRepSurfacesTopology::surface_facet_has_wrong_component_facets_around( const Surface3D& surface, const index_t facet_index ) const { - const auto cmp = component_mesh_polygons( brep_, surface, facet_index ); - for( const auto& [block_id, block_facets] : cmp.block_polygons ) - { - const auto& block = brep_.block( block_id ); - if( !block.is_active() ) - { - continue; - } - if( brep_.is_boundary( surface, block ) ) - { - if( block_facets.size() != 1 ) - { - return absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), " (", - surface.id().string(), ") is boundary of block ", - block.name().value_or( block.id().string() ), " (", - block_id.string(), "), but has ", block_facets.size(), - " facets of this Block around it, it should be 1." ); - } - continue; - } - if( brep_.is_internal( surface, block ) ) - { - if( block_facets.size() != 2 ) - { - return absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), " (", - surface.id().string(), ") is internal to block ", - block.name().value_or( block.id().string() ), " (", - block_id.string(), "), but has ", block_facets.size(), - " facets of this Block around it, it should be 2." ); - } - continue; - } - return absl::StrCat( "Surface ", - surface.name().value_or( surface.id().string() ), " (", - surface.id().string(), ") has facet with id ", facet_index, - " in common with Block", - block.name().value_or( block.id().string() ), " (", - block_id.string(), - "), but is neither boundary of nor internal to it." ); - } - return std::nullopt; + SurfaceBlocksRelations relations; + return polygon_has_wrong_component_facets_around( brep_, surface, + facet_index, polygon_unique_vertices( brep_, surface, facet_index ), + relations ); } BRepSurfacesTopologyInspectionResult @@ -447,11 +504,26 @@ namespace geode surface.name().value_or( surface.id().string() ), " (", surface.id().string(), ")" ) }; - for( const auto facet_id : Range{ surface.mesh().nb_polygons() } ) + const auto& mesh = surface.mesh(); + std::vector< index_t > unique_vertices( mesh.nb_vertices() ); + for( const auto v : Indices{ unique_vertices } ) + { + unique_vertices[v] = + brep_.unique_vertex( { surface.component_id(), v } ); + } + SurfaceBlocksRelations relations; + for( const auto facet_id : Range{ mesh.nb_polygons() } ) { + auto polygon_unique_vertices = + mesh.polygon_vertices( facet_id ); + for( auto& vertex : polygon_unique_vertices ) + { + vertex = unique_vertices[vertex]; + } if( const auto problem_message = - surface_facet_has_wrong_component_facets_around( - surface, facet_id ) ) + polygon_has_wrong_component_facets_around( brep_, + surface, facet_id, polygon_unique_vertices, + relations ) ) { surface_facets_with_wrong_cme.add_issue( facet_id, problem_message.value() ); From d02440303855507e5465692a8fee56939ec50aa9 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 15 Oct 2026 19:11:47 +0200 Subject: [PATCH 11/12] fix(Topology): run each unique vertex check independently A check throwing on a wrong link to a component no longer prevents the following checks on the same unique vertex from filling the result. Catch std::exception when looking up the unique vertex of a component mesh vertex, so it does not depend on the exact exception type. Co-Authored-By: Claude Opus 5.5 --- .../topology/internal/topology_helpers.hpp | 24 ++++++ .../topology/brep_blocks_topology.cpp | 80 ++++++++----------- .../topology/brep_corners_topology.cpp | 59 ++++++-------- .../topology/brep_lines_topology.cpp | 67 ++++++---------- .../topology/brep_surfaces_topology.cpp | 55 +++++-------- .../inspection/topology/brep_topology.cpp | 2 +- .../topology/section_corners_topology.cpp | 59 ++++++-------- .../topology/section_lines_topology.cpp | 69 ++++++---------- .../topology/section_surfaces_topology.cpp | 40 ++++------ .../inspection/topology/section_topology.cpp | 2 +- tests/inspection/test-brep-topology.cpp | 50 +++++++++++- 11 files changed, 242 insertions(+), 265 deletions(-) diff --git a/include/geode/inspector/inspection/topology/internal/topology_helpers.hpp b/include/geode/inspector/inspection/topology/internal/topology_helpers.hpp index 3b26328d..c21cf92c 100644 --- a/include/geode/inspector/inspection/topology/internal/topology_helpers.hpp +++ b/include/geode/inspector/inspection/topology/internal/topology_helpers.hpp @@ -107,5 +107,29 @@ namespace geode } return result; } + + // Adds the issue found by the check on the unique vertex, if any. + // Each check is protected on its own: if it throws because of wrong + // links between the unique vertex and the components (reported by the + // unique vertices inspection), the other checks still fill the result. + template < typename Check > + void add_unique_vertex_issue( InspectionIssues< index_t >& issues, + index_t unique_vertex_id, + const Check& check ) + { + try + { + if( const auto problem_message = check() ) + { + issues.add_issue( + unique_vertex_id, problem_message.value() ); + } + } + catch( const std::exception& ) + { + // Wrong links to components are reported by the unique + // vertices inspection + } + } } // namespace internal } // namespace geode \ No newline at end of file diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 381d6e94..c0b0404d 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -994,53 +994,39 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - try - { - if( const auto problem_message = - unique_vertex_is_part_of_two_blocks_and_no_boundary_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_part_of_two_blocks_and_no_boundary_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - unique_vertex_block_cmvs_count_is_incorrect( - unique_vertex_id ) ) - { - result.unique_vertices_with_incorrect_block_cmvs_count - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_invalid_single_surface( - unique_vertex_id, not_boundary_surfaces ) ) - { - result - .unique_vertices_linked_to_a_single_and_invalid_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_surface_with_wrong_relationships_to_block( - unique_vertex_id, not_boundary_surfaces, - dangling_surfaces ) ) - { - result - .unique_vertices_linked_to_surface_with_wrong_relationship_to_blocks - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_invalid_multiple_surfaces( - unique_vertex_id ) ) - { - result.unique_vertex_linked_to_multiple_invalid_surfaces - .add_issue( unique_vertex_id, problem_message.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result + .unique_vertices_part_of_two_blocks_and_no_boundary_surface, + unique_vertex_id, [&] { + return unique_vertex_is_part_of_two_blocks_and_no_boundary_surface( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_with_incorrect_block_cmvs_count, + unique_vertex_id, [&] { + return unique_vertex_block_cmvs_count_is_incorrect( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_a_single_and_invalid_surface, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_single_surface( + unique_vertex_id, not_boundary_surfaces ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_surface_with_wrong_relationship_to_blocks, + unique_vertex_id, [&] { + return vertex_is_part_of_surface_with_wrong_relationships_to_block( + unique_vertex_id, not_boundary_surfaces, + dangling_surfaces ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertex_linked_to_multiple_invalid_surfaces, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_multiple_surfaces( + unique_vertex_id ); + } ); } for( const auto& block : brep_.active_blocks() ) { diff --git a/src/geode/inspector/inspection/topology/brep_corners_topology.cpp b/src/geode/inspector/inspection/topology/brep_corners_topology.cpp index 189cdbb4..bd03144b 100644 --- a/src/geode/inspector/inspection/topology/brep_corners_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_corners_topology.cpp @@ -335,41 +335,30 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - try - { - if( const auto problem_message = - unique_vertex_has_multiple_corners( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiple_corners.add_issue( - unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_multiply_embedded( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiply_embedded_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_not_internal_nor_boundary( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_not_internal_nor_boundary_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_part_of_line_but_not_boundary( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_not_boundary_line_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_multiple_corners, + unique_vertex_id, [&] { + return unique_vertex_has_multiple_corners( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_multiply_embedded_corner, + unique_vertex_id, [&] { + return corner_is_multiply_embedded( unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_not_internal_nor_boundary_corner, + unique_vertex_id, [&] { + return corner_is_not_internal_nor_boundary( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_not_boundary_line_corner, + unique_vertex_id, [&] { + return corner_is_part_of_line_but_not_boundary( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp index 240e2647..7576ed64 100644 --- a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp @@ -615,47 +615,32 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - try - { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_line( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_line_with_invalid_embeddings - .add_issue( unique_vertex_id, - invalid_internal_topology.value() ); - } - if( const auto problem_message = - vertex_is_part_of_line_with_wrong_relationships_to_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_line_with_wrong_relationship_to_surface - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - vertex_is_part_of_invalid_single_line( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_a_single_and_invalid_line - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto lines_but_is_not_corner = - vertex_has_lines_but_is_not_a_corner( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner - .add_issue( - unique_vertex_id, lines_but_is_not_corner.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_a_line_with_invalid_embeddings, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_embedded_line( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_line_with_wrong_relationship_to_surface, + unique_vertex_id, [&] { + return vertex_is_part_of_line_with_wrong_relationships_to_surface( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_a_single_and_invalid_line, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_single_line( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner, + unique_vertex_id, [&] { + return vertex_has_lines_but_is_not_a_corner( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp index 72940520..8ee1a08a 100644 --- a/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_surfaces_topology.cpp @@ -538,41 +538,26 @@ namespace geode } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) { - try - { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_surface_with_invalid_embbedings - .add_issue( unique_vertex_id, - invalid_internal_topology.value() ); - } - if( const auto invalid_multiple_surfaces = - vertex_is_part_of_invalid_multiple_surfaces( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_several_and_invalid_surfaces - .add_issue( unique_vertex_id, - invalid_multiple_surfaces.value() ); - } - if( const auto line_and_not_on_surface_border = - vertex_is_part_of_line_and_not_on_surface_border( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border - .add_issue( unique_vertex_id, - line_and_not_on_surface_border.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_a_surface_with_invalid_embbedings, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_embedded_surface( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_several_and_invalid_surfaces, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_multiple_surfaces( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border, + unique_vertex_id, [&] { + return vertex_is_part_of_line_and_not_on_surface_border( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/brep_topology.cpp b/src/geode/inspector/inspection/topology/brep_topology.cpp index 52041ed7..869104ee 100644 --- a/src/geode/inspector/inspection/topology/brep_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_topology.cpp @@ -78,7 +78,7 @@ namespace geode { return brep_.unique_vertex( cmv ); } - catch( const std::out_of_range& ) + catch( const std::exception& ) { return std::nullopt; } diff --git a/src/geode/inspector/inspection/topology/section_corners_topology.cpp b/src/geode/inspector/inspection/topology/section_corners_topology.cpp index 159fa171..48df259e 100644 --- a/src/geode/inspector/inspection/topology/section_corners_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_corners_topology.cpp @@ -303,41 +303,30 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - try - { - if( const auto problem_message = - unique_vertex_has_multiple_corners( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiple_corners.add_issue( - unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_has_multiple_embeddings( unique_vertex_id ) ) - { - result.unique_vertices_linked_to_multiple_internals_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_not_internal_nor_boundary( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_not_internal_nor_boundary_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - if( const auto problem_message = - corner_is_part_of_line_but_not_boundary( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_not_boundary_line_corner - .add_issue( unique_vertex_id, problem_message.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_multiple_corners, + unique_vertex_id, [&] { + return unique_vertex_has_multiple_corners( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_multiple_internals_corner, + unique_vertex_id, [&] { + return corner_has_multiple_embeddings( unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_not_internal_nor_boundary_corner, + unique_vertex_id, [&] { + return corner_is_not_internal_nor_boundary( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_not_boundary_line_corner, + unique_vertex_id, [&] { + return corner_is_part_of_line_but_not_boundary( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/section_lines_topology.cpp b/src/geode/inspector/inspection/topology/section_lines_topology.cpp index a93f2ac2..06c85c31 100644 --- a/src/geode/inspector/inspection/topology/section_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_lines_topology.cpp @@ -368,49 +368,32 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - try - { - if( const auto boundary_nor_internal_line = - vertex_is_part_of_line_with_wrong_relationships_to_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_line_with_wrong_relationship_to_surface - .add_issue( unique_vertex_id, - boundary_nor_internal_line.value() ); - } - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_line( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_line_with_invalid_embeddings - .add_issue( unique_vertex_id, - invalid_internal_topology.value() ); - } - if( const auto invalid_unique_line = - vertex_is_part_of_invalid_single_line( - unique_vertex_id ) ) - { - result.unique_vertices_linked_to_a_single_and_invalid_line - .add_issue( - unique_vertex_id, invalid_unique_line.value() ); - } - if( const auto lines_but_is_not_corner = - vertex_has_lines_but_is_not_a_corner( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner - .add_issue( - unique_vertex_id, lines_but_is_not_corner.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_line_with_wrong_relationship_to_surface, + unique_vertex_id, [&] { + return vertex_is_part_of_line_with_wrong_relationships_to_surface( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_a_line_with_invalid_embeddings, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_embedded_line( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result.unique_vertices_linked_to_a_single_and_invalid_line, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_single_line( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_several_lines_but_not_linked_to_a_corner, + unique_vertex_id, [&] { + return vertex_has_lines_but_is_not_a_corner( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp b/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp index f379d081..75e63ba3 100644 --- a/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_surfaces_topology.cpp @@ -238,32 +238,20 @@ namespace geode for( const auto unique_vertex_id : Range{ section_.nb_unique_vertices() } ) { - try - { - if( const auto invalid_internal_topology = - vertex_is_part_of_invalid_embedded_surface( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_surface_with_invalid_embbedings - .add_issue( unique_vertex_id, - invalid_internal_topology.value() ); - } - if( const auto line_and_not_on_surface_border = - vertex_is_part_of_line_and_not_on_surface_border( - unique_vertex_id ) ) - { - result - .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border - .add_issue( unique_vertex_id, - line_and_not_on_surface_border.value() ); - } - } - catch( const std::exception& ) - { - // Wrong links to components are reported by the unique - // vertices inspection - } + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_a_surface_with_invalid_embbedings, + unique_vertex_id, [&] { + return vertex_is_part_of_invalid_embedded_surface( + unique_vertex_id ); + } ); + internal::add_unique_vertex_issue( + result + .unique_vertices_linked_to_a_line_but_is_not_on_a_surface_border, + unique_vertex_id, [&] { + return vertex_is_part_of_line_and_not_on_surface_border( + unique_vertex_id ); + } ); } return result; } diff --git a/src/geode/inspector/inspection/topology/section_topology.cpp b/src/geode/inspector/inspection/topology/section_topology.cpp index acbbc62a..b348278b 100644 --- a/src/geode/inspector/inspection/topology/section_topology.cpp +++ b/src/geode/inspector/inspection/topology/section_topology.cpp @@ -75,7 +75,7 @@ namespace geode { return section_.unique_vertex( cmv ); } - catch( const std::out_of_range& ) + catch( const std::exception& ) { return std::nullopt; } diff --git a/tests/inspection/test-brep-topology.cpp b/tests/inspection/test-brep-topology.cpp index eca8245b..4665ff4e 100644 --- a/tests/inspection/test-brep-topology.cpp +++ b/tests/inspection/test-brep-topology.cpp @@ -21,6 +21,8 @@ * */ +#include + #include #include @@ -45,6 +47,13 @@ namespace { + class CorruptingBRepBuilder : public geode::BRepBuilder + { + public: + using geode::BRepBuilder::BRepBuilder; + using geode::CornersBuilder3D::delete_corner; + }; + bool has_message( const geode::InspectionIssues< geode::index_t >& issues, std::string_view text ) { @@ -121,11 +130,49 @@ namespace "[line in block] Wrong message: ", issues.string() ); } + void test_checks_independent_of_failing_check() + { + geode::BRep brep; + CorruptingBRepBuilder builder{ brep }; + builder.create_unique_vertices( 1 ); + std::vector< geode::uuid > corner_ids; + for( const auto c : geode::LRange{ 3 } ) + { + const auto& corner = brep.corner( builder.add_corner() ); + builder.corner_mesh_builder( corner )->create_point( + geode::Point3D{ { static_cast< double >( c ), 0, 0 } } ); + builder.set_unique_vertex( { corner.component_id(), 0 }, 0 ); + corner_ids.push_back( corner.id() ); + } + // The unique vertex keeps a link to the deleted corner: checks + // reaching this link throw + builder.delete_corner( brep.corner( corner_ids[2] ) ); + const geode::BRepTopologyInspector inspector{ brep }; + const auto result = inspector.inspect_brep_topology(); + geode::OpenGeodeInspectorInspectionException::test( + result.unique_vertices_linked_to_inexistant_cmv.nb_issues() == 1, + "[failing check] Link to deleted corner should be reported" ); + geode::OpenGeodeInspectorInspectionException::test( + result.corners.unique_vertices_linked_to_multiple_corners + .nb_issues() + == 1, + "[failing check] Multiple corners should be reported" ); + // The multiply embedded check throws on the deleted corner, the + // following check must still be run + geode::OpenGeodeInspectorInspectionException::test( + result.corners + .unique_vertices_linked_to_not_internal_nor_boundary_corner + .nb_issues() + == 1, + "[failing check] Corner neither internal nor boundary should be " + "reported" ); + } + void test_empty_model_boundary() { geode::BRep brep; geode::BRepBuilder builder{ brep }; - builder.add_model_boundary(); + std::ignore = builder.add_model_boundary(); const auto& corner = brep.corner( builder.add_corner() ); builder.corner_mesh_builder( corner )->create_point( geode::Point3D{ { 0, 0, 0 } } ); @@ -150,6 +197,7 @@ int main() test_non_bijective_link_message(); test_line_embedded_in_block_message(); test_empty_model_boundary(); + test_checks_independent_of_failing_check(); geode::Logger::info( "TEST SUCCESS" ); return 0; } From a98d60171c41976d7e30f00c85dc3476e5d94164 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 15 Oct 2026 19:11:47 +0200 Subject: [PATCH 12/12] fix(Topology): restore warning on topological non-manifold block cmvs count The expected number of block cmvs formula is known to fail in some non manifold configurations, keep warning the user about it. Co-Authored-By: Claude Opus 5.5 --- .../inspection/topology/internal/expected_nb_cmvs.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp index dc26f874..867d22d7 100644 --- a/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp +++ b/src/geode/inspector/inspection/topology/internal/expected_nb_cmvs.cpp @@ -191,6 +191,15 @@ namespace /// On one side of a topological non-manifold => 2 cases possible, /// depending on wether the topological non-manifold is on the /// interior or exterior of the block + geode::Logger::warning( absl::StrCat( + "[expected_block_cmvs_and_error] Unique vertex ", + unique_vertex_id, " at position [", + brep.block( unique_vertex_cmvs.block_cmvs[0].component_id.id ) + .mesh() + .point( unique_vertex_cmvs.block_cmvs[0].vertex ) + .string(), + "] is on a topological non-manifold, which makes the correct " + "number of block cmvs unsure." ) ); if( nb_block_cmvs_signed == predicted_nb_block_cmvs + nb_lines_on_several_boundaries ) {