Differential D13520 Diff 46037 source/blender/nodes/geometry/nodes/node_geo_attribute_statistic.cc

Changeset View

Standalone View

source/blender/nodes/geometry/nodes/node_geo_attribute_statistic.cc

Show All 25 Lines

namespace blender::nodes::node_geo_attribute_statistic_cc {		namespace blender::nodes::node_geo_attribute_statistic_cc {

static void node_declare(NodeDeclarationBuilder &b)		static void node_declare(NodeDeclarationBuilder &b)
{		{
b.add_input<decl::Geometry>(N_("Geometry"));		b.add_input<decl::Geometry>(N_("Geometry"));
b.add_input<decl::Float>(N_("Attribute")).hide_value().supports_field();		b.add_input<decl::Float>(N_("Attribute")).hide_value().supports_field();
b.add_input<decl::Vector>(N_("Attribute"), "Attribute_001").hide_value().supports_field();		b.add_input<decl::Vector>(N_("Attribute"), "Attribute_001").hide_value().supports_field();
		b.add_input<decl::Bool>(N_("Selection")).default_value(true).supports_field().hide_value();
		HooglyBooglyUnsubmitted Done Inline Actions Selection inputs should go below the geometry input. HooglyBoogly: Selection inputs should go below the geometry input.

b.add_output<decl::Float>(N_("Mean"));		b.add_output<decl::Float>(N_("Mean"));
b.add_output<decl::Float>(N_("Median"));		b.add_output<decl::Float>(N_("Median"));
b.add_output<decl::Float>(N_("Sum"));		b.add_output<decl::Float>(N_("Sum"));
b.add_output<decl::Float>(N_("Min"));		b.add_output<decl::Float>(N_("Min"));
b.add_output<decl::Float>(N_("Max"));		b.add_output<decl::Float>(N_("Max"));
b.add_output<decl::Float>(N_("Range"));		b.add_output<decl::Float>(N_("Range"));
b.add_output<decl::Float>(N_("Standard Deviation"));		b.add_output<decl::Float>(N_("Standard Deviation"));
▲ Show 20 Lines • Show All 109 Lines • ▼ Show 20 Lines	static void node_geo_exec(GeoNodeExecParams params)

const bNode &node = params.node();		const bNode &node = params.node();
const CustomDataType data_type = static_cast<CustomDataType>(node.custom1);		const CustomDataType data_type = static_cast<CustomDataType>(node.custom1);
const AttributeDomain domain = static_cast<AttributeDomain>(node.custom2);		const AttributeDomain domain = static_cast<AttributeDomain>(node.custom2);

int64_t total_size = 0;		int64_t total_size = 0;
Vector<const GeometryComponent *> components = geometry_set.get_components_for_read();		Vector<const GeometryComponent *> components = geometry_set.get_components_for_read();

for (const GeometryComponent *component : components) {		for (const GeometryComponent *component : components) {
if (component->attribute_domain_supported(domain)) {		if (component->attribute_domain_supported(domain)) {
total_size += component->attribute_domain_size(domain);		total_size += component->attribute_domain_size(domain);
}		}
}		}
if (total_size == 0) {		if (total_size == 0) {
params.set_default_remaining_outputs();		params.set_default_remaining_outputs();
return;		return;
}		}
		const Field<bool> selection_field = params.get_input<Field<bool>>("Selection");
		HooglyBooglyUnsubmitted Done Inline Actions Maximum size is never used, so this calculation could be completely removed. HooglyBoogly: Maximum size is never used, so this calculation could be completely removed.

switch (data_type) {		switch (data_type) {
case CD_PROP_FLOAT: {		case CD_PROP_FLOAT: {
const Field<float> input_field = params.get_input<Field<float>>("Attribute");		const Field<float> input_field = params.get_input<Field<float>>("Attribute");
Array<float> data = Array<float>(total_size);		Vector<float> data(total_size, 0.0f);
int offset = 0;		total_size = 0;
		HooglyBooglyUnsubmitted Done Inline Actions I don't think there's any need to initialize the values. HooglyBoogly: I don't think there's any need to initialize the values.
for (const GeometryComponent *component : components) {		for (const GeometryComponent *component : components) {
if (component->attribute_domain_supported(domain)) {		if (component->attribute_domain_supported(domain)) {
GeometryComponentFieldContext field_context{*component, domain};		GeometryComponentFieldContext field_context{*component, domain};
const int domain_size = component->attribute_domain_size(domain);		const int domain_size = component->attribute_domain_size(domain);
		Array<float> data_in(domain_size);
		HooglyBooglyUnsubmitted Done Inline Actions No need for a temporary array here if it's just going to be copied into the `data` vector after. HooglyBoogly: No need for a temporary array here if it's just going to be copied into the `data` vector after.
fn::FieldEvaluator data_evaluator{field_context, domain_size};		fn::FieldEvaluator data_evaluator{field_context, domain_size};
MutableSpan<float> component_result = data.as_mutable_span().slice(offset, domain_size);		MutableSpan<float> component_result = data_in.as_mutable_span();
data_evaluator.add_with_destination(input_field, component_result);		data_evaluator.add_with_destination(input_field, component_result);
		data_evaluator.add(selection_field);
		HooglyBooglyUnsubmitted Done Inline Actions Moving from the selection field won't work if there is more than one component HooglyBoogly: Moving from the selection field won't work if there is more than one component
data_evaluator.evaluate();		data_evaluator.evaluate();
offset += domain_size;		const IndexMask selection = data_evaluator.get_evaluated_as_mask(1);

		int position = 0;
		for (const int i : selection) {
		HooglyBooglyUnsubmitted Done Inline Actions I think these could be simplified by slicing `data` beforehand, then iterating over `selection.index_range()` rather than keeping track of a "position" HooglyBoogly: I think these could be simplified by slicing `data` beforehand, then iterating over `selection.
		HooglyBooglyUnsubmitted Done Inline Actions I still think this would be an improvement. HooglyBoogly: I still think this would be an improvement.
		HooglyBooglyUnsubmitted Not Done Inline Actions `const` (same with the other) HooglyBoogly: `const` (same with the other)
		data[total_size + position++] = component_result[i];
		HooglyBooglyUnsubmitted Done Inline Actions I doesn't seem correct to subtract one here, what's the reasoning for that? HooglyBoogly: I doesn't seem correct to subtract one here, what's the reasoning for that?
		guitargeekAuthorUnsubmitted Done Inline Actions just a severe case of copy-and-paste-itis guitargeek: just a severe case of copy-and-paste-itis
		}
		total_size += selection.size();
}		}
}		}
		data.resize(total_size);
		HooglyBooglyUnsubmitted Done Inline Actions I'm not sure how resizing this after adding the data to it works, I think `data` could just be an array instead, since the size is known beforehand. HooglyBoogly: I'm not sure how resizing this after adding the data to it works, I think `data` could just…
		guitargeekAuthorUnsubmitted Done Inline Actions Maybe I'm misunderstanding something then. Here was my logic: Make a temp array large enough for all the source data Evaluate the source data into the temp array get the mask use the mask to only copy the selected source data into the Vector that will have calculations done on it. Since there may now be room at the end of this vector, chop it off to the right length Move along to the stats. guitargeek: Maybe I'm misunderstanding something then. Here was my logic: - Make a temp array large…
		HooglyBooglyUnsubmitted Done Inline Actions That works, but there's no need to evaluate into an array directly, you can just use `const VArray<float3> &component_data = data_evaluator.get_evaluated<float3>()` And you resized the vector to `total_size` earlier in the function, there shouldn't be any extra room at the end. (You never call `append`) HooglyBoogly: That works, but there's no need to evaluate into an array directly, you can just use `const…

float mean = 0.0f;		float mean = 0.0f;
float median = 0.0f;		float median = 0.0f;
float sum = 0.0f;		float sum = 0.0f;
float min = 0.0f;		float min = 0.0f;
float max = 0.0f;		float max = 0.0f;
float range = 0.0f;		float range = 0.0f;
float standard_deviation = 0.0f;		float standard_deviation = 0.0f;
Show All 40 Lines	case CD_PROP_FLOAT: {
if (variance_required) {		if (variance_required) {
params.set_output("Standard Deviation", standard_deviation);		params.set_output("Standard Deviation", standard_deviation);
params.set_output("Variance", variance);		params.set_output("Variance", variance);
}		}
break;		break;
}		}
case CD_PROP_FLOAT3: {		case CD_PROP_FLOAT3: {
const Field<float3> input_field = params.get_input<Field<float3>>("Attribute_001");		const Field<float3> input_field = params.get_input<Field<float3>>("Attribute_001");
		Vector<float3> data(total_size);
Array<float3> data = Array<float3>(total_size);		total_size = 0;
		HooglyBooglyUnsubmitted Done Inline Actions Don't reuse a variable and give it a different meaning. HooglyBoogly: Don't reuse a variable and give it a different meaning.
int offset = 0;
for (const GeometryComponent *component : components) {		for (const GeometryComponent *component : components) {
if (component->attribute_domain_supported(domain)) {		if (component->attribute_domain_supported(domain)) {
GeometryComponentFieldContext field_context{*component, domain};		GeometryComponentFieldContext field_context{*component, domain};
const int domain_size = component->attribute_domain_size(domain);		const int domain_size = component->attribute_domain_size(domain);
		Array<float3> data_in(domain_size);
fn::FieldEvaluator data_evaluator{field_context, domain_size};		fn::FieldEvaluator data_evaluator{field_context, domain_size};
MutableSpan<float3> component_result = data.as_mutable_span().slice(offset, domain_size);		MutableSpan<float3> component_result = data_in.as_mutable_span();
data_evaluator.add_with_destination(input_field, component_result);		data_evaluator.add_with_destination(input_field, component_result);
		data_evaluator.add(selection_field);
data_evaluator.evaluate();		data_evaluator.evaluate();
offset += domain_size;		const IndexMask selection = data_evaluator.get_evaluated_as_mask(1);

		int position = 0;
		for (const int i : selection) {
		data[total_size + position++] = component_result[i];
		}
		total_size += selection.size();
}		}
}		}
		data.resize(total_size);
		HooglyBooglyUnsubmitted Done Inline Actions It looks simpler to just skip this variable and call `data.size()` below. HooglyBoogly: It looks simpler to just skip this variable and call `data.size()` below.

float3 median{0};		float3 median{0};
float3 min{0};		float3 min{0};
float3 max{0};		float3 max{0};
float3 range{0};		float3 range{0};
float3 sum{0};		float3 sum{0};
float3 mean{0};		float3 mean{0};
float3 variance{0};		float3 variance{0};
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines