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.
This commit is contained in:
Hans Goudey 2022-03-11 11:34:03 -06:00
parent 3b28c785d4
commit 22807d2075
5 changed files with 112 additions and 1 deletions

View File

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

View File

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

View File

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

View File

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

View File

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