[Bf-blender-cvs] [52e3608fe9b] geometry-nodes: Geometry Nodes: simplify GeometrySet ownership handling
Jacques Lucke
noreply at git.blender.org
Mon Nov 16 13:48:41 CET 2020
Commit: 52e3608fe9b6c72739570ac6abe1473953c10f1d
Author: Jacques Lucke
Date: Mon Nov 16 13:48:33 2020 +0100
Branches: geometry-nodes
https://developer.blender.org/rB52e3608fe9b6c72739570ac6abe1473953c10f1d
Geometry Nodes: simplify GeometrySet ownership handling
Previously, GeometrySets and GeometryComponents has reference
counters and could be shared. This commit changes it so that
only GeometryComponents are shared. A GeometrySet is a fairly
small type that is cheap to copy.
A lot of code simplifies when we can assume that GeometrySet
is cheap to copy.
===================================================================
M source/blender/blenkernel/BKE_geometry_set.h
M source/blender/blenkernel/BKE_geometry_set.hh
M source/blender/blenkernel/BKE_modifier.h
M source/blender/blenkernel/intern/geometry_set.cc
M source/blender/blenkernel/intern/object.c
M source/blender/blenkernel/intern/pointcloud.cc
M source/blender/modifiers/intern/MOD_nodes.cc
M source/blender/nodes/geometry/node_geometry_exec.cc
M source/blender/nodes/geometry/nodes/node_geo_boolean.cc
M source/blender/nodes/geometry/nodes/node_geo_edge_split.cc
M source/blender/nodes/geometry/nodes/node_geo_object_info.cc
M source/blender/nodes/geometry/nodes/node_geo_point_distribute.cc
M source/blender/nodes/geometry/nodes/node_geo_point_instance.cc
M source/blender/nodes/geometry/nodes/node_geo_subdivision_surface.cc
M source/blender/nodes/geometry/nodes/node_geo_transform.cc
M source/blender/nodes/geometry/nodes/node_geo_triangulate.cc
M source/blender/nodes/intern/node_socket.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_geometry_set.h b/source/blender/blenkernel/BKE_geometry_set.h
index 902995f6aa5..c3ca02a1831 100644
--- a/source/blender/blenkernel/BKE_geometry_set.h
+++ b/source/blender/blenkernel/BKE_geometry_set.h
@@ -27,8 +27,7 @@ extern "C" {
struct Object;
struct GeometrySet;
-void BKE_geometry_set_user_add(struct GeometrySet *geometry_set);
-void BKE_geometry_set_user_remove(struct GeometrySet *geometry_set);
+void BKE_geometry_set_free(struct GeometrySet *geometry_set);
bool BKE_geometry_set_has_instances(const struct GeometrySet *geometry_set);
diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh
index 0bc9352de37..d4aa090960f 100644
--- a/source/blender/blenkernel/BKE_geometry_set.hh
+++ b/source/blender/blenkernel/BKE_geometry_set.hh
@@ -34,9 +34,6 @@ struct Mesh;
struct PointCloud;
struct Object;
-/* An automatically reference counted geometry set. */
-using GeometrySetPtr = blender::UserCounter<class GeometrySet>;
-
/* Each geometry component has a specific type. The type determines what kind of data the component
* stores. Functions modifying a geometry will usually just modify a subset of the component types.
*/
@@ -91,35 +88,18 @@ inline constexpr bool is_geometry_component_v = std::is_base_of_v<GeometryCompon
/**
* A geometry set contains zero or more geometry components. There is at most one component of each
- * type. Individual components might be shared between multiple geometries.
+ * type. Individual components might be shared between multiple geometries. Shared components are
+ * copied automatically when write access is requested.
*
- * Geometries are reference counted. This allows them to be shared without making unnecessary
- * copies. A geometry that is shared is immutable. If some code wants to change it,
- * #make_geometry_set_mutable should be called first.
+ * Copying a geometry set is a relatively cheap operation, because it does not copy the referenced
+ * geometry components.
*/
class GeometrySet {
private:
- /* Number of users of this geometry set. If this number goes to zero, the set is freed. If
- * it is above 1, the geometry set is immutable. */
- std::atomic<int> users_ = 1;
-
using GeometryComponentPtr = blender::UserCounter<class GeometryComponent>;
blender::Map<GeometryComponentType, GeometryComponentPtr> components_;
public:
- GeometrySet() = default;
- GeometrySet(const GeometrySet &other);
- GeometrySet(GeometrySet &&other) = delete;
- ~GeometrySet() = default;
-
- /* Disable copy and move assignment operators. */
- GeometrySet &operator=(const GeometrySet &other) = delete;
- GeometrySet &operator=(GeometrySet &&other) = delete;
-
- void user_add();
- void user_remove();
- bool is_mutable() const;
-
GeometryComponent &get_component_for_write(GeometryComponentType component_type);
template<typename Component> Component &get_component_for_write()
{
@@ -134,10 +114,14 @@ class GeometrySet {
return static_cast<const Component *>(get_component_for_read(Component::type));
}
+ friend std::ostream &operator<<(std::ostream &stream, const GeometrySet &geometry_set);
+ friend bool operator==(const GeometrySet &a, const GeometrySet &b);
+ uint64_t hash() const;
+
/* Utility methods for creation. */
- static GeometrySetPtr create_with_mesh(
+ static GeometrySet create_with_mesh(
Mesh *mesh, GeometryOwnershipType ownership = GeometryOwnershipType::Owned);
- static GeometrySetPtr create_with_pointcloud(
+ static GeometrySet create_with_pointcloud(
PointCloud *pointcloud, GeometryOwnershipType ownership = GeometryOwnershipType::Owned);
/* Utility methods for access. */
@@ -155,8 +139,6 @@ class GeometrySet {
GeometryOwnershipType ownership = GeometryOwnershipType::Owned);
};
-void make_geometry_set_mutable(GeometrySetPtr &geometry);
-
/** A geometry component that can store a mesh. */
class MeshComponent : public GeometryComponent {
private:
diff --git a/source/blender/blenkernel/BKE_modifier.h b/source/blender/blenkernel/BKE_modifier.h
index 5032e1985c4..2491f0953c0 100644
--- a/source/blender/blenkernel/BKE_modifier.h
+++ b/source/blender/blenkernel/BKE_modifier.h
@@ -247,9 +247,9 @@ typedef struct ModifierTypeInfo {
struct Hair *(*modifyHair)(struct ModifierData *md,
const struct ModifierEvalContext *ctx,
struct Hair *hair);
- struct GeometrySet *(*modifyPointCloud)(struct ModifierData *md,
- const struct ModifierEvalContext *ctx,
- struct GeometrySet *geometry_set);
+ void (*modifyPointCloud)(struct ModifierData *md,
+ const struct ModifierEvalContext *ctx,
+ struct GeometrySet *geometry_set);
struct Volume *(*modifyVolume)(struct ModifierData *md,
const struct ModifierEvalContext *ctx,
struct Volume *volume);
diff --git a/source/blender/blenkernel/intern/geometry_set.cc b/source/blender/blenkernel/intern/geometry_set.cc
index 6d7cce34e7e..ab83ccc4a3d 100644
--- a/source/blender/blenkernel/intern/geometry_set.cc
+++ b/source/blender/blenkernel/intern/geometry_set.cc
@@ -77,39 +77,11 @@ bool GeometryComponent::is_mutable() const
/** \name Geometry Set
* \{ */
-/* Makes a copy of the geometry set. The individual components are shared with the original
- * geometry set. Therefore, this is a relatively cheap operation.
- */
-GeometrySet::GeometrySet(const GeometrySet &other) : components_(other.components_)
-{
-}
-
-void GeometrySet::user_add()
-{
- users_.fetch_add(1);
-}
-
-void GeometrySet::user_remove()
-{
- const int new_users = users_.fetch_sub(1) - 1;
- if (new_users == 0) {
- delete this;
- }
-}
-
-bool GeometrySet::is_mutable() const
-{
- /* If the geometry set is shared, it is read-only. */
- /* The user count can be 0, when this is called from the destructor. */
- return users_ <= 1;
-}
-
/* This method can only be used when the geometry set is mutable. It returns a mutable geometry
* component of the given type.
*/
GeometryComponent &GeometrySet::get_component_for_write(GeometryComponentType component_type)
{
- BLI_assert(this->is_mutable());
return components_.add_or_modify(
component_type,
[&](GeometryComponentPtr *value_ptr) -> GeometryComponent & {
@@ -142,6 +114,27 @@ const GeometryComponent *GeometrySet::get_component_for_read(
return nullptr;
}
+std::ostream &operator<<(std::ostream &stream, const GeometrySet &geometry_set)
+{
+ stream << "<GeometrySet at " << &geometry_set << ", " << geometry_set.components_.size()
+ << " components>";
+ return stream;
+}
+
+/* This generally should not be used. It is necessary currently, so that GeometrySet can by used by
+ * the CPPType system. */
+bool operator==(const GeometrySet &UNUSED(a), const GeometrySet &UNUSED(b))
+{
+ return false;
+}
+
+/* This generally should not be used. It is necessary currently, so that GeometrySet can by used by
+ * the CPPType system. */
+uint64_t GeometrySet::hash() const
+{
+ return reinterpret_cast<uint64_t>(this);
+}
+
/* Returns a read-only mesh or null. */
const Mesh *GeometrySet::get_mesh_for_read() const
{
@@ -178,20 +171,20 @@ bool GeometrySet::has_instances() const
}
/* Create a new geometry set that only contains the given mesh. */
-GeometrySetPtr GeometrySet::create_with_mesh(Mesh *mesh, GeometryOwnershipType ownership)
+GeometrySet GeometrySet::create_with_mesh(Mesh *mesh, GeometryOwnershipType ownership)
{
- GeometrySet *geometry_set = new GeometrySet();
- MeshComponent &component = geometry_set->get_component_for_write<MeshComponent>();
+ GeometrySet geometry_set;
+ MeshComponent &component = geometry_set.get_component_for_write<MeshComponent>();
component.replace(mesh, ownership);
return geometry_set;
}
/* Create a new geometry set that only contains the given point cloud. */
-GeometrySetPtr GeometrySet::create_with_pointcloud(PointCloud *pointcloud,
- GeometryOwnershipType ownership)
+GeometrySet GeometrySet::create_with_pointcloud(PointCloud *pointcloud,
+ GeometryOwnershipType ownership)
{
- GeometrySet *geometry_set = new GeometrySet();
- PointCloudComponent &component = geometry_set->get_component_for_write<PointCloudComponent>();
+ GeometrySet geometry_set;
+ PointCloudComponent &component = geometry_set.get_component_for_write<PointCloudComponent>();
component.replace(pointcloud, ownership);
return geometry_set;
}
@@ -224,28 +217,6 @@ PointCloud *GeometrySet::get_pointcloud_for_write()
return component.get_for_write();
}
-/**
- * Changes the given pointer so that it points to a mutable geometry set. This might do nothing,
- * create a new empty geometry set or copy the entire geometry set.
- */
-void make_geometry_set_mutable(GeometrySetPtr &geometry_set)
-{
- if (!geometry_set) {
- /* If the pointer is null, create a new instance. */
- geometry_set = GeometrySetPtr{new GeometrySet()};
- }
- else if (geometry_set->is_mutable()) {
- /* If the instance is mutable already, do nothing. */
- }
- else {
- /* This geometry is shared, make a copy that is independent of the other users. */
- GeometrySet *shared_geometry_set = geometry_set.release();
- GeometrySet *new_geometry_set = new GeometrySet(*shared_geometry_set);
- shared_geometry_set->user_remove();
- geometry_set = GeometrySetPtr{new_geometry_set};
- }
-}
-
/** \} */
/* -------------------------------------------------------------------- */
@@ -470,14 +441,9 @@ int InstancesComponent::instances_amount() const
/** \name C API
* \{ */
-void BKE_geometry_set_user_add(GeometrySet *geometry_set)
-{
- geometry_set->user_add();
-}
-
-void BKE_geometry_set_user_remove(GeometrySet *geometry_set)
+void BKE_geometry_set_free(GeometrySet *geometry_set)
{
- geometry_set->user_remove();
+ delete geometry_set;
}
bool BKE_geometry_set_has_instances(co
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list