GPv3: Join Geometry/Realize Instances node #113844

Closed
Falk David wants to merge 11 commits from filedescriptor/blender:gpv3-join-geometry-node into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
4 changed files with 336 additions and 18 deletions

View File

@ -33,6 +33,8 @@ struct Material;
namespace blender::bke {
GreasePencil *grease_pencil_new_nomain(int layer_num);
namespace greasepencil {
struct DrawingTransforms {
@ -203,6 +205,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
explicit TreeNode(GreasePencilLayerTreeNodeType type);
explicit TreeNode(GreasePencilLayerTreeNodeType type, StringRefNull name);
TreeNode(const TreeNode &other);
TreeNode &operator=(const TreeNode &other);
~TreeNode();
public:
@ -331,6 +334,7 @@ class Layer : public ::GreasePencilLayer {
Layer();
explicit Layer(StringRefNull name);
Layer(const Layer &other);
Layer &operator=(const Layer &other);
~Layer();
public:
@ -455,6 +459,8 @@ class LayerGroupRuntime {
*/
class LayerGroup : public ::GreasePencilLayerTreeGroup {
friend struct ::GreasePencil;
/* Note: This function uses the internal `add_node` API. So we mark it as a friend. */
friend GreasePencil *blender::bke::grease_pencil_new_nomain(int num_layers);
public:
LayerGroup();

View File

@ -497,11 +497,23 @@ TreeNode::TreeNode(GreasePencilLayerTreeNodeType type, StringRefNull name) : Tre
this->GreasePencilLayerTreeNode::name = BLI_strdup(name.c_str());
}
static void copy_tree_node(const TreeNode &src_node, TreeNode &dst_node)
{
dst_node.GreasePencilLayerTreeNode::name = BLI_strdup_null(
src_node.GreasePencilLayerTreeNode::name);
dst_node.flag = src_node.flag;
copy_v3_v3_uchar(dst_node.color, src_node.color);
}
TreeNode::TreeNode(const TreeNode &other) : TreeNode(GreasePencilLayerTreeNodeType(other.type))
{
this->GreasePencilLayerTreeNode::name = BLI_strdup_null(other.GreasePencilLayerTreeNode::name);
this->flag = other.flag;
copy_v3_v3_uchar(this->color, other.color);
copy_tree_node(other, *this);
}
TreeNode &TreeNode::operator=(const TreeNode &other)
{
copy_tree_node(other, *this);
Review

Few issues here:

  • Does not protect against self-assignment.
  • Has a memory leak because it does not free the original data in the TreeNode that is being replaced.

Currently, I'd recommend against using copy-assignment for layers. It's not always trivial to get these things right. And here it's even harder because the layer has a pointer to its parent, and the parent also has a pointer to the children.

Few issues here: * Does not protect against self-assignment. * Has a memory leak because it does not free the original data in the `TreeNode` that is being replaced. Currently, I'd recommend against using copy-assignment for layers. It's not always trivial to get these things right. And here it's even harder because the layer has a pointer to its parent, and the parent also has a pointer to the children.
Review

Ok apart from fixing the issues you mentioned, what would be the alternative to using the copy assignment?

Ok apart from fixing the issues you mentioned, what would be the alternative to using the copy assignment?
Review

A method or normal function that copies that data you care about from one layer to another (but probably does not change the parent pointer).

A method or normal function that copies that data you care about from one layer to another (but probably does not change the parent pointer).
return *this;
}
TreeNode::~TreeNode()
@ -602,22 +614,33 @@ Layer::Layer(StringRefNull name) : Layer()
new (&this->base) TreeNode(GP_LAYER_TREE_LEAF, name);
}
Layer::Layer(const Layer &other) : Layer()
static void copy_layer(const Layer &src_layer, Layer &dst_layer)
{
new (&this->base) TreeNode(other.base.wrap());
/* TODO: duplicate masks. */
/* Note: We do not duplicate the frame storage since it is only needed for writing. */
this->blend_mode = other.blend_mode;
this->opacity = other.opacity;
dst_layer.blend_mode = src_layer.blend_mode;
dst_layer.opacity = src_layer.opacity;
this->runtime->frames_ = other.runtime->frames_;
this->runtime->sorted_keys_cache_ = other.runtime->sorted_keys_cache_;
dst_layer.runtime->frames_ = src_layer.runtime->frames_;
dst_layer.runtime->sorted_keys_cache_ = src_layer.runtime->sorted_keys_cache_;
/* TODO: what about masks cache? */
}
Layer::Layer(const Layer &other) : Layer()
{
new (&this->base) TreeNode(other.base.wrap());
copy_layer(other, *this);
}
Layer &Layer::operator=(const Layer &other)
{
this->base.wrap() = other.base.wrap();
copy_layer(other, *this);
return *this;
}
Layer::~Layer()
{
this->base.wrap().~TreeNode();
@ -1246,6 +1269,22 @@ void BKE_grease_pencil_duplicate_drawing_array(const GreasePencil *grease_pencil
grease_pencil_dst->drawings());
}
namespace blender::bke {
GreasePencil *grease_pencil_new_nomain(const int layer_num)
{
using namespace blender;
GreasePencil *grease_pencil = reinterpret_cast<GreasePencil *>(
BKE_id_new_nomain(ID_GP, nullptr));
for ([[maybe_unused]] const int _ : IndexRange(layer_num)) {
bke::greasepencil::Layer *new_layer = MEM_new<bke::greasepencil::Layer>(__func__);
grease_pencil->root_group().add_node(new_layer->as_node());
}
return grease_pencil;
}
} // namespace blender::bke
/** \} */
/* ------------------------------------------------------------------- */

View File

@ -194,12 +194,14 @@ GeometrySet join_geometries(const Span<GeometrySet> geometries,
const bke::AnonymousAttributePropagationInfo &propagation_info)
{
filedescriptor marked this conversation as resolved Outdated

Remove blender::

Remove `blender::`
GeometrySet result;
static const Array<GeometryComponent::Type> supported_types({GeometryComponent::Type::Mesh,
GeometryComponent::Type::PointCloud,
GeometryComponent::Type::Instance,
GeometryComponent::Type::Volume,
GeometryComponent::Type::Curve,
GeometryComponent::Type::Edit});
static const Array<GeometryComponent::Type> supported_types(
{GeometryComponent::Type::Mesh,
GeometryComponent::Type::PointCloud,
GeometryComponent::Type::Instance,
GeometryComponent::Type::Volume,
GeometryComponent::Type::Curve,
GeometryComponent::Type::Edit,
GeometryComponent::Type::GreasePencil});
for (const GeometryComponent::Type type : supported_types) {
join_component_type(type, geometries, propagation_info, result);
}

View File

@ -14,12 +14,15 @@
#include "BLI_listbase.h"
#include "BLI_math_matrix.hh"
#include "BLI_noise.hh"
#include "BLI_string.h"
#include "BLI_string_utils.h"
#include "BLI_task.hh"
#include "BKE_collection.h"
#include "BKE_curves.hh"
#include "BKE_deform.h"
#include "BKE_geometry_set_instances.hh"
#include "BKE_grease_pencil.hh"
#include "BKE_instances.hh"
#include "BKE_material.h"
#include "BKE_mesh.hh"
@ -180,6 +183,30 @@ struct RealizeCurveTask {
uint32_t id = 0;
};
/** Start indices in the final output grease pencil data-block. */
struct GreasePencilElementStartIndices {
int layer = 0;
int drawing = 0;
};
struct RealizeGreasePencilInfo {
const GreasePencil *grease_pencil = nullptr;
/** Matches the order in #AllGreasePencilsInfo.attributes. */
Array<std::optional<GVArraySpan>> attributes;
Span<int> stored_ids;
};
struct RealizeGreasePencilTask {
GreasePencilElementStartIndices start_indices;
const RealizeGreasePencilInfo *grease_pencil_info;
AttributeFallbacksArray attribute_fallbacks;
/** Only used when the output contains an output attribute. */
uint32_t id = 0;
};
struct AllPointCloudsInfo {
/** Ordering of all attributes that are propagated to the output point cloud generically. */
OrderedAttributes attributes;
@ -222,11 +249,24 @@ struct AllCurvesInfo {
bool create_nurbs_weight_attribute = false;
};
struct AllGreasePencilsInfo {
/** Ordering of all attributes that are propagated to the output grease pencil generically. */
OrderedAttributes attributes;
/** Ordering of the original grease pencils that are joined. */
VectorSet<const GreasePencil *> order;
/** Preprocessed data about every original grease pencil. This is ordered by #order. */
Array<RealizeGreasePencilInfo> realize_info;
/** All layer names (changed to be unique). */
Array<std::string> all_layer_names;
bool create_id_attribute = false;
};
/** Collects all tasks that need to be executed to realize all instances. */
struct GatherTasks {
Vector<RealizePointCloudTask> pointcloud_tasks;
Vector<RealizeMeshTask> mesh_tasks;
Vector<RealizeCurveTask> curve_tasks;
Vector<RealizeGreasePencilTask> grease_pencil_tasks;
/* Volumes only have very simple support currently. Only the first found volume is put into the
* output. */
@ -239,6 +279,7 @@ struct GatherOffsets {
int pointcloud_offset = 0;
MeshElementStartIndices mesh_offsets;
CurvesElementStartIndices curves_offsets;
GreasePencilElementStartIndices grease_pencil_offsets;
};
struct GatherTasksInfo {
@ -246,6 +287,7 @@ struct GatherTasksInfo {
const AllPointCloudsInfo &pointclouds;
const AllMeshesInfo &meshes;
const AllCurvesInfo &curves;
const AllGreasePencilsInfo &grease_pencils;
bool create_id_attribute_on_any_component = false;
/**
@ -272,13 +314,16 @@ struct InstanceContext {
AttributeFallbacksArray meshes;
/** Ordered by #AllCurvesInfo.attributes. */
AttributeFallbacksArray curves;
/** Ordered by #AllGreasePencilsInfo.attributes. */
AttributeFallbacksArray grease_pencils;
/** Id mixed from all parent instances. */
uint32_t id = 0;
InstanceContext(const GatherTasksInfo &gather_info)
: pointclouds(gather_info.pointclouds.attributes.size()),
meshes(gather_info.meshes.attributes.size()),
curves(gather_info.curves.attributes.size())
curves(gather_info.curves.attributes.size()),
grease_pencils(gather_info.grease_pencils.attributes.size())
{
}
};
@ -630,7 +675,21 @@ static void gather_realize_tasks_recursive(GatherTasksInfo &gather_info,
break;
}
case bke::GeometryComponent::Type::GreasePencil: {
/* TODO. Do nothing for now. */
const auto &grease_pencil_component = *static_cast<const bke::GreasePencilComponent *>(
component);
const GreasePencil *grease_pencil = grease_pencil_component.get();
if (grease_pencil->layers().size() > 0) {
const int grease_pencil_index = gather_info.grease_pencils.order.index_of(grease_pencil);
const RealizeGreasePencilInfo &grease_pencil_info =
gather_info.grease_pencils.realize_info[grease_pencil_index];
gather_info.r_tasks.grease_pencil_tasks.append(
{gather_info.r_offsets.grease_pencil_offsets,
&grease_pencil_info,
base_instance_context.grease_pencils,
base_instance_context.id});
gather_info.r_offsets.grease_pencil_offsets.layer += grease_pencil->layers().size();
gather_info.r_offsets.grease_pencil_offsets.drawing += grease_pencil->drawings().size();
}
break;
}
}
@ -1491,6 +1550,211 @@ static void execute_realize_curve_tasks(const RealizeInstancesOptions &options,
/** \} */
/* -------------------------------------------------------------------- */
/** \name Grease Pencil
* \{ */
static OrderedAttributes gather_generic_grease_pencil_layer_attributes_to_propagate(
const bke::GeometrySet &in_geometry_set,
const RealizeInstancesOptions &options,
bool &r_create_id)
{
Vector<bke::GeometryComponent::Type> src_component_types;
src_component_types.append(bke::GeometryComponent::Type::GreasePencil);
if (options.realize_instance_attributes) {
src_component_types.append(bke::GeometryComponent::Type::Instance);
}
Map<AttributeIDRef, AttributeKind> attributes_to_propagate;
in_geometry_set.gather_attributes_for_propagation(src_component_types,
bke::GeometryComponent::Type::GreasePencil,
true,
options.propagation_info,
attributes_to_propagate);
r_create_id = attributes_to_propagate.pop_try("id").has_value();
OrderedAttributes ordered_attributes;
for (const auto item : attributes_to_propagate.items()) {
ordered_attributes.ids.add_new(item.key);
ordered_attributes.kinds.append(item.value);
}
return ordered_attributes;
}
static int gather_grease_pencils_to_realize(const bke::GeometrySet &geometry_set,
VectorSet<const GreasePencil *> &r_grease_pencils)
{
int total_layers = 0;
if (const GreasePencil *grease_pencil = geometry_set.get_grease_pencil()) {
if (grease_pencil->layers().size() > 0) {
total_layers += grease_pencil->layers().size();
r_grease_pencils.add(grease_pencil);
}
}
if (const Instances *instances = geometry_set.get_instances()) {
instances->foreach_referenced_geometry([&](const bke::GeometrySet &instance_geometry_set) {
total_layers += gather_grease_pencils_to_realize(instance_geometry_set, r_grease_pencils);
});
}
return total_layers;
}
static bool unique_layer_name_check(void *arg, const char *name)
{
VectorSet<StringRefNull> &names = *reinterpret_cast<VectorSet<StringRefNull> *>(arg);
return names.contains(name);
}
static AllGreasePencilsInfo preprocess_grease_pencils(const bke::GeometrySet &geometry_set,
const RealizeInstancesOptions &options)
{
AllGreasePencilsInfo info;
info.attributes = gather_generic_grease_pencil_layer_attributes_to_propagate(
geometry_set, options, info.create_id_attribute);
const int total_layers = gather_grease_pencils_to_realize(geometry_set, info.order);
VectorSet<StringRefNull> current_names;
int current_name_i = 0;
info.all_layer_names.reinitialize(total_layers);
info.realize_info.reinitialize(info.order.size());
for (const int grease_pencil_index : info.realize_info.index_range()) {
RealizeGreasePencilInfo &grease_pencil_info = info.realize_info[grease_pencil_index];
const GreasePencil *grease_pencil = info.order[grease_pencil_index];
grease_pencil_info.grease_pencil = grease_pencil;
const Span<const bke::greasepencil::Layer *> layers = grease_pencil->layers();
for (const int layer_i : layers.index_range()) {
char unique_name[MAX_NAME];
Review

Seems like layer names can be quite a bit longer in generally? At least based on the DNA definition.

Seems like layer names can be quite a bit longer in generally? At least based on the DNA definition.
Review

Right, I think we'd need a better BLI_uniquename function like #113901 to support arbitrary long unique names.

Right, I think we'd need a better `BLI_uniquename` function like #113901 to support arbitrary long unique names.
BLI_strncpy(unique_name, layers[layer_i]->name().c_str(), MAX_NAME);
BLI_uniquename_cb(
unique_layer_name_check, &current_names, "Layer", '.', unique_name, MAX_NAME);
info.all_layer_names[current_name_i] = unique_name;
current_names.add(info.all_layer_names[current_name_i]);
current_name_i++;
}
bke::AttributeAccessor attributes = grease_pencil->attributes();
grease_pencil_info.attributes.reinitialize(info.attributes.size());
for (const int attribute_index : info.attributes.index_range()) {
const eAttrDomain domain = info.attributes.kinds[attribute_index].domain;
const AttributeIDRef &attribute_id = info.attributes.ids[attribute_index];
const eCustomDataType data_type = info.attributes.kinds[attribute_index].data_type;
if (attributes.contains(attribute_id)) {
GVArray attribute = *attributes.lookup_or_default(attribute_id, domain, data_type);
grease_pencil_info.attributes[attribute_index].emplace(std::move(attribute));
}
}
if (info.create_id_attribute) {
bke::GAttributeReader id_attribute = attributes.lookup("id");
if (id_attribute) {
grease_pencil_info.stored_ids = id_attribute.varray.get_internal_span().typed<int>();
}
}
}
return info;
}
static void execute_realize_grease_pencil_task(
const RealizeInstancesOptions &options,
const AllGreasePencilsInfo &all_grease_pencils_info,
const RealizeGreasePencilTask &task,
const OrderedAttributes &ordered_attributes,
GreasePencil &dst_grease_pencil,
MutableSpan<GSpanAttributeWriter> dst_attribute_writers)
{
const RealizeGreasePencilInfo &grease_pencil_info = *task.grease_pencil_info;
const GreasePencil &src_grease_pencil = *grease_pencil_info.grease_pencil;
const int start_layer_offset = task.start_indices.layer;
const int start_drawing_offset = task.start_indices.drawing;
const Span<std::string> all_layer_names = all_grease_pencils_info.all_layer_names;
/* Copy all the drawings. */
const Span<const GreasePencilDrawingBase *> src_drawings = src_grease_pencil.drawings();
const IndexRange dst_range = IndexRange(start_drawing_offset, src_drawings.size());
bke::greasepencil::copy_drawing_array(src_drawings,
dst_grease_pencil.drawings().slice(dst_range));
/* Copy the layers data and adjust the drawing indices for the frame mappings. */
const Span<const bke::greasepencil::Layer *> src_layers = src_grease_pencil.layers();
for (const int layer_index : src_layers.index_range()) {
const bke::greasepencil::Layer *src_layer = src_layers[layer_index];
bke::greasepencil::Layer *dst_layer =
dst_grease_pencil.layers_for_write()[start_layer_offset + layer_index];
*dst_layer = *src_layer;
dst_layer->set_name(all_layer_names[start_layer_offset + layer_index]);
for (auto [key, value] : dst_layer->frames_for_write().items()) {
value.drawing_index += start_drawing_offset;
}
}
copy_generic_attributes_to_result(
grease_pencil_info.attributes,
task.attribute_fallbacks,
ordered_attributes,
[&](const eAttrDomain domain) {
BLI_assert(domain == ATTR_DOMAIN_LAYER);
UNUSED_VARS_NDEBUG(domain);
return src_layers.index_range();
},
dst_attribute_writers);
}
static void execute_realize_grease_pencil_tasks(
const RealizeInstancesOptions &options,
const AllGreasePencilsInfo &all_grease_pencils_info,
const Span<RealizeGreasePencilTask> tasks,
const OrderedAttributes &ordered_attributes,
bke::GeometrySet &r_realized_geometry)
{
if (tasks.is_empty()) {
return;
}
const RealizeGreasePencilTask &last_task = tasks.last();
const GreasePencil &last_grease_pencil = *last_task.grease_pencil_info->grease_pencil;
const int layers_num = last_task.start_indices.layer + last_grease_pencil.layers().size();
const int drawings_num = last_task.start_indices.drawing + last_grease_pencil.drawings().size();
/* Allocate a new Grease Pencil data-block. */
GreasePencil *dst_grease_pencil = bke::grease_pencil_new_nomain(layers_num);
r_realized_geometry.replace_grease_pencil(dst_grease_pencil);
bke::MutableAttributeAccessor dst_attributes = dst_grease_pencil->attributes_for_write();
/* Allocate space for all the drawings. */
dst_grease_pencil->drawing_array_num = drawings_num;
dst_grease_pencil->drawing_array = MEM_cnew_array<GreasePencilDrawingBase *>(drawings_num,
__func__);
/* Prepare generic output attributes. */
Vector<GSpanAttributeWriter> dst_attribute_writers;
for (const int attribute_index : ordered_attributes.index_range()) {
const AttributeIDRef &attribute_id = ordered_attributes.ids[attribute_index];
const eAttrDomain domain = ordered_attributes.kinds[attribute_index].domain;
const eCustomDataType data_type = ordered_attributes.kinds[attribute_index].data_type;
dst_attribute_writers.append(
dst_attributes.lookup_or_add_for_write_only_span(attribute_id, domain, data_type));
}
/* Actually execute all tasks. */
threading::parallel_for(tasks.index_range(), 100, [&](const IndexRange task_range) {
for (const int task_index : task_range) {
const RealizeGreasePencilTask &task = tasks[task_index];
execute_realize_grease_pencil_task(options,
all_grease_pencils_info,
task,
ordered_attributes,
*dst_grease_pencil,
dst_attribute_writers);
}
});
/* Tag modified attributes. */
for (GSpanAttributeWriter &dst_attribute : dst_attribute_writers) {
dst_attribute.finish();
}
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Realize Instances
* \{ */
@ -1525,6 +1789,7 @@ bke::GeometrySet realize_instances(bke::GeometrySet geometry_set,
AllPointCloudsInfo all_pointclouds_info = preprocess_pointclouds(geometry_set, options);
AllMeshesInfo all_meshes_info = preprocess_meshes(geometry_set, options);
AllCurvesInfo all_curves_info = preprocess_curves(geometry_set, options);
AllGreasePencilsInfo all_grease_pencils_info = preprocess_grease_pencils(geometry_set, options);
Vector<std::unique_ptr<GArray<>>> temporary_arrays;
const bool create_id_attribute = all_pointclouds_info.create_id_attribute ||
@ -1533,6 +1798,7 @@ bke::GeometrySet realize_instances(bke::GeometrySet geometry_set,
GatherTasksInfo gather_info = {all_pointclouds_info,
all_meshes_info,
all_curves_info,
all_grease_pencils_info,
create_id_attribute,
temporary_arrays};
const float4x4 transform = float4x4::identity();
@ -1556,6 +1822,11 @@ bke::GeometrySet realize_instances(bke::GeometrySet geometry_set,
gather_info.r_tasks.curve_tasks,
all_curves_info.attributes,
new_geometry_set);
execute_realize_grease_pencil_tasks(options,
all_grease_pencils_info,
gather_info.r_tasks.grease_pencil_tasks,
all_grease_pencils_info.attributes,
new_geometry_set);
if (gather_info.r_tasks.first_volume) {
new_geometry_set.add(*gather_info.r_tasks.first_volume);