From 1dae11ccb5f9aea099d8f52ae32138bed02d946f Mon Sep 17 00:00:00 2001 From: Mattias Fredriksson Date: Wed, 24 Aug 2022 18:11:55 -0400 Subject: Cleanup: Improve comments Add to comments in curves header, fix typo in attribute header. Ref D14481 --- source/blender/blenkernel/BKE_attribute.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 1e61e477759..d29c60a7373 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -150,7 +150,7 @@ template struct AttributeReader { }; /** - * Result when looking up an attribute from some geometry with read an write access. After writing + * Result when looking up an attribute from some geometry with read and write access. After writing * to the attribute, the #finish method has to be called. This may invalidate caches based on this * attribute. */ -- cgit v1.2.3 From 82a46ea6f8829fc40205d0d3cabf4017eb738d9a Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Tue, 30 Aug 2022 11:08:27 -0500 Subject: Geometry Nodes: Use separate field context for each geometry type Using the same `GeometryComponentFieldContext` for all situations, even when only one geometry type is supported is misleading, and mixes too many different abstraction levels into code that could be simpler. With the attribute API moved out of geometry components recently, the "component" system is just getting in the way here. This commit adds specific field contexts for geometry types: meshes, curves, point clouds, and instances. There are also separate field input helper classes, to help reduce boilerplate for fields that only support specific geometry types. Another benefit of this change is that it separates geometry components from fields, which makes it easier to see the purpose of the two concepts, and how they relate. Because we want to be able to evaluate a field on just `CurvesGeometry` rather than the full `Curves` data-block, the generic "geometry context" had to be changed to avoid using `GeometryComponent`, since there is no corresponding geometry component type. The resulting void pointer is ugly, but only turns up in three places in practice. When Apple clang supports `std::variant`, that could be used instead. Differential Revision: https://developer.blender.org/D15519 --- source/blender/blenkernel/BKE_attribute.hh | 2 ++ 1 file changed, 2 insertions(+) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index d29c60a7373..c2f65c93cbe 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -2,6 +2,8 @@ #pragma once +#include + #include "BLI_color.hh" #include "BLI_function_ref.hh" #include "BLI_generic_span.hh" -- cgit v1.2.3 From 25237d2625078c6d14d744f288776299efd3c7c8 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Tue, 30 Aug 2022 14:54:53 -0500 Subject: Attributes: Improve custom data initialization options When allocating new `CustomData` layers, often we do redundant initialization of arrays. For example, it's common that values are allocated, set to their default value, and then set to some other value. This is wasteful, and it negates the benefits of optimizations to the allocator like D15082. There are two reasons for this. The first is array-of-structs storage that makes it annoying to initialize values manually, and the second is confusing options in the Custom Data API. This patch addresses the latter. The `CustomData` "alloc type" options are rearranged. Now, besides the options that use existing layers, there are two remaining: * `CD_SET_DEFAULT` sets the default value. * Usually zeroes, but for colors this is white (how it was before). * Should be used when you add the layer but don't set all values. * `CD_CONSTRUCT` refers to the "default construct" C++ term. * Only necessary or defined for non-trivial types like vertex groups. * Doesn't do anything for trivial types like `int` or `float3`. * Should be used every other time, when all values will be set. The attribute API's `AttributeInit` types are updated as well. To update code, replace `CD_CALLOC` with `CD_SET_DEFAULT` and `CD_DEFAULT` with `CD_CONSTRUCT`. This doesn't cause any functional changes yet. Follow-up commits will change to avoid initializing new layers where the correctness is clear. Differential Revision: https://developer.blender.org/D15617 --- source/blender/blenkernel/BKE_attribute.hh | 43 +++++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index c2f65c93cbe..6284cce9dc0 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -73,8 +73,13 @@ struct AttributeKind { */ struct AttributeInit { enum class Type { - Default, + /** #AttributeInitConstruct. */ + Construct, + /** #AttributeInitDefaultValue. */ + DefaultValue, + /** #AttributeInitVArray. */ VArray, + /** #AttributeInitMoveArray. */ MoveArray, }; Type type; @@ -84,11 +89,20 @@ struct AttributeInit { }; /** - * Create an attribute using the default value for the data type. - * The default values may depend on the attribute provider implementation. + * Default construct new attribute values. Does nothing for trivial types. This should be used + * if all attribute element values will be set by the caller after creating the attribute. */ -struct AttributeInitDefault : public AttributeInit { - AttributeInitDefault() : AttributeInit(Type::Default) +struct AttributeInitConstruct : public AttributeInit { + AttributeInitConstruct() : AttributeInit(Type::Construct) + { + } +}; + +/** + * Create an attribute using the default value for the data type (almost always "zero"). + */ +struct AttributeInitDefaultValue : public AttributeInit { + AttributeInitDefaultValue() : AttributeInit(Type::DefaultValue) { } }; @@ -96,14 +110,11 @@ struct AttributeInitDefault : public AttributeInit { /** * Create an attribute by copying data from an existing virtual array. The virtual array * must have the same type as the newly created attribute. - * - * Note that this can be used to fill the new attribute with the default */ struct AttributeInitVArray : public AttributeInit { - blender::GVArray varray; + GVArray varray; - AttributeInitVArray(blender::GVArray varray) - : AttributeInit(Type::VArray), varray(std::move(varray)) + AttributeInitVArray(GVArray varray) : AttributeInit(Type::VArray), varray(std::move(varray)) { } }; @@ -119,10 +130,10 @@ struct AttributeInitVArray : public AttributeInit { * The array must be allocated with MEM_*, since `attribute_try_create` will free the array if it * can't be used directly, and that is generally how Blender expects custom data to be allocated. */ -struct AttributeInitMove : public AttributeInit { +struct AttributeInitMoveArray : public AttributeInit { void *data = nullptr; - AttributeInitMove(void *data) : AttributeInit(Type::MoveArray), data(data) + AttributeInitMoveArray(void *data) : AttributeInit(Type::MoveArray), data(data) { } }; @@ -579,7 +590,7 @@ class MutableAttributeAccessor : public AttributeAccessor { const AttributeIDRef &attribute_id, const eAttrDomain domain, const eCustomDataType data_type, - const AttributeInit &initializer = AttributeInitDefault()); + const AttributeInit &initializer = AttributeInitDefaultValue()); /** * Same as above, but returns a type that makes it easier to work with the attribute as a span. @@ -590,7 +601,7 @@ class MutableAttributeAccessor : public AttributeAccessor { const AttributeIDRef &attribute_id, const eAttrDomain domain, const eCustomDataType data_type, - const AttributeInit &initializer = AttributeInitDefault()); + const AttributeInit &initializer = AttributeInitDefaultValue()); /** * Same as above, but should be used when the type is known at compile time. @@ -599,7 +610,7 @@ class MutableAttributeAccessor : public AttributeAccessor { AttributeWriter lookup_or_add_for_write( const AttributeIDRef &attribute_id, const eAttrDomain domain, - const AttributeInit &initializer = AttributeInitDefault()) + const AttributeInit &initializer = AttributeInitDefaultValue()) { const CPPType &cpp_type = CPPType::get(); const eCustomDataType data_type = cpp_type_to_custom_data_type(cpp_type); @@ -613,7 +624,7 @@ class MutableAttributeAccessor : public AttributeAccessor { SpanAttributeWriter lookup_or_add_for_write_span( const AttributeIDRef &attribute_id, const eAttrDomain domain, - const AttributeInit &initializer = AttributeInitDefault()) + const AttributeInit &initializer = AttributeInitDefaultValue()) { AttributeWriter attribute = this->lookup_or_add_for_write( attribute_id, domain, initializer); -- cgit v1.2.3 From 1fcc673230bf585b2b0239d4476b94523bd0ec21 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 5 Sep 2022 15:48:36 -0500 Subject: Cleanup: Remove unused function This had a specific use case relating to the `CurveEval` type which shouldn't be necessary anymore. --- source/blender/blenkernel/BKE_attribute.hh | 5 ----- 1 file changed, 5 deletions(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 6284cce9dc0..83e1a3208ae 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -752,11 +752,6 @@ class CustomDataAttributes { bool create_by_move(const AttributeIDRef &attribute_id, eCustomDataType data_type, void *buffer); bool remove(const AttributeIDRef &attribute_id); - /** - * Change the order of the attributes to match the order of IDs in the argument. - */ - void reorder(Span new_order); - bool foreach_attribute(const AttributeForeachCallback callback, eAttrDomain domain) const; }; -- cgit v1.2.3 From d5934974219135102f364f57c45a8b1465e2b8d9 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 7 Sep 2022 21:41:39 -0500 Subject: Cleanup: Use C++ methods to retrieve attribute accessors Replace `mesh_attributes`, `mesh_attributes_for_write` and the point cloud versions with methods on the `Mesh` and `PointCloud` types. This makes them friendlier to use and improves readability. Differential Revision: https://developer.blender.org/D15907 --- source/blender/blenkernel/BKE_attribute.hh | 6 ------ 1 file changed, 6 deletions(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 83e1a3208ae..4aa6c133e9e 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -755,12 +755,6 @@ class CustomDataAttributes { bool foreach_attribute(const AttributeForeachCallback callback, eAttrDomain domain) const; }; -AttributeAccessor mesh_attributes(const Mesh &mesh); -MutableAttributeAccessor mesh_attributes_for_write(Mesh &mesh); - -AttributeAccessor pointcloud_attributes(const PointCloud &pointcloud); -MutableAttributeAccessor pointcloud_attributes_for_write(PointCloud &pointcloud); - /* -------------------------------------------------------------------- */ /** \name #AttributeIDRef Inline Methods * \{ */ -- cgit v1.2.3 From e37f3388b1563591153fc82259cf549f7942dcf0 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 12 Sep 2022 10:36:56 -0500 Subject: Attributes: Add function to retrieve span without adding attribute Previously, the only versions of attribute access that gave a span would also add the attribute when it doesn't exist, which isn't always wanted. --- source/blender/blenkernel/BKE_attribute.hh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 4aa6c133e9e..21d91af55d5 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -552,6 +552,11 @@ class MutableAttributeAccessor : public AttributeAccessor { */ GAttributeWriter lookup_for_write(const AttributeIDRef &attribute_id); + /** + * Same as above, but returns a type that makes it easier to work with the attribute as a span. + */ + GSpanAttributeWriter lookup_for_write_span(const AttributeIDRef &attribute_id); + /** * Get a writable attribute or non if it does not exist. * Make sure to call #finish after changes are done. @@ -568,6 +573,19 @@ class MutableAttributeAccessor : public AttributeAccessor { return attribute.typed(); } + /** + * Same as above, but returns a type that makes it easier to work with the attribute as a span. + */ + template + SpanAttributeWriter lookup_for_write_span(const AttributeIDRef &attribute_id) + { + AttributeWriter attribute = this->lookup_for_write(attribute_id); + if (attribute) { + return SpanAttributeWriter{std::move(attribute), true}; + } + return {}; + } + /** * Create a new attribute. * \return True, when a new attribute has been created. False, when it's not possible to create -- cgit v1.2.3 From eaf416693dcb431ec122fc559788e6c930038c23 Mon Sep 17 00:00:00 2001 From: Mattias Fredriksson Date: Tue, 13 Sep 2022 11:36:14 -0500 Subject: Geometry Nodes: Port the trim curve node to the new data-block The trim functionality is implemented in the geometry module, and generalized a bit to be potentially useful for bisecting in the future. The implementation is based on a helper type called `IndexRangeCyclic` which allows iteration over all control points between two points on a curve. Catmull Rom curves are now supported-- trimmed without resampling first. However, maintaining the exact shape is not possible. NURBS splines are still converted to polylines using the evaluated curve concept. Performance is equivalent or faster then a 3.1 build with regards to node timings. Compared to 3.3 and 3.2, it's easy to observe test cases where the node is at least 3 or 4 times faster. Differential Revision: https://developer.blender.org/D14481 --- source/blender/blenkernel/BKE_attribute.hh | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 21d91af55d5..fbdacee139c 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -710,6 +710,19 @@ Vector retrieve_attributes_for_transfer( eAttrDomainMask domain_mask, const Set &skip = {}); +/** + * Copy attributes for the domain based on the elementwise mask. + * + * \param mask_indices: Indexed elements to copy from the source data-block. + * \param domain: Attribute domain to transfer. + * \param skip: Named attributes to ignore/skip. + */ +void copy_attribute_domain(AttributeAccessor src_attributes, + MutableAttributeAccessor dst_attributes, + IndexMask selection, + eAttrDomain domain, + const Set &skip = {}); + bool allow_procedural_attribute_access(StringRef attribute_name); extern const char *no_procedural_access_message; -- cgit v1.2.3 From 8934f00ac5701ea349f2bcccab32e252c79aa730 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Sat, 17 Sep 2022 14:38:30 -0500 Subject: Attributes: Validate some builtin attributes for untrusted inputs We expect some builtin attributes to have positive values or values within a certain range, but currently there some cases where users can set attributes to arbitrary values: the store named attribute node, and the output attributes of the geometry nodes modifier. The set material index node also needs validation. This patch adds an `AttributeValidator` to the attribute API, which can be used to correct values from these untrusted inputs if necessary. As an alternative to D15548, this approach makes it much easier to understand when validation is being applied, without the need to add arguments to every attribute API method or complicate the virtual array system. Currently validation is provided with a multi-function. That integrates well with the field evaluations that set these values now, but it could be wrapped to be friendlier to other areas of Blender in the future. The Python API is not handled here either. Currently I would prefer to wait until we can integrate the C++ and C attribute APIs better before addressing that. Fixes T100952 Differential Revision: https://developer.blender.org/D15990 --- source/blender/blenkernel/BKE_attribute.hh | 35 +++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index fbdacee139c..946a7d21580 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -16,6 +16,10 @@ struct Mesh; struct PointCloud; +namespace blender::fn { +class MultiFunction; +class GField; +} // namespace blender::fn namespace blender::bke { @@ -162,6 +166,27 @@ template struct AttributeReader { } }; +/** + * A utility to make sure attribute values are valid, for attributes like "material_index" which + * can only be positive, or attributes that represent enum options. This is usually only necessary + * when writing attributes from an untrusted/arbitrary user input. + */ +struct AttributeValidator { + /** + * Single input, single output function that corrects attribute values if necessary. + */ + const fn::MultiFunction *function; + + operator bool() const + { + return this->function != nullptr; + } + /** + * Return a field that creates corrected attribute values. + */ + fn::GField validate_field_if_necessary(const fn::GField &field) const; +}; + /** * Result when looking up an attribute from some geometry with read and write access. After writing * to the attribute, the #finish method has to be called. This may invalidate caches based on this @@ -343,7 +368,7 @@ struct AttributeAccessorFunctions { eAttrDomain to_domain); bool (*for_all)(const void *owner, FunctionRef fn); - + AttributeValidator (*lookup_validator)(const void *owner, const AttributeIDRef &attribute_id); GAttributeWriter (*lookup_for_write)(void *owner, const AttributeIDRef &attribute_id); bool (*remove)(void *owner, const AttributeIDRef &attribute_id); bool (*add)(void *owner, @@ -497,6 +522,14 @@ class AttributeAccessor { return VArray::ForSingle(default_value, this->domain_size(domain)); } + /** + * Same as the generic version above, but should be used when the type is known at compile time. + */ + AttributeValidator lookup_validator(const AttributeIDRef &attribute_id) const + { + return fn_->lookup_validator(owner_, attribute_id); + } + /** * Interpolate data from one domain to another. */ -- cgit v1.2.3 From a82e52102b0f7ddfe3741fccdaa9de5f480c59fe Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 21 Sep 2022 12:06:53 -0500 Subject: Mesh: Avoid uninitialized values when converting BMesh to Mesh I didn't observe this issue in practice, but since the write_only version of the attribute API isn't meant to initialize trivial types, theoretically this could be a problem if the attribute is created halfway through converting the BMesh to a Mesh. --- source/blender/blenkernel/BKE_attribute.hh | 2 ++ 1 file changed, 2 insertions(+) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 946a7d21580..b1f4039f788 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -692,6 +692,8 @@ class MutableAttributeAccessor : public AttributeAccessor { * The "only" in the name indicates that the caller should not read existing values from the * span. If the attribute is not stored as span internally, the existing values won't be copied * over to the span. + * + * For trivial types, the values in a newly created attribute will not be initialized. */ GSpanAttributeWriter lookup_or_add_for_write_only_span(const AttributeIDRef &attribute_id, const eAttrDomain domain, -- cgit v1.2.3 From 600c069e0ef5334575164e5c4d758efe7324b0c7 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 21 Sep 2022 12:19:04 -0500 Subject: Attributes: Correct implementation of typed "write_only" method The typed "lookup_or_add_for_write_only" function is meant to do the same thing as the non-typed version of the function. Instead, it still initialized values of new attribute arrays, which isn't meant to happen. Missed in 4c91c24bc7cbe2c4f97be373. I also had to correct one place that used the "write_only" function but didn't intialize all values. --- source/blender/blenkernel/BKE_attribute.hh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index b1f4039f788..97a8a91d0e4 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -706,7 +706,9 @@ class MutableAttributeAccessor : public AttributeAccessor { SpanAttributeWriter lookup_or_add_for_write_only_span(const AttributeIDRef &attribute_id, const eAttrDomain domain) { - AttributeWriter attribute = this->lookup_or_add_for_write(attribute_id, domain); + AttributeWriter attribute = this->lookup_or_add_for_write( + attribute_id, domain, AttributeInitConstruct()); + if (attribute) { return SpanAttributeWriter{std::move(attribute), false}; } -- cgit v1.2.3 From 91dd29fd45d190728f29cc2f6cf9cd5549392f61 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 21 Sep 2022 13:17:05 -0500 Subject: Attributes: Allow calling "finish" on empty accessors This removes some boilerplate when creating many optional attributes. --- source/blender/blenkernel/BKE_attribute.hh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'source/blender/blenkernel/BKE_attribute.hh') diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index 97a8a91d0e4..7b13b8a2b09 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -264,7 +264,9 @@ template struct SpanAttributeWriter { */ void finish() { - this->span.save(); + if (this->span.varray()) { + this->span.save(); + } if (this->tag_modified_fn) { this->tag_modified_fn(); } @@ -339,7 +341,9 @@ struct GSpanAttributeWriter { void finish() { - this->span.save(); + if (this->span.varray()) { + this->span.save(); + } if (this->tag_modified_fn) { this->tag_modified_fn(); } -- cgit v1.2.3