diff --git a/src/OpenMesh/Core/Utils/PropertyManager.hh b/src/OpenMesh/Core/Utils/PropertyManager.hh index 529ee120..db546d4d 100644 --- a/src/OpenMesh/Core/Utils/PropertyManager.hh +++ b/src/OpenMesh/Core/Utils/PropertyManager.hh @@ -60,7 +60,23 @@ namespace OpenMesh { * It also defines convenience operators to access the encapsulated * property's value. * - * Usage example: + * For C++11, it is recommended to use the factory functions + * makePropertyManagerFromNew, makePropertyManagerFromExisting, + * makePropertyManagerFromExistingOrNew to construct a PropertyManager, e.g. + * + * \code + * TriMesh mesh; + * auto visited = makePropertyManagerFromNew>(mesh, "visited.plugin-example.i8.informatik.rwth-aachen.de"); + * + * for (auto vh : mesh.vertices()) { + * if (!visited[vh]) { + * visitComponent(mesh, vh, visited); + * } + * } + * \endcode + * + * For C++98, it is usually more convenient to use the constructor explicitly, + * i.e. * * \code * TriMesh mesh; @@ -107,6 +123,9 @@ class PropertyManager { * the property is deleted upon destruction of the PropertyManager instance). If true, * the instance merely acts as a convenience wrapper around an existing property with no * lifecycle management whatsoever. + * + * @see PropertyManager::createIfNotExists, makePropertyManagerFromNew, + * makePropertyManagerFromExisting, makePropertyManagerFromExistingOrNew */ PropertyManager(MeshT &mesh, const char *propname, bool existing = false) : mesh_(&mesh), retain_(existing), name_(propname) { if (existing) { @@ -164,15 +183,14 @@ class PropertyManager { * Move assignment. Transfers ownership (delete responsibility). */ PropertyManager &operator=(PropertyManager &&rhs) { - - deleteProperty(); - - mesh_ = rhs.mesh_; - prop_ = rhs.prop_; - retain_ = rhs.retain_; - name_ = rhs.name_; - rhs.retain_ = true; - + if (&rhs != this) { + deleteProperty(); + mesh_ = rhs.mesh_; + prop_ = rhs.prop_; + retain_ = rhs.retain_; + name_ = rhs.name_; + rhs.retain_ = true; + } return *this; } @@ -180,6 +198,8 @@ class PropertyManager { * Create a property manager for the supplied property and mesh. * If the property doesn't exist, it is created. In any case, * lifecycle management is disabled. + * + * @see makePropertyManagerFromExistingOrNew */ static PropertyManager createIfNotExists(MeshT &mesh, const char *propname) { PROPTYPE dummy_prop; @@ -192,7 +212,7 @@ class PropertyManager { PropertyManager duplicate(const char *clone_name) { PropertyManager pm(*mesh_, clone_name, false); pm.mesh_->property(pm.prop_) = mesh_->property(prop_); - return std::move(pm); + return pm; } /** @@ -237,6 +257,8 @@ class PropertyManager { * Create a property manager for the supplied property and mesh. * If the property doesn't exist, it is created. In any case, * lifecycle management is disabled. + * + * @see makePropertyManagerFromExistingOrNew */ static Proxy createIfNotExists(MeshT &mesh, const char *propname) { PROPTYPE dummy_prop; @@ -406,5 +428,43 @@ class PropertyManager { std::string name_; }; +/** + * Creates a new property whose lifecycle is managed by the returned + * PropertyManager. + * + * Intended for temporary properties. Shadows any existsing properties of + * matching name and type. + */ +template +PropertyManager makePropertyManagerFromNew(MeshT &mesh, const char *propname) { + return PropertyManager(mesh, propname, false); +} + +/** + * Creates a non-owning wrapper for an existing mesh property (no lifecycle + * management). + * + * Intended for convenient access. + * + * @pre Property with the name \p propname of matching type exists. + * @throws std::runtime_error if no property with the name \p propname of + * matching type exists. + */ +template +PropertyManager makePropertyManagerFromExisting(MeshT &mesh, const char *propname) { + return PropertyManager(mesh, propname, true); +} + +/** + * Creates a non-owning wrapper for a mesh property (no lifecycle management). + * If the given property does not exist, it is created. + * + * Intended for creating or accessing persistent properties. + */ +template +PropertyManager makePropertyManagerFromExistingOrNew(MeshT &mesh, const char *propname) { + return PropertyManager::createIfNotExists(mesh, propname); +} + } /* namespace OpenMesh */ #endif /* PROPERTYMANAGER_HH_ */ diff --git a/src/Unittests/unittests_propertymanager.cc b/src/Unittests/unittests_propertymanager.cc index 24035186..5ee75c62 100644 --- a/src/Unittests/unittests_propertymanager.cc +++ b/src/Unittests/unittests_propertymanager.cc @@ -26,7 +26,7 @@ class OpenMeshPropertyManager : public OpenMeshBase { /* * ==================================================================== - * Define tests below + * General Tests * ==================================================================== */ @@ -92,11 +92,112 @@ TEST_F(OpenMeshPropertyManager, set_range_bool) { ASSERT_TRUE(pm_f_bool[*f_it]); } +/* + * ==================================================================== + * C++11 Specific Tests + * ==================================================================== + */ +#if _MSC_VER >= 1900 || __cplusplus > 199711L || defined(__GXX_EXPERIMENTAL_CXX0X__) +template +bool has_property(const Mesh& _mesh, const std::string& _name) { + auto dummy_handle = PropHandle{}; + return _mesh.get_property_handle(dummy_handle, _name); +} +/* + * Temporary property + */ +TEST_F(OpenMeshPropertyManager, cpp11_temp_property) { + using handle_type = OpenMesh::VPropHandleT; + const auto prop_name = "pm_v_test_property"; + ASSERT_FALSE(has_property(mesh_, prop_name)); + { + auto vprop = OpenMesh::makePropertyManagerFromNew(mesh_, prop_name); + static_cast(vprop); // Unused variable + ASSERT_TRUE(has_property(mesh_, prop_name)); + } + ASSERT_FALSE(has_property(mesh_, prop_name)); +} +/* + * Two temporary properties on a mesh using the same name and type. The second + * (inner) one shadows the first (outer) one instead of aliasing. + */ +TEST_F(OpenMeshPropertyManager, cpp11_temp_property_shadowing) { + auto vh = mesh_.add_vertex({0,0,0}); // Dummy vertex to attach properties to + using handle_type = OpenMesh::VPropHandleT; + const auto prop_name = "pm_v_test_property"; + + auto outer_prop = OpenMesh::makePropertyManagerFromNew(mesh_, prop_name); + outer_prop[vh] = 100; + ASSERT_EQ(100, outer_prop[vh]); + + { + // inner_prop uses same type and name as outer_prop + auto inner_prop = OpenMesh::makePropertyManagerFromNew(mesh_, prop_name); + inner_prop[vh] = 200; + ASSERT_EQ(200, inner_prop[vh]); + // End of scope: inner_prop is removed from mesh_ + } + + // Ensure outer_prop still exists and its data has not been overwritten by inner_prop + ASSERT_TRUE(has_property(mesh_, prop_name)); + ASSERT_EQ(100, outer_prop[vh]); +} + +/* + * In sequence: + * - add a persistent property to a mesh + * - retrieve an existing property of a mesh and modify it + * - obtain a non-owning property handle + * - attempt to obtain a non-owning handle to a non-existing property (throws) + */ +TEST_F(OpenMeshPropertyManager, cpp11_persistent_and_non_owning_properties) { + auto vh = mesh_.add_vertex({0,0,0}); // Dummy vertex to attach properties to + + using handle_type = OpenMesh::VPropHandleT; + const auto prop_name = "pm_v_test_property"; + + ASSERT_FALSE(has_property(mesh_, prop_name)); + + { + auto prop = OpenMesh::makePropertyManagerFromExistingOrNew(mesh_, prop_name); + prop[vh] = 100; + // End of scope, property persists + } + + ASSERT_TRUE(has_property(mesh_, prop_name)); + + { + // Since a property of the same name and type already exists, this refers to the existing property. + auto prop = OpenMesh::makePropertyManagerFromExistingOrNew(mesh_, prop_name); + ASSERT_EQ(100, prop[vh]); + prop[vh] = 200; + // End of scope, property persists + } + + ASSERT_TRUE(has_property(mesh_, prop_name)); + + { + // Acquire non-owning handle to the property, knowing it exists + auto prop = OpenMesh::makePropertyManagerFromExisting(mesh_, prop_name); + ASSERT_EQ(200, prop[vh]); + } + + ASSERT_TRUE(has_property(mesh_, prop_name)); + + { + // Attempt to acquire non-owning handle for a non-existing property + ASSERT_THROW(OpenMesh::makePropertyManagerFromExisting(mesh_, "wrong_property_name"), std::runtime_error); + } + + ASSERT_TRUE(has_property(mesh_, prop_name)); +} + +#endif }