[Bf-blender-cvs] [b32f9bf801e] master: Geometry Nodes: use array instead of map in GeometrySet

Jacques Lucke noreply at git.blender.org
Sun Dec 5 15:10:24 CET 2021


Commit: b32f9bf801ec6cc0e3b40b87de16f34eba3f593e
Author: Jacques Lucke
Date:   Sun Dec 5 15:10:11 2021 +0100
Branches: master
https://developer.blender.org/rBb32f9bf801ec6cc0e3b40b87de16f34eba3f593e

Geometry Nodes: use array instead of map in GeometrySet

`GeometrySet` contains at most one component of each type.
Previously, a map was used to make sure that each component
type only exists once. The overhead of a map (especially with
inline storage) is rather large though. Since all component types
are known at compile time and the number of types is low,
a simple `std::array` works as well.

Some benefits of using `std::array` here:
* Looking up the component of a specific type is a bit faster.
* The size of `GeometrySet` becomes much smaller from 192 to 40 bytes.
* Debugging a `GeometrySet` in many tools becomes simpler because
  one can  easily see which components exists and which don't

===================================================================

M	source/blender/blenkernel/BKE_geometry_set.h
M	source/blender/blenkernel/BKE_geometry_set.hh
M	source/blender/blenkernel/intern/geometry_set.cc

===================================================================

diff --git a/source/blender/blenkernel/BKE_geometry_set.h b/source/blender/blenkernel/BKE_geometry_set.h
index 17cdb9d6a42..2d6218f1036 100644
--- a/source/blender/blenkernel/BKE_geometry_set.h
+++ b/source/blender/blenkernel/BKE_geometry_set.h
@@ -39,6 +39,8 @@ typedef enum GeometryComponentType {
   GEO_COMPONENT_TYPE_CURVE = 4,
 } GeometryComponentType;
 
+#define GEO_COMPONENT_TYPE_ENUM_SIZE 5
+
 void BKE_geometry_set_free(struct GeometrySet *geometry_set);
 
 bool BKE_object_has_geometry_set_instances(const struct Object *ob);
diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh
index ead62827104..84564ca5114 100644
--- a/source/blender/blenkernel/BKE_geometry_set.hh
+++ b/source/blender/blenkernel/BKE_geometry_set.hh
@@ -262,7 +262,8 @@ inline constexpr bool is_geometry_component_v = std::is_base_of_v<GeometryCompon
 struct GeometrySet {
  private:
   using GeometryComponentPtr = blender::UserCounter<class GeometryComponent>;
-  blender::Map<GeometryComponentType, GeometryComponentPtr> components_;
+  /* Indexed by #GeometryComponentType. */
+  std::array<GeometryComponentPtr, GEO_COMPONENT_TYPE_ENUM_SIZE> components_;
 
  public:
   GeometrySet();
diff --git a/source/blender/blenkernel/intern/geometry_set.cc b/source/blender/blenkernel/intern/geometry_set.cc
index c250c14f1d7..f65e3eeb2d0 100644
--- a/source/blender/blenkernel/intern/geometry_set.cc
+++ b/source/blender/blenkernel/intern/geometry_set.cc
@@ -118,25 +118,20 @@ GeometrySet &GeometrySet::operator=(GeometrySet &&other) = default;
  */
 GeometryComponent &GeometrySet::get_component_for_write(GeometryComponentType component_type)
 {
-  return components_.add_or_modify(
-      component_type,
-      [&](GeometryComponentPtr *value_ptr) -> GeometryComponent & {
-        /* If the component did not exist before, create a new one. */
-        new (value_ptr) GeometryComponentPtr(GeometryComponent::create(component_type));
-        return **value_ptr;
-      },
-      [&](GeometryComponentPtr *value_ptr) -> GeometryComponent & {
-        GeometryComponentPtr &value = *value_ptr;
-        if (value->is_mutable()) {
-          /* If the referenced component is already mutable, return it directly. */
-          return *value;
-        }
-        /* If the referenced component is shared, make a copy. The copy is not shared and is
-         * therefore mutable. */
-        GeometryComponent *copied_component = value->copy();
-        value = GeometryComponentPtr{copied_component};
-        return *copied_component;
-      });
+  GeometryComponentPtr &component_ptr = components_[component_type];
+  if (!component_ptr) {
+    /* If the component did not exist before, create a new one. */
+    component_ptr = GeometryComponent::create(component_type);
+    return *component_ptr;
+  }
+  if (component_ptr->is_mutable()) {
+    /* If the referenced component is already mutable, return it directly. */
+    return *component_ptr;
+  }
+  /* If the referenced component is shared, make a copy. The copy is not shared and is
+   * therefore mutable. */
+  component_ptr = component_ptr->copy();
+  return *component_ptr;
 }
 
 /**
@@ -155,21 +150,17 @@ GeometryComponent *GeometrySet::get_component_ptr(GeometryComponentType type)
 const GeometryComponent *GeometrySet::get_component_for_read(
     GeometryComponentType component_type) const
 {
-  const GeometryComponentPtr *component = components_.lookup_ptr(component_type);
-  if (component != nullptr) {
-    return component->get();
-  }
-  return nullptr;
+  return components_[component_type].get();
 }
 
 bool GeometrySet::has(const GeometryComponentType component_type) const
 {
-  return components_.contains(component_type);
+  return components_[component_type].has_value();
 }
 
 void GeometrySet::remove(const GeometryComponentType component_type)
 {
-  components_.remove(component_type);
+  components_[component_type].reset();
 }
 
 /**
@@ -177,20 +168,20 @@ void GeometrySet::remove(const GeometryComponentType component_type)
  */
 void GeometrySet::keep_only(const blender::Span<GeometryComponentType> component_types)
 {
-  for (auto it = components_.keys().begin(); it != components_.keys().end(); ++it) {
-    const GeometryComponentType type = *it;
-    if (!component_types.contains(type)) {
-      components_.remove(it);
+  for (GeometryComponentPtr &component_ptr : components_) {
+    if (component_ptr) {
+      if (!component_types.contains(component_ptr->type())) {
+        component_ptr.reset();
+      }
     }
   }
 }
 
 void GeometrySet::add(const GeometryComponent &component)
 {
-  BLI_assert(!components_.contains(component.type()));
+  BLI_assert(!components_[component.type()]);
   component.user_add();
-  GeometryComponentPtr component_ptr{const_cast<GeometryComponent *>(&component)};
-  components_.add_new(component.type(), std::move(component_ptr));
+  components_[component.type()] = const_cast<GeometryComponent *>(&component);
 }
 
 /**
@@ -199,8 +190,10 @@ void GeometrySet::add(const GeometryComponent &component)
 Vector<const GeometryComponent *> GeometrySet::get_components_for_read() const
 {
   Vector<const GeometryComponent *> components;
-  for (const GeometryComponentPtr &ptr : components_.values()) {
-    components.append(ptr.get());
+  for (const GeometryComponentPtr &component_ptr : components_) {
+    if (component_ptr) {
+      components.append(component_ptr.get());
+    }
   }
   return components;
 }
@@ -236,27 +229,34 @@ std::ostream &operator<<(std::ostream &stream, const GeometrySet &geometry_set)
 /* Remove all geometry components from the geometry set. */
 void GeometrySet::clear()
 {
-  components_.clear();
+  for (GeometryComponentPtr &component_ptr : components_) {
+    component_ptr.reset();
+  }
 }
 
 /* Make sure that the geometry can be cached. This does not ensure ownership of object/collection
  * instances. */
 void GeometrySet::ensure_owns_direct_data()
 {
-  for (GeometryComponentType type : components_.keys()) {
-    const GeometryComponent *component = this->get_component_for_read(type);
-    if (!component->owns_direct_data()) {
-      GeometryComponent &component_for_write = this->get_component_for_write(type);
-      component_for_write.ensure_owns_direct_data();
+  for (GeometryComponentPtr &component_ptr : components_) {
+    if (!component_ptr) {
+      continue;
     }
+    if (component_ptr->owns_direct_data()) {
+      continue;
+    }
+    GeometryComponent &component_for_write = this->get_component_for_write(component_ptr->type());
+    component_for_write.ensure_owns_direct_data();
   }
 }
 
 bool GeometrySet::owns_direct_data() const
 {
-  for (const GeometryComponentPtr &component : components_.values()) {
-    if (!component->owns_direct_data()) {
-      return false;
+  for (const GeometryComponentPtr &component_ptr : components_) {
+    if (component_ptr) {
+      if (!component_ptr->owns_direct_data()) {
+        return false;
+      }
     }
   }
   return true;
@@ -328,23 +328,20 @@ bool GeometrySet::has_curve() const
 /* Returns true when the geometry set has any data that is not an instance. */
 bool GeometrySet::has_realized_data() const
 {
-  if (components_.is_empty()) {
-    return false;
-  }
-  if (components_.size() > 1) {
-    return true;
+  for (const GeometryComponentPtr &component_ptr : components_) {
+    if (component_ptr) {
+      if (component_ptr->type() != GEO_COMPONENT_TYPE_INSTANCES) {
+        return true;
+      }
+    }
   }
-  /* Check if the only component is an #InstancesComponent. */
-  return this->get_component_for_read<InstancesComponent>() == nullptr;
+  return false;
 }
 
 /* Return true if the geometry set has any component that isn't empty. */
 bool GeometrySet::is_empty() const
 {
-  if (components_.is_empty()) {
-    return true;
-  }
-  return !(this->has_mesh() || this->has_curve() || this->has_pointcloud() ||
+  return !(this->has_mesh() || this->has_curve() || this->has_pointcloud() || this->has_volume() ||
            this->has_instances());
 }



More information about the Bf-blender-cvs mailing list