From 3961df1f5609becf554fc94bd6ac50dd9fb96bae Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 17:43:11 +0200 Subject: [PATCH 01/14] fix(Intersections): use mesh1 index for Section shared edge vertex When two Section triangles share two vertices, the second shared point was read in mesh1 with its mesh2 index, giving a wrong point and false intersections between adjacent surfaces numbered differently. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 2 +- tests/inspection/CMakeLists.txt | 10 + tests/inspection/test-model-intersections.cpp | 206 ++++++++++++++++++ 3 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 tests/inspection/test-model-intersections.cpp diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 7dff4a2f..6da5eb6f 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -360,7 +360,7 @@ namespace if( common_vertices.size() == 2 ) { const auto& common_pt0 = mesh1_.point( common_vertices[0][0] ); - const auto& common_pt1 = mesh1_.point( common_vertices[1][1] ); + const auto& common_pt1 = mesh1_.point( common_vertices[1][0] ); const auto& t1_third_pt = mesh1_.point( third_point_index( t1_vertices, common_vertices, 0 ) ); const auto& t2_third_pt = mesh2_.point( diff --git a/tests/inspection/CMakeLists.txt b/tests/inspection/CMakeLists.txt index f51c1a42..00396eaf 100755 --- a/tests/inspection/CMakeLists.txt +++ b/tests/inspection/CMakeLists.txt @@ -107,6 +107,16 @@ add_geode_test( ${PROJECT_NAME}::inspection ) +add_geode_test( + SOURCE "test-model-intersections.cpp" + DEPENDENCIES + OpenGeode::basic + OpenGeode::geometry + OpenGeode::mesh + OpenGeode::model + ${PROJECT_NAME}::inspection +) + add_geode_test( SOURCE "test-surface-curve-intersections.cpp" DEPENDENCIES diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp new file mode 100644 index 00000000..d04aeb95 --- /dev/null +++ b/tests/inspection/test-model-intersections.cpp @@ -0,0 +1,206 @@ +/* + * 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 +#include +#include +#include + +#include + +namespace +{ + template < geode::index_t dimension > + struct SurfaceDefinition + { + std::vector< geode::Point< dimension > > points; + std::vector< std::vector< geode::index_t > > polygons; + // Unique vertex of each mesh vertex, NO_ID to leave it unset + std::vector< geode::index_t > unique_vertices; + bool polygonal{ false }; + }; + + template < geode::index_t dimension > + struct LineDefinition + { + std::vector< geode::Point< dimension > > points; + std::vector< std::array< geode::index_t, 2 > > edges; + std::vector< geode::index_t > unique_vertices; + }; + + template < typename Model, typename Builder > + void fill_model( const Model& model, + Builder& builder, + absl::Span< const SurfaceDefinition< Model::dim > > surfaces, + absl::Span< const LineDefinition< Model::dim > > lines, + geode::index_t nb_unique_vertices ) + { + builder.create_unique_vertices( nb_unique_vertices ); + for( const auto& definition : surfaces ) + { + const auto& surface_id = + definition.polygonal + ? builder.add_surface( geode::OpenGeodePolygonalSurface< + Model::dim >::impl_name_static() ) + : builder.add_surface(); + const auto& surface = model.surface( surface_id ); + auto mesh_builder = builder.surface_mesh_builder( surface ); + for( const auto& point : definition.points ) + { + mesh_builder->create_point( point ); + } + for( const auto& polygon : definition.polygons ) + { + mesh_builder->create_polygon( polygon ); + } + for( const auto v : geode::Indices{ definition.unique_vertices } ) + { + if( definition.unique_vertices[v] != geode::NO_ID ) + { + builder.set_unique_vertex( { surface.component_id(), v }, + definition.unique_vertices[v] ); + } + } + } + for( const auto& definition : lines ) + { + const auto& line = model.line( builder.add_line() ); + auto mesh_builder = builder.line_mesh_builder( line ); + for( const auto& point : definition.points ) + { + mesh_builder->create_point( point ); + } + for( const auto& edge : definition.edges ) + { + mesh_builder->create_edge( edge[0], edge[1] ); + } + for( const auto v : geode::Indices{ definition.unique_vertices } ) + { + if( definition.unique_vertices[v] != geode::NO_ID ) + { + builder.set_unique_vertex( { line.component_id(), v }, + definition.unique_vertices[v] ); + } + } + } + } + + geode::Section build_section( + absl::Span< const SurfaceDefinition< 2 > > surfaces, + absl::Span< const LineDefinition< 2 > > lines, + geode::index_t nb_unique_vertices ) + { + geode::Section section; + geode::SectionBuilder builder{ section }; + fill_model( section, builder, surfaces, lines, nb_unique_vertices ); + return section; + } + + geode::BRep build_brep( absl::Span< const SurfaceDefinition< 3 > > surfaces, + absl::Span< const LineDefinition< 3 > > lines, + geode::index_t nb_unique_vertices ) + { + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + fill_model( brep, builder, surfaces, lines, nb_unique_vertices ); + // Lines are only checked against surfaces when embedded in a block + const auto& block = brep.block( builder.add_block() ); + for( const auto& line : brep.lines() ) + { + builder.add_line_block_internal_relationship( line, block ); + } + return brep; + } + + template < typename Model > + geode::index_t nb_intersections( const Model& model ) + { + const geode::ModelMeshesIntersections< Model > inspector{ model }; + return inspector.inspect_intersections() + .elements_intersections.nb_issues(); + } + + template < typename Model > + geode::index_t nb_self_intersections( const Model& model ) + { + const geode::ModelMeshesIntersections< Model > inspector{ model }; + return inspector.inspect_surfaces_self_intersections() + .elements_intersections.nb_issues(); + } + + void check_nb_intersections( geode::index_t nb_found, + geode::index_t nb_expected, + std::string_view test_name ) + { + geode::OpenGeodeInspectorInspectionException::test( + nb_found == nb_expected, "[", test_name, "] ", nb_found, + " intersections found instead of ", nb_expected, "." ); + } +} // namespace + +void check_section_adjacent_surfaces_different_numbering() +{ + // Two triangles from two surfaces sharing edge (1,0)-(0,1), the shared + // vertices having different mesh indices in both surfaces + const SurfaceDefinition< 2 > surface1{ { geode::Point2D{ { 0., 0. } }, + geode::Point2D{ { 1., 0. } }, + geode::Point2D{ { 0., 1. } } }, + { { 0, 1, 2 } }, { 0, 1, 2 } }; + const SurfaceDefinition< 2 > surface2{ { geode::Point2D{ { 1., 1. } }, + geode::Point2D{ { 0., 1. } }, + geode::Point2D{ { 1., 0. } } }, + { { 0, 1, 2 } }, { 3, 2, 1 } }; + const auto section = build_section( { surface1, surface2 }, {}, 4 ); + check_nb_intersections( nb_intersections( section ), 0, + "Section adjacent surfaces with different numbering" ); +} + +int main() +{ + try + { + geode::OpenGeodeInspectorInspectionLibrary::initialize(); + check_section_adjacent_surfaces_different_numbering(); + + geode::Logger::info( "TEST SUCCESS" ); + return 0; + } + catch( ... ) + { + return geode::geode_lippincott(); + } +} From 275a391fc26d24be5add6495d66753a56aa521d9 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 17:45:03 +0200 Subject: [PATCH 02/14] fix(Intersections): never match vertices without unique vertex Mesh vertices not linked to a unique vertex all compared equal (NO_ID), so unrelated triangles were seen as sharing vertices. Ignore NO_ID and fall back to mesh vertex indices within the same surface. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 12 +++++-- tests/inspection/test-model-intersections.cpp | 34 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 6da5eb6f..29489e1d 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -228,9 +228,15 @@ namespace { surface1_.component_id(), v1_id } ); for( const auto v2_id : t2_vertices ) { - if( v1_unique_vertex - == this->model().unique_vertex( - { surface2_.component_id(), v2_id } ) ) + if( same_surface_ && v1_id == v2_id ) + { + common_vertices.push_back( { v1_id, v2_id } ); + break; + } + if( v1_unique_vertex != geode::NO_ID + && v1_unique_vertex + == this->model().unique_vertex( + { surface2_.component_id(), v2_id } ) ) { common_vertices.push_back( { v1_id, v2_id } ); break; diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index d04aeb95..74cf06f1 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -189,12 +189,46 @@ void check_section_adjacent_surfaces_different_numbering() "Section adjacent surfaces with different numbering" ); } +void check_brep_vertices_without_unique_vertex() +{ + // Two non-intersecting triangles with overlapping bounding boxes, no mesh + // vertex being linked to a unique vertex + const SurfaceDefinition< 3 > surface1{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 1., 0., 0. } }, + geode::Point3D{ { 0., 1., 0. } } }, + { { 0, 1, 2 } }, {} + }; + const SurfaceDefinition< 3 > surface2{ + { geode::Point3D{ { 0.8, 0.8, -1. } }, + geode::Point3D{ { 0.8, 0.8, 1. } }, + geode::Point3D{ { 1., 1., 0. } } }, + { { 0, 1, 2 } }, {} + }; + const auto brep = build_brep( { surface1, surface2 }, {}, 0 ); + check_nb_intersections( + nb_intersections( brep ), 0, "BRep surfaces without unique vertices" ); + + // Two adjacent triangles of the same surface without unique vertices + const SurfaceDefinition< 3 > surface3{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 1., 0., 0. } }, + geode::Point3D{ { 0., 1., 0. } }, + geode::Point3D{ { 1., 1., 0. } } }, + { { 0, 1, 2 }, { 1, 3, 2 } }, {} + }; + const auto brep2 = build_brep( { surface3 }, {}, 0 ); + check_nb_intersections( + nb_intersections( brep2 ), 0, "BRep surface without unique vertices" ); + check_nb_intersections( nb_self_intersections( brep2 ), 0, + "BRep surface without unique vertices (self)" ); +} + int main() { try { geode::OpenGeodeInspectorInspectionLibrary::initialize(); check_section_adjacent_surfaces_different_numbering(); + check_brep_vertices_without_unique_vertex(); geode::Logger::info( "TEST SUCCESS" ); return 0; From 1ed8820204e48f5eb1baec7ced469d8921664a7b Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:23:08 +0200 Subject: [PATCH 03/14] fix(Intersections): an empty component mesh no longer disables the inspection When any surface had no polygon (or any line no edge), the whole model intersection inspection returned without checking anything. Components are now iterated through the AABB trees, which skip empty meshes, and the mesh_tree_ids_ lookups are gone. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 87 ++++++------------- tests/inspection/test-model-intersections.cpp | 23 +++++ 2 files changed, 48 insertions(+), 62 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 29489e1d..a743146c 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -697,33 +697,22 @@ namespace geode intersecting_polygons() const { IntersectionsResult component_intersections; - for( const auto& surface : model_.active_surfaces() ) - { - if( surface.mesh().nb_polygons() == 0 ) - { - geode::Logger::warning( - "One of the surface meshes has an empty mesh, cannot " - "compute the AABBTree used for detecting the mesh " - "intersections, no intersections will be " - "computed." ); - return component_intersections; - } - } using Task = async::task< IntersectionsResult >; std::vector< Task > tasks; - for( const auto& surface : model_.active_surfaces() ) + for( const auto tree_id : Indices{ surfaces_model_tree_.uuids_ } ) { - tasks.emplace_back( async::spawn( [this, &surface] { + tasks.emplace_back( async::spawn( [this, tree_id] { + const auto& surface_uuid = + surfaces_model_tree_.uuids_[tree_id]; const Action surfaces_intersection_action{ model_, - surface.id(), surface.id() }; + surface_uuid, surface_uuid }; const auto intersecting_elements = - surfaces_model_tree_ - .mesh_trees_[surfaces_model_tree_.mesh_tree_ids_.at( - surface.id() )] + surfaces_model_tree_.mesh_trees_[tree_id] .compute_self_element_bbox_intersections( surfaces_intersection_action ); IntersectionsResult result; - const auto surface_id = surface.component_id(); + const auto surface_id = + model_.surface( surface_uuid ).component_id(); for( const auto& [polygon1, polygon2] : intersecting_elements ) { @@ -783,32 +772,22 @@ namespace geode std::vector< std::pair< ComponentMeshElement, ComponentMeshElement > > component_intersections; - for( const auto& line : model_.active_lines() ) - { - if( line.mesh().nb_edges() == 0 ) - { - geode::Logger::warning( - "One of the line meshes has an empty mesh, " - "skipping line-surface intersection detection." ); - return component_intersections; - } - } using Task = async::task< IntersectionsResult >; std::vector< Task > tasks; - for( const auto& line : model_.active_lines() ) + for( const auto tree_id : Indices{ lines_model_tree_.uuids_ } ) { - tasks.emplace_back( async::spawn( [this, &line] { + tasks.emplace_back( async::spawn( [this, tree_id] { + const auto& line_uuid = lines_model_tree_.uuids_[tree_id]; const LineLineIntersection lines_intersection_action{ - model_, line.id(), line.id() + model_, line_uuid, line_uuid }; const auto intersecting_elements = - lines_model_tree_ - .mesh_trees_[lines_model_tree_.mesh_tree_ids_.at( - line.id() )] + lines_model_tree_.mesh_trees_[tree_id] .compute_self_element_bbox_intersections( lines_intersection_action ); IntersectionsResult result; - const auto line_id = line.component_id(); + const auto line_id = + model_.line( line_uuid ).component_id(); for( const auto& [edge1, edge2] : intersecting_elements ) { result.emplace_back( @@ -867,33 +846,18 @@ namespace geode std::vector< std::pair< ComponentMeshElement, ComponentMeshElement > > component_intersections; - for( const auto& surface : brep.active_surfaces() ) - { - if( surface.mesh().nb_polygons() == 0 ) - { - geode::Logger::warning( - "One of the surface meshes has an empty mesh, " - "skipping line-surface intersection detection." ); - return component_intersections; - } - } - for( const auto& line : brep.active_lines() ) - { - if( line.mesh().nb_edges() == 0 ) - { - geode::Logger::warning( - "One of the line meshes has an empty mesh, " - "skipping line-surface intersection detection." ); - return component_intersections; - } - } - for( const auto& surface : brep.active_surfaces() ) + for( const auto surface_tree_id : + Indices{ surfaces_model_tree_.uuids_ } ) { + const auto& surface = brep.surface( + surfaces_model_tree_.uuids_[surface_tree_id] ); const auto& surface_tree = - surfaces_model_tree_.mesh_trees_[surfaces_model_tree_ - .mesh_tree_ids_.at( surface.id() )]; - for( const auto& line : brep.active_lines() ) + surfaces_model_tree_.mesh_trees_[surface_tree_id]; + for( const auto line_tree_id : + Indices{ lines_model_tree_.uuids_ } ) { + const auto& line = + brep.line( lines_model_tree_.uuids_[line_tree_id] ); if( brep.nb_embedding_blocks( line ) == 0 ) { continue; @@ -901,8 +865,7 @@ namespace geode const BRepLineSurfacesIntersection action{ brep, surface.id(), line.id() }; const auto& line_tree = - lines_model_tree_.mesh_trees_[lines_model_tree_ - .mesh_tree_ids_.at( line.id() )]; + lines_model_tree_.mesh_trees_[line_tree_id]; for( const auto& element_pair : surface_tree.compute_other_element_bbox_intersections( line_tree, action ) ) diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index 74cf06f1..5d8e7e85 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -222,6 +222,28 @@ void check_brep_vertices_without_unique_vertex() "BRep surface without unique vertices (self)" ); } +void check_brep_with_empty_meshes() +{ + // Two crossing triangles plus a surface and a line without vertices + const SurfaceDefinition< 3 > horizontal{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 2., 0., 0. } }, + geode::Point3D{ { 0., 2., 0. } } }, + { { 0, 1, 2 } }, {} + }; + const SurfaceDefinition< 3 > vertical{ + { geode::Point3D{ { 0.5, 0.5, -1. } }, + geode::Point3D{ { 0.5, 0.5, 1. } }, + geode::Point3D{ { 1., 0., 0.5 } } }, + { { 0, 1, 2 } }, {} + }; + const SurfaceDefinition< 3 > empty_surface{}; + const LineDefinition< 3 > empty_line{}; + const auto brep = build_brep( + { horizontal, empty_surface, vertical }, { empty_line }, 0 ); + check_nb_intersections( + nb_intersections( brep ), 1, "BRep with empty meshes" ); +} + int main() { try @@ -229,6 +251,7 @@ int main() geode::OpenGeodeInspectorInspectionLibrary::initialize(); check_section_adjacent_surfaces_different_numbering(); check_brep_vertices_without_unique_vertex(); + check_brep_with_empty_meshes(); geode::Logger::info( "TEST SUCCESS" ); return 0; From c0dc862dec550ea6ad1fe85025af4b449ede93b6 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:24:07 +0200 Subject: [PATCH 04/14] fix(Intersections): Section triangles nested in another are detected With zero or one common vertex, 2D triangles were only tested for crossing edges, so a triangle lying inside another one (or nested in its corner at the common vertex) was not reported. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 31 +++++++++++++++- tests/inspection/test-model-intersections.cpp | 35 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index a743146c..f81a92cf 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -355,6 +355,30 @@ namespace }; } + [[nodiscard]] bool non_common_vertex_inside( + const geode::SurfaceMesh2D& mesh, + const geode::PolygonVertices& triangle_vertices, + const geode::Triangle2D& other_triangle, + absl::Span< const std::array< geode::index_t, 2 > > common_vertices, + geode::local_index_t vertex_position ) + { + for( const auto vertex_id : triangle_vertices ) + { + if( absl::c_any_of( common_vertices, + [vertex_id, vertex_position]( + const std::array< geode::index_t, 2 >& common ) { + return common[vertex_position] == vertex_id; + } ) ) + { + continue; + } + return geode::point_triangle_position( + mesh.point( vertex_id ), other_triangle ) + != geode::POSITION::outside; + } + return false; + } + template <> [[nodiscard]] bool ModelSurfacesIntersectionBase< geode::Section >::triangles_intersect( @@ -434,7 +458,12 @@ namespace } } } - return false; + // No edge crossing: one triangle may still lie inside the other + // (possibly nested in its corner at the common vertex) + return non_common_vertex_inside( mesh1_, t1_vertices, + mesh_triangle( mesh2_, t2_vertices ), common_vertices, 0 ) + || non_common_vertex_inside( mesh2_, t2_vertices, + mesh_triangle( mesh1_, t1_vertices ), common_vertices, 1 ); } [[nodiscard]] bool triangle_intersects_other( diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index 5d8e7e85..46e2d2ca 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -189,6 +189,40 @@ void check_section_adjacent_surfaces_different_numbering() "Section adjacent surfaces with different numbering" ); } +void check_section_nested_triangles() +{ + // A small triangle strictly inside a big one, no common vertex + const SurfaceDefinition< 2 > big{ { geode::Point2D{ { 0., 0. } }, + geode::Point2D{ { 10., 0. } }, + geode::Point2D{ { 0., 10. } } }, + { { 0, 1, 2 } }, { 0, 1, 2 } }; + const SurfaceDefinition< 2 > inside{ { geode::Point2D{ { 1., 1. } }, + geode::Point2D{ { 2., 1. } }, + geode::Point2D{ { 1., 2. } } }, + { { 0, 1, 2 } }, { 3, 4, 5 } }; + const auto section = build_section( { big, inside }, {}, 6 ); + check_nb_intersections( + nb_intersections( section ), 1, "Section triangle inside another" ); + + // A small triangle nested in the corner of the big one at vertex 0 + const SurfaceDefinition< 2 > corner{ { geode::Point2D{ { 0., 0. } }, + geode::Point2D{ { 3., 1. } }, + geode::Point2D{ { 1., 3. } } }, + { { 0, 1, 2 } }, { 0, 3, 4 } }; + const auto section2 = build_section( { big, corner }, {}, 5 ); + check_nb_intersections( nb_intersections( section2 ), 1, + "Section triangle nested in a corner" ); + + // Triangle outside the big one, only sharing vertex 0 + const SurfaceDefinition< 2 > outside{ { geode::Point2D{ { 0., 0. } }, + geode::Point2D{ { -3., -1. } }, + geode::Point2D{ { -1., -3. } } }, + { { 0, 1, 2 } }, { 0, 3, 4 } }; + const auto section3 = build_section( { big, outside }, {}, 5 ); + check_nb_intersections( + nb_intersections( section3 ), 0, "Section triangles sharing a vertex" ); +} + void check_brep_vertices_without_unique_vertex() { // Two non-intersecting triangles with overlapping bounding boxes, no mesh @@ -250,6 +284,7 @@ int main() { geode::OpenGeodeInspectorInspectionLibrary::initialize(); check_section_adjacent_surfaces_different_numbering(); + check_section_nested_triangles(); check_brep_vertices_without_unique_vertex(); check_brep_with_empty_meshes(); From 03da6f9594b40284004fc97ba8c3e7e57f18225e Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:25:27 +0200 Subject: [PATCH 05/14] fix(Intersections): detect lines crossing a surface in its plane A line segment lying in a triangle plane and crossing it is reported as 'parallel' by the segment/triangle detection, which was ignored. It is now an intersection unless the contact is reduced to triangle vertices. The edge0/1/2 tests on the segment position, which can never happen, are removed. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 32 +++++++++++++++---- tests/inspection/test-model-intersections.cpp | 25 +++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index f81a92cf..f0f0a38c 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -74,6 +74,25 @@ namespace return triangles; } + // The segment lies in the triangle plane and overlaps it: it is an + // intersection unless the contact is reduced to triangle vertices + [[nodiscard]] bool segment_in_triangle_plane_intersects( + const geode::Segment3D& segment, const geode::Triangle3D& triangle ) + { + for( const auto& vertex : segment.vertices() ) + { + const auto position = + geode::point_triangle_position( vertex.get(), triangle ); + if( position != geode::POSITION::vertex0 + && position != geode::POSITION::vertex1 + && position != geode::POSITION::vertex2 ) + { + return true; + } + } + return false; + } + class BRepLineSurfacesIntersection { public: @@ -115,21 +134,22 @@ namespace == geode::POSITION::inside || segment_triangle_intersection.second == geode::POSITION::inside - || segment_triangle_intersection.first - == geode::POSITION::edge0 - || segment_triangle_intersection.first - == geode::POSITION::edge1 || segment_triangle_intersection.second == geode::POSITION::edge0 || segment_triangle_intersection.second == geode::POSITION::edge1 || segment_triangle_intersection.second - == geode::POSITION::edge2 - || segment_triangle_intersection.first == geode::POSITION::edge2 ) { return true; } + if( segment_triangle_intersection.first + == geode::POSITION::parallel + && segment_in_triangle_plane_intersects( + line_segment, triangle ) ) + { + return true; + } } return false; } diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index 46e2d2ca..ad6dd340 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -256,6 +256,30 @@ void check_brep_vertices_without_unique_vertex() "BRep surface without unique vertices (self)" ); } +void check_brep_line_in_surface_plane() +{ + // A line lying in the plane of a triangle and crossing it + const SurfaceDefinition< 3 > triangle{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 2., 0., 0. } }, + geode::Point3D{ { 0., 2., 0. } } }, + { { 0, 1, 2 } }, {} + }; + const LineDefinition< 3 > crossing{ { geode::Point3D{ { -1., 0.5, 0. } }, + geode::Point3D{ { 3., 0.5, 0. } } }, + { { 0, 1 } }, {} }; + const auto brep = build_brep( { triangle }, { crossing }, 0 ); + check_nb_intersections( nb_intersections( brep ), 1, + "BRep line crossing a surface in its plane" ); + + // A line in the plane only touching a triangle vertex + const LineDefinition< 3 > touching{ { geode::Point3D{ { -1., 0., 0. } }, + geode::Point3D{ { 0., 0., 0. } } }, + { { 0, 1 } }, {} }; + const auto brep2 = build_brep( { triangle }, { touching }, 0 ); + check_nb_intersections( nb_intersections( brep2 ), 0, + "BRep line touching a surface vertex in its plane" ); +} + void check_brep_with_empty_meshes() { // Two crossing triangles plus a surface and a line without vertices @@ -287,6 +311,7 @@ int main() check_section_nested_triangles(); check_brep_vertices_without_unique_vertex(); check_brep_with_empty_meshes(); + check_brep_line_in_surface_plane(); geode::Logger::info( "TEST SUCCESS" ); return 0; From cf5e731eb0afe1530e54bd7286862c9340f6af15 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:27:10 +0200 Subject: [PATCH 06/14] fix(Intersections): detect overlapping collinear line edges Collinear edges are reported as 'parallel' by the segment/segment detection and were ignored, so overlapping or duplicated lines (and lines folding back on themselves) were never reported. model_A1 has 1254 such pairs of duplicated line edges. Co-Authored-By: Claude Opus 5.5 --- .../python/tests/inspection/test-py-brep.py | 8 ++-- .../intersections/model_intersections.cpp | 42 +++++++++++++++++-- tests/inspection/test-brep.cpp | 8 ++-- tests/inspection/test-model-intersections.cpp | 32 ++++++++++++++ 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/bindings/python/tests/inspection/test-py-brep.py b/bindings/python/tests/inspection/test-py-brep.py index 25728e2f..816794c4 100644 --- a/bindings/python/tests/inspection/test-py-brep.py +++ b/bindings/python/tests/inspection/test-py-brep.py @@ -228,9 +228,9 @@ def check_a1(verbose): nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, verbose ) - if nb_component_meshes_issues != 13494: + if nb_component_meshes_issues != 14748: raise ValueError( - "[Test] model model_A1_valid should have 13494 component meshes issues (pairs of component meshes triangles intersecting)." + "[Test] model model_A1_valid should have 14748 component meshes issues (pairs of component meshes triangles intersecting)." ) @@ -249,9 +249,9 @@ def inspect_model_A1(model_brep, verbose): nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, verbose ) - if nb_component_meshes_issues != 13494: + if nb_component_meshes_issues != 14748: raise ValueError( - "[Test] model model_A1_valid should have 13494 component meshes issues (pairs of component meshes triangles intersecting)." + "[Test] model model_A1_valid should have 14748 component meshes issues (pairs of component meshes triangles intersecting)." ) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index f0f0a38c..f6f98f44 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -590,11 +590,47 @@ namespace private: bool lines_intersect( geode::index_t p1_id, geode::index_t p2_id ) const { + const auto segment1 = mesh1_.segment( p1_id ); + const auto segment2 = mesh2_.segment( p2_id ); const auto [position1, position2] = geode::segment_segment_intersection_detection( - mesh1_.segment( p1_id ), mesh2_.segment( p2_id ) ); - return position1 == geode::POSITION::inside - || position2 == geode::POSITION::inside; + segment1, segment2 ); + if( position1 == geode::POSITION::inside + || position2 == geode::POSITION::inside ) + { + return true; + } + return position1 == geode::POSITION::parallel + && collinear_segments_overlap( segment1, segment2 ); + } + + // Collinear segments overlap if an end point is strictly inside the + // other segment, or if they share both end points + [[nodiscard]] static bool collinear_segments_overlap( + const geode::Segment< dimension >& segment1, + const geode::Segment< dimension >& segment2 ) + { + geode::local_index_t nb_shared_vertices{ 0 }; + for( const auto& [segment, other] : + { std::pair{ &segment1, &segment2 }, + std::pair{ &segment2, &segment1 } } ) + { + for( const auto& vertex : segment->vertices() ) + { + const auto position = + geode::point_segment_position( vertex.get(), *other ); + if( position == geode::POSITION::inside ) + { + return true; + } + if( position == geode::POSITION::vertex0 + || position == geode::POSITION::vertex1 ) + { + nb_shared_vertices++; + } + } + } + return nb_shared_vertices == 4; } private: diff --git a/tests/inspection/test-brep.cpp b/tests/inspection/test-brep.cpp index 70e73c49..aed87d9d 100644 --- a/tests/inspection/test-brep.cpp +++ b/tests/inspection/test-brep.cpp @@ -332,8 +332,8 @@ void check_model_a1( bool string ) const auto nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, string ); geode::OpenGeodeInspectorInspectionException::test( - nb_component_meshes_issues == 13494, "model_A1 has ", - nb_component_meshes_issues, " meshes problems instead of 13494." ); + nb_component_meshes_issues == 14748, "model_A1 has ", + nb_component_meshes_issues, " meshes problems instead of 14748." ); } void check_model_a1_valid( bool string ) @@ -355,8 +355,8 @@ void check_model_a1_valid( bool string ) const auto nb_component_meshes_issues = launch_component_meshes_validity_checks( result.meshes, string ); geode::OpenGeodeInspectorInspectionException::test( - nb_component_meshes_issues == 13494, "model_A1_valid has ", - nb_component_meshes_issues, " meshes problems instead of 13494." ); + nb_component_meshes_issues == 14748, "model_A1_valid has ", + nb_component_meshes_issues, " meshes problems instead of 14748." ); } void check_model_mss( bool string ) diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index ad6dd340..df5b7222 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -280,6 +280,37 @@ void check_brep_line_in_surface_plane() "BRep line touching a surface vertex in its plane" ); } +void check_collinear_lines() +{ + // Two lines overlapping along the same direction + const LineDefinition< 2 > line1{ { geode::Point2D{ { 0., 0. } }, + geode::Point2D{ { 2., 0. } } }, + { { 0, 1 } }, {} }; + const LineDefinition< 2 > line2{ { geode::Point2D{ { 1., 0. } }, + geode::Point2D{ { 3., 0. } } }, + { { 0, 1 } }, {} }; + const auto section = build_section( {}, { line1, line2 }, 0 ); + check_nb_intersections( + nb_intersections( section ), 1, "Section overlapping lines" ); + + // Two collinear lines only touching at an end point + const LineDefinition< 2 > line3{ { geode::Point2D{ { 2., 0. } }, + geode::Point2D{ { 3., 0. } } }, + { { 0, 1 } }, {} }; + const auto section2 = build_section( {}, { line1, line3 }, 0 ); + check_nb_intersections( + nb_intersections( section2 ), 0, "Section touching collinear lines" ); + + // A line folding back on itself in 3D + const LineDefinition< 3 > folded{ { geode::Point3D{ { 0., 0., 0. } }, + geode::Point3D{ { 2., 2., 2. } }, + geode::Point3D{ { 1., 1., 1. } } }, + { { 0, 1 }, { 1, 2 } }, {} }; + const auto brep = build_brep( {}, { folded }, 0 ); + check_nb_intersections( + nb_intersections( brep ), 1, "BRep line folding back on itself" ); +} + void check_brep_with_empty_meshes() { // Two crossing triangles plus a surface and a line without vertices @@ -311,6 +342,7 @@ int main() check_section_nested_triangles(); check_brep_vertices_without_unique_vertex(); check_brep_with_empty_meshes(); + check_collinear_lines(); check_brep_line_in_surface_plane(); geode::Logger::info( "TEST SUCCESS" ); From d4a3551a72a441e9479148224c853f6b80bbfc32 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:28:32 +0200 Subject: [PATCH 07/14] fix(Intersections): common vertices use the same point in both triangles A vertex shared by two surfaces (same unique vertex) can have slightly different coordinates in each mesh. The exact predicates then saw a tiny crossing at the common corner and reported a false intersection. The second triangle now uses the first mesh points for its common vertices. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 43 ++++++++++++++++--- tests/inspection/test-model-intersections.cpp | 21 +++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index f6f98f44..3a540a31 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -291,6 +291,34 @@ namespace mesh.point( triangle_vertices[2] ) }; } + // Point of a mesh2 vertex, taken from mesh1 when the vertex is common + // to both triangles: common vertices must be exactly the same point + // even if their coordinates slightly differ between the meshes. + [[nodiscard]] const geode::Point< Model::dim >& mesh2_point( + geode::index_t vertex2, + absl::Span< const std::array< geode::index_t, 2 > > + common_vertices ) const + { + for( const auto& common : common_vertices ) + { + if( common[1] == vertex2 ) + { + return mesh1_.point( common[0] ); + } + } + return mesh2_.point( vertex2 ); + } + + [[nodiscard]] geode::Triangle< Model::dim > mesh2_triangle( + const geode::PolygonVertices& triangle_vertices, + absl::Span< const std::array< geode::index_t, 2 > > + common_vertices ) const + { + return { mesh2_point( triangle_vertices[0], common_vertices ), + mesh2_point( triangle_vertices[1], common_vertices ), + mesh2_point( triangle_vertices[2], common_vertices ) }; + } + private: bool same_surface_; const geode::Surface< Model::dim >& surface1_; @@ -428,7 +456,8 @@ namespace return true; } const auto triangle1 = mesh_triangle( mesh1_, t1_vertices ); - const auto triangle2 = mesh_triangle( mesh2_, t2_vertices ); + const auto triangle2 = + mesh2_triangle( t2_vertices, common_vertices ); return geode::point_triangle_position( t1_third_pt, triangle2 ) != geode::POSITION::outside || geode::point_triangle_position( t2_third_pt, triangle1 ) @@ -449,9 +478,10 @@ namespace t2_vertices[t2_edge_v == 2 ? 0 : t2_edge_v + 1] }; const auto edge_edge_inter = - geode::segment_segment_intersection_detection( - t1_edge, { mesh2_.point( edge2_vertices[0] ), - mesh2_.point( edge2_vertices[1] ) } ); + geode::segment_segment_intersection_detection( t1_edge, + { mesh2_point( edge2_vertices[0], common_vertices ), + mesh2_point( + edge2_vertices[1], common_vertices ) } ); if( edge_edge_inter.first == geode::POSITION::outside || edge_edge_inter.first == geode::POSITION::parallel ) { @@ -481,7 +511,8 @@ namespace // No edge crossing: one triangle may still lie inside the other // (possibly nested in its corner at the common vertex) return non_common_vertex_inside( mesh1_, t1_vertices, - mesh_triangle( mesh2_, t2_vertices ), common_vertices, 0 ) + mesh2_triangle( t2_vertices, common_vertices ), + common_vertices, 0 ) || non_common_vertex_inside( mesh2_, t2_vertices, mesh_triangle( mesh1_, t1_vertices ), common_vertices, 1 ); } @@ -535,7 +566,7 @@ namespace absl::Span< const std::array< geode::index_t, 2 > > common_vertices ) const { - const auto triangle2 = mesh_triangle( mesh2_, t2_vertices ); + const auto triangle2 = mesh2_triangle( t2_vertices, common_vertices ); if( common_vertices.size() == 2 ) { const auto& t1_third_pt = mesh1_.point( diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index df5b7222..bb8fc24e 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -311,6 +311,26 @@ void check_collinear_lines() nb_intersections( brep ), 1, "BRep line folding back on itself" ); } +void check_brep_common_vertex_with_different_coordinates() +{ + // Two triangles sharing unique vertex 0, whose coordinates slightly + // differ between both surfaces: they only touch at this vertex + const SurfaceDefinition< 3 > horizontal{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 1., 0., 0. } }, + geode::Point3D{ { 0., 1., 0. } } }, + { { 0, 1, 2 } }, { 0, 1, 2 } + }; + const SurfaceDefinition< 3 > vertical{ + { geode::Point3D{ { 1e-9, 1e-9, 0. } }, + geode::Point3D{ { -1., -1., 1. } }, + geode::Point3D{ { -1., -1., -1. } } }, + { { 0, 1, 2 } }, { 0, 3, 4 } + }; + const auto brep = build_brep( { horizontal, vertical }, {}, 5 ); + check_nb_intersections( nb_intersections( brep ), 0, + "BRep common vertex with different coordinates" ); +} + void check_brep_with_empty_meshes() { // Two crossing triangles plus a surface and a line without vertices @@ -342,6 +362,7 @@ int main() check_section_nested_triangles(); check_brep_vertices_without_unique_vertex(); check_brep_with_empty_meshes(); + check_brep_common_vertex_with_different_coordinates(); check_collinear_lines(); check_brep_line_in_surface_plane(); From 2b52c5332f5a1a696b63e62651aa338fbbf81603 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:31:34 +0200 Subject: [PATCH 08/14] perf(Intersections): precompute the unique vertices of surface meshes Finding the common vertices of each candidate triangle pair made up to 12 model.unique_vertex() calls (uuid hash map lookup + attribute read). The mesh vertex to unique vertex mapping of each surface is now computed once. corbi_tet_mesh 0.61s -> 0.21s, cva/brep_from_topo 1.76s -> 1.33s, fimiston 11.7s -> 8.2s (identical results). Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 67 ++++++++++++++----- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 3a540a31..fcfb211b 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -183,13 +183,17 @@ namespace public: ModelSurfacesIntersectionBase( const Model& model, const geode::uuid& surface_id1, - const geode::uuid& surface_id2 ) + const geode::uuid& surface_id2, + absl::Span< const geode::index_t > unique_vertices1, + absl::Span< const geode::index_t > unique_vertices2 ) : ModelIntersectionBase< Model >{ model }, same_surface_{ surface_id1 == surface_id2 }, surface1_( model.surface( surface_id1 ) ), surface2_( model.surface( surface_id2 ) ), mesh1_( surface1_.mesh() ), - mesh2_( same_surface_ ? mesh1_ : surface2_.mesh() ) + mesh2_( same_surface_ ? mesh1_ : surface2_.mesh() ), + unique_vertices1_( unique_vertices1 ), + unique_vertices2_( unique_vertices2 ) { } @@ -244,8 +248,7 @@ namespace common_vertices; for( const auto v1_id : t1_vertices ) { - const auto v1_unique_vertex = this->model().unique_vertex( - { surface1_.component_id(), v1_id } ); + const auto v1_unique_vertex = unique_vertices1_[v1_id]; for( const auto v2_id : t2_vertices ) { if( same_surface_ && v1_id == v2_id ) @@ -254,9 +257,7 @@ namespace break; } if( v1_unique_vertex != geode::NO_ID - && v1_unique_vertex - == this->model().unique_vertex( - { surface2_.component_id(), v2_id } ) ) + && v1_unique_vertex == unique_vertices2_[v2_id] ) { common_vertices.push_back( { v1_id, v2_id } ); break; @@ -325,6 +326,8 @@ namespace const geode::Surface< Model::dim >& surface2_; const geode::SurfaceMesh< Model::dim >& mesh1_; const geode::SurfaceMesh< Model::dim >& mesh2_; + absl::Span< const geode::index_t > unique_vertices1_; + absl::Span< const geode::index_t > unique_vertices2_; }; template < typename Model > @@ -334,9 +337,14 @@ namespace public: AllModelSurfacesIntersection( const Model& model, const geode::uuid& surface_id1, - const geode::uuid& surface_id2 ) - : ModelSurfacesIntersectionBase< Model >( - model, surface_id1, surface_id2 ), + const geode::uuid& surface_id2, + absl::Span< const geode::index_t > unique_vertices1, + absl::Span< const geode::index_t > unique_vertices2 ) + : ModelSurfacesIntersectionBase< Model >( model, + surface_id1, + surface_id2, + unique_vertices1, + unique_vertices2 ), same_surface_{ surface_id1 == surface_id2 } { } @@ -362,9 +370,14 @@ namespace public: AllModelSurfacesAutoIntersection( const Model& model, const geode::uuid& surface_id1, - const geode::uuid& surface_id2 ) - : ModelSurfacesIntersectionBase< Model >( - model, surface_id1, surface_id2 ), + const geode::uuid& surface_id2, + absl::Span< const geode::index_t > unique_vertices1, + absl::Span< const geode::index_t > unique_vertices2 ) + : ModelSurfacesIntersectionBase< Model >( model, + surface_id1, + surface_id2, + unique_vertices1, + unique_vertices2 ), same_surface_{ surface_id1 == surface_id2 } { } @@ -697,8 +710,24 @@ namespace geode Impl( const Model& model ) : model_( model ), surfaces_model_tree_{ create_surface_meshes_aabb_trees( model ) }, - lines_model_tree_{ create_line_meshes_aabb_trees( model ) } + lines_model_tree_{ create_line_meshes_aabb_trees( model ) }, + surfaces_unique_vertices_( surfaces_model_tree_.uuids_.size() ) { + async::parallel_for( async::irange( index_t{ 0 }, + static_cast< index_t >( + surfaces_unique_vertices_.size() ) ), + [this]( index_t tree_id ) { + const auto& surface = + model_.surface( surfaces_model_tree_.uuids_[tree_id] ); + const auto surface_id = surface.component_id(); + auto& unique_vertices = surfaces_unique_vertices_[tree_id]; + unique_vertices.resize( surface.mesh().nb_vertices() ); + for( const auto v : Indices{ unique_vertices } ) + { + unique_vertices[v] = + model_.unique_vertex( { surface_id, v } ); + } + } ); } [[nodiscard]] bool model_has_intersecting_surfaces() const @@ -821,7 +850,9 @@ namespace geode const auto& surface_uuid = surfaces_model_tree_.uuids_[tree_id]; const Action surfaces_intersection_action{ model_, - surface_uuid, surface_uuid }; + surface_uuid, surface_uuid, + surfaces_unique_vertices_[tree_id], + surfaces_unique_vertices_[tree_id] }; const auto intersecting_elements = surfaces_model_tree_.mesh_trees_[tree_id] .compute_self_element_bbox_intersections( @@ -851,7 +882,9 @@ namespace geode const auto surface_uuid2 = surfaces_model_tree_.uuids_[components.second]; const Action surfaces_intersection_action{ model_, - surface_uuid1, surface_uuid2 }; + surface_uuid1, surface_uuid2, + surfaces_unique_vertices_[components.first], + surfaces_unique_vertices_[components.second] }; const auto intersecting_elements = surfaces_model_tree_.mesh_trees_[components.first] .compute_other_element_bbox_intersections( @@ -1001,6 +1034,8 @@ namespace geode const Model& model_; ModelMeshesAABBTree< Model::dim > surfaces_model_tree_; ModelMeshesAABBTree< Model::dim > lines_model_tree_; + // Unique vertex of each mesh vertex, per surface mesh tree + std::vector< std::vector< index_t > > surfaces_unique_vertices_; }; template < typename Model > From 2c37c681069218eabe97018fd9a33820e233ac9b Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:33:04 +0200 Subject: [PATCH 09/14] perf(Intersections): reject 3D triangle pairs separated by a plane Before the segment/triangle tests (up to 12 exact orientations), check if the non-common vertices of a triangle are strictly on one side of the other triangle plane: the triangles can then only touch at their common vertex. cva/brep_from_topo 1.33s -> 1.05s, fimiston 8.2s -> 6.5s (identical results). Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 28 +++++++++++++++++ tests/inspection/test-model-intersections.cpp | 30 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index fcfb211b..52dd518a 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -571,6 +571,28 @@ namespace return false; } + // True if the triangle vertices, except the common ones lying on the + // other triangle plane, are all strictly on the same side of this plane: + // both triangles can then only touch at their common vertex (if any). + [[nodiscard]] bool strictly_on_one_side( const geode::Triangle3D& triangle, + const geode::Triangle3D& other, + geode::index_t nb_common_vertices ) + { + std::array< geode::SIDE, 3 > sides; + for( const auto v : geode::LRange{ 3 } ) + { + sides[v] = geode::point_side_to_triangle( + triangle.vertices()[v].get(), other ); + } + const auto nb_zeros = absl::c_count( sides, geode::SIDE::zero ); + if( nb_zeros > nb_common_vertices ) + { + return false; + } + const auto nb_positives = absl::c_count( sides, geode::SIDE::positive ); + return nb_positives == 3 - nb_zeros || nb_positives == 0; + } + template <> [[nodiscard]] bool ModelSurfacesIntersectionBase< geode::BRep >::triangles_intersect( @@ -597,6 +619,12 @@ namespace == geode::POSITION::parallel; } const auto triangle1 = mesh_triangle( mesh1_, t1_vertices ); + if( strictly_on_one_side( triangle1, triangle2, common_vertices.size() ) + || strictly_on_one_side( + triangle2, triangle1, common_vertices.size() ) ) + { + return false; + } return triangle_intersects_other( triangle1, triangle2, t1_vertices, t2_vertices, common_vertices, 1 ) || triangle_intersects_other( triangle2, triangle1, t2_vertices, diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index bb8fc24e..ff350b28 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -331,6 +331,35 @@ void check_brep_common_vertex_with_different_coordinates() "BRep common vertex with different coordinates" ); } +void check_brep_triangle_touching_plane() +{ + // A triangle with one vertex touching the inside of another triangle, + // its two other vertices being on the same side of the plane + const SurfaceDefinition< 3 > horizontal{ + { geode::Point3D{ { 0., 0., 0. } }, geode::Point3D{ { 2., 0., 0. } }, + geode::Point3D{ { 0., 2., 0. } } }, + { { 0, 1, 2 } }, {} + }; + const SurfaceDefinition< 3 > touching{ + { geode::Point3D{ { 0.5, 0.5, 0. } }, + geode::Point3D{ { 0.5, 0.5, 1. } }, + geode::Point3D{ { 1., 0., 1. } } }, + { { 0, 1, 2 } }, {} + }; + const auto brep = build_brep( { horizontal, touching }, {}, 0 ); + check_nb_intersections( + nb_intersections( brep ), 1, "BRep triangle touching another" ); + + // Same configuration, slightly above the plane + const SurfaceDefinition< 3 > above{ { geode::Point3D{ { 0.5, 0.5, 0.1 } }, + geode::Point3D{ { 0.5, 0.5, 1. } }, + geode::Point3D{ { 1., 0., 1. } } }, + { { 0, 1, 2 } }, {} }; + const auto brep2 = build_brep( { horizontal, above }, {}, 0 ); + check_nb_intersections( + nb_intersections( brep2 ), 0, "BRep triangle above another" ); +} + void check_brep_with_empty_meshes() { // Two crossing triangles plus a surface and a line without vertices @@ -362,6 +391,7 @@ int main() check_section_nested_triangles(); check_brep_vertices_without_unique_vertex(); check_brep_with_empty_meshes(); + check_brep_triangle_touching_plane(); check_brep_common_vertex_with_different_coordinates(); check_collinear_lines(); check_brep_line_in_surface_plane(); From 8f27752231cee0361da21a02d49fde0bd564a9e6 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:34:44 +0200 Subject: [PATCH 10/14] perf(Intersections): self intersections skip pairs of different surfaces inspect_surfaces_self_intersections still traversed the AABB trees of every pair of overlapping surfaces, only to reject every polygon pair in the action. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 75 ++++++++++--------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 52dd518a..d6d92118 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -898,41 +898,48 @@ namespace geode return result; } ) ); } - const auto surfaces_overlap = - surfaces_model_tree_.components_tree_ - .compute_self_element_bbox_intersections( - ComponentOverlap{} ); - for( const auto& components : surfaces_overlap ) + // Self intersections only: polygons of different surfaces are + // never tested + std::vector< std::pair< index_t, index_t > > surfaces_overlap; + if constexpr( !std::is_same_v< Action, + AllModelSurfacesAutoIntersection< Model > > ) { - tasks.emplace_back( async::spawn( [this, &components] { - const auto surface_uuid1 = - surfaces_model_tree_.uuids_[components.first]; - const auto surface_uuid2 = - surfaces_model_tree_.uuids_[components.second]; - const Action surfaces_intersection_action{ model_, - surface_uuid1, surface_uuid2, - surfaces_unique_vertices_[components.first], - surfaces_unique_vertices_[components.second] }; - const auto intersecting_elements = - surfaces_model_tree_.mesh_trees_[components.first] - .compute_other_element_bbox_intersections( - surfaces_model_tree_ - .mesh_trees_[components.second], - surfaces_intersection_action ); - IntersectionsResult result; - const auto component_id1 = - model_.surface( surface_uuid1 ).component_id(); - const auto component_id2 = - model_.surface( surface_uuid2 ).component_id(); - for( const auto& [polygon1, polygon2] : - intersecting_elements ) - { - result.emplace_back( - ComponentMeshElement{ component_id1, polygon1 }, - ComponentMeshElement{ component_id2, polygon2 } ); - } - return result; - } ) ); + surfaces_overlap = surfaces_model_tree_.components_tree_ + .compute_self_element_bbox_intersections( + ComponentOverlap{} ); + for( const auto& components : surfaces_overlap ) + { + tasks.emplace_back( async::spawn( [this, &components] { + const auto surface_uuid1 = + surfaces_model_tree_.uuids_[components.first]; + const auto surface_uuid2 = + surfaces_model_tree_.uuids_[components.second]; + const Action surfaces_intersection_action{ model_, + surface_uuid1, surface_uuid2, + surfaces_unique_vertices_[components.first], + surfaces_unique_vertices_[components.second] }; + const auto intersecting_elements = + surfaces_model_tree_.mesh_trees_[components.first] + .compute_other_element_bbox_intersections( + surfaces_model_tree_ + .mesh_trees_[components.second], + surfaces_intersection_action ); + IntersectionsResult result; + const auto component_id1 = + model_.surface( surface_uuid1 ).component_id(); + const auto component_id2 = + model_.surface( surface_uuid2 ).component_id(); + for( const auto& [polygon1, polygon2] : + intersecting_elements ) + { + result.emplace_back( + ComponentMeshElement{ component_id1, polygon1 }, + ComponentMeshElement{ + component_id2, polygon2 } ); + } + return result; + } ) ); + } } for( auto& task : async::when_all( tasks ).get() ) { From e3f772e0fc00f555742fab32ec29eca5eb5dae44 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:36:27 +0200 Subject: [PATCH 11/14] perf(Intersections): cull and parallelize line/surface checks Each surface was tested serially against every embedded line, without checking their bounding boxes first, and nb_embedding_blocks was recomputed for each surface. Overlapping (surface, line) pairs now come from the components AABB trees and are processed in parallel, in the same order as before. No change on the test models (few embedded lines); the cost was quadratic in the number of surfaces and lines. Co-Authored-By: Claude Opus 5.5 --- .../intersections/model_intersections.cpp | 79 ++++++++++++------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index d6d92118..61bfe5a5 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -1030,37 +1030,60 @@ namespace geode std::vector< std::pair< ComponentMeshElement, ComponentMeshElement > > component_intersections; - for( const auto surface_tree_id : - Indices{ surfaces_model_tree_.uuids_ } ) + std::vector< bool > embedded_lines( + lines_model_tree_.uuids_.size() ); + for( const auto line_tree_id : Indices{ lines_model_tree_.uuids_ } ) { - const auto& surface = brep.surface( - surfaces_model_tree_.uuids_[surface_tree_id] ); - const auto& surface_tree = - surfaces_model_tree_.mesh_trees_[surface_tree_id]; - for( const auto line_tree_id : - Indices{ lines_model_tree_.uuids_ } ) + embedded_lines[line_tree_id] = + brep.nb_embedding_blocks( + brep.line( lines_model_tree_.uuids_[line_tree_id] ) ) + != 0; + } + auto overlapping_components = + surfaces_model_tree_.components_tree_ + .compute_other_element_bbox_intersections( + lines_model_tree_.components_tree_, + ComponentOverlap{} ); + absl::c_sort( overlapping_components ); + using Task = async::task< IntersectionsResult >; + std::vector< Task > tasks; + tasks.reserve( overlapping_components.size() ); + for( const auto& components : overlapping_components ) + { + const auto surface_tree_id = components.first; + const auto line_tree_id = components.second; + if( !embedded_lines[line_tree_id] ) { - const auto& line = - brep.line( lines_model_tree_.uuids_[line_tree_id] ); - if( brep.nb_embedding_blocks( line ) == 0 ) - { - continue; - } - const BRepLineSurfacesIntersection action{ brep, - surface.id(), line.id() }; - const auto& line_tree = - lines_model_tree_.mesh_trees_[line_tree_id]; - for( const auto& element_pair : - surface_tree.compute_other_element_bbox_intersections( - line_tree, action ) ) - { - component_intersections.emplace_back( - ComponentMeshElement{ - surface.component_id(), element_pair.first }, - ComponentMeshElement{ - line.component_id(), element_pair.second } ); - } + continue; } + tasks.emplace_back( + async::spawn( [this, &brep, surface_tree_id, line_tree_id] { + const auto& surface = brep.surface( + surfaces_model_tree_.uuids_[surface_tree_id] ); + const auto& line = + brep.line( lines_model_tree_.uuids_[line_tree_id] ); + const BRepLineSurfacesIntersection action{ brep, + surface.id(), line.id() }; + IntersectionsResult result; + for( const auto& element_pair : + surfaces_model_tree_.mesh_trees_[surface_tree_id] + .compute_other_element_bbox_intersections( + lines_model_tree_.mesh_trees_[line_tree_id], + action ) ) + { + result.emplace_back( + ComponentMeshElement{ surface.component_id(), + element_pair.first }, + ComponentMeshElement{ line.component_id(), + element_pair.second } ); + } + return result; + } ) ); + } + for( auto& task : async::when_all( tasks ).get() ) + { + absl::c_move( + task.get(), std::back_inserter( component_intersections ) ); } return component_intersections; } From b1b4d8a7799d0f8c5051aaa7867803050644fb05 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:38:22 +0200 Subject: [PATCH 12/14] perf(Intersections): remove redundant point positions in surface/curve test The two point_triangle_position calls before the segment/triangle detection are covered by it (inside second position, or parallel then end point check). Brute-force check on 2M random integer segment/triangle pairs, including degenerate ones: identical results. Co-Authored-By: Claude Opus 5.5 --- .../intersections/surface_curve_intersections.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/surface_curve_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/surface_curve_intersections.cpp index a8834d27..9b104c98 100644 --- a/src/geode/inspector/inspection/criterion/intersections/surface_curve_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/surface_curve_intersections.cpp @@ -109,15 +109,6 @@ namespace { const auto triangle = surface_.triangle( triangle_id ); const auto segment = curve_.segment( edge_id ); - for( const auto ev : geode::LRange{ 2 } ) - { - if( geode::point_triangle_position( - segment.vertices()[ev].get(), triangle ) - == geode::POSITION::inside ) - { - return true; - } - } const auto result = geode::segment_triangle_intersection_detection( segment, triangle ); if( result.first == geode::POSITION::outside From 86b5eee6cafe9f55f653e3c3857ac81bad0ff229 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Wed, 14 Oct 2026 18:38:43 +0200 Subject: [PATCH 13/14] perf(Intersections): no heap allocation for quad fan triangles polygon_fan_triangles stored a single triangle inline, so every quad (two fan triangles) allocated for each candidate polygon pair. Co-Authored-By: Claude Opus 5.5 --- .../criterion/intersections/model_intersections.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index 61bfe5a5..be4ad657 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -58,11 +58,11 @@ namespace } }; - [[nodiscard]] absl::InlinedVector< geode::PolygonVertices, 1 > + [[nodiscard]] absl::InlinedVector< geode::PolygonVertices, 2 > polygon_fan_triangles( const geode::PolygonVertices& polygon, geode::local_index_t apex ) { - absl::InlinedVector< geode::PolygonVertices, 1 > triangles; + absl::InlinedVector< geode::PolygonVertices, 2 > triangles; for( const auto edge : geode::LRange{ 1, polygon.size() - 1 } ) { const auto next_apex = ( apex + edge ) % polygon.size(); From 0a2aa9374e0c008f4b403eae9430310943b264e3 Mon Sep 17 00:00:00 2001 From: Arnaud Botella Date: Thu, 15 Oct 2026 16:36:40 +0200 Subject: [PATCH 14/14] test(Intersections): straight line with collinear edges, explain nested check Review follow-up: a straight line made of several collinear edges reports no self intersection, and a comment explains why a single non common vertex is tested for 2D nested triangles. Co-Authored-By: Claude Opus 5.5 --- .../criterion/intersections/model_intersections.cpp | 3 +++ tests/inspection/test-model-intersections.cpp | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp index be4ad657..f0bfcda9 100644 --- a/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp +++ b/src/geode/inspector/inspection/criterion/intersections/model_intersections.cpp @@ -416,6 +416,9 @@ namespace }; } + // Only called when no edge of the triangle crosses the other triangle: + // its non common vertices are then either all inside or all outside + // the other triangle, so testing the first one is enough [[nodiscard]] bool non_common_vertex_inside( const geode::SurfaceMesh2D& mesh, const geode::PolygonVertices& triangle_vertices, diff --git a/tests/inspection/test-model-intersections.cpp b/tests/inspection/test-model-intersections.cpp index ff350b28..415bf188 100644 --- a/tests/inspection/test-model-intersections.cpp +++ b/tests/inspection/test-model-intersections.cpp @@ -309,6 +309,17 @@ void check_collinear_lines() const auto brep = build_brep( {}, { folded }, 0 ); check_nb_intersections( nb_intersections( brep ), 1, "BRep line folding back on itself" ); + + // A straight line made of several collinear edges: an edge is never + // compared with itself and consecutive edges share a single end point + const LineDefinition< 3 > straight{ { geode::Point3D{ { 0., 0., 0. } }, + geode::Point3D{ { 1., 1., 1. } }, + geode::Point3D{ { 2., 2., 2. } }, + geode::Point3D{ { 3., 3., 3. } } }, + { { 0, 1 }, { 1, 2 }, { 2, 3 } }, {} }; + const auto brep2 = build_brep( {}, { straight }, 0 ); + check_nb_intersections( nb_intersections( brep2 ), 0, + "BRep straight line with collinear edges" ); } void check_brep_common_vertex_with_different_coordinates()