[Bf-blender-cvs] [e9f82d3dc7e] master: Curves: Remove redundant custom data pointers
Hans Goudey
noreply at git.blender.org
Wed Jul 20 01:16:27 CEST 2022
Commit: e9f82d3dc7eebadcc52fdc43858d060c3a8214b2
Author: Hans Goudey
Date: Tue Jul 19 18:01:04 2022 -0500
Branches: master
https://developer.blender.org/rBe9f82d3dc7eebadcc52fdc43858d060c3a8214b2
Curves: Remove redundant custom data pointers
These mutable pointers present problems with ownership in relation to
proper copy-on-write for attributes. The simplest solution is to just
remove them and retrieve the layers from `CustomData` when they are
needed. This also removes the complexity and redundancy of having to
update the pointers as the curves change. A similar change will apply
to meshes and point clouds.
One downside of this change is that it makes random access with RNA
slower. However, it's simple to just use the RNA attribute API instead,
which is unaffected. In this patch I updated Cycles to do that. With
the future attribute CoW changes, this generic approach makes sense
because Cycles can just request ownership of the existing arrays.
Differential Revision: https://developer.blender.org/D15486
===================================================================
M intern/cycles/blender/curves.cpp
M source/blender/blenkernel/BKE_curves.hh
M source/blender/blenkernel/intern/curves.cc
M source/blender/blenkernel/intern/curves_geometry.cc
M source/blender/blenkernel/intern/geometry_component_curves.cc
M source/blender/geometry/intern/resample_curves.cc
M source/blender/makesdna/DNA_curves_types.h
M source/blender/makesrna/intern/rna_curves.c
===================================================================
diff --git a/intern/cycles/blender/curves.cpp b/intern/cycles/blender/curves.cpp
index 263a5fc0e02..c4154bce022 100644
--- a/intern/cycles/blender/curves.cpp
+++ b/intern/cycles/blender/curves.cpp
@@ -630,6 +630,25 @@ static std::optional<BL::FloatAttribute> find_curves_radius_attribute(BL::Curves
return std::nullopt;
}
+static BL::FloatVectorAttribute find_curves_position_attribute(BL::Curves b_curves)
+{
+ for (BL::Attribute &b_attribute : b_curves.attributes) {
+ if (b_attribute.name() != "position") {
+ continue;
+ }
+ if (b_attribute.domain() != BL::Attribute::domain_POINT) {
+ continue;
+ }
+ if (b_attribute.data_type() != BL::Attribute::data_type_FLOAT_VECTOR) {
+ continue;
+ }
+ return BL::FloatVectorAttribute{b_attribute};
+ }
+ /* The position attribute must exist. */
+ assert(false);
+ return BL::FloatVectorAttribute{b_curves.attributes[0]};
+}
+
template<typename TypeInCycles, typename GetValueAtIndex>
static void fill_generic_attribute(BL::Curves &b_curves,
TypeInCycles *data,
@@ -793,16 +812,16 @@ static void attr_create_generic(Scene *scene,
}
}
-static float4 hair_point_as_float4(BL::Curves b_curves,
+static float4 hair_point_as_float4(BL::FloatVectorAttribute b_attr_position,
std::optional<BL::FloatAttribute> b_attr_radius,
const int index)
{
- float4 mP = float3_to_float4(get_float3(b_curves.position_data[index].vector()));
+ float4 mP = float3_to_float4(get_float3(b_attr_position.data[index].vector()));
mP.w = b_attr_radius ? b_attr_radius->data[index].value() : 0.0f;
return mP;
}
-static float4 interpolate_hair_points(BL::Curves b_curves,
+static float4 interpolate_hair_points(BL::FloatVectorAttribute b_attr_position,
std::optional<BL::FloatAttribute> b_attr_radius,
const int first_point_index,
const int num_points,
@@ -812,8 +831,8 @@ static float4 interpolate_hair_points(BL::Curves b_curves,
const int point_a = clamp((int)curve_t, 0, num_points - 1);
const int point_b = min(point_a + 1, num_points - 1);
const float t = curve_t - (float)point_a;
- return lerp(hair_point_as_float4(b_curves, b_attr_radius, first_point_index + point_a),
- hair_point_as_float4(b_curves, b_attr_radius, first_point_index + point_b),
+ return lerp(hair_point_as_float4(b_attr_position, b_attr_radius, first_point_index + point_a),
+ hair_point_as_float4(b_attr_position, b_attr_radius, first_point_index + point_b),
t);
}
@@ -846,6 +865,7 @@ static void export_hair_curves(Scene *scene,
hair->reserve_curves(num_curves, num_keys);
+ BL::FloatVectorAttribute b_attr_position = find_curves_position_attribute(b_curves);
std::optional<BL::FloatAttribute> b_attr_radius = find_curves_radius_attribute(b_curves);
/* Export curves and points. */
@@ -864,7 +884,7 @@ static void export_hair_curves(Scene *scene,
/* Position and radius. */
for (int i = 0; i < num_points; i++) {
- const float3 co = get_float3(b_curves.position_data[first_point_index + i].vector());
+ const float3 co = get_float3(b_attr_position.data[first_point_index + i].vector());
const float radius = b_attr_radius ? b_attr_radius->data[first_point_index + i].value() :
0.005f;
hair->add_curve_key(co, radius);
@@ -921,6 +941,7 @@ static void export_hair_curves_motion(Hair *hair, BL::Curves b_curves, int motio
int num_motion_keys = 0;
int curve_index = 0;
+ BL::FloatVectorAttribute b_attr_position = find_curves_position_attribute(b_curves);
std::optional<BL::FloatAttribute> b_attr_radius = find_curves_radius_attribute(b_curves);
for (int i = 0; i < num_curves; i++) {
@@ -936,7 +957,7 @@ static void export_hair_curves_motion(Hair *hair, BL::Curves b_curves, int motio
int point_index = first_point_index + i;
if (point_index < num_keys) {
- mP[num_motion_keys] = hair_point_as_float4(b_curves, b_attr_radius, point_index);
+ mP[num_motion_keys] = hair_point_as_float4(b_attr_position, b_attr_radius, point_index);
num_motion_keys++;
if (!have_motion) {
@@ -956,7 +977,7 @@ static void export_hair_curves_motion(Hair *hair, BL::Curves b_curves, int motio
for (int i = 0; i < curve.num_keys; i++) {
const float step = i * step_size;
mP[num_motion_keys] = interpolate_hair_points(
- b_curves, b_attr_radius, first_point_index, num_points, step);
+ b_attr_position, b_attr_radius, first_point_index, num_points, step);
num_motion_keys++;
}
have_motion = true;
diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh
index 25a912b8825..68c90a45031 100644
--- a/source/blender/blenkernel/BKE_curves.hh
+++ b/source/blender/blenkernel/BKE_curves.hh
@@ -385,8 +385,6 @@ class CurvesGeometry : public ::CurvesGeometry {
void calculate_bezier_auto_handles();
- void update_customdata_pointers();
-
void remove_points(IndexMask points_to_delete);
void remove_curves(IndexMask curves_to_delete);
diff --git a/source/blender/blenkernel/intern/curves.cc b/source/blender/blenkernel/intern/curves.cc
index 7e9f994313b..5684a2e5b07 100644
--- a/source/blender/blenkernel/intern/curves.cc
+++ b/source/blender/blenkernel/intern/curves.cc
@@ -53,8 +53,6 @@ using blender::Vector;
static const char *ATTR_POSITION = "position";
-static void update_custom_data_pointers(Curves &curves);
-
static void curves_init_data(ID *id)
{
Curves *curves = (Curves *)id;
@@ -97,8 +95,6 @@ static void curves_copy_data(Main *UNUSED(bmain), ID *id_dst, const ID *id_src,
dst.runtime->type_counts = src.runtime->type_counts;
- dst.update_customdata_pointers();
-
curves_dst->batch_cache = nullptr;
}
@@ -170,7 +166,6 @@ static void curves_blend_read_data(BlendDataReader *reader, ID *id)
/* Geometry */
CustomData_blend_read(reader, &curves->geometry.point_data, curves->geometry.point_num);
CustomData_blend_read(reader, &curves->geometry.curve_data, curves->geometry.curve_num);
- update_custom_data_pointers(*curves);
BLO_read_int32_array(reader, curves->geometry.curve_num + 1, &curves->geometry.curve_offsets);
@@ -233,11 +228,6 @@ IDTypeInfo IDType_ID_CV = {
/*lib_override_apply_post */ nullptr,
};
-static void update_custom_data_pointers(Curves &curves)
-{
- blender::bke::CurvesGeometry::wrap(curves.geometry).update_customdata_pointers();
-}
-
void *BKE_curves_add(Main *bmain, const char *name)
{
Curves *curves = static_cast<Curves *>(BKE_id_new(bmain, ID_CV, name));
diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc
index 8d955a46275..7fc660cfbfc 100644
--- a/source/blender/blenkernel/intern/curves_geometry.cc
+++ b/source/blender/blenkernel/intern/curves_geometry.cc
@@ -68,8 +68,6 @@ CurvesGeometry::CurvesGeometry(const int point_num, const int curve_num)
#endif
this->offsets_for_write().first() = 0;
- this->update_customdata_pointers();
-
this->runtime = MEM_new<CurvesGeometryRuntime>(__func__);
/* Fill the type counts with the default so they're in a valid state. */
this->runtime->type_counts[CURVE_TYPE_CATMULL_ROM] = curve_num;
@@ -95,8 +93,6 @@ static void copy_curves_geometry(CurvesGeometry &dst, const CurvesGeometry &src)
/* Though type counts are a cache, they must be copied because they are calculated eagerly. */
dst.runtime->type_counts = src.runtime->type_counts;
-
- dst.update_customdata_pointers();
}
CurvesGeometry::CurvesGeometry(const CurvesGeometry &other)
@@ -130,9 +126,6 @@ static void move_curves_geometry(CurvesGeometry &dst, CurvesGeometry &src)
MEM_SAFE_FREE(src.curve_offsets);
std::swap(dst.runtime, src.runtime);
-
- src.update_customdata_pointers();
- dst.update_customdata_pointers();
}
CurvesGeometry::CurvesGeometry(CurvesGeometry &&other)
@@ -306,13 +299,11 @@ void CurvesGeometry::update_curve_types()
Span<float3> CurvesGeometry::positions() const
{
- return {(const float3 *)this->position, this->point_num};
+ return get_span_attribute<float3>(*this, ATTR_DOMAIN_POINT, ATTR_POSITION);
}
MutableSpan<float3> CurvesGeometry::positions_for_write()
{
- this->position = (float(*)[3])CustomData_duplicate_referenced_layer_named(
- &this->point_data, CD_PROP_FLOAT3, ATTR_POSITION.c_str(), this->point_num);
- return {(float3 *)this->position, this->point_num};
+ return get_mutable_attribute<float3>(*this, ATTR_DOMAIN_POINT, ATTR_POSITION);
}
Span<int> CurvesGeometry::offsets() const
@@ -961,7 +952,6 @@ void CurvesGeometry::resize(const int points_num, const int curves_num)
this->curve_offsets = (int *)MEM_reallocN(this->curve_offsets, sizeof(int) * (curves_num + 1));
}
this->tag_topology_changed();
- this->update_customdata_pointers();
}
void CurvesGeometry::tag_positions_changed()
@@ -1060,10 +1050,11 @@ void CurvesGeometry::transform(const float4x4 &matrix)
static std::optional<bounds::MinMaxResult<float3>> curves_bounds(const CurvesGeometry &curves)
{
- Span<float3> positions = curves.positions();
- if (curves.radius) {
- Span<float> radii{curves.radius, curves.points_num()};
- return bounds::min_max_with_radii(positions, radii);
+ const Span<float3> positions = curves.positions();
+ const VArray<float> radii = curves.attributes().lookup_or_default<float>(
+ ATTR_RADIUS, ATTR_DOMAIN_POINT, 0.0f);
+ if (!(radii.is_single() && radii.get_internal_single() == 0.0f)) {
+ return bounds::min_max_with_radii(positions, radii.get_internal_span());
}
return bounds::min_max(positions);
}
@@ -1079,16 +1070,6 @@ bool CurvesGeometry::bounds_min_max(float3 &min, float3 &max) const
return true;
}
-void CurvesGeometry::update_customdata_pointers()
-{
- this->position = (float(*)[3])CustomData_get_layer_named(
- &this->point_data, CD_PROP_FLOAT3, ATTR_POSITION.c_str());
- this->radius = (float *)Cus
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list