blender
/
blender
129
397
Fork 571
Code Issues 6.2k Pull Requests 750 Projects 19 Wiki Activity

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.
This commit is contained in:
Hans Goudey 2022-11-22 11:29:12 -06:00
parent 822dab9a42
commit a5e7657cee
Notes: blender-bot 2023-05-03 18:13:10 +02:00 
Referenced by commit 3cebc58936, Fix: Assert in subdivide curves node after span slicing change
Referenced by commit 584089879c, BLI: Follow up and fix recent span slicing change
Referenced by commit 1f76863f80, BLI: Remove clamping from generic span slicing
4 changed files with 36 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
source/blender/blenlib/BLI_span.hh
View File

@ -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());
}
/**

2
source/blender/blenlib/tests/BLI_span_test.cc
View File

@ -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);

4
source/blender/draw/intern/draw_command.cc
View File

@ -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());

30
source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
View File

@ -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