[Bf-blender-cvs] [584089879c5] master: BLI: Follow up and fix recent span slicing change

Hans Goudey noreply at git.blender.org
Wed Nov 23 18:40:50 CET 2022


Commit: 584089879c57747251578db073b876c03d74f60b
Author: Hans Goudey
Date:   Wed Nov 23 11:35:59 2022 -0600
Branches: master
https://developer.blender.org/rB584089879c57747251578db073b876c03d74f60b

BLI: Follow up and fix recent span slicing change

a5e7657ceeb6cc didn't account for slices of zero sizes, and the asserts
were slightly incorrect otherwise. Also, the change didn't apply to
`Span`, only `MutableSpan`, which was a mistake. This also adds "safe"
methods to `IndexMask`, and switches function calls where necessary.

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

M	source/blender/blenkernel/intern/curves_geometry.cc
M	source/blender/blenlib/BLI_index_mask.hh
M	source/blender/blenlib/BLI_span.hh
M	source/blender/blenlib/intern/index_mask.cc
M	source/blender/blenlib/tests/BLI_span_test.cc
M	source/blender/functions/FN_multi_function_builder.hh
M	source/blender/geometry/intern/resample_curves.cc

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

diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc
index 16b2bd1b3af..9612930b4f3 100644
--- a/source/blender/blenkernel/intern/curves_geometry.cc
+++ b/source/blender/blenkernel/intern/curves_geometry.cc
@@ -654,7 +654,7 @@ Span<float3> CurvesGeometry::evaluated_positions() const
           case CURVE_TYPE_NURBS: {
             curves::nurbs::interpolate_to_evaluated(this->runtime->nurbs_basis_cache[curve_index],
                                                     nurbs_orders[curve_index],
-                                                    nurbs_weights.slice(points),
+                                                    nurbs_weights.slice_safe(points),
                                                     positions.slice(points),
                                                     evaluated_positions.slice(evaluated_points));
             break;
@@ -812,7 +812,7 @@ void CurvesGeometry::interpolate_to_evaluated(const int curve_index,
     case CURVE_TYPE_NURBS:
       curves::nurbs::interpolate_to_evaluated(this->runtime->nurbs_basis_cache[curve_index],
                                               this->nurbs_orders()[curve_index],
-                                              this->nurbs_weights().slice(points),
+                                              this->nurbs_weights().slice_safe(points),
                                               src,
                                               dst);
       return;
@@ -853,7 +853,7 @@ void CurvesGeometry::interpolate_to_evaluated(const GSpan src, GMutableSpan dst)
         case CURVE_TYPE_NURBS:
           curves::nurbs::interpolate_to_evaluated(this->runtime->nurbs_basis_cache[curve_index],
                                                   nurbs_orders[curve_index],
-                                                  nurbs_weights.slice(points),
+                                                  nurbs_weights.slice_safe(points),
                                                   src.slice(points),
                                                   dst.slice(evaluated_points));
           continue;
diff --git a/source/blender/blenlib/BLI_index_mask.hh b/source/blender/blenlib/BLI_index_mask.hh
index 22bdf090181..6fa3fdb963e 100644
--- a/source/blender/blenlib/BLI_index_mask.hh
+++ b/source/blender/blenlib/BLI_index_mask.hh
@@ -236,6 +236,9 @@ class IndexMask {
 
   IndexMask slice(int64_t start, int64_t size) const;
   IndexMask slice(IndexRange slice) const;
+
+  IndexMask slice_safe(int64_t start, int64_t size) const;
+  IndexMask slice_safe(IndexRange slice) const;
   /**
    * Create a sub-mask that is also shifted to the beginning.
    * The shifting to the beginning allows code to work with smaller indices,
diff --git a/source/blender/blenlib/BLI_span.hh b/source/blender/blenlib/BLI_span.hh
index a513c58d6ac..94464ef5088 100644
--- a/source/blender/blenlib/BLI_span.hh
+++ b/source/blender/blenlib/BLI_span.hh
@@ -141,8 +141,8 @@ template<typename T> class Span {
   {
     BLI_assert(start >= 0);
     BLI_assert(size >= 0);
-    const int64_t new_size = std::max<int64_t>(0, std::min(size, size_ - start));
-    return Span(data_ + start, new_size);
+    BLI_assert(start + size <= size_ || size == 0);
+    return Span(data_ + start, size);
   }
 
   constexpr Span slice(IndexRange range) const
@@ -150,6 +150,23 @@ template<typename T> class Span {
     return this->slice(range.start(), range.size());
   }
 
+  /**
+   * Returns a contiguous part of the array. This invokes undefined behavior when the start or size
+   * is negative. Clamps the size of the new new span so it fits in the current one.
+   */
+  constexpr Span slice_safe(const int64_t start, const int64_t size) const
+  {
+    BLI_assert(start >= 0);
+    BLI_assert(size >= 0);
+    const int64_t new_size = std::max<int64_t>(0, std::min(size, size_ - start));
+    return Span(data_ + start, new_size);
+  }
+
+  constexpr Span slice_safe(IndexRange range) const
+  {
+    return this->slice_safe(range.start(), range.size());
+  }
+
   /**
    * Returns a new Span with n elements removed from the beginning. This invokes undefined
    * behavior when n is negative.
@@ -578,8 +595,9 @@ template<typename T> class MutableSpan {
    */
   constexpr MutableSpan slice(const int64_t start, const int64_t size) const
   {
-    BLI_assert(this->index_range().contains(start));
-    BLI_assert(this->index_range().contains(IndexRange(start, size).last()));
+    BLI_assert(start >= 0);
+    BLI_assert(size >= 0);
+    BLI_assert(start + size <= size_ || size == 0);
     return MutableSpan(data_ + start, size);
   }
 
diff --git a/source/blender/blenlib/intern/index_mask.cc b/source/blender/blenlib/intern/index_mask.cc
index e9af183d60d..72282bc69f3 100644
--- a/source/blender/blenlib/intern/index_mask.cc
+++ b/source/blender/blenlib/intern/index_mask.cc
@@ -15,6 +15,16 @@ IndexMask IndexMask::slice(IndexRange slice) const
   return IndexMask(indices_.slice(slice));
 }
 
+IndexMask IndexMask::slice_safe(int64_t start, int64_t size) const
+{
+  return this->slice_safe(IndexRange(start, size));
+}
+
+IndexMask IndexMask::slice_safe(IndexRange slice) const
+{
+  return IndexMask(indices_.slice_safe(slice));
+}
+
 IndexMask IndexMask::slice_and_offset(const IndexRange slice, Vector<int64_t> &r_new_indices) const
 {
   const int slice_size = slice.size();
diff --git a/source/blender/blenlib/tests/BLI_span_test.cc b/source/blender/blenlib/tests/BLI_span_test.cc
index 01f458f3c04..1fcbdfae567 100644
--- a/source/blender/blenlib/tests/BLI_span_test.cc
+++ b/source/blender/blenlib/tests/BLI_span_test.cc
@@ -142,7 +142,7 @@ TEST(span, SliceRange)
 TEST(span, SliceLargeN)
 {
   Vector<int> a = {1, 2, 3, 4, 5};
-  Span<int> slice1 = Span<int>(a).slice(3, 100);
+  Span<int> slice1 = Span<int>(a).slice_safe(3, 100);
   MutableSpan<int> slice2 = MutableSpan<int>(a).slice_safe(3, 100);
   EXPECT_EQ(slice1.size(), 2);
   EXPECT_EQ(slice2.size(), 2);
diff --git a/source/blender/functions/FN_multi_function_builder.hh b/source/blender/functions/FN_multi_function_builder.hh
index eeadb1938bb..7a936f34bd2 100644
--- a/source/blender/functions/FN_multi_function_builder.hh
+++ b/source/blender/functions/FN_multi_function_builder.hh
@@ -232,7 +232,7 @@ void execute_materialized(TypeSequence<ParamTags...> /* param_tags */,
 
   /* Outer loop over all chunks. */
   for (int64_t chunk_start = 0; chunk_start < mask_size; chunk_start += MaxChunkSize) {
-    const IndexMask sliced_mask = mask.slice(chunk_start, MaxChunkSize);
+    const IndexMask sliced_mask = mask.slice_safe(chunk_start, MaxChunkSize);
     const int64_t chunk_size = sliced_mask.size();
     const bool sliced_mask_is_range = sliced_mask.is_range();
 
diff --git a/source/blender/geometry/intern/resample_curves.cc b/source/blender/geometry/intern/resample_curves.cc
index 3be850ec097..c8f6e9a9226 100644
--- a/source/blender/geometry/intern/resample_curves.cc
+++ b/source/blender/geometry/intern/resample_curves.cc
@@ -254,7 +254,7 @@ static CurvesGeometry resample_to_uniform(const CurvesGeometry &src_curves,
   bke::CurvesFieldContext field_context{src_curves, ATTR_DOMAIN_CURVE};
   fn::FieldEvaluator evaluator{field_context, src_curves.curves_num()};
   evaluator.set_selection(selection_field);
-  evaluator.add_with_destination(count_field, dst_offsets);
+  evaluator.add_with_destination(count_field, dst_offsets.drop_back(1));
   evaluator.evaluate();
   const IndexMask selection = evaluator.get_evaluated_selection_as_mask();
   const Vector<IndexRange> unselected_ranges = selection.extract_ranges_invert(



More information about the Bf-blender-cvs mailing list