[Bf-blender-cvs] [cf50a3eabce] master: Cleanup: remove is_same method for virtual arrays

Jacques Lucke noreply at git.blender.org
Wed Jan 18 13:24:34 CET 2023


Commit: cf50a3eabce47810d38c093be63919487dfbe42c
Author: Jacques Lucke
Date:   Wed Jan 18 13:24:00 2023 +0100
Branches: master
https://developer.blender.org/rBcf50a3eabce47810d38c093be63919487dfbe42c

Cleanup: remove is_same method for virtual arrays

This abstraction is rarely used. It could be replaced by some more
general "query" API in the future. For now it's easier to just compare
pointers in the Set Position node where this was used.

This is possible now, because mesh positions are stored as flat `float3`
arrays (previously, they were stored as `MVert` with some other data
interleaved).

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

M	source/blender/blenlib/BLI_virtual_array.hh
M	source/blender/nodes/geometry/nodes/node_geo_set_position.cc

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

diff --git a/source/blender/blenlib/BLI_virtual_array.hh b/source/blender/blenlib/BLI_virtual_array.hh
index 819807843df..c57c1dae961 100644
--- a/source/blender/blenlib/BLI_virtual_array.hh
+++ b/source/blender/blenlib/BLI_virtual_array.hh
@@ -156,15 +156,6 @@ template<typename T> class VArrayImpl {
   {
     return false;
   }
-
-  /**
-   * Return true when the other virtual array should be considered to be the same, e.g. because it
-   * shares the same underlying memory.
-   */
-  virtual bool is_same(const VArrayImpl<T> & /*other*/) const
-  {
-    return false;
-  }
 };
 
 /** Similar to #VArrayImpl, but adds methods that allow modifying the referenced elements. */
@@ -238,18 +229,6 @@ template<typename T> class VArrayImpl_For_Span : public VMutableArrayImpl<T> {
     return CommonVArrayInfo(CommonVArrayInfo::Type::Span, true, data_);
   }
 
-  bool is_same(const VArrayImpl<T> &other) const final
-  {
-    if (other.size() != this->size_) {
-      return false;
-    }
-    const CommonVArrayInfo other_info = other.common_info();
-    if (other_info.type != CommonVArrayInfo::Type::Span) {
-      return false;
-    }
-    return data_ == static_cast<const T *>(other_info.data);
-  }
-
   void materialize(IndexMask mask, T *dst) const override
   {
     mask.foreach_index([&](const int64_t i) { dst[i] = data_[i]; });
@@ -482,23 +461,6 @@ class VArrayImpl_For_DerivedSpan final : public VMutableArrayImpl<ElemT> {
       }
     });
   }
-
-  bool is_same(const VArrayImpl<ElemT> &other) const override
-  {
-    if (other.size() != this->size_) {
-      return false;
-    }
-    if (const VArrayImpl_For_DerivedSpan<StructT, ElemT, GetFunc> *other_typed =
-            dynamic_cast<const VArrayImpl_For_DerivedSpan<StructT, ElemT, GetFunc> *>(&other)) {
-      return other_typed->data_ == data_;
-    }
-    if (const VArrayImpl_For_DerivedSpan<StructT, ElemT, GetFunc, SetFunc> *other_typed =
-            dynamic_cast<const VArrayImpl_For_DerivedSpan<StructT, ElemT, GetFunc, SetFunc> *>(
-                &other)) {
-      return other_typed->data_ == data_;
-    }
-    return false;
-  }
 };
 
 template<typename StructT,
@@ -778,25 +740,6 @@ template<typename T> class VArrayCommon {
     return *static_cast<const T *>(info.data);
   }
 
-  /**
-   * Return true when the other virtual references the same underlying memory.
-   */
-  bool is_same(const VArrayCommon<T> &other) const
-  {
-    if (!*this || !other) {
-      return false;
-    }
-    /* Check in both directions in case one does not know how to compare to the other
-     * implementation. */
-    if (impl_->is_same(*other.impl_)) {
-      return true;
-    }
-    if (other.impl_->is_same(*impl_)) {
-      return true;
-    }
-    return false;
-  }
-
   /** Copy the entire virtual array into a span. */
   void materialize(MutableSpan<T> r_span) const
   {
diff --git a/source/blender/nodes/geometry/nodes/node_geo_set_position.cc b/source/blender/nodes/geometry/nodes/node_geo_set_position.cc
index caa911a5f45..2ad126567d1 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_set_position.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_set_position.cc
@@ -29,9 +29,16 @@ static void set_computed_position_and_offset(GeometryComponent &component,
                                              const IndexMask selection)
 {
   MutableAttributeAccessor attributes = *component.attributes_for_write();
+
+  /* Optimize the case when `in_positions` references the original positions array. */
   const VArray<float3> positions_read_only = attributes.lookup<float3>("position");
+  bool positions_are_original = false;
+  if (positions_read_only.is_span() && in_positions.is_span()) {
+    positions_are_original = positions_read_only.get_internal_span().data() ==
+                             in_positions.get_internal_span().data();
+  }
 
-  if (in_positions.is_same(positions_read_only)) {
+  if (positions_are_original) {
     if (const std::optional<float3> offset = in_offsets.get_if_single()) {
       if (math::is_zero(*offset)) {
         return;
@@ -81,7 +88,7 @@ static void set_computed_position_and_offset(GeometryComponent &component,
     default: {
       AttributeWriter<float3> positions = attributes.lookup_for_write<float3>("position");
       MutableVArraySpan<float3> out_positions_span = positions.varray;
-      if (in_positions.is_same(positions_read_only)) {
+      if (positions_are_original) {
         devirtualize_varray(in_offsets, [&](const auto in_offsets) {
           threading::parallel_for(
               selection.index_range(), grain_size, [&](const IndexRange range) {



More information about the Bf-blender-cvs mailing list