Cleanup: Store instances id attribute with other attributes

Now that we can store any dynamic attribute on the instances component,
we don't need the special case for `id`, it can just be handled by the
generic attribute storage. Mostly this just allows removing a bunch
of redundant code.

I had to add a null check for `update_custom_data_pointers` because
the instances component doesn't have any pointers to inside of
custom data.

Differential Revision: https://developer.blender.org/D13430

5 files changed, 57 insertions, 142 deletions

diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh
index 35e66908d54..ead62827104 100644
--- a/source/blender/blenkernel/BKE_geometry_set.hh
+++ b/source/blender/blenkernel/BKE_geometry_set.hh
@@ -636,13 +636,6 @@ class InstancesComponent : public GeometryComponent {
   blender::Vector<int> instance_reference_handles_;
   /** Transformation of the instances. */
   blender::Vector<blender::float4x4> instance_transforms_;
-  /**
-   * IDs of the instances. They are used for consistency over multiple frames for things like
-   * motion blur. Proper stable ID data that actually helps when rendering can only be generated
-   * in some situations, so this vector is allowed to be empty, in which case the index of each
-   * instance will be used for the final ID.
-   */
-  blender::Vector<int> instance_ids_;
 
   /* These almost unique ids are generated based on `ids_`, which might not contain unique ids at
    * all. They are *almost* unique, because under certain very unlikely circumstances, they are not
@@ -676,11 +669,6 @@ class InstancesComponent : public GeometryComponent {
   blender::MutableSpan<int> instance_reference_handles();
   blender::MutableSpan<blender::float4x4> instance_transforms();
   blender::Span<blender::float4x4> instance_transforms() const;
-  blender::MutableSpan<int> instance_ids();
-  blender::Span<int> instance_ids() const;
-
-  blender::MutableSpan<int> instance_ids_ensure();
-  void instance_ids_clear();
 
   int instances_amount() const;
   int references_amount() const;
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index 902e08f1b28..5663f6b8756 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -400,7 +400,9 @@ WriteAttributeLookup BuiltinCustomDataLayerProvider::try_get_for_write(
   }
 
   if (data != new_data) {
-    custom_data_access_.update_custom_data_pointers(component);
+    if (custom_data_access_.update_custom_data_pointers) {
+      custom_data_access_.update_custom_data_pointers(component);
+    }
     data = new_data;
   }
 
@@ -441,7 +443,9 @@ bool BuiltinCustomDataLayerProvider::try_delete(GeometryComponent &component) co
   const bool delete_success = CustomData_free_layer(
       custom_data, stored_type_, domain_size, layer_index);
   if (delete_success) {
-    custom_data_access_.update_custom_data_pointers(component);
+    if (custom_data_access_.update_custom_data_pointers) {
+      custom_data_access_.update_custom_data_pointers(component);
+    }
   }
   return delete_success;
 }
@@ -476,7 +480,9 @@ bool BuiltinCustomDataLayerProvider::try_create(GeometryComponent &component,
         *custom_data, stored_type_, domain_size, initializer);
   }
   if (success) {
-    custom_data_access_.update_custom_data_pointers(component);
+    if (custom_data_access_.update_custom_data_pointers) {
+      custom_data_access_.update_custom_data_pointers(component);
+    }
   }
   return success;
 }
@@ -644,7 +650,9 @@ WriteAttributeLookup NamedLegacyCustomDataProvider::try_get_for_write(
         void *data_new = CustomData_duplicate_referenced_layer_named(
             custom_data, stored_type_, layer.name, domain_size);
         if (data_old != data_new) {
-          custom_data_access_.update_custom_data_pointers(component);
+          if (custom_data_access_.update_custom_data_pointers) {
+            custom_data_access_.update_custom_data_pointers(component);
+          }
         }
         return {as_write_attribute_(layer.data, domain_size), domain_};
       }
@@ -666,7 +674,9 @@ bool NamedLegacyCustomDataProvider::try_delete(GeometryComponent &component,
       if (custom_data_layer_matches_attribute_id(layer, attribute_id)) {
         const int domain_size = component.attribute_domain_size(domain_);
         CustomData_free_layer(custom_data, stored_type_, domain_size, i);
-        custom_data_access_.update_custom_data_pointers(component);
+        if (custom_data_access_.update_custom_data_pointers) {
+          custom_data_access_.update_custom_data_pointers(component);
+        }
         return true;
       }
     }
diff --git a/source/blender/blenkernel/intern/geometry_component_instances.cc b/source/blender/blenkernel/intern/geometry_component_instances.cc
index 29c28a875c1..aa80a2aacd2 100644
--- a/source/blender/blenkernel/intern/geometry_component_instances.cc
+++ b/source/blender/blenkernel/intern/geometry_component_instances.cc
@@ -37,6 +37,7 @@ using blender::MutableSpan;
 using blender::Set;
 using blender::Span;
 using blender::VectorSet;
+using blender::fn::GSpan;
 
 /* -------------------------------------------------------------------- */
 /** \name Geometry Component Implementation
@@ -51,7 +52,6 @@ GeometryComponent *InstancesComponent::copy() const
   InstancesComponent *new_component = new InstancesComponent();
   new_component->instance_reference_handles_ = instance_reference_handles_;
   new_component->instance_transforms_ = instance_transforms_;
-  new_component->instance_ids_ = instance_ids_;
   new_component->references_ = references_;
   return new_component;
 }
@@ -60,9 +60,6 @@ void InstancesComponent::reserve(int min_capacity)
 {
   instance_reference_handles_.reserve(min_capacity);
   instance_transforms_.reserve(min_capacity);
-  if (!instance_ids_.is_empty()) {
-    this->instance_ids_ensure();
-  }
   attributes_.reallocate(min_capacity);
 }
 
@@ -76,9 +73,6 @@ void InstancesComponent::resize(int capacity)
 {
   instance_reference_handles_.resize(capacity);
   instance_transforms_.resize(capacity);
-  if (!instance_ids_.is_empty()) {
-    this->instance_ids_ensure();
-  }
   attributes_.reallocate(capacity);
 }
 
@@ -86,9 +80,7 @@ void InstancesComponent::clear()
 {
   instance_reference_handles_.clear();
   instance_transforms_.clear();
-  instance_ids_.clear();
   attributes_.clear();
-
   references_.clear();
 }
 
@@ -98,9 +90,6 @@ void InstancesComponent::add_instance(const int instance_handle, const float4x4
   BLI_assert(instance_handle < references_.size());
   instance_reference_handles_.append(instance_handle);
   instance_transforms_.append(transform);
-  if (!instance_ids_.is_empty()) {
-    this->instance_ids_ensure();
-  }
   attributes_.reallocate(this->instances_amount());
 }
 
@@ -123,31 +112,6 @@ blender::Span<blender::float4x4> InstancesComponent::instance_transforms() const
   return instance_transforms_;
 }
 
-blender::MutableSpan<int> InstancesComponent::instance_ids()
-{
-  return instance_ids_;
-}
-blender::Span<int> InstancesComponent::instance_ids() const
-{
-  return instance_ids_;
-}
-
-/**
- * Make sure the ID storage size matches the number of instances. By directly resizing the
- * component's vectors internally, it is possible to be in a situation where the IDs are not
- * empty but they do not have the correct size; this function resolves that.
- */
-blender::MutableSpan<int> InstancesComponent::instance_ids_ensure()
-{
-  instance_ids_.append_n_times(0, this->instances_amount() - instance_ids_.size());
-  return instance_ids_;
-}
-
-void InstancesComponent::instance_ids_clear()
-{
-  instance_ids_.clear_and_make_inline();
-}
-
 /**
  * With write access to the instances component, the data in the instanced geometry sets can be
  * changed. This is a function on the component rather than each reference to ensure `const`
@@ -351,15 +315,17 @@ static blender::Array<int> generate_unique_instance_ids(Span<int> original_ids)
 blender::Span<int> InstancesComponent::almost_unique_ids() const
 {
   std::lock_guard lock(almost_unique_ids_mutex_);
-  if (instance_ids().is_empty()) {
-    almost_unique_ids_.reinitialize(this->instances_amount());
-    for (const int i : almost_unique_ids_.index_range()) {
-      almost_unique_ids_[i] = i;
+  std::optional<GSpan> instance_ids_gspan = attributes_.get_for_read("id");
+  if (instance_ids_gspan) {
+    Span<int> instance_ids = instance_ids_gspan->typed<int>();
+    if (almost_unique_ids_.size() != instance_ids.size()) {
+      almost_unique_ids_ = generate_unique_instance_ids(instance_ids);
     }
   }
   else {
-    if (almost_unique_ids_.size() != instance_ids_.size()) {
-      almost_unique_ids_ = generate_unique_instance_ids(instance_ids_);
+    almost_unique_ids_.reinitialize(this->instances_amount());
+    for (const int i : almost_unique_ids_.index_range()) {
+      almost_unique_ids_[i] = i;
     }
   }
   return almost_unique_ids_;
@@ -438,81 +404,21 @@ class InstancePositionAttributeProvider final : public BuiltinAttributeProvider
   }
 };
 
-class InstanceIDAttributeProvider final : public BuiltinAttributeProvider {
- public:
-  InstanceIDAttributeProvider()
-      : BuiltinAttributeProvider(
-            "id", ATTR_DOMAIN_INSTANCE, CD_PROP_INT32, Creatable, Writable, Deletable)
-  {
-  }
-
-  GVArray try_get_for_read(const GeometryComponent &component) const final
-  {
-    const InstancesComponent &instances = static_cast<const InstancesComponent &>(component);
-    if (instances.instance_ids().is_empty()) {
-      return {};
-    }
-    return VArray<int>::ForSpan(instances.instance_ids());
-  }
-
-  WriteAttributeLookup try_get_for_write(GeometryComponent &component) const final
-  {
-    InstancesComponent &instances = static_cast<InstancesComponent &>(component);
-    if (instances.instance_ids().is_empty()) {
-      return {};
-    }
-    return {VMutableArray<int>::ForSpan(instances.instance_ids()), domain_};
-  }
-
-  bool try_delete(GeometryComponent &component) const final
-  {
-    InstancesComponent &instances = static_cast<InstancesComponent &>(component);
-    if (instances.instance_ids().is_empty()) {
-      return false;
-    }
-    instances.instance_ids_clear();
-    return true;
-  }
-
-  bool try_create(GeometryComponent &component, const AttributeInit &initializer) const final
-  {
-    InstancesComponent &instances = static_cast<InstancesComponent &>(component);
-    if (instances.instances_amount() == 0) {
-      return false;
-    }
-    MutableSpan<int> ids = instances.instance_ids_ensure();
-    switch (initializer.type) {
-      case AttributeInit::Type::Default: {
-        ids.fill(0);
-        break;
-      }
-      case AttributeInit::Type::VArray: {
-        const GVArray &varray = static_cast<const AttributeInitVArray &>(initializer).varray;
-        varray.materialize_to_uninitialized(varray.index_range(), ids.data());
-        break;
-      }
-      case AttributeInit::Type::MoveArray: {
-        void *source_data = static_cast<const AttributeInitMove &>(initializer).data;
-        ids.copy_from({static_cast<int *>(source_data), instances.instances_amount()});
-        MEM_freeN(source_data);
-        break;
-      }
-    }
-    return true;
-  }
+template<typename T>
+static GVArray make_array_read_attribute(const void *data, const int domain_size)
+{
+  return VArray<T>::ForSpan(Span<T>((const T *)data, domain_size));
+}
 
-  bool exists(const GeometryComponent &component) const final
-  {
-    const InstancesComponent &instances = static_cast<const InstancesComponent &>(component);
-    return !instances.instance_ids().is_empty();
-  }
-};
+template<typename T>
+static GVMutableArray make_array_write_attribute(void *data, const int domain_size)
+{
+  return VMutableArray<T>::ForSpan(MutableSpan<T>((T *)data, domain_size));
+}
 
 static ComponentAttributeProviders create_attribute_providers_for_instances()
 {
   static InstancePositionAttributeProvider position;
-  static InstanceIDAttributeProvider id;
-
   static CustomDataAccessInfo instance_custom_data_access = {
       [](GeometryComponent &component) -> CustomData * {
         InstancesComponent &inst = static_cast<InstancesComponent &>(component);
@@ -524,6 +430,24 @@ static ComponentAttributeProviders create_attribute_providers_for_instances()
       },
       nullptr};
 
+  /**
+   * IDs of the instances. They are used for consistency over multiple frames for things like
+   * motion blur. Proper stable ID data that actually helps when rendering can only be generated
+   * in some situations, so this vector is allowed to be empty, in which case the index of each
+   * instance will be used for the final ID.
+   */
+  static BuiltinCustomDataLayerProvider id("id",
+                                           ATTR_DOMAIN_INSTANCE,
+                                           CD_PROP_INT32,
+                                           CD_PROP_INT32,
+                                           BuiltinAttributeProvider::Creatable,
+                                           BuiltinAttributeProvider::Writable,
+                                           BuiltinAttributeProvider::Deletable,
+                                           instance_custom_data_access,
+                                           make_array_read_attribute<int>,
+                                           make_array_write_attribute<int>,
+                                           nullptr);
+
   static CustomDataAttributeProvider instance_custom_data(ATTR_DOMAIN_INSTANCE,
                                                           instance_custom_data_access);
 
diff --git a/source/blender/nodes/geometry/nodes/legacy/node_geo_legacy_point_instance.cc b/source/blender/nodes/geometry/nodes/legacy/node_geo_legacy_point_instance.cc
index be8294cda90..ef35bef96aa 100644
--- a/source/blender/nodes/geometry/nodes/legacy/node_geo_legacy_point_instance.cc
+++ b/source/blender/nodes/geometry/nodes/legacy/node_geo_legacy_point_instance.cc
@@ -186,7 +186,9 @@ static void add_instances_from_component(InstancesComponent &instances,
   instances.resize(start_len + domain_size);
   MutableSpan<int> handles = instances.instance_reference_handles().slice(start_len, domain_size);
   MutableSpan<float4x4> transforms = instances.instance_transforms().slice(start_len, domain_size);
-  MutableSpan<int> instance_ids = instances.instance_ids_ensure().slice(start_len, domain_size);
+  OutputAttribute_Typed<int> instance_id_attribute =
+      instances.attribute_try_get_for_output_only<int>("id", ATTR_DOMAIN_INSTANCE);
+  MutableSpan<int> instance_ids = instance_id_attribute.as_span();
 
   /* Skip all of the randomness handling if there is only a single possible instance
    * (anything except for collection mode with "Whole Collection" turned off). */
@@ -213,6 +215,8 @@ static void add_instances_from_component(InstancesComponent &instances,
       }
     });
   }
+
+  instance_id_attribute.save();
 }
 
 static void node_geo_exec(GeoNodeExecParams params)
diff --git a/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc b/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc
index 637c2c59da6..4e75f1f60d9 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc
@@ -158,17 +158,6 @@ static void add_instances_from_component(
     }
   });
 
-  VArray<int> ids = src_component
-                        .attribute_try_get_for_read("id", ATTR_DOMAIN_POINT, CD_PROP_INT32)
-                        .typed<int>();
-  if (ids) {
-    VArray_Span<int> ids_span{ids};
-    MutableSpan<int> dst_ids = dst_component.instance_ids_ensure();
-    for (const int64_t i : selection.index_range()) {
-      dst_ids[i] = ids_span[selection[i]];
-    }
-  }
-
   if (pick_instance.is_single()) {
     if (pick_instance.get_internal_single()) {
       if (instance.has_realized_data()) {