From 5836701e71fb8d01984c5b9969d586aa1c7deee3 Mon Sep 17 00:00:00 2001 From: MelchiorSchuh Date: Wed, 17 Jun 2026 15:35:53 +0200 Subject: [PATCH 1/6] fix(BlockTopology): Fixed call to checks on block surface boundaries --- .../topology/brep_blocks_topology.hpp | 15 +++-- .../topology/brep_blocks_topology.cpp | 67 ++++++++++--------- .../topology/brep_lines_topology.cpp | 20 +++--- 3 files changed, 57 insertions(+), 45 deletions(-) diff --git a/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp b/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp index aedb3362..5297ad8f 100644 --- a/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp +++ b/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp @@ -49,11 +49,18 @@ namespace geode "Blocks without mesh (uuid listed)" }; InspectionIssues< uuid > wrong_block_boundary_surface{ - "Surfaces incorrectly bounding Blocks (uuids listed)" + "Surfaces boundary of Blocks but not linked to another boundary " + "Surface by a line (uuids listed)" }; InspectionIssuesMap< index_t > blocks_not_linked_to_a_unique_vertex{ "Blocks containing mesh vertices not linked to unique vertices" }; + InspectionIssues< uuid > blocks_with_not_closed_boundary_surfaces{ + "Blocks with non-closed boundary Surfaces" + }; + InspectionIssues< index_t > model_boundaries_dont_form_a_closed_surface{ + "ModelBoundaries do not form a valid closed surface" + }; InspectionIssues< index_t > unique_vertices_part_of_two_blocks_and_no_boundary_surface{ "unique vertices shared by two Blocks without a boundary " @@ -73,12 +80,6 @@ namespace geode unique_vertices_linked_to_a_single_and_invalid_surface{ "unique vertices linked to a single Surface that is invalid" }; - InspectionIssues< uuid > blocks_with_not_closed_boundary_surfaces{ - "Blocks with non-closed boundary Surfaces" - }; - InspectionIssues< index_t > model_boundaries_dont_form_a_closed_surface{ - "ModelBoundaries are not valid" - }; InspectionIssues< index_t > unique_vertex_linked_to_multiple_invalid_surfaces{ "unique vertices linked to multiple Surfaces in an invalid way" diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 8d610984..64716a30 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -87,11 +87,8 @@ namespace { for( const auto& incident_surface : brep.incidences( line ) ) { - if( incident_surface.id() == boundary_surface_id ) - { - continue; - } - if( brep.is_boundary( incident_surface, block ) ) + if( incident_surface.id() != boundary_surface_id + && brep.is_boundary( incident_surface, block ) ) { return true; } @@ -99,7 +96,7 @@ namespace return false; } - bool surface_should_not_be_boundary_to_block( + bool surface_boundaries_are_not_linked_to_another_block_boundary( const geode::Surface3D& surface, const geode::BRep& brep, const geode::Block3D& block ) @@ -379,10 +376,11 @@ namespace geode { index_t BRepBlocksTopologyInspectionResult::nb_issues() const { - return wrong_block_boundary_surface.nb_issues() - + model_boundaries_dont_form_a_closed_surface.nb_issues() - + some_blocks_not_meshed.nb_issues() + return some_blocks_not_meshed.nb_issues() + + wrong_block_boundary_surface.nb_issues() + blocks_not_linked_to_a_unique_vertex.nb_issues() + + blocks_with_not_closed_boundary_surfaces.nb_issues() + + model_boundaries_dont_form_a_closed_surface.nb_issues() + unique_vertices_part_of_two_blocks_and_no_boundary_surface .nb_issues() + unique_vertices_with_incorrect_block_cmvs_count.nb_issues() @@ -396,23 +394,28 @@ namespace geode std::string BRepBlocksTopologyInspectionResult::string() const { std::string message; + if( some_blocks_not_meshed.nb_issues() != 0 ) + { + absl::StrAppend( &message, some_blocks_not_meshed.string(), "\n" ); + } if( wrong_block_boundary_surface.nb_issues() != 0 ) { absl::StrAppend( &message, wrong_block_boundary_surface.string() ); } - if( model_boundaries_dont_form_a_closed_surface.nb_issues() != 0 ) + if( blocks_not_linked_to_a_unique_vertex.nb_issues() != 0 ) { - absl::StrAppend( &message, - model_boundaries_dont_form_a_closed_surface.string() ); + absl::StrAppend( + &message, blocks_not_linked_to_a_unique_vertex.string() ); } - if( some_blocks_not_meshed.nb_issues() != 0 ) + if( blocks_with_not_closed_boundary_surfaces.nb_issues() != 0 ) { - absl::StrAppend( &message, some_blocks_not_meshed.string(), "\n" ); + absl::StrAppend( + &message, blocks_with_not_closed_boundary_surfaces.string() ); } - if( blocks_not_linked_to_a_unique_vertex.nb_issues() != 0 ) + if( model_boundaries_dont_form_a_closed_surface.nb_issues() != 0 ) { - absl::StrAppend( - &message, blocks_not_linked_to_a_unique_vertex.string() ); + absl::StrAppend( &message, + model_boundaries_dont_form_a_closed_surface.string() ); } if( unique_vertices_part_of_two_blocks_and_no_boundary_surface .nb_issues() @@ -811,21 +814,13 @@ namespace geode result.blocks_not_linked_to_a_unique_vertex.add_issues_to_map( block.id(), std::move( block_result ) ); } - if( verify_blocks_boundaries( brep_, block ) ) - { - result.blocks_with_not_closed_boundary_surfaces.add_issue( - block.id(), - absl::StrCat( "Block ", - block.name().value_or( block.id().string() ), " (", - block.id().string(), ") boundaries are valid." ) ); - } } if( brep_.nb_model_boundaries() != 0 ) { if( !are_brep_boundaries_closed( brep_ ) ) { result.model_boundaries_dont_form_a_closed_surface.add_issue( - 0, "boundaries don't form a valid closed surface." ); + 0, "ModelBoundaries don't form a valid closed surface." ); } } for( const auto unique_vertex_id : Range{ brep_.nb_unique_vertices() } ) @@ -871,24 +866,36 @@ namespace geode } for( const auto& block : brep_.active_blocks() ) { + if( !verify_blocks_boundaries( brep_, block ) ) + { + result.blocks_with_not_closed_boundary_surfaces.add_issue( + block.id(), + absl::StrCat( "Block ", + block.name().value_or( block.id().string() ), " (", + block.id().string(), + ") boundaries are not closed or form several closed " + "volumes." ) ); + } for( const auto& surface : brep_.boundaries( block ) ) { if( !surface.is_active() ) { continue; } - if( surface_should_not_be_boundary_to_block( + if( surface_boundaries_are_not_linked_to_another_block_boundary( surface, brep_, block ) ) { result.wrong_block_boundary_surface.add_issue( surface.id(), absl::StrCat( "Surface ", surface.name().value_or( surface.id().string() ), " (", surface.id().string(), - ") should not be boundary of Block ", + ") is boundary of Block ", block.name().value_or( block.id().string() ), " (", block.id().string(), - ") : it has a boundary Line not incident to any " - "other Block boundary Surface." ) ); + ") but it has a boundary Line not incident to any " + "other Block boundary Surface. Either the surface " + "shouldn't be boundary, or there is a hole in the " + "block boundaries" ) ); } } } diff --git a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp index b2e71455..239e7891 100644 --- a/src/geode/inspector/inspection/topology/brep_lines_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_lines_topology.cpp @@ -340,11 +340,11 @@ 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 should be boundary of Surface ", + line.id().string(), "), which is boundary of Surface ", incident_surface.name().value_or( incident_surface.id().string() ), - " (", incident_surface.id().string(), ")" ); + " (", incident_surface.id().string(), + "), but Line edge 0 is not part of this surface" ); } if( cme.surface_edges.at( incident_surface.id() ).size() != 1 ) { @@ -352,10 +352,12 @@ namespace geode " is part of Line ", line.name().value_or( line.id().string() ), " (", cmv.component_id.id().string(), - "), which should not be boundary of Surface ", + "), which is boundary of Surface ", incident_surface.name().value_or( incident_surface.id().string() ), - " (", incident_surface.id().string(), ")" ); + " (", incident_surface.id().string(), + ") but has several surface edges of this surface " + "around Line edge 0" ); } } for( const auto& embedding_surface : @@ -371,10 +373,11 @@ namespace geode " is part of Line ", line.name().value_or( line.id().string() ), " (", cmv.component_id.id().string(), - "', which should be embedded in Surface ", + "), which is embedded in Surface ", embedding_surface.name().value_or( embedding_surface.id().string() ), - " (", embedding_surface.id().string(), "'" ); + " (", embedding_surface.id().string(), + "), but has no surface edge around Line edge 0" ); } if( cme.surface_edges.at( embedding_surface.id() ).size() <= 1 ) { @@ -386,7 +389,8 @@ namespace geode embedding_surface.name().value_or( embedding_surface.id().string() ), " (", embedding_surface.id().string(), - ") but doesn't cut it" ); + ") but doesn't cut it (should have more than 1 Surface " + "edge around Line edge 0)" ); } } if( brep_.nb_incidences( cmv.component_id.id() ) == 0 From d1e476811edd25723d6ea8252083cfe8a10de724 Mon Sep 17 00:00:00 2001 From: MelchiorSchuh <26920036+MelchiorSchuh@users.noreply.github.com> Date: Wed, 17 Jun 2026 13:36:59 +0000 Subject: [PATCH 2/6] Apply prepare changes --- .clang-format | 2 -- 1 file changed, 2 deletions(-) diff --git a/.clang-format b/.clang-format index 659a8593..b76813f9 100644 --- a/.clang-format +++ b/.clang-format @@ -1,5 +1,3 @@ -# DO NOT MODIFY DIRECTLY THIS FILE -# LOOK AT https://github.com/Geode-solutions/actions AccessModifierOffset: -4 AlignAfterOpenBracket: DontAlign AlignConsecutiveAssignments: false From 058ab8ebd137eb6d94b9d584f045a64aae8aa42c Mon Sep 17 00:00:00 2001 From: MelchiorSchuh Date: Wed, 17 Jun 2026 17:00:58 +0200 Subject: [PATCH 3/6] fix boundary surfaces test --- .../topology/brep_blocks_topology.hpp | 7 +- .../topology/brep_blocks_topology.cpp | 175 +++++++++--------- 2 files changed, 87 insertions(+), 95 deletions(-) diff --git a/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp b/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp index 5297ad8f..cf4cc770 100644 --- a/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp +++ b/include/geode/inspector/inspection/topology/brep_blocks_topology.hpp @@ -49,11 +49,12 @@ namespace geode "Blocks without mesh (uuid listed)" }; InspectionIssues< uuid > wrong_block_boundary_surface{ - "Surfaces boundary of Blocks but not linked to another boundary " - "Surface by a line (uuids listed)" + "Blocks with boundary surface(s) not linked to another boundary " + "Surface by a line (Surface shouldn't be boundary or there is a " + "hole in the block boundaries)" }; InspectionIssuesMap< index_t > blocks_not_linked_to_a_unique_vertex{ - "Blocks containing mesh vertices not linked to unique vertices" + "Blocks mesh vertices not linked to unique vertices" }; InspectionIssues< uuid > blocks_with_not_closed_boundary_surfaces{ "Blocks with non-closed boundary Surfaces" diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 64716a30..d1992e91 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -24,6 +24,7 @@ #include #include +#include #include @@ -271,104 +272,94 @@ namespace return dangling_surfaces; } - bool verify_blocks_boundaries( - const geode::BRep& brep, const geode::Block3D& block ) + bool find_linked_processed_surface( const geode::BRep& brep, + const geode::uuid& surface_uuid, + const absl::flat_hash_set< geode::uuid >& processed ) { - auto polygonal_surface = geode::PolygonalSurface3D::create(); - auto polygonal_surface_builder = - geode::PolygonalSurfaceBuilder3D::create( *polygonal_surface ); - polygonal_surface->enable_edges(); - geode::BijectiveMapping< geode::index_t, geode::index_t > - unique_vertices_to_polygonal_surface_vertices; - for( const auto& boundary : brep.boundaries( block ) ) - { - const auto& surface_mesh = boundary.mesh(); - for( const auto polygon : - geode::Range{ surface_mesh.nb_polygons() } ) - { - auto polygon_vertices = - surface_mesh.polygon_vertices( polygon ); - for( auto& polygon_vertex : polygon_vertices ) + const auto& surface = brep.surface( surface_uuid ); + for( const auto& line : brep.boundaries( surface ) ) + { + for( const auto& other_surface : brep.incidences( line ) ) + { + if( processed.contains( other_surface.id() ) ) { - const auto unique_vertex = brep.unique_vertex( - { boundary.component_id(), polygon_vertex } ); - if( !unique_vertices_to_polygonal_surface_vertices - .has_mapping_input( unique_vertex ) ) - { - const auto new_vertex = - polygonal_surface_builder->create_point( - surface_mesh.point( polygon_vertex ) ); - unique_vertices_to_polygonal_surface_vertices.map( - unique_vertex, new_vertex ); - polygon_vertex = new_vertex; - } - else - { - polygon_vertex = - unique_vertices_to_polygonal_surface_vertices - .in2out( unique_vertex ); - } + return true; } - polygonal_surface_builder->create_polygon( polygon_vertices ); } } - polygonal_surface_builder->compute_polygon_adjacencies(); - return polygonal_surface->nb_vertices() - - polygonal_surface->edges().nb_edges() - + polygonal_surface->nb_polygons() - == 2; + return false; } - bool are_brep_boundaries_closed( const geode::BRep& brep ) + bool surfaces_are_closed( + const geode::BRep& brep, std::queue< geode::uuid > to_process ) + { + absl::flat_hash_set< geode::uuid > processed; + const auto first_surface_uuid = to_process.front(); + to_process.pop(); + processed.emplace( first_surface_uuid ); + geode::index_t skip_counter{ 0 }; + while( !to_process.empty() ) + { + if( skip_counter == to_process.size() ) + { + return false; + } + const auto surface_uuid = to_process.front(); + to_process.pop(); + if( processed.contains( surface_uuid ) ) + { + continue; + } + if( find_linked_processed_surface( brep, surface_uuid, processed ) ) + { + processed.emplace( surface_uuid ); + skip_counter = 0; + } + else + { + to_process.emplace( surface_uuid ); + skip_counter++; + } + } + return true; + } + + std::queue< geode::uuid > queue_with_block_boundaries( + const geode::BRep& brep, const geode::Block3D& block ) { - auto polygonal_surface = geode::PolygonalSurface3D::create(); - auto polygonal_surface_builder = - geode::PolygonalSurfaceBuilder3D::create( *polygonal_surface ); - polygonal_surface->enable_edges(); - geode::BijectiveMapping< geode::index_t, geode::index_t > - unique_vertices_to_polygonal_surface_vertices; + std::queue< geode::uuid > to_process; + for( const auto& surface : brep.boundaries( block ) ) + { + to_process.emplace( surface.id() ); + } + return to_process; + } + + std::queue< geode::uuid > queue_with_model_boundaries( + const geode::BRep& brep ) + { + std::queue< geode::uuid > to_process; for( const auto& model_boundary : brep.model_boundaries() ) { - for( const auto& boundary : + for( const auto& surface : brep.model_boundary_items( model_boundary ) ) { - const auto& surface_mesh = boundary.mesh(); - for( const auto polygon : - geode::Range{ surface_mesh.nb_polygons() } ) - { - auto polygon_vertices = - surface_mesh.polygon_vertices( polygon ); - for( auto& polygon_vertex : polygon_vertices ) - { - const auto unique_vertex = brep.unique_vertex( - { boundary.component_id(), polygon_vertex } ); - if( !unique_vertices_to_polygonal_surface_vertices - .has_mapping_input( unique_vertex ) ) - { - const auto new_vertex = - polygonal_surface_builder->create_point( - surface_mesh.point( polygon_vertex ) ); - unique_vertices_to_polygonal_surface_vertices.map( - unique_vertex, new_vertex ); - polygon_vertex = new_vertex; - } - else - { - polygon_vertex = - unique_vertices_to_polygonal_surface_vertices - .in2out( unique_vertex ); - } - } - polygonal_surface_builder->create_polygon( - polygon_vertices ); - } + to_process.emplace( surface.id() ); } } - polygonal_surface_builder->compute_polygon_adjacencies(); - return polygonal_surface->nb_vertices() - - polygonal_surface->edges().nb_edges() - + polygonal_surface->nb_polygons() - == 2; + return to_process; + } + + bool block_boundaries_are_closed( + const geode::BRep& brep, const geode::Block3D& block ) + { + return surfaces_are_closed( + brep, queue_with_block_boundaries( brep, block ) ); + } + + bool brep_boundaries_are_closed( const geode::BRep& brep ) + { + return surfaces_are_closed( brep, queue_with_model_boundaries( brep ) ); } } // namespace @@ -817,7 +808,7 @@ namespace geode } if( brep_.nb_model_boundaries() != 0 ) { - if( !are_brep_boundaries_closed( brep_ ) ) + if( !brep_boundaries_are_closed( brep_ ) ) { result.model_boundaries_dont_form_a_closed_surface.add_issue( 0, "ModelBoundaries don't form a valid closed surface." ); @@ -866,7 +857,7 @@ namespace geode } for( const auto& block : brep_.active_blocks() ) { - if( !verify_blocks_boundaries( brep_, block ) ) + if( !block_boundaries_are_closed( brep_, block ) ) { result.blocks_with_not_closed_boundary_surfaces.add_issue( block.id(), @@ -885,17 +876,17 @@ namespace geode if( surface_boundaries_are_not_linked_to_another_block_boundary( surface, brep_, block ) ) { - result.wrong_block_boundary_surface.add_issue( surface.id(), - absl::StrCat( "Surface ", + result.wrong_block_boundary_surface.add_issue( block.id(), + absl::StrCat( "Block ", + block.name().value_or( block.id().string() ), " (", + block.id().string(), ") boundary 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 it has a boundary Line not incident to any " + ") has a boundary Line not incident to any " "other Block boundary Surface. Either the surface " "shouldn't be boundary, or there is a hole in the " "block boundaries" ) ); + break; } } } From 0611057676c27482005912b8336e81f0fafdd4df Mon Sep 17 00:00:00 2001 From: MelchiorSchuh Date: Wed, 17 Jun 2026 17:53:51 +0200 Subject: [PATCH 4/6] fix tests --- .../python/tests/inspection/test-py-brep.py | 12 +++--- .../tests/validity/test-py-brep-validity.py | 12 +++--- tests/inspection/test-brep.cpp | 25 ++++++------ tests/validity/test-brep-validity.cpp | 38 +++++++++++-------- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/bindings/python/tests/inspection/test-py-brep.py b/bindings/python/tests/inspection/test-py-brep.py index 6fbb36c8..25728e2f 100644 --- a/bindings/python/tests/inspection/test-py-brep.py +++ b/bindings/python/tests/inspection/test-py-brep.py @@ -221,9 +221,9 @@ def check_a1(verbose): else: print("model_A1 topology is invalid.") nb_model_issues = launch_topological_validity_checks(result.topology, verbose) - if nb_model_issues != 5201: + if nb_model_issues != 5191: raise ValueError( - "[Test] model model_A1 should have 5201 unique vertices with topological problems." + "[Test] model model_A1 should have 5191 unique vertices with topological problems." ) nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, verbose @@ -242,9 +242,9 @@ def inspect_model_A1(model_brep, verbose): else: print("model_A1_valid topology is invalid.") nb_model_issues = launch_topological_validity_checks(result.topology, verbose) - if nb_model_issues != 5201: + if nb_model_issues != 5191: raise ValueError( - "[Test] model model_A1_valid should have 5201 topological problems." + "[Test] model model_A1_valid should have 5191 topological problems." ) nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, verbose @@ -268,9 +268,9 @@ def inspect_model_mss(model_brep, verbose): else: print("model mss topology is invalid.") nb_model_issues = launch_topological_validity_checks(result.topology, verbose) - if nb_model_issues != 52: + if nb_model_issues != 50: raise ValueError( - "[Test] model mss.og_strm should have 37 topological problems, not " + "[Test] model mss.og_strm should have 50 topological problems, not " + str(nb_model_issues) ) nb_component_meshes_issues = launch_component_meshes_validity_checks( diff --git a/bindings/python/tests/validity/test-py-brep-validity.py b/bindings/python/tests/validity/test-py-brep-validity.py index 66284512..b178e522 100644 --- a/bindings/python/tests/validity/test-py-brep-validity.py +++ b/bindings/python/tests/validity/test-py-brep-validity.py @@ -38,22 +38,22 @@ def data_dir(): def check_a1(): model_brep = geode.load_brep(data_dir() + "/model_A1.og_brep") result = validity.is_brep_valid(model_brep) - if result.nb_issues()!=7: - raise ValueError( "[Test] model model_A1 should have 7 issues." ) + if result.nb_issues()!=8: + raise ValueError( "[Test] model model_A1 should have 8 issues." ) def check_a1_valid(): model_brep = geode.load_brep(data_dir() + "/model_A1_valid.og_brep") result = validity.is_brep_valid(model_brep) - if result.nb_issues()!=7: - raise ValueError( "[Test] model model_A1_valid should have 7 issues." ) + if result.nb_issues()!=8: + raise ValueError( "[Test] model model_A1_valid should have 8 issues." ) def check_model_mss(): model_brep = geode.load_brep(data_dir() + "/mss.og_brep") result = validity.is_brep_valid(model_brep) - if result.nb_issues()!=5: - raise ValueError( "[Test] model_mss should have 5 issues." ) + if result.nb_issues()!=4: + raise ValueError( "[Test] model_mss should have 4 issues." ) def check_model_D(): diff --git a/tests/inspection/test-brep.cpp b/tests/inspection/test-brep.cpp index 82b6c0b6..70e73c49 100644 --- a/tests/inspection/test-brep.cpp +++ b/tests/inspection/test-brep.cpp @@ -326,8 +326,8 @@ void check_model_a1( bool string ) const auto nb_topological_issues = launch_topological_validity_checks( result.topology, string ); geode::OpenGeodeInspectorInspectionException::test( - nb_topological_issues == 5201, "model_A1 has ", nb_topological_issues, - " topological problems instead of 5201." ); + nb_topological_issues == 5191, "model_A1 has ", nb_topological_issues, + " topological problems instead of 5191." ); const auto nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, string ); @@ -349,8 +349,8 @@ void check_model_a1_valid( bool string ) const auto nb_topological_issues = launch_topological_validity_checks( result.topology, string ); geode::OpenGeodeInspectorInspectionException::test( - nb_topological_issues == 5201, "model_A1_valid has ", - nb_topological_issues, " topological problems instead of 5201." ); + nb_topological_issues == 5191, "model_A1_valid has ", + nb_topological_issues, " topological problems instead of 5191." ); const auto nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, string ); @@ -372,8 +372,8 @@ void check_model_mss( bool string ) const auto nb_topological_issues = launch_topological_validity_checks( result.topology, string ); geode::OpenGeodeInspectorInspectionException::test( - nb_topological_issues == 52, "mss has ", nb_topological_issues, - " topological problems instead of 37." ); + nb_topological_issues == 50, "mss has ", nb_topological_issues, + " topological problems instead of 50." ); const auto nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, string ); @@ -415,17 +415,18 @@ void check_wrong_bsurfaces_model() "should be 3, and it is ", result.topology.blocks.wrong_block_boundary_surface.nb_issues(), "." ) ); - std::vector< geode::uuid > wrong_bsurf{ - geode::uuid{ "00000000-78d4-4e10-8000-0000cb3a3a27" }, - geode::uuid{ "00000000-7a4e-4a1c-8000-00003732de1f" }, - geode::uuid{ "00000000-980f-49d4-8000-00002f79374e" } + std::vector< geode::uuid > wrong_bsurf_blocks{ + geode::uuid{ "00000000-e821-4e90-8000-00000089237c" }, + geode::uuid{ "00000000-3c71-472b-8000-0000fa07558c" }, + geode::uuid{ "00000000-fbad-4231-8000-000085ca0c23" } }; for( const auto& issue : result.topology.blocks.wrong_block_boundary_surface.issues() ) { + DEBUG( issue.string() ); geode::OpenGeodeInspectorInspectionException::test( - absl::c_find( wrong_bsurf, issue ) != wrong_bsurf.end(), - "Surface (", issue.string(), + absl::c_contains( wrong_bsurf_blocks, issue ), "Block (", + issue.string(), ") is detected as a wrong boundary surface but is not one." ); } } diff --git a/tests/validity/test-brep-validity.cpp b/tests/validity/test-brep-validity.cpp index ebabcf38..18f43122 100644 --- a/tests/validity/test-brep-validity.cpp +++ b/tests/validity/test-brep-validity.cpp @@ -36,10 +36,11 @@ void check_model_a1() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "model_A1.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( "model_A1 invalidities: \n", invalidities.string() + // ); geode::OpenGeodeInspectorValidityException::test( - invalidities.nb_issues() == 7, "model_A1 has ", - invalidities.nb_issues(), " issues instead of 7." ); + invalidities.nb_issues() == 8, "model_A1 has ", + invalidities.nb_issues(), " issues instead of 8." ); } void check_model_a1_valid() @@ -47,10 +48,11 @@ void check_model_a1_valid() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "model_A1_valid.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( + // "model_A1_valid invalidities: \n", invalidities.string() ); geode::OpenGeodeInspectorValidityException::test( - invalidities.nb_issues() == 7, "model_A1_valid has ", - invalidities.nb_issues(), " issues instead of 7." ); + invalidities.nb_issues() == 8, "model_A1_valid has ", + invalidities.nb_issues(), " issues instead of 8." ); } void check_model_mss() @@ -58,10 +60,11 @@ void check_model_mss() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "mss.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( "model mss invalidities: \n", invalidities.string() + // ); geode::OpenGeodeInspectorValidityException::test( - invalidities.nb_issues() == 5, "model_mss has ", - invalidities.nb_issues(), " issues instead of 5." ); + invalidities.nb_issues() == 4, "model_mss has ", + invalidities.nb_issues(), " issues instead of 4." ); } void check_model_D() @@ -69,7 +72,8 @@ void check_model_D() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "model_D.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( "model_D invalidities: \n", invalidities.string() + // ); geode::OpenGeodeInspectorValidityException::test( invalidities.nb_issues() == 0, "model_D has ", invalidities.nb_issues(), " issues instead of 0." ); @@ -80,10 +84,11 @@ void check_wrong_bsurfaces_model() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "wrong_boundary_surface_model.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( "wrong_boundary_surface_model invalidities: \n", + // invalidities.string() ); geode::OpenGeodeInspectorValidityException::test( - invalidities.nb_issues() == 2, "wrong_boundary_surface_model has ", - invalidities.nb_issues(), " issues instead of 2." ); + invalidities.nb_issues() == 1, "wrong_boundary_surface_model has ", + invalidities.nb_issues(), " issues instead of 1." ); } void check_segmented_cube() @@ -91,10 +96,11 @@ void check_segmented_cube() const auto model_brep = geode::load_brep( absl::StrCat( geode::DATA_PATH, "cube_segmented.og_brep" ) ); const auto invalidities = geode::is_brep_valid( model_brep ); - + // geode::Logger::debug( + // "cube_segmented invalidities: \n", invalidities.string() ); geode::OpenGeodeInspectorValidityException::test( - invalidities.nb_issues() == 1, "cube_segmented has ", - invalidities.nb_issues(), " issues instead of 1." ); + invalidities.nb_issues() == 0, "cube_segmented has ", + invalidities.nb_issues(), " issues instead of 0." ); } int main() From e43c38615c94a7e6f3d0e8bea54149220736d04d Mon Sep 17 00:00:00 2001 From: MelchiorSchuh Date: Thu, 18 Jun 2026 14:21:02 +0200 Subject: [PATCH 5/6] fix test in case of inner blocks --- .../topology/brep_blocks_topology.cpp | 166 ++++++++++++++++-- 1 file changed, 153 insertions(+), 13 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index d1992e91..986a1315 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -32,6 +32,8 @@ #include #include +#include +#include #include #include @@ -40,6 +42,7 @@ #include #include #include +#include #include #include @@ -274,14 +277,14 @@ namespace bool find_linked_processed_surface( const geode::BRep& brep, const geode::uuid& surface_uuid, - const absl::flat_hash_set< geode::uuid >& processed ) + absl::Span< const geode::uuid > processed ) { const auto& surface = brep.surface( surface_uuid ); for( const auto& line : brep.boundaries( surface ) ) { for( const auto& other_surface : brep.incidences( line ) ) { - if( processed.contains( other_surface.id() ) ) + if( absl::c_contains( processed, other_surface.id() ) ) { return true; } @@ -290,29 +293,29 @@ namespace return false; } - bool surfaces_are_closed( - const geode::BRep& brep, std::queue< geode::uuid > to_process ) + std::vector< geode::uuid > closed_linked_surfaces( + const geode::BRep& brep, std::queue< geode::uuid >& to_process ) { - absl::flat_hash_set< geode::uuid > processed; + std::vector< geode::uuid > processed; const auto first_surface_uuid = to_process.front(); to_process.pop(); - processed.emplace( first_surface_uuid ); + processed.emplace_back( first_surface_uuid ); geode::index_t skip_counter{ 0 }; while( !to_process.empty() ) { if( skip_counter == to_process.size() ) { - return false; + return processed; } const auto surface_uuid = to_process.front(); to_process.pop(); - if( processed.contains( surface_uuid ) ) + if( absl::c_contains( processed, surface_uuid ) ) { continue; } if( find_linked_processed_surface( brep, surface_uuid, processed ) ) { - processed.emplace( surface_uuid ); + processed.emplace_back( surface_uuid ); skip_counter = 0; } else @@ -321,7 +324,7 @@ namespace skip_counter++; } } - return true; + return processed; } std::queue< geode::uuid > queue_with_block_boundaries( @@ -350,16 +353,153 @@ namespace return to_process; } + geode::BoundingBox3D surface_list_box( + const geode::BRep& brep, absl::Span< const geode::uuid > surface_list ) + { + geode::BoundingBox3D result; + for( const auto& surface_id : surface_list ) + { + result.add_box( brep.surface( surface_id ).mesh().bounding_box() ); + } + return result; + } + + std::optional< geode::index_t > enclosing_surface_list( + const geode::BRep& brep, + const std::vector< std::vector< geode::uuid > >& linked_surfaces_list ) + { + geode::index_t enclosing_surface_index{ 0 }; + auto current_enclosing_bbox = + surface_list_box( brep, linked_surfaces_list[0] ); + for( const auto& surface_part_id : + geode::Range{ 1, linked_surfaces_list.size() } ) + { + auto surface_part_box = + surface_list_box( brep, linked_surfaces_list[surface_part_id] ); + if( current_enclosing_bbox.contains( surface_part_box ) ) + { + continue; + } + if( !surface_part_box.contains( current_enclosing_bbox ) ) + { + return std::nullopt; + } + enclosing_surface_index = surface_part_id; + current_enclosing_bbox = std::move( surface_part_box ); + } + return enclosing_surface_index; + } + + std::unique_ptr< geode::PolygonalSurface3D > fuse_brep_surfaces_from_list( + const geode::BRep& brep, + const std::vector< geode::uuid >& surface_list ) + { + auto polygonal_surface = geode::PolygonalSurface3D::create(); + auto polygonal_surface_builder = + geode::PolygonalSurfaceBuilder3D::create( *polygonal_surface ); + geode::BijectiveMapping< geode::index_t, geode::index_t > + unique_vertices_to_polygonal_surface_vertices; + for( const auto& surface_id : surface_list ) + { + const auto& surface = brep.surface( surface_id ); + const auto& surface_mesh = surface.mesh(); + for( const auto polygon : + geode::Range{ surface_mesh.nb_polygons() } ) + { + auto polygon_vertices = + surface_mesh.polygon_vertices( polygon ); + for( auto& polygon_vertex : polygon_vertices ) + { + const auto unique_vertex = brep.unique_vertex( + { surface.component_id(), polygon_vertex } ); + if( !unique_vertices_to_polygonal_surface_vertices + .has_mapping_input( unique_vertex ) ) + { + const auto new_vertex = + polygonal_surface_builder->create_point( + surface_mesh.point( polygon_vertex ) ); + unique_vertices_to_polygonal_surface_vertices.map( + unique_vertex, new_vertex ); + polygon_vertex = new_vertex; + } + else + { + polygon_vertex = + unique_vertices_to_polygonal_surface_vertices + .in2out( unique_vertex ); + } + } + polygonal_surface_builder->create_polygon( polygon_vertices ); + } + } + polygonal_surface_builder->compute_polygon_adjacencies(); + return polygonal_surface; + } + bool block_boundaries_are_closed( const geode::BRep& brep, const geode::Block3D& block ) { - return surfaces_are_closed( - brep, queue_with_block_boundaries( brep, block ) ); + auto to_process = queue_with_block_boundaries( brep, block ); + std::vector< std::vector< geode::uuid > > linked_boundary_parts; + while( !to_process.empty() ) + { + linked_boundary_parts.emplace_back( + closed_linked_surfaces( brep, to_process ) ); + } + if( linked_boundary_parts.size() < 2 ) + { + return true; + } + const auto enclosing_surface_index = + enclosing_surface_list( brep, linked_boundary_parts ); + if( !enclosing_surface_index ) + { + return false; + } + const auto enclosing_surface_mesh = fuse_brep_surfaces_from_list( + brep, linked_boundary_parts[enclosing_surface_index.value()] ); + const auto enclosing_surface_aabb = + geode::create_aabb_tree( *enclosing_surface_mesh ); + for( const auto surface_list_id : + geode::Indices{ linked_boundary_parts } ) + { + if( surface_list_id == enclosing_surface_index.value() ) + { + continue; + } + for( const auto& surface_id : + linked_boundary_parts[surface_list_id] ) + { + const auto& surface_mesh = brep.surface( surface_id ).mesh(); + for( const auto vertex_id : + geode::Range{ surface_mesh.nb_vertices() } ) + { + try + { + if( !geode::is_point_inside_closed_surface( + surface_mesh.point( vertex_id ), + *enclosing_surface_mesh, + enclosing_surface_aabb ) ) + { + return false; + } + } + catch( const geode::OpenGeodeException& ) + { + return false; + } + } + } + } + return true; } bool brep_boundaries_are_closed( const geode::BRep& brep ) { - return surfaces_are_closed( brep, queue_with_model_boundaries( brep ) ); + auto to_process = queue_with_model_boundaries( brep ); + auto compute_closed_linked_surfaces = + closed_linked_surfaces( brep, to_process ); + return to_process.empty(); } } // namespace From c2eaa411de6017f070f52942e17e158191fd803e Mon Sep 17 00:00:00 2001 From: MelchiorSchuh Date: Thu, 18 Jun 2026 17:38:41 +0200 Subject: [PATCH 6/6] fix block boundaries test --- .../topology/brep_blocks_topology.cpp | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp index 986a1315..ae4b49bf 100644 --- a/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp +++ b/src/geode/inspector/inspection/topology/brep_blocks_topology.cpp @@ -368,26 +368,33 @@ namespace const geode::BRep& brep, const std::vector< std::vector< geode::uuid > >& linked_surfaces_list ) { - geode::index_t enclosing_surface_index{ 0 }; - auto current_enclosing_bbox = - surface_list_box( brep, linked_surfaces_list[0] ); - for( const auto& surface_part_id : - geode::Range{ 1, linked_surfaces_list.size() } ) + std::vector< geode::BoundingBox3D > bounding_boxes; + for( const auto& surface_list : linked_surfaces_list ) { - auto surface_part_box = - surface_list_box( brep, linked_surfaces_list[surface_part_id] ); - if( current_enclosing_bbox.contains( surface_part_box ) ) + bounding_boxes.emplace_back( + surface_list_box( brep, surface_list ) ); + } + for( const auto first_surface_list_id : + geode::Indices{ linked_surfaces_list } ) + { + bool contains_others{ true }; + for( const auto second_surface_list_id : + geode::Indices{ linked_surfaces_list } ) { - continue; + if( second_surface_list_id != first_surface_list_id + && !bounding_boxes[first_surface_list_id].contains( + bounding_boxes[second_surface_list_id] ) ) + { + contains_others = false; + break; + } } - if( !surface_part_box.contains( current_enclosing_bbox ) ) + if( contains_others ) { - return std::nullopt; + return first_surface_list_id; } - enclosing_surface_index = surface_part_id; - current_enclosing_bbox = std::move( surface_part_box ); } - return enclosing_surface_index; + return std::nullopt; } std::unique_ptr< geode::PolygonalSurface3D > fuse_brep_surfaces_from_list(