[Bf-blender-cvs] [a5e7657ceeb] master: BLI: Remove clamping from span slicing

Hans Goudey noreply at git.blender.org
Tue Nov 22 18:47:20 CET 2022


Commit: a5e7657ceeb6cc5b6a67209ab1120e45ffa39e6f
Author: Hans Goudey
Date:   Tue Nov 22 11:29:12 2022 -0600
Branches: master
https://developer.blender.org/rBa5e7657ceeb6cc5b6a67209ab1120e45ffa39e6f

BLI: Remove clamping from span slicing

Currently slicing a span clamped the final size so that it would be
within bounds of the input. However, in the vast majority of cases
that is already the case anyway, and we can use asserts to detect
when that assumption fails.

The clamping had a performance cost. On a test interpolating a boolean
attribute from 1 million curves to 4 million points, removing the
clamping saved about 10% of the time. That's an extreme case but
this probably slightly improves performance in other cases too.
Slicing is used a lot in the new curve code.

This commit introduces `slice_safe` which still does the clamping,
and uses it in the few places that needed it or where I wasn't
sure.

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

M	source/blender/blenlib/BLI_span.hh
M	source/blender/blenlib/tests/BLI_span_test.cc
M	source/blender/draw/intern/draw_command.cc
M	source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc

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

diff --git a/source/blender/blenlib/BLI_span.hh b/source/blender/blenlib/BLI_span.hh
index adfccd8d6fe..a513c58d6ac 100644
--- a/source/blender/blenlib/BLI_span.hh
+++ b/source/blender/blenlib/BLI_span.hh
@@ -577,6 +577,22 @@ template<typename T> class MutableSpan {
    * is negative.
    */
   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()));
+    return MutableSpan(data_ + start, size);
+  }
+
+  constexpr MutableSpan slice(IndexRange range) const
+  {
+    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 MutableSpan slice_safe(const int64_t start, const int64_t size) const
   {
     BLI_assert(start >= 0);
     BLI_assert(size >= 0);
@@ -584,9 +600,9 @@ template<typename T> class MutableSpan {
     return MutableSpan(data_ + start, new_size);
   }
 
-  constexpr MutableSpan slice(IndexRange range) const
+  constexpr MutableSpan slice_safe(IndexRange range) const
   {
-    return this->slice(range.start(), range.size());
+    return this->slice_safe(range.start(), range.size());
   }
 
   /**
diff --git a/source/blender/blenlib/tests/BLI_span_test.cc b/source/blender/blenlib/tests/BLI_span_test.cc
index 2dec84c4206..01f458f3c04 100644
--- a/source/blender/blenlib/tests/BLI_span_test.cc
+++ b/source/blender/blenlib/tests/BLI_span_test.cc
@@ -143,7 +143,7 @@ TEST(span, SliceLargeN)
 {
   Vector<int> a = {1, 2, 3, 4, 5};
   Span<int> slice1 = Span<int>(a).slice(3, 100);
-  MutableSpan<int> slice2 = MutableSpan<int>(a).slice(3, 100);
+  MutableSpan<int> slice2 = MutableSpan<int>(a).slice_safe(3, 100);
   EXPECT_EQ(slice1.size(), 2);
   EXPECT_EQ(slice2.size(), 2);
   EXPECT_EQ(slice1[0], 4);
diff --git a/source/blender/draw/intern/draw_command.cc b/source/blender/draw/intern/draw_command.cc
index f7bf8d20547..7a91ecdc108 100644
--- a/source/blender/draw/intern/draw_command.cc
+++ b/source/blender/draw/intern/draw_command.cc
@@ -420,7 +420,7 @@ std::string DrawMulti::serialize(std::string line_prefix) const
     intptr_t offset = grp.start;
 
     if (grp.back_proto_len > 0) {
-      for (DrawPrototype &proto : prototypes.slice({offset, grp.back_proto_len})) {
+      for (DrawPrototype &proto : prototypes.slice_safe({offset, grp.back_proto_len})) {
         BLI_assert(proto.group_id == group_index);
         ResourceHandle handle(proto.resource_handle);
         BLI_assert(handle.has_inverted_handedness());
@@ -432,7 +432,7 @@ std::string DrawMulti::serialize(std::string line_prefix) const
     }
 
     if (grp.front_proto_len > 0) {
-      for (DrawPrototype &proto : prototypes.slice({offset, grp.front_proto_len})) {
+      for (DrawPrototype &proto : prototypes.slice_safe({offset, grp.front_proto_len})) {
         BLI_assert(proto.group_id == group_index);
         ResourceHandle handle(proto.resource_handle);
         BLI_assert(!handle.has_inverted_handedness());
diff --git a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
index 0d3b48fcadd..5a1a4e2c796 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
@@ -272,10 +272,10 @@ static void extrude_mesh_vertices(Mesh &mesh,
   });
 
   MutableSpan<int> vert_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_POINT);
-  vert_orig_indices.slice(new_vert_range).fill(ORIGINDEX_NONE);
+  vert_orig_indices.slice_safe(new_vert_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> new_edge_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_EDGE);
-  new_edge_orig_indices.slice(new_edge_range).fill(ORIGINDEX_NONE);
+  new_edge_orig_indices.slice_safe(new_edge_range).fill(ORIGINDEX_NONE);
 
   if (attribute_outputs.top_id) {
     save_selection_as_attribute(
@@ -608,14 +608,14 @@ static void extrude_mesh_edges(Mesh &mesh,
   }
 
   MutableSpan<int> vert_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_POINT);
-  vert_orig_indices.slice(new_vert_range).fill(ORIGINDEX_NONE);
+  vert_orig_indices.slice_safe(new_vert_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> edge_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_EDGE);
-  edge_orig_indices.slice(connect_edge_range).fill(ORIGINDEX_NONE);
-  edge_orig_indices.slice(duplicate_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(connect_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(duplicate_edge_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> poly_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_FACE);
-  poly_orig_indices.slice(new_poly_range).fill(ORIGINDEX_NONE);
+  poly_orig_indices.slice_safe(new_poly_range).fill(ORIGINDEX_NONE);
 
   if (attribute_outputs.top_id) {
     save_selection_as_attribute(
@@ -992,15 +992,15 @@ static void extrude_mesh_face_regions(Mesh &mesh,
   }
 
   MutableSpan<int> vert_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_POINT);
-  vert_orig_indices.slice(new_vert_range).fill(ORIGINDEX_NONE);
+  vert_orig_indices.slice_safe(new_vert_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> edge_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_EDGE);
-  edge_orig_indices.slice(connect_edge_range).fill(ORIGINDEX_NONE);
-  edge_orig_indices.slice(new_inner_edge_range).fill(ORIGINDEX_NONE);
-  edge_orig_indices.slice(boundary_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(connect_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(new_inner_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(boundary_edge_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> poly_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_FACE);
-  poly_orig_indices.slice(side_poly_range).fill(ORIGINDEX_NONE);
+  poly_orig_indices.slice_safe(side_poly_range).fill(ORIGINDEX_NONE);
 
   if (attribute_outputs.top_id) {
     save_selection_as_attribute(
@@ -1256,14 +1256,14 @@ static void extrude_individual_mesh_faces(Mesh &mesh,
   });
 
   MutableSpan<int> vert_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_POINT);
-  vert_orig_indices.slice(new_vert_range).fill(ORIGINDEX_NONE);
+  vert_orig_indices.slice_safe(new_vert_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> edge_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_EDGE);
-  edge_orig_indices.slice(connect_edge_range).fill(ORIGINDEX_NONE);
-  edge_orig_indices.slice(duplicate_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(connect_edge_range).fill(ORIGINDEX_NONE);
+  edge_orig_indices.slice_safe(duplicate_edge_range).fill(ORIGINDEX_NONE);
 
   MutableSpan<int> poly_orig_indices = get_orig_index_layer(mesh, ATTR_DOMAIN_FACE);
-  poly_orig_indices.slice(side_poly_range).fill(ORIGINDEX_NONE);
+  poly_orig_indices.slice_safe(side_poly_range).fill(ORIGINDEX_NONE);
 
   /* Finally update each extruded polygon's loops to point to the new edges and vertices.
    * This must be done last, because they were used to find original indices for attribute



More information about the Bf-blender-cvs mailing list