Skip to content

feat(VertexIdentifier): add a unique_vertex_attribute_id #1324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BotellaA merged 2 commits into from
Aug 26, 2026
Merged

feat(VertexIdentifier): add a unique_vertex_attribute_id #1324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8aa91fb
feat(VertexIdentifier): add a unique_vertex_attribute_id
BenPinet Aug 25, 2026
a91ec66
comments
BenPinet Aug 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/geode/model/mixin/core/vertex_identifier.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
/*!
* Identify a vertex in a geometric component
*/
struct opengeode_model_api ComponentMeshVertex

Check warning on line 50 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:50:32 [cppcoreguidelines-special-member-functions] 

class 'ComponentMeshVertex' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator
{
ComponentMeshVertex(
ComponentID component_id_in, index_t vertex_id_in );
Expand All @@ -69,7 +69,7 @@
}

template < typename H >
friend H AbslHashValue( H h, const ComponentMeshVertex& value )

Check warning on line 72 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:72:35 [readability-identifier-length] 

parameter name 'h' is too short, expected at least 3 characters

Check warning on line 72 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:72:18 [readability-identifier-naming] 

invalid case style for global function 'AbslHashValue'
{
return H::combine(
std::move( h ), value.component_id, value.vertex );
Expand All @@ -88,12 +88,12 @@
* as unique vertices.
* This is a only topological information.
*/
class opengeode_model_api VertexIdentifier

Check warning on line 91 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:91:31 [cppcoreguidelines-special-member-functions] 

class 'VertexIdentifier' defines a destructor, a move constructor and a move assignment operator but does not define a copy constructor or a copy assignment operator
{
public:
PASSKEY( VertexIdentifierBuilder, BuilderKey /*key*/ );
VertexIdentifier();
VertexIdentifier( BITSERY );

Check warning on line 96 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:96:9 [google-explicit-constructor] 

single-argument constructors must be marked explicit to avoid unintentional implicit conversions
~VertexIdentifier();

[[nodiscard]] index_t nb_unique_vertices() const;
Expand Down Expand Up @@ -134,6 +134,8 @@
*/
void save_unique_vertices( std::string_view directory ) const;

[[nodiscard]] const uuid& unique_vertex_attribute_id() const;

public:
/*!
* Add a component in the VertexIdentifier
Expand Down Expand Up @@ -167,7 +169,7 @@
* Create several empty unique vertices
* @return Index of the first created unique vertex
*/
index_t create_unique_vertices( index_t nb, BuilderKey /*key*/ );

Check warning on line 172 in include/geode/model/mixin/core/vertex_identifier.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/model/mixin/core/vertex_identifier.hpp:172:49 [readability-identifier-length] 

parameter name 'nb' is too short, expected at least 3 characters

/*!
* Identify a component vertex to an existing unique vertex index.
Expand Down
18 changes: 15 additions & 3 deletions src/geode/model/helpers/convert_model_meshes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@
}
const auto unique_vertices =
save_unique_vertices( model, mesh, surface.component_id() );
const auto& unique_vertex_attribute_id =
model.unique_vertex_attribute_id();
if( mesh_type
== geode::TriangulatedSurface< Model::dim >::type_name_static() )
{
Expand All @@ -94,15 +96,19 @@
geode::OpenGeodeException::TYPE::internal,
"[do_convert_surface] Cannot convert SurfaceMesh "
"to TriangulatedSurface" );
tri_surface.value()->vertex_attribute_manager().delete_attribute(

Check warning on line 99 in src/geode/model/helpers/convert_model_meshes.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/helpers/convert_model_meshes.cpp:99:13 [bugprone-unchecked-optional-access] 

unchecked access to optional value
unique_vertex_attribute_id );
builder.update_surface_mesh(
surface, std::move( tri_surface ).value() );

Check warning on line 102 in src/geode/model/helpers/convert_model_meshes.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/helpers/convert_model_meshes.cpp:102:26 [bugprone-unchecked-optional-access] 

unchecked access to optional value
}
else if( mesh_type
== geode::PolygonalSurface< Model::dim >::type_name_static() )
{
builder.update_surface_mesh( surface,
std::move( geode::convert_surface_mesh_into_polygonal_surface(
mesh ) ) );
auto poly_surface =
geode::convert_surface_mesh_into_polygonal_surface( mesh );
poly_surface->vertex_attribute_manager().delete_attribute(
Comment thread
BenPinet marked this conversation as resolved.
unique_vertex_attribute_id );
builder.update_surface_mesh( surface, std::move( poly_surface ) );
}
set_unique_vertices( builder, unique_vertices, surface.component_id() );
}
Expand Down Expand Up @@ -149,6 +155,8 @@
}
const auto unique_vertices =
save_unique_vertices( model, mesh, block.component_id() );
const auto& unique_vertex_attribute_id =
model.unique_vertex_attribute_id();
if( mesh_type == geode::TetrahedralSolid3D::type_name_static() )
{
auto tet_solid =
Expand All @@ -158,7 +166,9 @@
geode::OpenGeodeException::TYPE::internal,
"[do_convert_block] Cannot convert "
"SolidMesh to TetrahedralSolid" );
tet_solid.value()->vertex_attribute_manager().delete_attribute(

Check warning on line 169 in src/geode/model/helpers/convert_model_meshes.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/helpers/convert_model_meshes.cpp:169:13 [bugprone-unchecked-optional-access] 

unchecked access to optional value
unique_vertex_attribute_id );
builder.update_block_mesh( block, std::move( tet_solid ).value() );

Check warning on line 171 in src/geode/model/helpers/convert_model_meshes.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/helpers/convert_model_meshes.cpp:171:47 [bugprone-unchecked-optional-access] 

unchecked access to optional value
}
else if( mesh_type == geode::HybridSolid3D::type_name_static() )
{
Expand All @@ -168,6 +178,8 @@
hybrid_solid.has_value(), nullptr,
geode::OpenGeodeException::TYPE::internal,
"[do_convert_block] Cannot convert SolidMesh to HybridSolid" );
hybrid_solid.value()->vertex_attribute_manager().delete_attribute(
unique_vertex_attribute_id );
builder.update_block_mesh(
block, std::move( hybrid_solid ).value() );
}
Expand Down
89 changes: 71 additions & 18 deletions src/geode/model/mixin/core/vertex_identifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,27 +186,31 @@
return false;
}

const uuid& unique_vertex_attribute_id() const
{
return unique_vertex_id_;
}

template < typename MeshComponent >
void register_component( const MeshComponent& component )
{
AttributeProperties attribute_properties;
attribute_properties.assignable = false;
attribute_properties.interpolable = false;
attribute_properties.transferable = false;
AttributeValues< index_t > unqiue_vertex_attribute_values;
unqiue_vertex_attribute_values.default_value = NO_ID;
unqiue_vertex_attribute_values.no_value = NO_ID;
AttributeValues< index_t > unique_vertex_attribute_values;
unique_vertex_attribute_values.default_value = NO_ID;
unique_vertex_attribute_values.no_value = NO_ID;
const auto& mesh = component.mesh();
const auto unique_vertices_attribute_id =
mesh.vertex_attribute_manager()
.template create_attribute< VariableAttribute, index_t >(
UNIQUE_VERTICES_NAME, unqiue_vertex_attribute_values,
attribute_properties );
mesh.vertex_attribute_manager()
.template create_attribute< VariableAttribute, index_t >(
UNIQUE_VERTICES_NAME, unique_vertex_id_,
unique_vertex_attribute_values, attribute_properties );
const auto [_, inserted] =
vertex2unique_vertex_.emplace( component.id(),
mesh.vertex_attribute_manager()
.template find_attribute< VariableAttribute, index_t >(
unique_vertices_attribute_id ) );
unique_vertex_id_ ) );
OpenGeodeModelException::check_exception( inserted,
component.component_id(), OpenGeodeException::TYPE::data,
"[VertexIdentifier::register_component] Component ",
Expand All @@ -217,19 +221,17 @@
void load_component( const MeshComponent& component )
{
const auto& mesh = component.mesh();
const auto unique_vertices_ids =
mesh.vertex_attribute_manager().attribute_ids_matching_name(
UNIQUE_VERTICES_NAME );
OpenGeodeModelException::check_exception(
unique_vertices_ids.has_value(), nullptr,
OpenGeodeException::TYPE::data,
"[VertexIdentifier::load_component] Unique vertices "
"attribute not found." );
if( !mesh.vertex_attribute_manager().attribute_exists(
unique_vertex_id_ ) )
{
import_old_attribute_based_on_name(
mesh.vertex_attribute_manager(), mesh.nb_vertices() );
}
const auto [_, inserted] =
vertex2unique_vertex_.emplace( component.id(),
mesh.vertex_attribute_manager()
.template find_attribute< VariableAttribute, index_t >(
unique_vertices_ids.value().front() ) );
unique_vertex_id_ ) );
OpenGeodeModelException::check_exception( inserted,
component.component_id(), OpenGeodeException::TYPE::data,
"[VertexIdentifier::load_component] Component ",
Expand Down Expand Up @@ -442,7 +444,7 @@
std::shared_ptr< VariableAttribute< index_t > > >
old_map;
archive.ext( old_map,
bitsery::ext::StdMap{ old_map.max_size() },

Check failure on line 447 in src/geode/model/mixin/core/vertex_identifier.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/mixin/core/vertex_identifier.cpp:447:44 [clang-diagnostic-error] 

no member named 'StdMap' in namespace 'bitsery::ext'
[]( Archive& archive2, uuid& id,
std::shared_ptr< VariableAttribute<
index_t > >& attribute ) {
Expand All @@ -466,7 +468,7 @@
VariableAttribute< index_t > > >
old_map;
archive.ext( old_map,
bitsery::ext::StdMap{ old_map.max_size() },

Check failure on line 471 in src/geode/model/mixin/core/vertex_identifier.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/model/mixin/core/vertex_identifier.cpp:471:47 [clang-diagnostic-error] 

no member named 'StdMap' in namespace 'bitsery::ext'
[]( Archive& archive2, uuid& id,
std::shared_ptr< VariableAttribute<
index_t > >& attribute ) {
Expand All @@ -479,6 +481,12 @@
archive.object( impl.unique_vertices_ );
archive.ext( impl.component_vertices_,
bitsery::ext::StdSmartPtr{} );
},
[]( Archive& archive, Impl& impl ) {
archive.object( impl.unique_vertices_ );
archive.ext( impl.component_vertices_,
bitsery::ext::StdSmartPtr{} );
archive.object( impl.unique_vertex_id_ );
} } } );
}

Expand Down Expand Up @@ -510,6 +518,45 @@
} );
}

void import_old_attribute_based_on_name(
AttributeManager& vertex_attribute_manager,
const index_t nb_vertices )
{
const auto unique_vertices_ids =
vertex_attribute_manager.attribute_ids_matching_name(
UNIQUE_VERTICES_NAME );
OpenGeodeModelException::check_exception(
unique_vertices_ids.has_value(), nullptr,
OpenGeodeException::TYPE::data,
"[VertexIdentifier::load_component] Unique vertices "
"attribute not found." );
OpenGeodeModelException::check_exception(
unique_vertices_ids.value().size() == 1, nullptr,
OpenGeodeException::TYPE::data,
"[VertexIdentifier::load_component] Unique vertices "
"attribute is not unique." );
const auto old_attribute =
vertex_attribute_manager
.template find_attribute< VariableAttribute, index_t >(
unique_vertices_ids.value().front() );
vertex_attribute_manager
.template create_attribute< VariableAttribute, index_t >(
UNIQUE_VERTICES_NAME, unique_vertex_id_,
old_attribute->default_values(),
old_attribute->properties() );
const auto new_attribute =
vertex_attribute_manager
.template find_attribute< VariableAttribute, index_t >(
unique_vertex_id_ );
for( const auto vertex : Range{ nb_vertices } )
{
new_attribute->set_value(
vertex, old_attribute->value( vertex ) );
}
vertex_attribute_manager.delete_attribute(
unique_vertices_ids.value().front() );
}

private:
OpenGeodeVertexSet unique_vertices_;
std::shared_ptr<
Expand All @@ -518,6 +565,7 @@
absl::flat_hash_map< uuid,
std::shared_ptr< VariableAttribute< index_t > > >
vertex2unique_vertex_;
geode::uuid unique_vertex_id_;
};

VertexIdentifier::VertexIdentifier() = default;
Expand Down Expand Up @@ -567,6 +615,11 @@
unique_vertex_id, component_id );
}

const uuid& VertexIdentifier::unique_vertex_attribute_id() const
{
return impl_->unique_vertex_attribute_id();
}

template < typename MeshComponent >
void VertexIdentifier::load_mesh_component(
const MeshComponent& component, BuilderKey /*key*/ )
Expand Down
Binary file added tests/data/backward_io/v18_0/v18_0.og_brep
View file
Open in desktop
Binary file not shown.
41 changes: 36 additions & 5 deletions tests/model/test-brep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,42 @@ void test_backward_io()
"[Backward_IO] Incorrect brep unique_vertex_id." );
}
test_registry( brep_v17, 4, 6, 9, 5, 1, 5, 2, 2, 2, 1, 3 );
auto brep_v18_0 = geode::load_brep(
absl::StrCat( geode::DATA_PATH, "backward_io/v18_0/v18_0.og_brep" ) );
geode::BRepBuilder brep_builder_v18{ brep_v18_0 };
for( const auto& block : brep_v18_0.blocks() )
{
geode::OpenGeodeModelException::test( block.id() == block.mesh().id(),
"[Backward_IO] Brep block should have the same uuid as its mesh." );
}
for( const auto& surface : brep_v18_0.surfaces() )
{
geode::OpenGeodeModelException::test(
surface.id() == surface.mesh().id(),
"[Backward_IO] Brep surface should have the same uuid as its "
"mesh." );
}
for( const auto& line : brep_v18_0.lines() )
{
geode::OpenGeodeModelException::test( line.id() == line.mesh().id(),
"[Backward_IO] Brep line should have the same uuid as its mesh." );
}
for( const auto& corner : brep_v18_0.corners() )
{
geode::OpenGeodeModelException::test( corner.id() == corner.mesh().id(),
"[Backward_IO] Brep corner should have the same uuid as its "
"mesh." );
}
for( const auto& surface : brep_v18_0.surfaces() )
{
auto vertex_index =
brep_builder_v18.surface_mesh_builder( surface )->create_vertex();
geode::OpenGeodeModelException::test(
brep_v18_0.unique_vertex( { surface.component_id(), vertex_index } )
== geode::NO_ID,
"[Backward_IO] Incorrect brep unique_vertex_id." );
}
test_registry( brep_v18_0, 4, 6, 9, 5, 1, 5, 2, 2, 2, 1, 3 );
}

void test_components_filter()
Expand Down Expand Up @@ -1717,19 +1753,14 @@ void test()
test_block_collection_ranges( model, block_uuid, block_collection_uuid );
test_clone( model );
test_steal_mesh( model );
DEBUG( "io" );
const auto file_io = absl::StrCat( "test.", model.native_extension() );
geode::save_brep( model, file_io );
DEBUG( "start load" );
auto model2 = geode::load_brep( file_io );
geode::BRepBuilder model2_builder{ model2 };
for( const auto& surface : model2.surfaces() )
{
auto vertex_index =
model2_builder.surface_mesh_builder( surface )->create_vertex();
DEBUG( vertex_index );
DEBUG(
model2.unique_vertex( { surface.component_id(), vertex_index } ) );
geode::OpenGeodeModelException::test(
model2.unique_vertex( { surface.component_id(), vertex_index } )
== geode::NO_ID,
Expand Down
Loading