[Bf-blender-cvs] [22807d2075b] master: Curves: Move constructor/assignment
Hans Goudey
noreply at git.blender.org
Fri Mar 11 18:34:11 CET 2022
Commit: 22807d2075b0bcc7b5ae655ba43b08a4e44b002b
Author: Hans Goudey
Date: Fri Mar 11 11:34:03 2022 -0600
Branches: master
https://developer.blender.org/rB22807d2075b0bcc7b5ae655ba43b08a4e44b002b
Curves: Move constructor/assignment
Add the ability to move `CurvesGeometry` without copying its attributes
and data. The benefit is more intuitive management of the data-block
copying, and less overhead for copying in some cases. The "moved-from"
source is left in an empty but valid state. A test file is added to test
the move constructor.
===================================================================
M source/blender/blenkernel/BKE_curves.hh
M source/blender/blenkernel/CMakeLists.txt
M source/blender/blenkernel/intern/curves_geometry.cc
A source/blender/blenkernel/intern/curves_geometry_test.cc
M source/blender/makesdna/DNA_curves_types.h
===================================================================
diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh
index 6cab9c8ff90..875cf48ed43 100644
--- a/source/blender/blenkernel/BKE_curves.hh
+++ b/source/blender/blenkernel/BKE_curves.hh
@@ -62,7 +62,9 @@ class CurvesGeometry : public ::CurvesGeometry {
*/
CurvesGeometry(int point_size, int curve_size);
CurvesGeometry(const CurvesGeometry &other);
+ CurvesGeometry(CurvesGeometry &&other);
CurvesGeometry &operator=(const CurvesGeometry &other);
+ CurvesGeometry &operator=(CurvesGeometry &&other);
~CurvesGeometry();
static CurvesGeometry &wrap(::CurvesGeometry &dna_struct)
diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt
index 3828d442f58..bd632380ada 100644
--- a/source/blender/blenkernel/CMakeLists.txt
+++ b/source/blender/blenkernel/CMakeLists.txt
@@ -807,6 +807,7 @@ if(WITH_GTESTS)
intern/asset_test.cc
intern/bpath_test.cc
intern/cryptomatte_test.cc
+ intern/curves_geometry_test.cc
intern/fcurve_test.cc
intern/idprop_serialize_test.cc
intern/image_partial_update_test.cc
diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc
index afa0e6e452d..dd91e788e5a 100644
--- a/source/blender/blenkernel/intern/curves_geometry.cc
+++ b/source/blender/blenkernel/intern/curves_geometry.cc
@@ -84,6 +84,42 @@ CurvesGeometry &CurvesGeometry::operator=(const CurvesGeometry &other)
return *this;
}
+/* The source should be empty, but in a valid state so that using it further will work. */
+static void move_curves_geometry(CurvesGeometry &dst, CurvesGeometry &src)
+{
+ dst.point_size = src.point_size;
+ std::swap(dst.point_data, src.point_data);
+ CustomData_free(&src.point_data, src.point_size);
+ src.point_size = 0;
+
+ dst.curve_size = src.curve_size;
+ std::swap(dst.curve_data, dst.curve_data);
+ CustomData_free(&src.curve_data, src.curve_size);
+ src.curve_size = 0;
+
+ std::swap(dst.curve_offsets, src.curve_offsets);
+ MEM_SAFE_FREE(src.curve_offsets);
+
+ std::swap(dst.runtime, src.runtime);
+
+ src.update_customdata_pointers();
+ dst.update_customdata_pointers();
+}
+
+CurvesGeometry::CurvesGeometry(CurvesGeometry &&other)
+ : CurvesGeometry(other.point_size, other.curve_size)
+{
+ move_curves_geometry(*this, other);
+}
+
+CurvesGeometry &CurvesGeometry::operator=(CurvesGeometry &&other)
+{
+ if (this != &other) {
+ move_curves_geometry(*this, other);
+ }
+ return *this;
+}
+
CurvesGeometry::~CurvesGeometry()
{
CustomData_free(&this->point_data, this->point_size);
@@ -124,6 +160,8 @@ int CurvesGeometry::evaluated_points_size() const
IndexRange CurvesGeometry::range_for_curve(const int index) const
{
+ BLI_assert(this->curve_size > 0);
+ BLI_assert(this->curve_offsets != nullptr);
const int offset = this->curve_offsets[index];
const int offset_next = this->curve_offsets[index + 1];
return {offset, offset_next - offset};
@@ -131,6 +169,8 @@ IndexRange CurvesGeometry::range_for_curve(const int index) const
IndexRange CurvesGeometry::range_for_curves(const IndexRange curves) const
{
+ BLI_assert(this->curve_size > 0);
+ BLI_assert(this->curve_offsets != nullptr);
const int offset = this->curve_offsets[curves.start()];
const int offset_next = this->curve_offsets[curves.one_after_last()];
return {offset, offset_next - offset};
diff --git a/source/blender/blenkernel/intern/curves_geometry_test.cc b/source/blender/blenkernel/intern/curves_geometry_test.cc
new file mode 100644
index 00000000000..3a43c0c8102
--- /dev/null
+++ b/source/blender/blenkernel/intern/curves_geometry_test.cc
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/** \file
+ * \ingroup bke
+ */
+
+#include "BKE_curves.hh"
+
+#include "testing/testing.h"
+
+namespace blender::bke::tests {
+
+static CurvesGeometry create_basic_curves(const int points_size, const int curves_size)
+{
+ CurvesGeometry curves(points_size, curves_size);
+
+ const int curve_length = points_size / curves_size;
+ for (const int i : curves.curves_range()) {
+ curves.offsets()[i] = points_size * curve_length;
+ }
+ curves.offsets().last() = points_size;
+
+ for (const int i : curves.points_range()) {
+ curves.positions()[i] = {float(i), float(i % curve_length), 0.0f};
+ }
+
+ return curves;
+}
+
+TEST(curves_geometry, Empty)
+{
+ CurvesGeometry empty(0, 0);
+ empty.cyclic();
+ float3 min;
+ float3 max;
+ EXPECT_FALSE(empty.bounds_min_max(min, max));
+}
+
+TEST(curves_geometry, Move)
+{
+ CurvesGeometry curves = create_basic_curves(100, 10);
+
+ const int *offsets_data = curves.offsets().data();
+ const float3 *positions_data = curves.positions().data();
+
+ CurvesGeometry other = std::move(curves);
+
+ /* The old curves should be empty, and the offsets are expected to be null. */
+ EXPECT_EQ(curves.points_size(), 0); /* NOLINT: bugprone-use-after-move */
+ EXPECT_EQ(curves.curve_offsets, nullptr); /* NOLINT: bugprone-use-after-move */
+
+ /* Just a basic check that the new curves work okay. */
+ float3 min;
+ float3 max;
+ EXPECT_TRUE(other.bounds_min_max(min, max));
+
+ curves = std::move(other);
+
+ CurvesGeometry second_other(std::move(curves));
+
+ /* The data should not have been reallocated ever. */
+ EXPECT_EQ(second_other.positions().data(), positions_data);
+ EXPECT_EQ(second_other.offsets().data(), offsets_data);
+}
+
+} // namespace blender::bke::tests
diff --git a/source/blender/makesdna/DNA_curves_types.h b/source/blender/makesdna/DNA_curves_types.h
index 98d2aa4b295..d3e69315265 100644
--- a/source/blender/makesdna/DNA_curves_types.h
+++ b/source/blender/makesdna/DNA_curves_types.h
@@ -69,7 +69,8 @@ typedef struct CurvesGeometry {
/**
* The start index of each curve in the point data. The size of each curve can be calculated by
* subtracting the offset from the next offset. That is valid even for the last curve because
- * this array is allocated with a length one larger than the number of splines.
+ * this array is allocated with a length one larger than the number of splines. This is allowed
+ * to be null when there are no curves.
*
* \note This is *not* stored in #CustomData because its size is one larger than #curve_data.
*/
@@ -77,6 +78,7 @@ typedef struct CurvesGeometry {
/**
* All attributes stored on control points (#ATTR_DOMAIN_POINT).
+ * This might not contain a layer for positions if there are no points.
*/
CustomData point_data;
More information about the Bf-blender-cvs
mailing list