[Bf-blender-cvs] [cf771807b79] blender-v3.0-release: Geometry Nodes: do cache invalidation after writing attributes

Jacques Lucke noreply at git.blender.org
Fri Oct 29 09:49:20 CEST 2021


Commit: cf771807b7997949611dbf76b43150592c9977cb
Author: Jacques Lucke
Date:   Fri Oct 29 09:28:31 2021 +0200
Branches: blender-v3.0-release
https://developer.blender.org/rBcf771807b7997949611dbf76b43150592c9977cb

Geometry Nodes: do cache invalidation after writing attributes

This is a better and more general fix for T92511 and T92508 than
the ones that I committed before.

Previously, we tagged caches dirty when first accessing attributes.
This led to incorrect caches when under some circumstances. Now
cache invalidation is part of `OutputAttribute.save()`.

A nice side benefit of this change is that it may make things more
efficient in some cases, because we don't invalidate caches when
they don't have to be invalidated.

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

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

M	source/blender/blenkernel/BKE_attribute_access.hh
M	source/blender/blenkernel/BKE_spline.hh
M	source/blender/blenkernel/intern/attribute_access.cc
M	source/blender/blenkernel/intern/attribute_access_intern.hh
M	source/blender/blenkernel/intern/curve_eval.cc
M	source/blender/blenkernel/intern/geometry_component_curve.cc
M	source/blender/blenkernel/intern/geometry_component_instances.cc
M	source/blender/blenkernel/intern/geometry_component_mesh.cc

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

diff --git a/source/blender/blenkernel/BKE_attribute_access.hh b/source/blender/blenkernel/BKE_attribute_access.hh
index 38f5497ed92..cf19eb29a0e 100644
--- a/source/blender/blenkernel/BKE_attribute_access.hh
+++ b/source/blender/blenkernel/BKE_attribute_access.hh
@@ -183,6 +183,8 @@ struct WriteAttributeLookup {
   GVMutableArrayPtr varray;
   /* Domain the attributes lives on in the geometry. */
   AttributeDomain domain;
+  /* Call this after changing the attribute to invalidate caches that depend on this attribute. */
+  std::function<void()> tag_modified_fn;
 
   /* Convenience function to check if the attribute has been found. */
   operator bool() const
diff --git a/source/blender/blenkernel/BKE_spline.hh b/source/blender/blenkernel/BKE_spline.hh
index 97e0d8415a5..8509b730709 100644
--- a/source/blender/blenkernel/BKE_spline.hh
+++ b/source/blender/blenkernel/BKE_spline.hh
@@ -570,6 +570,8 @@ struct CurveEval {
   blender::Array<int> evaluated_point_offsets() const;
   blender::Array<float> accumulated_spline_lengths() const;
 
+  void mark_cache_invalid();
+
   void assert_valid_point_attributes() const;
 };
 
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index 1ea7f522bef..01b53baf237 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -360,7 +360,7 @@ GVArrayPtr BuiltinCustomDataLayerProvider::try_get_for_read(
   return as_read_attribute_(data, domain_size);
 }
 
-GVMutableArrayPtr BuiltinCustomDataLayerProvider::try_get_for_write(
+WriteAttributeLookup BuiltinCustomDataLayerProvider::try_get_for_write(
     GeometryComponent &component) const
 {
   if (writable_ != Writable) {
@@ -397,10 +397,14 @@ GVMutableArrayPtr BuiltinCustomDataLayerProvider::try_get_for_write(
     data = new_data;
   }
 
+  std::function<void()> tag_modified_fn;
   if (update_on_write_ != nullptr) {
-    update_on_write_(component);
+    tag_modified_fn = [component = &component, update = update_on_write_]() {
+      update(*component);
+    };
   }
-  return as_write_attribute_(data, domain_size);
+
+  return {as_write_attribute_(data, domain_size), domain_, std::move(tag_modified_fn)};
 }
 
 bool BuiltinCustomDataLayerProvider::try_delete(GeometryComponent &component) const
@@ -925,7 +929,7 @@ blender::bke::WriteAttributeLookup GeometryComponent::attribute_try_get_for_writ
     const BuiltinAttributeProvider *builtin_provider =
         providers->builtin_attribute_providers().lookup_default_as(attribute_id.name(), nullptr);
     if (builtin_provider != nullptr) {
-      return {builtin_provider->try_get_for_write(*this), builtin_provider->domain()};
+      return builtin_provider->try_get_for_write(*this);
     }
   }
   for (const DynamicAttributesProvider *dynamic_provider :
@@ -1249,6 +1253,20 @@ static void save_output_attribute(OutputAttribute &output_attribute)
     varray.get(i, buffer);
     write_attribute.varray->set_by_relocate(i, buffer);
   }
+  if (write_attribute.tag_modified_fn) {
+    write_attribute.tag_modified_fn();
+  }
+}
+
+static std::function<void(OutputAttribute &)> get_simple_output_attribute_save_method(
+    const blender::bke::WriteAttributeLookup &attribute)
+{
+  if (!attribute.tag_modified_fn) {
+    return {};
+  }
+  return [tag_modified_fn = attribute.tag_modified_fn](OutputAttribute &UNUSED(attribute)) {
+    tag_modified_fn();
+  };
 }
 
 static OutputAttribute create_output_attribute(GeometryComponent &component,
@@ -1293,14 +1311,21 @@ static OutputAttribute create_output_attribute(GeometryComponent &component,
       /* Builtin attribute is on different domain. */
       return {};
     }
+
     GVMutableArrayPtr varray = std::move(attribute.varray);
     if (varray->type() == *cpp_type) {
       /* Builtin attribute matches exactly. */
-      return OutputAttribute(std::move(varray), domain, {}, ignore_old_values);
+      return OutputAttribute(std::move(varray),
+                             domain,
+                             get_simple_output_attribute_save_method(attribute),
+                             ignore_old_values);
     }
     /* Builtin attribute is on the same domain but has a different data type. */
     varray = conversions.try_convert(std::move(varray), *cpp_type);
-    return OutputAttribute(std::move(varray), domain, {}, ignore_old_values);
+    return OutputAttribute(std::move(varray),
+                           domain,
+                           get_simple_output_attribute_save_method(attribute),
+                           ignore_old_values);
   }
 
   const int domain_size = component.attribute_domain_size(domain);
@@ -1324,7 +1349,11 @@ static OutputAttribute create_output_attribute(GeometryComponent &component,
   }
   if (attribute.domain == domain && attribute.varray->type() == *cpp_type) {
     /* Existing generic attribute matches exactly. */
-    return OutputAttribute(std::move(attribute.varray), domain, {}, ignore_old_values);
+
+    return OutputAttribute(std::move(attribute.varray),
+                           domain,
+                           get_simple_output_attribute_save_method(attribute),
+                           ignore_old_values);
   }
 
   /* Allocate a new array that lives next to the existing attribute. It will overwrite the existing
diff --git a/source/blender/blenkernel/intern/attribute_access_intern.hh b/source/blender/blenkernel/intern/attribute_access_intern.hh
index 5cedcf69953..140498bdb01 100644
--- a/source/blender/blenkernel/intern/attribute_access_intern.hh
+++ b/source/blender/blenkernel/intern/attribute_access_intern.hh
@@ -87,7 +87,7 @@ class BuiltinAttributeProvider {
   }
 
   virtual GVArrayPtr try_get_for_read(const GeometryComponent &component) const = 0;
-  virtual GVMutableArrayPtr try_get_for_write(GeometryComponent &component) const = 0;
+  virtual WriteAttributeLookup try_get_for_write(GeometryComponent &component) const = 0;
   virtual bool try_delete(GeometryComponent &component) const = 0;
   virtual bool try_create(GeometryComponent &UNUSED(component),
                           const AttributeInit &UNUSED(initializer)) const = 0;
@@ -267,7 +267,7 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider {
   }
 
   GVArrayPtr try_get_for_read(const GeometryComponent &component) const final;
-  GVMutableArrayPtr try_get_for_write(GeometryComponent &component) const final;
+  WriteAttributeLookup try_get_for_write(GeometryComponent &component) const final;
   bool try_delete(GeometryComponent &component) const final;
   bool try_create(GeometryComponent &component, const AttributeInit &initializer) const final;
   bool exists(const GeometryComponent &component) const final;
diff --git a/source/blender/blenkernel/intern/curve_eval.cc b/source/blender/blenkernel/intern/curve_eval.cc
index 8eec7f5dfab..0e3da9e0789 100644
--- a/source/blender/blenkernel/intern/curve_eval.cc
+++ b/source/blender/blenkernel/intern/curve_eval.cc
@@ -160,6 +160,13 @@ blender::Array<float> CurveEval::accumulated_spline_lengths() const
   return spline_lengths;
 }
 
+void CurveEval::mark_cache_invalid()
+{
+  for (SplinePtr &spline : splines_) {
+    spline->mark_cache_invalid();
+  }
+}
+
 static BezierSpline::HandleType handle_type_from_dna_bezt(const eBezTriple_Handle dna_handle_type)
 {
   switch (dna_handle_type) {
diff --git a/source/blender/blenkernel/intern/geometry_component_curve.cc b/source/blender/blenkernel/intern/geometry_component_curve.cc
index d2a404b860a..d3c3fcc1e67 100644
--- a/source/blender/blenkernel/intern/geometry_component_curve.cc
+++ b/source/blender/blenkernel/intern/geometry_component_curve.cc
@@ -433,7 +433,7 @@ class BuiltinSplineAttributeProvider final : public BuiltinAttributeProvider {
     return as_read_attribute_(*curve);
   }
 
-  GVMutableArrayPtr try_get_for_write(GeometryComponent &component) const final
+  WriteAttributeLookup try_get_for_write(GeometryComponent &component) const final
   {
     if (writable_ != Writable) {
       return {};
@@ -442,7 +442,7 @@ class BuiltinSplineAttributeProvider final : public BuiltinAttributeProvider {
     if (curve == nullptr) {
       return {};
     }
-    return as_write_attribute_(*curve);
+    return {as_write_attribute_(*curve), domain_};
   }
 
   bool try_delete(GeometryComponent &UNUSED(component)) const final
@@ -1122,7 +1122,7 @@ template<typename T> class BuiltinPointAttributeProvider : public BuiltinAttribu
     return point_data_gvarray(spans, offsets);
   }
 
-  GVMutableArrayPtr try_get_for_write(GeometryComponent &component) const override
+  WriteAttributeLookup try_get_for_write(GeometryComponent &component) const override
   {
     CurveEval *curve = get_curve_from_component_for_write(component);
     if (curve == nullptr) {
@@ -1133,25 +1133,30 @@ template<typename T> class BuiltinPointAttributeProvider : public BuiltinAttribu
       return {};
     }
 
+    std::function<void()> tag_modified_fn;
+    if (update_on_write_ != nullptr) {
+      tag_modified_fn = [curve, update = update_on_write_]() {
+        for (SplinePtr &spline : curve->splines()) {
+          update(*spline);
+        }
+      };
+    }
+
     MutableSpan<SplinePtr> splines = curve->splines();
     if (splines.size() == 1) {
-      if (update_on_write_) {
-        update_on_write_(*splines[0]);
-      }
-      return std::make_unique<fn::GVMutableArray_For_GMutableSpan>(
-          get_mutable_span_(*splines.first()));
+      return {std::make_unique<fn::GVMutableArray_For_GMutableSpan>(
+                  get_mutable_span_(*splines.first())),
+              domain_,
+              std::move(tag_modified_fn)};
     }
 
     Array<int> offsets = curve->control_point_offsets();
     Array<MutableSpan<T>> spans(splines.size());
     for (const int i : splines.index_range()) {
       spans[i] = get_mutable_span_(*splines[i]);
-      if (update_on_write_) {
-        update_on_write_(*splines[i]);
-      }
     }
 
-    return point_data_gvarray(spans, offsets);
+    return {point_data_gvarray(spans, offsets), domain_, tag_modified_fn};
   }
 
   bool try_delete(GeometryComponent &component) const final
@@ -1223,7 +1228,7 @@ 

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list