From 8c44793228750537c08ea7b19fc18df0138f9501 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Wed, 23 Mar 2022 12:05:47 +0100 Subject: Implement C++ methods for DNA structures This change makes it possible to add implementation of common C++ methods for DNA structures which helps ensuring unsafe operations like shallow copy are done explicitly. For example, creating a shallow copy used to be: Object temp_object = *input_object; In the C++ context it was seen like the temp_object is properly decoupled from the input object, while in the reality is it not. Now this code becomes: Object temp_object = blender::dna::shallow_copy(*input_object); The copy and move constructor and assignment operators are now explicitly disabled. Other than a more explicit resource management this change also solves a lot of warnings generated by the implicitly defined copy constructors w.r.t dealing with deprecated fields. These warnings were generated by Apple Clang when a shallow object copy was created via implicitly defined copy constructor. In order to enable C++ methods for DNA structures a newly added macro `DNA_DEFINE_CXX_METHODS()` is to be used: tpyedef struct Object { DNA_DEFINE_CXX_METHODS(Object) ... } Object; For the shallow copy use `blender::dna::shallow_copy()`. The implementation of the memcpy is hidden via an internal DNA function to avoid pulling `string.h` into every DNA header. This means that the solution does not affect on the headers dependencies. --- Ideally `DNA_shallow_copy` would be defined in a more explicit header, but don;t think we have a suitable one already. Maybe we can introduce `DNA_access.h` ? Differential Revision: https://developer.blender.org/D14427 --- source/blender/blenkernel/intern/mesh_convert.cc | 6 +- source/blender/blenkernel/intern/object.cc | 2 +- .../depsgraph/intern/depsgraph_query_iter.cc | 2 +- source/blender/editors/object/object_transform.cc | 4 +- .../io/wavefront_obj/exporter/obj_export_mesh.cc | 2 +- source/blender/makesdna/DNA_defs.h | 81 ++++++++++++++++++++++ source/blender/makesdna/DNA_object_types.h | 2 + source/blender/makesdna/intern/dna_utils.c | 12 ++++ source/blender/makesdna/intern/makesdna.c | 12 ++++ 9 files changed, 116 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc index 40c6fbcf67e..1be2d06ce61 100644 --- a/source/blender/blenkernel/intern/mesh_convert.cc +++ b/source/blender/blenkernel/intern/mesh_convert.cc @@ -910,7 +910,8 @@ static void curve_to_mesh_eval_ensure(Object &object) * * So we create temporary copy of the object which will use same data as the original bevel, but * will have no modifiers. */ - Object bevel_object = {{nullptr}}; + Object bevel_object; + memset(&bevel_object, 0, sizeof(bevel_object)); if (curve.bevobj != nullptr) { memcpy(&bevel_object, curve.bevobj, sizeof(bevel_object)); BLI_listbase_clear(&bevel_object.modifiers); @@ -919,7 +920,8 @@ static void curve_to_mesh_eval_ensure(Object &object) } /* Same thing for taper. */ - Object taper_object = {{nullptr}}; + Object taper_object; + memset(&taper_object, 0, sizeof(taper_object)); if (curve.taperobj != nullptr) { memcpy(&taper_object, curve.taperobj, sizeof(taper_object)); BLI_listbase_clear(&taper_object.modifiers); diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc index 1e3b5d77fa7..6d1eac5af49 100644 --- a/source/blender/blenkernel/intern/object.cc +++ b/source/blender/blenkernel/intern/object.cc @@ -3946,7 +3946,7 @@ bool BKE_object_minmax_dupli(Depsgraph *depsgraph, /* pass */ } else { - Object temp_ob = *dob->ob; + Object temp_ob = blender::dna::shallow_copy(*dob->ob); /* Do not modify the original boundbox. */ temp_ob.runtime.bb = nullptr; BKE_object_replace_data_on_shallow_copy(&temp_ob, dob->ob_data); diff --git a/source/blender/depsgraph/intern/depsgraph_query_iter.cc b/source/blender/depsgraph/intern/depsgraph_query_iter.cc index 5788e8efa07..d5566e4be98 100644 --- a/source/blender/depsgraph/intern/depsgraph_query_iter.cc +++ b/source/blender/depsgraph/intern/depsgraph_query_iter.cc @@ -167,7 +167,7 @@ bool deg_iterator_duplis_step(DEGObjectIterData *data) /* Temporary object to evaluate. */ Object *dupli_parent = data->dupli_parent; Object *temp_dupli_object = &data->temp_dupli_object; - *temp_dupli_object = *dob->ob; + *temp_dupli_object = blender::dna::shallow_copy(*dob->ob); temp_dupli_object->base_flag = dupli_parent->base_flag | BASE_FROM_DUPLI; temp_dupli_object->base_local_view_bits = dupli_parent->base_local_view_bits; temp_dupli_object->runtime.local_collections_bits = diff --git a/source/blender/editors/object/object_transform.cc b/source/blender/editors/object/object_transform.cc index 24425b5a991..afd2c048379 100644 --- a/source/blender/editors/object/object_transform.cc +++ b/source/blender/editors/object/object_transform.cc @@ -1695,13 +1695,13 @@ static void object_apply_rotation(Object *ob, const float rmat[3][3]) static void object_apply_location(Object *ob, const float loc[3]) { /* quick but weak */ - Object ob_prev = *ob; + Object ob_prev = blender::dna::shallow_copy(*ob); float mat[4][4]; copy_m4_m4(mat, ob->obmat); copy_v3_v3(mat[3], loc); BKE_object_apply_mat4(ob, mat, true, true); copy_v3_v3(mat[3], ob->loc); - *ob = ob_prev; + *ob = blender::dna::shallow_copy(ob_prev); copy_v3_v3(ob->loc, mat[3]); } diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc index aa63e65b1e8..8c9b04a5ac3 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc +++ b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc @@ -33,7 +33,7 @@ OBJMesh::OBJMesh(Depsgraph *depsgraph, const OBJExportParams &export_params, Obj { /* We need to copy the object because it may be in temporary space. */ Object *obj_eval = DEG_get_evaluated_object(depsgraph, mesh_object); - export_object_eval_ = *obj_eval; + export_object_eval_ = dna::shallow_copy(*obj_eval); export_mesh_eval_ = export_params.apply_modifiers ? BKE_object_get_evaluated_mesh(&export_object_eval_) : BKE_object_get_pre_modified_mesh(&export_object_eval_); diff --git a/source/blender/makesdna/DNA_defs.h b/source/blender/makesdna/DNA_defs.h index b4230209dd5..283eacf1a16 100644 --- a/source/blender/makesdna/DNA_defs.h +++ b/source/blender/makesdna/DNA_defs.h @@ -46,3 +46,84 @@ /* non-id name variables should use this length */ #define MAX_NAME 64 + +/* #DNA_DEFINE_CXX_METHODS is used to define C++ methods which are needed for proper/safe resource + * management, making unsafe (from an ownership perspective: i.e. pointers which sometimes needs to + * be set to nullptr on copy, sometimes needs to be dupalloc-ed) operations explicit, and taking + * care of compiler specific warnings when dealing with members marked with DNA_DEPRECATED. + * + * The `class_name` argument is to match the structure name the macro is used from. + * + * Typical usage example: + * + * typedef struct Object { + * DNA_DEFINE_CXX_METHODS(Object) + * } Object; + */ +#ifndef __cplusplus +# define DNA_DEFINE_CXX_METHODS(class_name) +#else + +/* Forward-declared here since there is no simple header file to be pulled for this functionality. + * Avoids pulling `string.h` from this header to get access to #memcpy. */ +extern "C" void _DNA_internal_memcpy(void *dst, const void *src, size_t size); + +namespace blender::dna::internal { + +template class ShallowDataConstRef { + public: + constexpr explicit ShallowDataConstRef(const T &ref) : ref_(ref) + { + } + + inline const T *get_pointer() const + { + return &ref_; + } + + private: + const T &ref_; +}; + +} // namespace blender::dna::internal + +# define DNA_DEFINE_CXX_METHODS(class_name) \ + class_name() = default; \ + ~class_name() = default; \ + /* Delete copy and assignment, which are not safe for resource ownership. */ \ + class_name(const class_name &other) = delete; \ + class_name(class_name &&other) noexcept = delete; \ + class_name &operator=(const class_name &other) = delete; \ + class_name &operator=(class_name &&other) = delete; \ + /* Support for shallow copy. */ \ + class_name(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + class_name &operator=(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + if (this != ref.get_pointer()) { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + return *this; \ + } + +namespace blender::dna { + +/* Creates shallow copy of the given object. + * The entire object is copied as-is using memory copy. + * + * Typical usage: + * Object temp_object = blender::dna::shallow_copy(*input_object); + * + * From the implementation detail go via copy constructor/assign operator defined in the structure. + */ +template +[[nodiscard]] inline internal::ShallowDataConstRef shallow_copy(const T &other) +{ + return internal::ShallowDataConstRef(other); +} + +} // namespace blender::dna + +#endif diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h index 9e0bf7dcc5a..c3708e25ee7 100644 --- a/source/blender/makesdna/DNA_object_types.h +++ b/source/blender/makesdna/DNA_object_types.h @@ -233,6 +233,8 @@ enum eObjectLineArt_Flags { }; typedef struct Object { + DNA_DEFINE_CXX_METHODS(Object) + ID id; /** Animation data (must be immediately after id for utilities to use it). */ struct AnimData *adt; diff --git a/source/blender/makesdna/intern/dna_utils.c b/source/blender/makesdna/intern/dna_utils.c index 8c86ef69ebd..560b91b925e 100644 --- a/source/blender/makesdna/intern/dna_utils.c +++ b/source/blender/makesdna/intern/dna_utils.c @@ -308,3 +308,15 @@ const char *DNA_struct_rename_legacy_hack_alias_from_static(const char *name) } /** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Internal helpers for C++ + * \{ */ + +void _DNA_internal_memcpy(void *dst, const void *src, size_t size); +void _DNA_internal_memcpy(void *dst, const void *src, const size_t size) +{ + memcpy(dst, src, size); +} + +/** \} */ diff --git a/source/blender/makesdna/intern/makesdna.c b/source/blender/makesdna/intern/makesdna.c index 0d2f265a9b5..12ec7262906 100644 --- a/source/blender/makesdna/intern/makesdna.c +++ b/source/blender/makesdna/intern/makesdna.c @@ -620,6 +620,7 @@ static int preprocess_include(char *maindata, const int maindata_len) int newlen = 0; comment = 0; a = maindata_len; + bool skip_until_closing_brace = false; while (a--) { if (cp[0] == '/' && cp[1] == '*') { @@ -646,6 +647,17 @@ static int preprocess_include(char *maindata, const int maindata_len) a -= 13; cp += 13; } + else if (match_identifier(cp, "DNA_DEFINE_CXX_METHODS")) { + /* single values are skipped already, so decrement 1 less */ + a -= 21; + cp += 21; + skip_until_closing_brace = true; + } + else if (skip_until_closing_brace) { + if (cp[0] == ')') { + skip_until_closing_brace = false; + } + } else { md[0] = cp[0]; md++; -- cgit v1.2.3 From 484af996aa75750c8b419a25b68c9eb8d7efcf19 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Fri, 25 Mar 2022 10:57:13 +0100 Subject: Revert "Implement C++ methods for DNA structures" This reverts commit 8c44793228750537c08ea7b19fc18df0138f9501. Apparently, this generated a lot of warnings in GCC. Didn't find a quick solution and is it not something I want to be trading between (more quiet Clang in an expense of less quiet GCC). Will re-iterate on the patch are re-commit it. --- source/blender/blenkernel/intern/mesh_convert.cc | 6 +- source/blender/blenkernel/intern/object.cc | 2 +- .../depsgraph/intern/depsgraph_query_iter.cc | 2 +- source/blender/editors/object/object_transform.cc | 4 +- .../io/wavefront_obj/exporter/obj_export_mesh.cc | 2 +- source/blender/makesdna/DNA_defs.h | 81 ---------------------- source/blender/makesdna/DNA_object_types.h | 2 - source/blender/makesdna/intern/dna_utils.c | 12 ---- source/blender/makesdna/intern/makesdna.c | 12 ---- 9 files changed, 7 insertions(+), 116 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc index 1be2d06ce61..40c6fbcf67e 100644 --- a/source/blender/blenkernel/intern/mesh_convert.cc +++ b/source/blender/blenkernel/intern/mesh_convert.cc @@ -910,8 +910,7 @@ static void curve_to_mesh_eval_ensure(Object &object) * * So we create temporary copy of the object which will use same data as the original bevel, but * will have no modifiers. */ - Object bevel_object; - memset(&bevel_object, 0, sizeof(bevel_object)); + Object bevel_object = {{nullptr}}; if (curve.bevobj != nullptr) { memcpy(&bevel_object, curve.bevobj, sizeof(bevel_object)); BLI_listbase_clear(&bevel_object.modifiers); @@ -920,8 +919,7 @@ static void curve_to_mesh_eval_ensure(Object &object) } /* Same thing for taper. */ - Object taper_object; - memset(&taper_object, 0, sizeof(taper_object)); + Object taper_object = {{nullptr}}; if (curve.taperobj != nullptr) { memcpy(&taper_object, curve.taperobj, sizeof(taper_object)); BLI_listbase_clear(&taper_object.modifiers); diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc index 6d1eac5af49..1e3b5d77fa7 100644 --- a/source/blender/blenkernel/intern/object.cc +++ b/source/blender/blenkernel/intern/object.cc @@ -3946,7 +3946,7 @@ bool BKE_object_minmax_dupli(Depsgraph *depsgraph, /* pass */ } else { - Object temp_ob = blender::dna::shallow_copy(*dob->ob); + Object temp_ob = *dob->ob; /* Do not modify the original boundbox. */ temp_ob.runtime.bb = nullptr; BKE_object_replace_data_on_shallow_copy(&temp_ob, dob->ob_data); diff --git a/source/blender/depsgraph/intern/depsgraph_query_iter.cc b/source/blender/depsgraph/intern/depsgraph_query_iter.cc index d5566e4be98..5788e8efa07 100644 --- a/source/blender/depsgraph/intern/depsgraph_query_iter.cc +++ b/source/blender/depsgraph/intern/depsgraph_query_iter.cc @@ -167,7 +167,7 @@ bool deg_iterator_duplis_step(DEGObjectIterData *data) /* Temporary object to evaluate. */ Object *dupli_parent = data->dupli_parent; Object *temp_dupli_object = &data->temp_dupli_object; - *temp_dupli_object = blender::dna::shallow_copy(*dob->ob); + *temp_dupli_object = *dob->ob; temp_dupli_object->base_flag = dupli_parent->base_flag | BASE_FROM_DUPLI; temp_dupli_object->base_local_view_bits = dupli_parent->base_local_view_bits; temp_dupli_object->runtime.local_collections_bits = diff --git a/source/blender/editors/object/object_transform.cc b/source/blender/editors/object/object_transform.cc index afd2c048379..24425b5a991 100644 --- a/source/blender/editors/object/object_transform.cc +++ b/source/blender/editors/object/object_transform.cc @@ -1695,13 +1695,13 @@ static void object_apply_rotation(Object *ob, const float rmat[3][3]) static void object_apply_location(Object *ob, const float loc[3]) { /* quick but weak */ - Object ob_prev = blender::dna::shallow_copy(*ob); + Object ob_prev = *ob; float mat[4][4]; copy_m4_m4(mat, ob->obmat); copy_v3_v3(mat[3], loc); BKE_object_apply_mat4(ob, mat, true, true); copy_v3_v3(mat[3], ob->loc); - *ob = blender::dna::shallow_copy(ob_prev); + *ob = ob_prev; copy_v3_v3(ob->loc, mat[3]); } diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc index 8c9b04a5ac3..aa63e65b1e8 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc +++ b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc @@ -33,7 +33,7 @@ OBJMesh::OBJMesh(Depsgraph *depsgraph, const OBJExportParams &export_params, Obj { /* We need to copy the object because it may be in temporary space. */ Object *obj_eval = DEG_get_evaluated_object(depsgraph, mesh_object); - export_object_eval_ = dna::shallow_copy(*obj_eval); + export_object_eval_ = *obj_eval; export_mesh_eval_ = export_params.apply_modifiers ? BKE_object_get_evaluated_mesh(&export_object_eval_) : BKE_object_get_pre_modified_mesh(&export_object_eval_); diff --git a/source/blender/makesdna/DNA_defs.h b/source/blender/makesdna/DNA_defs.h index 283eacf1a16..b4230209dd5 100644 --- a/source/blender/makesdna/DNA_defs.h +++ b/source/blender/makesdna/DNA_defs.h @@ -46,84 +46,3 @@ /* non-id name variables should use this length */ #define MAX_NAME 64 - -/* #DNA_DEFINE_CXX_METHODS is used to define C++ methods which are needed for proper/safe resource - * management, making unsafe (from an ownership perspective: i.e. pointers which sometimes needs to - * be set to nullptr on copy, sometimes needs to be dupalloc-ed) operations explicit, and taking - * care of compiler specific warnings when dealing with members marked with DNA_DEPRECATED. - * - * The `class_name` argument is to match the structure name the macro is used from. - * - * Typical usage example: - * - * typedef struct Object { - * DNA_DEFINE_CXX_METHODS(Object) - * } Object; - */ -#ifndef __cplusplus -# define DNA_DEFINE_CXX_METHODS(class_name) -#else - -/* Forward-declared here since there is no simple header file to be pulled for this functionality. - * Avoids pulling `string.h` from this header to get access to #memcpy. */ -extern "C" void _DNA_internal_memcpy(void *dst, const void *src, size_t size); - -namespace blender::dna::internal { - -template class ShallowDataConstRef { - public: - constexpr explicit ShallowDataConstRef(const T &ref) : ref_(ref) - { - } - - inline const T *get_pointer() const - { - return &ref_; - } - - private: - const T &ref_; -}; - -} // namespace blender::dna::internal - -# define DNA_DEFINE_CXX_METHODS(class_name) \ - class_name() = default; \ - ~class_name() = default; \ - /* Delete copy and assignment, which are not safe for resource ownership. */ \ - class_name(const class_name &other) = delete; \ - class_name(class_name &&other) noexcept = delete; \ - class_name &operator=(const class_name &other) = delete; \ - class_name &operator=(class_name &&other) = delete; \ - /* Support for shallow copy. */ \ - class_name(const blender::dna::internal::ShallowDataConstRef ref) \ - { \ - _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ - } \ - class_name &operator=(const blender::dna::internal::ShallowDataConstRef ref) \ - { \ - if (this != ref.get_pointer()) { \ - _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ - } \ - return *this; \ - } - -namespace blender::dna { - -/* Creates shallow copy of the given object. - * The entire object is copied as-is using memory copy. - * - * Typical usage: - * Object temp_object = blender::dna::shallow_copy(*input_object); - * - * From the implementation detail go via copy constructor/assign operator defined in the structure. - */ -template -[[nodiscard]] inline internal::ShallowDataConstRef shallow_copy(const T &other) -{ - return internal::ShallowDataConstRef(other); -} - -} // namespace blender::dna - -#endif diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h index c3708e25ee7..9e0bf7dcc5a 100644 --- a/source/blender/makesdna/DNA_object_types.h +++ b/source/blender/makesdna/DNA_object_types.h @@ -233,8 +233,6 @@ enum eObjectLineArt_Flags { }; typedef struct Object { - DNA_DEFINE_CXX_METHODS(Object) - ID id; /** Animation data (must be immediately after id for utilities to use it). */ struct AnimData *adt; diff --git a/source/blender/makesdna/intern/dna_utils.c b/source/blender/makesdna/intern/dna_utils.c index 560b91b925e..8c86ef69ebd 100644 --- a/source/blender/makesdna/intern/dna_utils.c +++ b/source/blender/makesdna/intern/dna_utils.c @@ -308,15 +308,3 @@ const char *DNA_struct_rename_legacy_hack_alias_from_static(const char *name) } /** \} */ - -/* -------------------------------------------------------------------- */ -/** \name Internal helpers for C++ - * \{ */ - -void _DNA_internal_memcpy(void *dst, const void *src, size_t size); -void _DNA_internal_memcpy(void *dst, const void *src, const size_t size) -{ - memcpy(dst, src, size); -} - -/** \} */ diff --git a/source/blender/makesdna/intern/makesdna.c b/source/blender/makesdna/intern/makesdna.c index 12ec7262906..0d2f265a9b5 100644 --- a/source/blender/makesdna/intern/makesdna.c +++ b/source/blender/makesdna/intern/makesdna.c @@ -620,7 +620,6 @@ static int preprocess_include(char *maindata, const int maindata_len) int newlen = 0; comment = 0; a = maindata_len; - bool skip_until_closing_brace = false; while (a--) { if (cp[0] == '/' && cp[1] == '*') { @@ -647,17 +646,6 @@ static int preprocess_include(char *maindata, const int maindata_len) a -= 13; cp += 13; } - else if (match_identifier(cp, "DNA_DEFINE_CXX_METHODS")) { - /* single values are skipped already, so decrement 1 less */ - a -= 21; - cp += 21; - skip_until_closing_brace = true; - } - else if (skip_until_closing_brace) { - if (cp[0] == ')') { - skip_until_closing_brace = false; - } - } else { md[0] = cp[0]; md++; -- cgit v1.2.3 From 75b8c4fc18997851620f4abd491f7c418f6666ee Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Fri, 25 Mar 2022 11:38:38 +0100 Subject: LibOverride: Prevent some more potential modification of overridden collections. --- source/blender/blenkernel/intern/collection.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/collection.c b/source/blender/blenkernel/intern/collection.c index 891145b47c9..bdaea487cfb 100644 --- a/source/blender/blenkernel/intern/collection.c +++ b/source/blender/blenkernel/intern/collection.c @@ -432,7 +432,8 @@ void BKE_collection_add_from_object(Main *bmain, bool is_instantiated = false; FOREACH_SCENE_COLLECTION_BEGIN (scene, collection) { - if (!ID_IS_LINKED(collection) && BKE_collection_has_object(collection, ob_src)) { + if (!ID_IS_LINKED(collection) && !ID_IS_OVERRIDABLE_LIBRARY(collection) && + BKE_collection_has_object(collection, ob_src)) { collection_child_add(collection, collection_dst, 0, true); is_instantiated = true; } @@ -454,7 +455,8 @@ void BKE_collection_add_from_collection(Main *bmain, bool is_instantiated = false; FOREACH_SCENE_COLLECTION_BEGIN (scene, collection) { - if (!ID_IS_LINKED(collection) && collection_find_child(collection, collection_src)) { + if (!ID_IS_LINKED(collection) && !ID_IS_OVERRIDABLE_LIBRARY(collection) && + collection_find_child(collection, collection_src)) { collection_child_add(collection, collection_dst, 0, true); is_instantiated = true; } -- cgit v1.2.3 From 03df72ee4e7e7f9893df73de426cdc3af1c7a676 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Wed, 23 Mar 2022 12:05:47 +0100 Subject: Implement C++ methods for DNA structures This change makes it possible to add implementation of common C++ methods for DNA structures which helps ensuring unsafe operations like shallow copy are done explicitly. For example, creating a shallow copy used to be: Object temp_object = *input_object; In the C++ context it was seen like the temp_object is properly decoupled from the input object, while in the reality is it not. Now this code becomes: Object temp_object = blender::dna::shallow_copy(*input_object); The copy and move constructor and assignment operators are now explicitly disabled. Other than a more explicit resource management this change also solves a lot of warnings generated by the implicitly defined copy constructors w.r.t dealing with deprecated fields. These warnings were generated by Apple Clang when a shallow object copy was created via implicitly defined copy constructor. In order to enable C++ methods for DNA structures a newly added macro `DNA_DEFINE_CXX_METHODS()` is to be used: tpyedef struct Object { DNA_DEFINE_CXX_METHODS(Object) ... } Object; For the shallow copy use `blender::dna::shallow_copy()`. The implementation of the memcpy is hidden via an internal DNA function to avoid pulling `string.h` into every DNA header. This means that the solution does not affect on the headers dependencies. --- Ideally `DNA_shallow_copy` would be defined in a more explicit header, but don;t think we have a suitable one already. Maybe we can introduce `DNA_access.h` ? Differential Revision: https://developer.blender.org/D14427 --- source/blender/blenkernel/intern/mesh_convert.cc | 6 +- source/blender/blenkernel/intern/object.cc | 2 +- .../depsgraph/intern/depsgraph_query_iter.cc | 2 +- source/blender/editors/object/object_transform.cc | 4 +- .../io/wavefront_obj/exporter/obj_export_mesh.cc | 2 +- source/blender/makesdna/DNA_defs.h | 81 ++++++++++++++++++++++ source/blender/makesdna/DNA_object_types.h | 2 + source/blender/makesdna/intern/dna_utils.c | 12 ++++ source/blender/makesdna/intern/makesdna.c | 12 ++++ 9 files changed, 116 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc index 40c6fbcf67e..1be2d06ce61 100644 --- a/source/blender/blenkernel/intern/mesh_convert.cc +++ b/source/blender/blenkernel/intern/mesh_convert.cc @@ -910,7 +910,8 @@ static void curve_to_mesh_eval_ensure(Object &object) * * So we create temporary copy of the object which will use same data as the original bevel, but * will have no modifiers. */ - Object bevel_object = {{nullptr}}; + Object bevel_object; + memset(&bevel_object, 0, sizeof(bevel_object)); if (curve.bevobj != nullptr) { memcpy(&bevel_object, curve.bevobj, sizeof(bevel_object)); BLI_listbase_clear(&bevel_object.modifiers); @@ -919,7 +920,8 @@ static void curve_to_mesh_eval_ensure(Object &object) } /* Same thing for taper. */ - Object taper_object = {{nullptr}}; + Object taper_object; + memset(&taper_object, 0, sizeof(taper_object)); if (curve.taperobj != nullptr) { memcpy(&taper_object, curve.taperobj, sizeof(taper_object)); BLI_listbase_clear(&taper_object.modifiers); diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc index 1e3b5d77fa7..6d1eac5af49 100644 --- a/source/blender/blenkernel/intern/object.cc +++ b/source/blender/blenkernel/intern/object.cc @@ -3946,7 +3946,7 @@ bool BKE_object_minmax_dupli(Depsgraph *depsgraph, /* pass */ } else { - Object temp_ob = *dob->ob; + Object temp_ob = blender::dna::shallow_copy(*dob->ob); /* Do not modify the original boundbox. */ temp_ob.runtime.bb = nullptr; BKE_object_replace_data_on_shallow_copy(&temp_ob, dob->ob_data); diff --git a/source/blender/depsgraph/intern/depsgraph_query_iter.cc b/source/blender/depsgraph/intern/depsgraph_query_iter.cc index 5788e8efa07..d5566e4be98 100644 --- a/source/blender/depsgraph/intern/depsgraph_query_iter.cc +++ b/source/blender/depsgraph/intern/depsgraph_query_iter.cc @@ -167,7 +167,7 @@ bool deg_iterator_duplis_step(DEGObjectIterData *data) /* Temporary object to evaluate. */ Object *dupli_parent = data->dupli_parent; Object *temp_dupli_object = &data->temp_dupli_object; - *temp_dupli_object = *dob->ob; + *temp_dupli_object = blender::dna::shallow_copy(*dob->ob); temp_dupli_object->base_flag = dupli_parent->base_flag | BASE_FROM_DUPLI; temp_dupli_object->base_local_view_bits = dupli_parent->base_local_view_bits; temp_dupli_object->runtime.local_collections_bits = diff --git a/source/blender/editors/object/object_transform.cc b/source/blender/editors/object/object_transform.cc index 24425b5a991..afd2c048379 100644 --- a/source/blender/editors/object/object_transform.cc +++ b/source/blender/editors/object/object_transform.cc @@ -1695,13 +1695,13 @@ static void object_apply_rotation(Object *ob, const float rmat[3][3]) static void object_apply_location(Object *ob, const float loc[3]) { /* quick but weak */ - Object ob_prev = *ob; + Object ob_prev = blender::dna::shallow_copy(*ob); float mat[4][4]; copy_m4_m4(mat, ob->obmat); copy_v3_v3(mat[3], loc); BKE_object_apply_mat4(ob, mat, true, true); copy_v3_v3(mat[3], ob->loc); - *ob = ob_prev; + *ob = blender::dna::shallow_copy(ob_prev); copy_v3_v3(ob->loc, mat[3]); } diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc index aa63e65b1e8..8c9b04a5ac3 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc +++ b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc @@ -33,7 +33,7 @@ OBJMesh::OBJMesh(Depsgraph *depsgraph, const OBJExportParams &export_params, Obj { /* We need to copy the object because it may be in temporary space. */ Object *obj_eval = DEG_get_evaluated_object(depsgraph, mesh_object); - export_object_eval_ = *obj_eval; + export_object_eval_ = dna::shallow_copy(*obj_eval); export_mesh_eval_ = export_params.apply_modifiers ? BKE_object_get_evaluated_mesh(&export_object_eval_) : BKE_object_get_pre_modified_mesh(&export_object_eval_); diff --git a/source/blender/makesdna/DNA_defs.h b/source/blender/makesdna/DNA_defs.h index b4230209dd5..283eacf1a16 100644 --- a/source/blender/makesdna/DNA_defs.h +++ b/source/blender/makesdna/DNA_defs.h @@ -46,3 +46,84 @@ /* non-id name variables should use this length */ #define MAX_NAME 64 + +/* #DNA_DEFINE_CXX_METHODS is used to define C++ methods which are needed for proper/safe resource + * management, making unsafe (from an ownership perspective: i.e. pointers which sometimes needs to + * be set to nullptr on copy, sometimes needs to be dupalloc-ed) operations explicit, and taking + * care of compiler specific warnings when dealing with members marked with DNA_DEPRECATED. + * + * The `class_name` argument is to match the structure name the macro is used from. + * + * Typical usage example: + * + * typedef struct Object { + * DNA_DEFINE_CXX_METHODS(Object) + * } Object; + */ +#ifndef __cplusplus +# define DNA_DEFINE_CXX_METHODS(class_name) +#else + +/* Forward-declared here since there is no simple header file to be pulled for this functionality. + * Avoids pulling `string.h` from this header to get access to #memcpy. */ +extern "C" void _DNA_internal_memcpy(void *dst, const void *src, size_t size); + +namespace blender::dna::internal { + +template class ShallowDataConstRef { + public: + constexpr explicit ShallowDataConstRef(const T &ref) : ref_(ref) + { + } + + inline const T *get_pointer() const + { + return &ref_; + } + + private: + const T &ref_; +}; + +} // namespace blender::dna::internal + +# define DNA_DEFINE_CXX_METHODS(class_name) \ + class_name() = default; \ + ~class_name() = default; \ + /* Delete copy and assignment, which are not safe for resource ownership. */ \ + class_name(const class_name &other) = delete; \ + class_name(class_name &&other) noexcept = delete; \ + class_name &operator=(const class_name &other) = delete; \ + class_name &operator=(class_name &&other) = delete; \ + /* Support for shallow copy. */ \ + class_name(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + class_name &operator=(const blender::dna::internal::ShallowDataConstRef ref) \ + { \ + if (this != ref.get_pointer()) { \ + _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ + } \ + return *this; \ + } + +namespace blender::dna { + +/* Creates shallow copy of the given object. + * The entire object is copied as-is using memory copy. + * + * Typical usage: + * Object temp_object = blender::dna::shallow_copy(*input_object); + * + * From the implementation detail go via copy constructor/assign operator defined in the structure. + */ +template +[[nodiscard]] inline internal::ShallowDataConstRef shallow_copy(const T &other) +{ + return internal::ShallowDataConstRef(other); +} + +} // namespace blender::dna + +#endif diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h index 9e0bf7dcc5a..c3708e25ee7 100644 --- a/source/blender/makesdna/DNA_object_types.h +++ b/source/blender/makesdna/DNA_object_types.h @@ -233,6 +233,8 @@ enum eObjectLineArt_Flags { }; typedef struct Object { + DNA_DEFINE_CXX_METHODS(Object) + ID id; /** Animation data (must be immediately after id for utilities to use it). */ struct AnimData *adt; diff --git a/source/blender/makesdna/intern/dna_utils.c b/source/blender/makesdna/intern/dna_utils.c index 8c86ef69ebd..560b91b925e 100644 --- a/source/blender/makesdna/intern/dna_utils.c +++ b/source/blender/makesdna/intern/dna_utils.c @@ -308,3 +308,15 @@ const char *DNA_struct_rename_legacy_hack_alias_from_static(const char *name) } /** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Internal helpers for C++ + * \{ */ + +void _DNA_internal_memcpy(void *dst, const void *src, size_t size); +void _DNA_internal_memcpy(void *dst, const void *src, const size_t size) +{ + memcpy(dst, src, size); +} + +/** \} */ diff --git a/source/blender/makesdna/intern/makesdna.c b/source/blender/makesdna/intern/makesdna.c index 0d2f265a9b5..12ec7262906 100644 --- a/source/blender/makesdna/intern/makesdna.c +++ b/source/blender/makesdna/intern/makesdna.c @@ -620,6 +620,7 @@ static int preprocess_include(char *maindata, const int maindata_len) int newlen = 0; comment = 0; a = maindata_len; + bool skip_until_closing_brace = false; while (a--) { if (cp[0] == '/' && cp[1] == '*') { @@ -646,6 +647,17 @@ static int preprocess_include(char *maindata, const int maindata_len) a -= 13; cp += 13; } + else if (match_identifier(cp, "DNA_DEFINE_CXX_METHODS")) { + /* single values are skipped already, so decrement 1 less */ + a -= 21; + cp += 21; + skip_until_closing_brace = true; + } + else if (skip_until_closing_brace) { + if (cp[0] == ')') { + skip_until_closing_brace = false; + } + } else { md[0] = cp[0]; md++; -- cgit v1.2.3 From 0c33e84020deca84c987dffa1302651f59c27158 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Fri, 25 Mar 2022 11:36:08 +0100 Subject: Fix compilation warnings after previous change Thanks Jacques for finding solution for deprecation warning which was generated by GCC for constructor. The rest of the change is related on fixing memaccess warning which was happening when memset/memcpy was used directly on the DNA object pointer. Now there are two utility functions for this: - blender::dna::zero_memory - blender::dna::copy_memory --- source/blender/blenkernel/intern/mesh_convert.cc | 10 +++++----- source/blender/blenkernel/intern/object.cc | 2 +- source/blender/makesdna/DNA_defs.h | 18 +++++++++++++++++- source/blender/makesdna/intern/dna_utils.c | 6 ++++++ 4 files changed, 29 insertions(+), 7 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/mesh_convert.cc b/source/blender/blenkernel/intern/mesh_convert.cc index 1be2d06ce61..fc484e73967 100644 --- a/source/blender/blenkernel/intern/mesh_convert.cc +++ b/source/blender/blenkernel/intern/mesh_convert.cc @@ -911,9 +911,9 @@ static void curve_to_mesh_eval_ensure(Object &object) * So we create temporary copy of the object which will use same data as the original bevel, but * will have no modifiers. */ Object bevel_object; - memset(&bevel_object, 0, sizeof(bevel_object)); + blender::dna::zero_memory(bevel_object); if (curve.bevobj != nullptr) { - memcpy(&bevel_object, curve.bevobj, sizeof(bevel_object)); + blender::dna::copy_memory(bevel_object, *curve.bevobj); BLI_listbase_clear(&bevel_object.modifiers); BKE_object_runtime_reset(&bevel_object); curve.bevobj = &bevel_object; @@ -921,9 +921,9 @@ static void curve_to_mesh_eval_ensure(Object &object) /* Same thing for taper. */ Object taper_object; - memset(&taper_object, 0, sizeof(taper_object)); + blender::dna::zero_memory(taper_object); if (curve.taperobj != nullptr) { - memcpy(&taper_object, curve.taperobj, sizeof(taper_object)); + blender::dna::copy_memory(taper_object, *curve.taperobj); BLI_listbase_clear(&taper_object.modifiers); BKE_object_runtime_reset(&taper_object); curve.taperobj = &taper_object; @@ -1068,7 +1068,7 @@ static Mesh *mesh_new_from_mesh_object_with_layers(Depsgraph *depsgraph, } Object object_for_eval; - memcpy(&object_for_eval, object, sizeof(object_for_eval)); + blender::dna::zero_memory(object_for_eval); if (object_for_eval.runtime.data_orig != nullptr) { object_for_eval.data = object_for_eval.runtime.data_orig; } diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc index 6d1eac5af49..098b7c52664 100644 --- a/source/blender/blenkernel/intern/object.cc +++ b/source/blender/blenkernel/intern/object.cc @@ -1236,7 +1236,7 @@ IDTypeInfo IDType_ID_OB = { void BKE_object_workob_clear(Object *workob) { - memset(workob, 0, sizeof(Object)); + blender::dna::zero_memory(*workob); workob->scale[0] = workob->scale[1] = workob->scale[2] = 1.0f; workob->dscale[0] = workob->dscale[1] = workob->dscale[2] = 1.0f; diff --git a/source/blender/makesdna/DNA_defs.h b/source/blender/makesdna/DNA_defs.h index 283eacf1a16..bfeb809b369 100644 --- a/source/blender/makesdna/DNA_defs.h +++ b/source/blender/makesdna/DNA_defs.h @@ -67,6 +67,7 @@ /* Forward-declared here since there is no simple header file to be pulled for this functionality. * Avoids pulling `string.h` from this header to get access to #memcpy. */ extern "C" void _DNA_internal_memcpy(void *dst, const void *src, size_t size); +extern "C" void _DNA_internal_memzero(void *dst, size_t size); namespace blender::dna::internal { @@ -96,7 +97,8 @@ template class ShallowDataConstRef { class_name &operator=(const class_name &other) = delete; \ class_name &operator=(class_name &&other) = delete; \ /* Support for shallow copy. */ \ - class_name(const blender::dna::internal::ShallowDataConstRef ref) \ + /* NOTE: Calling the default constructor works-around deprecated warning generated by GCC. */ \ + class_name(const blender::dna::internal::ShallowDataConstRef ref) : class_name() \ { \ _DNA_internal_memcpy(this, ref.get_pointer(), sizeof(class_name)); \ } \ @@ -124,6 +126,20 @@ template return internal::ShallowDataConstRef(other); } +/* Fill underlying memory used by DNA object with zeroes. */ +template inline void zero_memory(T &object) +{ + /* TODO(sergey): Consider adding static assert for T being a trivial type. */ + _DNA_internal_memzero(&object, sizeof(T)); +} + +/* Copy memory from one DNA object to another. */ +template inline void copy_memory(T &dst, const T &src) +{ + /* TODO(sergey): Consider adding static assert for T being a trivial type. */ + _DNA_internal_memcpy(&dst, &src, sizeof(T)); +} + } // namespace blender::dna #endif diff --git a/source/blender/makesdna/intern/dna_utils.c b/source/blender/makesdna/intern/dna_utils.c index 560b91b925e..bc2584fe57a 100644 --- a/source/blender/makesdna/intern/dna_utils.c +++ b/source/blender/makesdna/intern/dna_utils.c @@ -319,4 +319,10 @@ void _DNA_internal_memcpy(void *dst, const void *src, const size_t size) memcpy(dst, src, size); } +void _DNA_internal_memzero(void *dst, size_t size); +void _DNA_internal_memzero(void *dst, const size_t size) +{ + memset(dst, 0, size); +} + /** \} */ -- cgit v1.2.3 From d4e1458db3a0e0eaf80219dc8e6d10cb27620793 Mon Sep 17 00:00:00 2001 From: Henrik Dick Date: Fri, 25 Mar 2022 11:51:45 +0100 Subject: GPencil: Improve smooth operation This patch makes the grease pencil smooth operation symmetric. It also increases the performance a lot if strong smoothing is required. Additionally there is an option for the position smooth operation to keep the shape closer to the original for more iterations. Since the result differs from the previous algorithm, versioning is used to change the iterations and factor to match the old result. Differential Revision: http://developer.blender.org/D14325 --- source/blender/blenkernel/BKE_gpencil_geom.h | 58 ++- source/blender/blenkernel/intern/gpencil_geom.cc | 467 ++++++++++++--------- source/blender/blenloader/intern/versioning_300.c | 18 + source/blender/editors/gpencil/gpencil_edit.c | 31 +- source/blender/editors/gpencil/gpencil_fill.c | 11 +- .../blender/editors/gpencil/gpencil_interpolate.c | 30 +- source/blender/editors/gpencil/gpencil_paint.c | 26 +- .../blender/editors/gpencil/gpencil_sculpt_paint.c | 8 +- .../intern/MOD_gpencilshrinkwrap.c | 10 +- .../gpencil_modifiers/intern/MOD_gpencilsmooth.c | 77 ++-- .../makesdna/DNA_gpencil_modifier_defaults.h | 2 +- .../blender/makesdna/DNA_gpencil_modifier_types.h | 1 + source/blender/makesrna/intern/rna_brush.c | 5 +- .../blender/makesrna/intern/rna_gpencil_modifier.c | 7 +- 14 files changed, 442 insertions(+), 309 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/BKE_gpencil_geom.h b/source/blender/blenkernel/BKE_gpencil_geom.h index 4127030e96f..ad3b1971ca9 100644 --- a/source/blender/blenkernel/BKE_gpencil_geom.h +++ b/source/blender/blenkernel/BKE_gpencil_geom.h @@ -220,30 +220,78 @@ bool BKE_gpencil_stroke_sample(struct bGPdata *gpd, * \param gps: Stroke to smooth * \param i: Point index * \param inf: Amount of smoothing to apply + * \param iterations: Radius of points to consider, equivalent to iterations * \param smooth_caps: Apply smooth to stroke extremes + * \param keep_shape: Smooth out fine details first + * \param r_gps: Stroke to put the result into */ -bool BKE_gpencil_stroke_smooth_point(struct bGPDstroke *gps, int i, float inf, bool smooth_caps); +bool BKE_gpencil_stroke_smooth_point(struct bGPDstroke *gps, + int point_index, + float influence, + int iterations, + bool smooth_caps, + bool keep_shape, + struct bGPDstroke *r_gps); /** * Apply smooth strength to stroke point. * \param gps: Stroke to smooth * \param point_index: Point index * \param influence: Amount of smoothing to apply + * \param iterations: Radius of points to consider, equivalent to iterations + * \param r_gps: Stroke to put the result into */ -bool BKE_gpencil_stroke_smooth_strength(struct bGPDstroke *gps, int point_index, float influence); +bool BKE_gpencil_stroke_smooth_strength(struct bGPDstroke *gps, + int point_index, + float influence, + int iterations, + struct bGPDstroke *r_gps); /** * Apply smooth for thickness to stroke point (use pressure). * \param gps: Stroke to smooth * \param point_index: Point index * \param influence: Amount of smoothing to apply + * \param iterations: Radius of points to consider, equivalent to iterations + * \param r_gps: Stroke to put the result into */ -bool BKE_gpencil_stroke_smooth_thickness(struct bGPDstroke *gps, int point_index, float influence); +bool BKE_gpencil_stroke_smooth_thickness(struct bGPDstroke *gps, + int point_index, + float influence, + int iterations, + struct bGPDstroke *r_gps); /** - * Apply smooth for UV rotation to stroke point (use pressure). + * Apply smooth for UV rotation/factor to stroke point. * \param gps: Stroke to smooth * \param point_index: Point index * \param influence: Amount of smoothing to apply + * \param iterations: Radius of points to consider, equivalent to iterations + * \param r_gps: Stroke to put the result into */ -bool BKE_gpencil_stroke_smooth_uv(struct bGPDstroke *gps, int point_index, float influence); +bool BKE_gpencil_stroke_smooth_uv(struct bGPDstroke *gps, + int point_index, + float influence, + int iterations, + struct bGPDstroke *r_gps); +/** + * Apply smooth operation to the stroke. + * \param gps: Stroke to smooth + * \param influence: The interpolation factor for the smooth and the original stroke + * \param iterations: Radius of points to consider, equivalent to iterations + * \param smooth_position: Smooth point locations + * \param smooth_strength: Smooth point strength + * \param smooth_thickness: Smooth point thickness + * \param smooth_uv: Smooth uv rotation/factor + * \param keep_shape: Use different distribution for smooth locations to keep the shape + * \param weights: per point weights to multiply influence with (optional, can be null) + */ +void BKE_gpencil_stroke_smooth(struct bGPDstroke *gps, + const float influence, + const int iterations, + const bool smooth_position, + const bool smooth_strength, + const bool smooth_thickness, + const bool smooth_uv, + const bool keep_shape, + const float *weights); /** * Close grease pencil stroke. * \param gps: Stroke to close diff --git a/source/blender/blenkernel/intern/gpencil_geom.cc b/source/blender/blenkernel/intern/gpencil_geom.cc index a5eff1f9d5a..a0b6ab2d654 100644 --- a/source/blender/blenkernel/intern/gpencil_geom.cc +++ b/source/blender/blenkernel/intern/gpencil_geom.cc @@ -980,74 +980,116 @@ bool BKE_gpencil_stroke_shrink(bGPDstroke *gps, const float dist, const short mo /** \name Stroke Smooth Positions * \{ */ -bool BKE_gpencil_stroke_smooth_point(bGPDstroke *gps, int i, float inf, const bool smooth_caps) +bool BKE_gpencil_stroke_smooth_point(bGPDstroke *gps, + int i, + float influence, + int iterations, + const bool smooth_caps, + const bool keep_shape, + bGPDstroke *r_gps) { - bGPDspoint *pt = &gps->points[i]; - float sco[3] = {0.0f}; - const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; - - /* Do nothing if not enough points to smooth out */ - if (gps->totpoints <= 2) { + /* If nothing to do, return early */ + if (gps->totpoints <= 2 || iterations <= 0) { return false; } - /* Only affect endpoints by a fraction of the normal strength, - * to prevent the stroke from shrinking too much + /* Overview of the algorithm here and in the following smooth functions: + * The smooth functions return the new attribute in question for a single point. + * The result is stored in r_gps->points[i], while the data is read from gps. + * To get a correct result, duplicate the stroke point data and read from the copy, + * while writing to the real stroke. Not doing that will result in acceptable, but + * asymmetric results. + * This algorithm works as long as all points are being smoothed. If there is + * points that should not get smoothed, use the old repeat smooth pattern with + * the parameter "iterations" set to 1 or 2. (2 matches the old algorithm). */ - if ((!smooth_caps) && (!is_cyclic && ELEM(i, 0, gps->totpoints - 1))) { - inf *= 0.1f; - } - - /* Compute smoothed coordinate by taking the ones nearby */ - /* XXX: This is potentially slow, - * and suffers from accumulation error as earlier points are handled before later ones. */ - { - /* XXX: this is hardcoded to look at 2 points on either side of the current one - * (i.e. 5 items total). */ - const int steps = 2; - const float average_fac = 1.0f / (float)(steps * 2 + 1); - int step; - - /* add the point itself */ - madd_v3_v3fl(sco, &pt->x, average_fac); - - /* n-steps before/after current point */ - /* XXX: review how the endpoints are treated by this algorithm. */ - /* XXX: falloff measures should also introduce some weighting variations, - * so that further-out points get less weight. */ - for (step = 1; step <= steps; step++) { - bGPDspoint *pt1, *pt2; - int before = i - step; - int after = i + step; - - if (is_cyclic) { - if (before < 0) { - /* Sub to end point (before is already negative). */ - before = gps->totpoints + before; - CLAMP(before, 0, gps->totpoints - 1); - } - if (after > gps->totpoints - 1) { - /* Add to start point. */ - after = after - gps->totpoints; - CLAMP(after, 0, gps->totpoints - 1); + + const bGPDspoint *pt = &gps->points[i]; + const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; + /* If smooth_caps is false, the caps will not be translated by smoothing. */ + if (!smooth_caps && !is_cyclic && ELEM(i, 0, gps->totpoints - 1)) { + copy_v3_v3(&r_gps->points[i].x, &pt->x); + return true; + } + + /* This function uses a binomial kernel, which is the discrete version of gaussian blur. + * The weight for a vertex at the relative index i is + * w = nCr(n, j + n/2) / 2^n = (n/1 * (n-1)/2 * ... * (n-j-n/2)/(j+n/2)) / 2^n + * All weights together sum up to 1 + * This is equivalent to doing multiple iterations of averaging neighbors, + * where n = iterations * 2 and -n/2 <= j <= n/2 + * + * Now the problem is that nCr(n, j + n/2) is very hard to compute for n > 500, since even + * double precision isn't sufficient. A very good robust approximation for n > 20 is + * nCr(n, j + n/2) / 2^n = sqrt(2/(pi*n)) * exp(-2*j*j/n) + * + * There is one more problem left: The old smooth algorithm was doing a more aggressive + * smooth. To solve that problem, choose a different n/2, which does not match the range and + * normalize the weights on finish. This may cause some artifacts at low values. + * + * keep_shape is a new option to stop the stroke from severly deforming. + * It uses different partially negative weights. + * w = 2 * (nCr(n, j + n/2) / 2^n) - (nCr(3*n, j + n) / 2^(3*n)) + * ~ 2 * sqrt(2/(pi*n)) * exp(-2*j*j/n) - sqrt(2/(pi*3*n)) * exp(-2*j*j/(3*n)) + * All weigths still sum up to 1. + * Note these weights only work because the averaging is done in relative coordinates. + */ + float sco[3] = {0.0f, 0.0f, 0.0f}; + float tmp[3]; + const int n_half = keep_shape ? (iterations * iterations) / 8 + iterations : + (iterations * iterations) / 4 + 2 * iterations + 12; + double w = keep_shape ? 2.0 : 1.0; + double w2 = keep_shape ? + (1.0 / M_SQRT3) * exp((2 * iterations * iterations) / (double)(n_half * 3)) : + 0.0; + double total_w = 0.0; + for (int step = iterations; step > 0; step--) { + int before = i - step; + int after = i + step; + float w_before = (float)(w - w2); + float w_after = (float)(w - w2); + + if (is_cyclic) { + before = (before % gps->totpoints + gps->totpoints) % gps->totpoints; + after = after % gps->totpoints; + } + else { + if (before < 0) { + if (!smooth_caps) { + w_before *= -before / (float)i; } + before = 0; } - else { - CLAMP_MIN(before, 0); - CLAMP_MAX(after, gps->totpoints - 1); + if (after > gps->totpoints - 1) { + if (!smooth_caps) { + w_after *= (after - (gps->totpoints - 1)) / (float)(gps->totpoints - 1 - i); + } + after = gps->totpoints - 1; } + } - pt1 = &gps->points[before]; - pt2 = &gps->points[after]; + /* Add both these points in relative coordinates to the weighted average sum. */ + sub_v3_v3v3(tmp, &gps->points[before].x, &pt->x); + madd_v3_v3fl(sco, tmp, w_before); + sub_v3_v3v3(tmp, &gps->points[after].x, &pt->x); + madd_v3_v3fl(sco, tmp, w_after); - /* add both these points to the average-sum (s += p[i]/n) */ - madd_v3_v3fl(sco, &pt1->x, average_fac); - madd_v3_v3fl(sco, &pt2->x, average_fac); - } + total_w += w_before; + total_w += w_after; + + w *= (n_half + step) / (double)(n_half + 1 - step); + w2 *= (n_half * 3 + step) / (double)(n_half * 3 + 1 - step); } + total_w += w - w2; + /* The accumulated weight total_w should be + * ~sqrt(M_PI * n_half) * exp((iterations * iterations) / n_half) < 100 + * here, but sometimes not quite. */ + mul_v3_fl(sco, (float)(1.0 / total_w)); + /* Shift back to global coordinates. */ + add_v3_v3(sco, &pt->x); - /* Based on influence factor, blend between original and optimal smoothed coordinate */ - interp_v3_v3v3(&pt->x, &pt->x, sco, inf); + /* Based on influence factor, blend between original and optimal smoothed coordinate. */ + interp_v3_v3v3(&r_gps->points[i].x, &pt->x, sco, influence); return true; } @@ -1058,74 +1100,54 @@ bool BKE_gpencil_stroke_smooth_point(bGPDstroke *gps, int i, float inf, const bo /** \name Stroke Smooth Strength * \{ */ -bool BKE_gpencil_stroke_smooth_strength(bGPDstroke *gps, int point_index, float influence) +bool BKE_gpencil_stroke_smooth_strength( + bGPDstroke *gps, int i, float influence, int iterations, bGPDstroke *r_gps) { - bGPDspoint *ptb = &gps->points[point_index]; - const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; - - /* Do nothing if not enough points */ - if ((gps->totpoints <= 2) || (point_index < 1)) { + /* If nothing to do, return early */ + if (gps->totpoints <= 2 || iterations <= 0) { return false; } - /* Only affect endpoints by a fraction of the normal influence */ - float inf = influence; - if (!is_cyclic && ELEM(point_index, 0, gps->totpoints - 1)) { - inf *= 0.01f; - } - /* Limit max influence to reduce pop effect. */ - CLAMP_MAX(inf, 0.98f); - - float total = 0.0f; - float max_strength = 0.0f; - const int steps = 4; - const float average_fac = 1.0f / (float)(steps * 2 + 1); - int step; - /* add the point itself */ - total += ptb->strength * average_fac; - max_strength = ptb->strength; + /* See BKE_gpencil_stroke_smooth_point for details on the algorithm. */ - /* n-steps before/after current point */ - for (step = 1; step <= steps; step++) { - bGPDspoint *pt1, *pt2; - int before = point_index - step; - int after = point_index + step; + const bGPDspoint *pt = &gps->points[i]; + const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; + float strength = 0.0f; + const int n_half = (iterations * iterations) / 4 + iterations; + double w = 1.0; + double total_w = 0.0; + for (int step = iterations; step > 0; step--) { + int before = i - step; + int after = i + step; + float w_before = (float)w; + float w_after = (float)w; if (is_cyclic) { - if (before < 0) { - /* Sub to end point (before is already negative). */ - before = gps->totpoints + before; - CLAMP(before, 0, gps->totpoints - 1); - } - if (after > gps->totpoints - 1) { - /* Add to start point. */ - after = after - gps->totpoints; - CLAMP(after, 0, gps->totpoints - 1); - } + before = (before % gps->totpoints + gps->totpoints) % gps->totpoints; + after = after % gps->totpoints; } else { CLAMP_MIN(before, 0); CLAMP_MAX(after, gps->totpoints - 1); } - pt1 = &gps->points[before]; - pt2 = &gps->points[after]; - /* add both these points to the average-sum (s += p[i]/n) */ - total += pt1->strength * average_fac; - total += pt2->strength * average_fac; - /* Save max value. */ - if (max_strength < pt1->strength) { - max_strength = pt1->strength; - } - if (max_strength < pt2->strength) { - max_strength = pt2->strength; - } + /* Add both these points in relative coordinates to the weighted average sum. */ + strength += w_before * (gps->points[before].strength - pt->strength); + strength += w_after * (gps->points[after].strength - pt->strength); + + total_w += w_before; + total_w += w_after; + + w *= (n_half + step) / (double)(n_half + 1 - step); } + total_w += w; + /* The accumulated weight total_w should be + * ~sqrt(M_PI * n_half) * exp((iterations * iterations) / n_half) < 100 + * here, but sometimes not quite. */ + strength /= total_w; /* Based on influence factor, blend between original and optimal smoothed value. */ - ptb->strength = interpf(ptb->strength, total, inf); - /* Clamp to maximum stroke strength to avoid weird results. */ - CLAMP_MAX(ptb->strength, max_strength); + r_gps->points[i].strength = pt->strength + strength * influence; return true; } @@ -1136,74 +1158,55 @@ bool BKE_gpencil_stroke_smooth_strength(bGPDstroke *gps, int point_index, float /** \name Stroke Smooth Thickness * \{ */ -bool BKE_gpencil_stroke_smooth_thickness(bGPDstroke *gps, int point_index, float influence) +bool BKE_gpencil_stroke_smooth_thickness( + bGPDstroke *gps, int i, float influence, int iterations, bGPDstroke *r_gps) { - bGPDspoint *ptb = &gps->points[point_index]; - const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; - - /* Do nothing if not enough points */ - if ((gps->totpoints <= 2) || (point_index < 1)) { + /* If nothing to do, return early */ + if (gps->totpoints <= 2 || iterations <= 0) { return false; } - /* Only affect endpoints by a fraction of the normal influence */ - float inf = influence; - if (!is_cyclic && ELEM(point_index, 0, gps->totpoints - 1)) { - inf *= 0.01f; - } - /* Limit max influence to reduce pop effect. */ - CLAMP_MAX(inf, 0.98f); - - float total = 0.0f; - float max_pressure = 0.0f; - const int steps = 4; - const float average_fac = 1.0f / (float)(steps * 2 + 1); - int step; - /* add the point itself */ - total += ptb->pressure * average_fac; - max_pressure = ptb->pressure; + /* See BKE_gpencil_stroke_smooth_point for details on the algorithm. */ - /* n-steps before/after current point */ - for (step = 1; step <= steps; step++) { - bGPDspoint *pt1, *pt2; - int before = point_index - step; - int after = point_index + step; + const bGPDspoint *pt = &gps->points[i]; + const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; + float pressure = 0.0f; + const int n_half = (iterations * iterations) / 4 + iterations; + double w = 1.0; + double total_w = 0.0; + for (int step = iterations; step > 0; step--) { + int before = i - step; + int after = i + step; + float w_before = (float)w; + float w_after = (float)w; if (is_cyclic) { - if (before < 0) { - /* Sub to end point (before is already negative). */ - before = gps->totpoints + before; - CLAMP(before, 0, gps->totpoints - 1); - } - if (after > gps->totpoints - 1) { - /* Add to start point. */ - after = after - gps->totpoints; - CLAMP(after, 0, gps->totpoints - 1); - } + before = (before % gps->totpoints + gps->totpoints) % gps->totpoints; + after = after % gps->totpoints; } else { CLAMP_MIN(before, 0); CLAMP_MAX(after, gps->totpoints - 1); } - pt1 = &gps->points[before]; - pt2 = &gps->points[after]; - /* add both these points to the average-sum (s += p[i]/n) */ - total += pt1->pressure * average_fac; - total += pt2->pressure * average_fac; - /* Save max value. */ - if (max_pressure < pt1->pressure) { - max_pressure = pt1->pressure; - } - if (max_pressure < pt2->pressure) { - max_pressure = pt2->pressure; - } + /* Add both these points in relative coordinates to the weighted average sum. */ + pressure += w_before * (gps->points[before].pressure - pt->pressure); + pressure += w_after * (gps->points[after].pressure - pt->pressure); + + total_w += w_before; + total_w += w_after; + + w *= (n_half + step) / (double)(n_half + 1 - step); } + total_w += w; + /* The accumulated weight total_w should be + * ~sqrt(M_PI * n_half) * exp((iterations * iterations) / n_half) < 100 + * here, but sometimes not quite. */ + pressure /= total_w; /* Based on influence factor, blend between original and optimal smoothed value. */ - ptb->pressure = interpf(ptb->pressure, total, inf); - /* Clamp to maximum stroke thickness to avoid weird results. */ - CLAMP_MAX(ptb->pressure, max_pressure); + r_gps->points[i].pressure = pt->pressure + pressure * influence; + return true; } @@ -1213,57 +1216,127 @@ bool BKE_gpencil_stroke_smooth_thickness(bGPDstroke *gps, int point_index, float /** \name Stroke Smooth UV * \{ */ -bool BKE_gpencil_stroke_smooth_uv(bGPDstroke *gps, int point_index, float influence) +bool BKE_gpencil_stroke_smooth_uv( + struct bGPDstroke *gps, int i, float influence, int iterations, struct bGPDstroke *r_gps) { - bGPDspoint *ptb = &gps->points[point_index]; + /* If nothing to do, return early */ + if (gps->totpoints <= 2 || iterations <= 0) { + return false; + } + + /* See BKE_gpencil_stroke_smooth_point for details on the algorithm. */ + + const bGPDspoint *pt = &gps->points[i]; const bool is_cyclic = (gps->flag & GP_STROKE_CYCLIC) != 0; - /* Do nothing if not enough points */ - if (gps->totpoints <= 2) { - return false; + /* If don't change the caps. */ + if (!is_cyclic && ELEM(i, 0, gps->totpoints - 1)) { + r_gps->points[i].uv_rot = pt->uv_rot; + r_gps->points[i].uv_fac = pt->uv_fac; + return true; } - /* Compute theoretical optimal value */ - bGPDspoint *pta, *ptc; - int before = point_index - 1; - int after = point_index + 1; + float uv_rot = 0.0f; + float uv_fac = 0.0f; + const int n_half = iterations * iterations + iterations; + double w = 1.0; + double total_w = 0.0; + for (int step = iterations; step > 0; step--) { + int before = i - step; + int after = i + step; + float w_before = (float)w; + float w_after = (float)w; - if (is_cyclic) { - if (before < 0) { - /* Sub to end point (before is already negative). */ - before = gps->totpoints + before; - CLAMP(before, 0, gps->totpoints - 1); + if (is_cyclic) { + before = (before % gps->totpoints + gps->totpoints) % gps->totpoints; + after = after % gps->totpoints; } - if (after > gps->totpoints - 1) { - /* Add to start point. */ - after = after - gps->totpoints; - CLAMP(after, 0, gps->totpoints - 1); + else { + if (before < 0) { + w_before *= -before / (float)i; + before = 0; + } + if (after > gps->totpoints - 1) { + w_after *= (after - (gps->totpoints - 1)) / (float)(gps->totpoints - 1 - i); + after = gps->totpoints - 1; + } } - } - else { - CLAMP_MIN(before, 0); - CLAMP_MAX(after, gps->totpoints - 1); - } - pta = &gps->points[before]; - ptc = &gps->points[after]; - /* the optimal value is the corresponding to the interpolation of the pressure - * at the distance of point b - */ - float fac = line_point_factor_v3(&ptb->x, &pta->x, &ptc->x); - /* sometimes the factor can be wrong due stroke geometry, so use middle point */ - if ((fac < 0.0f) || (fac > 1.0f)) { - fac = 0.5f; + /* Add both these points in relative coordinates to the weighted average sum. */ + uv_rot += w_before * (gps->points[before].uv_rot - pt->uv_rot); + uv_rot += w_after * (gps->points[after].uv_rot - pt->uv_rot); + uv_fac += w_before * (gps->points[before].uv_fac - pt->uv_fac); + uv_fac += w_after * (gps->points[after].uv_fac - pt->uv_fac); + + total_w += w_before; + total_w += w_after; + + w *= (n_half + step) / (double)(n_half + 1 - step); } - float optimal = interpf(ptc->uv_rot, pta->uv_rot, fac); + total_w += w; + /* The accumulated weight total_w should be + * ~sqrt(M_PI * n_half) * exp((iterations * iterations) / n_half) < 100 + * here, but sometimes not quite. */ + uv_rot /= total_w; + uv_fac /= total_w; - /* Based on influence factor, blend between original and optimal */ - ptb->uv_rot = interpf(optimal, ptb->uv_rot, influence); - CLAMP(ptb->uv_rot, -M_PI_2, M_PI_2); + /* Based on influence factor, blend between original and optimal smoothed value. */ + r_gps->points[i].uv_rot = pt->uv_rot + uv_rot * influence; + r_gps->points[i].uv_fac = pt->uv_fac + uv_fac * influence; return true; } +void BKE_gpencil_stroke_smooth(bGPDstroke *gps, + const float influence, + const int iterations, + const bool smooth_position, + const bool smooth_strength, + const bool smooth_thickness, + const bool smooth_uv, + const bool keep_shape, + const float *weights) +{ + if (influence <= 0 || iterations <= 0) { + return; + } + + /* Make a copy of the point data to avoid directionality of the smooth operation. */ + bGPDstroke gps_old = *gps; + gps_old.points = (bGPDspoint *)MEM_dupallocN(gps->points); + + /* Smooth stroke. */ + for (int i = 0; i < gps->totpoints; i++) { + float val = influence; + if (weights != NULL) { + val *= weights[i]; + if (val <= 0.0f) { + continue; + } + } + + /* TODO: Currently the weights only control the influence, but is would be much better if they + * would control the distribution used in smooth, similar to how the ends are handled. */ + + /* Perform smoothing. */ + if (smooth_position) { + BKE_gpencil_stroke_smooth_point(&gps_old, i, val, iterations, false, keep_shape, gps); + } + if (smooth_strength) { + BKE_gpencil_stroke_smooth_strength(&gps_old, i, val, iterations, gps); + } + if (smooth_thickness) { + BKE_gpencil_stroke_smooth_thickness(&gps_old, i, val, iterations, gps); + } + if (smooth_uv) { + BKE_gpencil_stroke_smooth_uv(&gps_old, i, val, iterations, gps); + } + } + + /* Free the copied points array. */ + MEM_freeN(gps_old.points); +} + void BKE_gpencil_stroke_2d_flat(const bGPDspoint *points, int totpoints, float (*points2d)[2], @@ -3443,7 +3516,7 @@ void BKE_gpencil_stroke_join(bGPDstroke *gps_a, for (i = start; i < end; i++) { pt = &gps_a->points[i]; pt->pressure += (avg_pressure - pt->pressure) * ratio; - BKE_gpencil_stroke_smooth_point(gps_a, i, ratio * 0.6f, false); + BKE_gpencil_stroke_smooth_point(gps_a, i, ratio * 0.6f, 2, false, true, gps_a); ratio += step; /* In the center, reverse the ratio. */ diff --git a/source/blender/blenloader/intern/versioning_300.c b/source/blender/blenloader/intern/versioning_300.c index 51b5cab1f7c..adc6b990869 100644 --- a/source/blender/blenloader/intern/versioning_300.c +++ b/source/blender/blenloader/intern/versioning_300.c @@ -2415,6 +2415,24 @@ void blo_do_versions_300(FileData *fd, Library *UNUSED(lib), Main *bmain) } } } + + /* Change grease pencil smooth iterations to match old results with new algorithm. */ + LISTBASE_FOREACH (Object *, ob, &bmain->objects) { + LISTBASE_FOREACH (GpencilModifierData *, md, &ob->greasepencil_modifiers) { + if (md->type == eGpencilModifierType_Smooth) { + SmoothGpencilModifierData *gpmd = (SmoothGpencilModifierData *)md; + if (gpmd->step == 1 && gpmd->factor <= 0.5f) { + gpmd->factor *= 2.0f; + } + else { + gpmd->step = 1 + (int)(gpmd->factor * max_ff(0.0f, + min_ff(5.1f * sqrtf(gpmd->step) - 3.0f, + gpmd->step + 2.0f))); + gpmd->factor = 1.0f; + } + } + } + } } /** diff --git a/source/blender/editors/gpencil/gpencil_edit.c b/source/blender/editors/gpencil/gpencil_edit.c index 8506e90191f..a8fb344f366 100644 --- a/source/blender/editors/gpencil/gpencil_edit.c +++ b/source/blender/editors/gpencil/gpencil_edit.c @@ -3924,31 +3924,36 @@ static void gpencil_smooth_stroke(bContext *C, wmOperator *op) GP_EDITABLE_STROKES_BEGIN (gpstroke_iter, C, gpl, gps) { if (gps->flag & GP_STROKE_SELECT) { - for (int r = 0; r < repeat; r++) { + /* TODO use `BKE_gpencil_stroke_smooth` when the weights are better used. */ + bGPDstroke gps_old = *gps; + gps_old.points = (bGPDspoint *)MEM_dupallocN(gps->points); + /* Here the iteration needs to be done outside the smooth functions, + * as there are points that don't get smoothed. */ + for (int n = 0; n < repeat; n++) { for (int i = 0; i < gps->totpoints; i++) { - bGPDspoint *pt = &gps->points[i]; - if ((only_selected) && ((pt->flag & GP_SPOINT_SELECT) == 0)) { + if (only_selected && (gps->points[i].flag & GP_SPOINT_SELECT) == 0) { continue; } - /* perform smoothing */ + /* Perform smoothing. */ if (smooth_position) { - BKE_gpencil_stroke_smooth_point(gps, i, factor, false); + BKE_gpencil_stroke_smooth_point(&gps_old, i, factor, 1, false, false, gps); } if (smooth_strength) { - BKE_gpencil_stroke_smooth_strength(gps, i, factor); + BKE_gpencil_stroke_smooth_strength(&gps_old, i, factor, 1, gps); } if (smooth_thickness) { - /* thickness need to repeat process several times */ - for (int r2 = 0; r2 < repeat * 2; r2++) { - BKE_gpencil_stroke_smooth_thickness(gps, i, 1.0f - factor); - } + BKE_gpencil_stroke_smooth_thickness(&gps_old, i, 1.0f - factor, 1, gps); } if (smooth_uv) { - BKE_gpencil_stroke_smooth_uv(gps, i, factor); + BKE_gpencil_stroke_smooth_uv(&gps_old, i, factor, 1, gps); } } + if (n < repeat - 1) { + memcpy(gps_old.points, gps->points, sizeof(bGPDspoint) * gps->totpoints); + } } + MEM_freeN(gps_old.points); } } GP_EDITABLE_STROKES_END(gpstroke_iter); @@ -4926,10 +4931,10 @@ void GPENCIL_OT_stroke_smooth(wmOperatorType *ot) ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; /* properties */ - prop = RNA_def_int(ot->srna, "repeat", 1, 1, 50, "Repeat", "", 1, 20); + prop = RNA_def_int(ot->srna, "repeat", 2, 1, 1000, "Repeat", "", 1, 1000); RNA_def_property_flag(prop, PROP_SKIP_SAVE); - RNA_def_float(ot->srna, "factor", 0.5f, 0.0f, 2.0f, "Factor", "", 0.0f, 2.0f); + RNA_def_float(ot->srna, "factor", 1.0f, 0.0f, 2.0f, "Factor", "", 0.0f, 1.0f); RNA_def_boolean(ot->srna, "only_selected", true, diff --git a/source/blender/editors/gpencil/gpencil_fill.c b/source/blender/editors/gpencil/gpencil_fill.c index 45a2247c65e..8fc300412d9 100644 --- a/source/blender/editors/gpencil/gpencil_fill.c +++ b/source/blender/editors/gpencil/gpencil_fill.c @@ -1638,14 +1638,9 @@ static void gpencil_stroke_from_buffer(tGPDfill *tgpf) } } - /* smooth stroke */ - float reduce = 0.0f; - float smoothfac = 1.0f; - for (int r = 0; r < 1; r++) { - for (int i = 0; i < gps->totpoints; i++) { - BKE_gpencil_stroke_smooth_point(gps, i, smoothfac - reduce, false); - } - reduce += 0.25f; /* reduce the factor */ + /* Smooth stroke. No copy of the stroke since there only a minor improvement here. */ + for (int i = 0; i < gps->totpoints; i++) { + BKE_gpencil_stroke_smooth_point(gps, i, 1.0f, 2, false, true, gps); } /* if axis locked, reproject to plane locked */ diff --git a/source/blender/editors/gpencil/gpencil_interpolate.c b/source/blender/editors/gpencil/gpencil_interpolate.c index 65060e1bab5..8630b7f23d4 100644 --- a/source/blender/editors/gpencil/gpencil_interpolate.c +++ b/source/blender/editors/gpencil/gpencil_interpolate.c @@ -310,23 +310,6 @@ static void gpencil_stroke_pair_table(bContext *C, } } -static void gpencil_interpolate_smooth_stroke(bGPDstroke *gps, - float smooth_factor, - int smooth_steps) -{ - if (smooth_factor == 0.0f) { - return; - } - - float reduce = 0.0f; - for (int r = 0; r < smooth_steps; r++) { - for (int i = 0; i < gps->totpoints - 1; i++) { - BKE_gpencil_stroke_smooth_point(gps, i, smooth_factor - reduce, false); - BKE_gpencil_stroke_smooth_strength(gps, i, smooth_factor); - } - reduce += 0.25f; /* reduce the factor */ - } -} /* Perform interpolation */ static void gpencil_interpolate_update_points(const bGPDstroke *gps_from, const bGPDstroke *gps_to, @@ -553,7 +536,15 @@ static void gpencil_interpolate_set_points(bContext *C, tGPDinterpolate *tgpi) /* Update points position. */ gpencil_interpolate_update_points(gps_from, gps_to, new_stroke, tgpil->factor); - gpencil_interpolate_smooth_stroke(new_stroke, tgpi->smooth_factor, tgpi->smooth_steps); + BKE_gpencil_stroke_smooth(new_stroke, + tgpi->smooth_factor, + tgpi->smooth_steps, + true, + true, + false, + false, + true, + NULL); /* Calc geometry data. */ BKE_gpencil_stroke_geometry_update(gpd, new_stroke); @@ -1385,7 +1376,8 @@ static int gpencil_interpolate_seq_exec(bContext *C, wmOperator *op) /* Update points position. */ gpencil_interpolate_update_points(gps_from, gps_to, new_stroke, factor); - gpencil_interpolate_smooth_stroke(new_stroke, smooth_factor, smooth_steps); + BKE_gpencil_stroke_smooth( + new_stroke, smooth_factor, smooth_steps, true, true, false, false, true, NULL); /* Calc geometry data. */ BKE_gpencil_stroke_geometry_update(gpd, new_stroke); diff --git a/source/blender/editors/gpencil/gpencil_paint.c b/source/blender/editors/gpencil/gpencil_paint.c index 18dc8d4bc72..93a4a784674 100644 --- a/source/blender/editors/gpencil/gpencil_paint.c +++ b/source/blender/editors/gpencil/gpencil_paint.c @@ -1197,29 +1197,21 @@ static void gpencil_stroke_newfrombuffer(tGPsdata *p) gpencil_subdivide_stroke(gpd, gps, subdivide); } - /* Smooth stroke after subdiv - only if there's something to do for each iteration, - * the factor is reduced to get a better smoothing - * without changing too much the original stroke. */ - if ((brush->gpencil_settings->flag & GP_BRUSH_GROUP_SETTINGS) && - (brush->gpencil_settings->draw_smoothfac > 0.0f)) { - float reduce = 0.0f; - for (int r = 0; r < brush->gpencil_settings->draw_smoothlvl; r++) { - for (i = 0; i < gps->totpoints - 1; i++) { - BKE_gpencil_stroke_smooth_point( - gps, i, brush->gpencil_settings->draw_smoothfac - reduce, false); - BKE_gpencil_stroke_smooth_strength(gps, i, brush->gpencil_settings->draw_smoothfac); - } - reduce += 0.25f; /* reduce the factor */ - } + /* Smooth stroke after subdiv - only if there's something to do for each iteration. + * Keep the original stroke shape as much as possible. */ + const float smoothfac = brush->gpencil_settings->draw_smoothfac; + const int iterations = brush->gpencil_settings->draw_smoothlvl; + if (brush->gpencil_settings->flag & GP_BRUSH_GROUP_SETTINGS) { + BKE_gpencil_stroke_smooth(gps, smoothfac, iterations, true, true, false, false, true, NULL); } /* If reproject the stroke using Stroke mode, need to apply a smooth because * the reprojection creates small jitter. */ if (ts->gpencil_v3d_align & GP_PROJECT_DEPTH_STROKE) { float ifac = (float)brush->gpencil_settings->input_samples / 10.0f; float sfac = interpf(1.0f, 0.2f, ifac); - for (i = 0; i < gps->totpoints - 1; i++) { - BKE_gpencil_stroke_smooth_point(gps, i, sfac, false); - BKE_gpencil_stroke_smooth_strength(gps, i, sfac); + for (i = 0; i < gps->totpoints; i++) { + BKE_gpencil_stroke_smooth_point(gps, i, sfac, 2, false, true, gps); + BKE_gpencil_stroke_smooth_strength(gps, i, sfac, 2, gps); } } diff --git a/source/blender/editors/gpencil/gpencil_sculpt_paint.c b/source/blender/editors/gpencil/gpencil_sculpt_paint.c index 216971e514b..1e7159392e2 100644 --- a/source/blender/editors/gpencil/gpencil_sculpt_paint.c +++ b/source/blender/editors/gpencil/gpencil_sculpt_paint.c @@ -329,16 +329,16 @@ static bool gpencil_brush_smooth_apply(tGP_BrushEditData *gso, /* perform smoothing */ if (gso->brush->gpencil_settings->sculpt_mode_flag & GP_SCULPT_FLAGMODE_APPLY_POSITION) { - BKE_gpencil_stroke_smooth_point(gps, pt_index, inf, false); + BKE_gpencil_stroke_smooth_point(gps, pt_index, inf, 2, false, false, gps); } if (gso->brush->gpencil_settings->sculpt_mode_flag & GP_SCULPT_FLAGMODE_APPLY_STRENGTH) { - BKE_gpencil_stroke_smooth_strength(gps, pt_index, inf); + BKE_gpencil_stroke_smooth_strength(gps, pt_index, inf, 2, gps); } if (gso->brush->gpencil_settings->sculpt_mode_flag & GP_SCULPT_FLAGMODE_APPLY_THICKNESS) { - BKE_gpencil_stroke_smooth_thickness(gps, pt_index, inf); + BKE_gpencil_stroke_smooth_thickness(gps, pt_index, inf, 2, gps); } if (gso->brush->gpencil_settings->sculpt_mode_flag & GP_SCULPT_FLAGMODE_APPLY_UV) { - BKE_gpencil_stroke_smooth_uv(gps, pt_index, inf); + BKE_gpencil_stroke_smooth_uv(gps, pt_index, inf, 2, gps); } return true; diff --git a/source/blender/gpencil_modifiers/intern/MOD_gpencilshrinkwrap.c b/source/blender/gpencil_modifiers/intern/MOD_gpencilshrinkwrap.c index 8eaed56dc58..d80224e6639 100644 --- a/source/blender/gpencil_modifiers/intern/MOD_gpencilshrinkwrap.c +++ b/source/blender/gpencil_modifiers/intern/MOD_gpencilshrinkwrap.c @@ -105,15 +105,15 @@ static void deformStroke(GpencilModifierData *md, /* Apply deformed coordinates. */ pt = gps->points; + bGPDstroke gps_old = *gps; + gps_old.points = (bGPDspoint *)MEM_dupallocN(gps->points); for (i = 0; i < gps->totpoints; i++, pt++) { copy_v3_v3(&pt->x, vert_coords[i]); /* Smooth stroke. */ - if (mmd->smooth_factor > 0.0f) { - for (int r = 0; r < mmd->smooth_step; r++) { - BKE_gpencil_stroke_smooth_point(gps, i, mmd->smooth_factor, true); - } - } + BKE_gpencil_stroke_smooth_point( + &gps_old, i, mmd->smooth_factor, mmd->smooth_step, true, false, gps); } + MEM_freeN(gps_old.points); MEM_freeN(vert_coords); diff --git a/source/blender/gpencil_modifiers/intern/MOD_gpencilsmooth.c b/source/blender/gpencil_modifiers/intern/MOD_gpencilsmooth.c index f8201eb6b4f..eb51a247d87 100644 --- a/source/blender/gpencil_modifiers/intern/MOD_gpencilsmooth.c +++ b/source/blender/gpencil_modifiers/intern/MOD_gpencilsmooth.c @@ -34,10 +34,14 @@ #include "UI_interface.h" #include "UI_resources.h" +#include "RNA_access.h" + #include "MOD_gpencil_modifiertypes.h" #include "MOD_gpencil_ui_common.h" #include "MOD_gpencil_util.h" +#include "MEM_guardedalloc.h" + static void initData(GpencilModifierData *md) { SmoothGpencilModifierData *gpmd = (SmoothGpencilModifierData *)md; @@ -94,45 +98,40 @@ static void deformStroke(GpencilModifierData *md, return; } - /* smooth stroke */ - if (mmd->factor > 0.0f) { - for (int r = 0; r < mmd->step; r++) { - for (int i = 0; i < gps->totpoints; i++) { - MDeformVert *dvert = gps->dvert != NULL ? &gps->dvert[i] : NULL; - - /* verify vertex group */ - float weight = get_modifier_point_weight( - dvert, (mmd->flag & GP_SMOOTH_INVERT_VGROUP) != 0, def_nr); - if (weight < 0.0f) { - continue; - } - - /* Custom curve to modulate value. */ - if (use_curve) { - float value = (float)i / (gps->totpoints - 1); - weight *= BKE_curvemapping_evaluateF(mmd->curve_intensity, 0, value); - } - - const float val = mmd->factor * weight; - /* perform smoothing */ - if (mmd->flag & GP_SMOOTH_MOD_LOCATION) { - BKE_gpencil_stroke_smooth_point(gps, i, val, false); - } - if (mmd->flag & GP_SMOOTH_MOD_STRENGTH) { - BKE_gpencil_stroke_smooth_strength(gps, i, val); - } - if ((mmd->flag & GP_SMOOTH_MOD_THICKNESS) && (val > 0.0f)) { - /* thickness need to repeat process several times */ - for (int r2 = 0; r2 < r * 10; r2++) { - BKE_gpencil_stroke_smooth_thickness(gps, i, val); - } - } - if (mmd->flag & GP_SMOOTH_MOD_UV) { - BKE_gpencil_stroke_smooth_uv(gps, i, val); - } + if (mmd->factor <= 0.0f || mmd->step <= 0) { + return; + } + + float *weights = NULL; + if (def_nr != -1 || use_curve) { + weights = MEM_malloc_arrayN(gps->totpoints, sizeof(*weights), __func__); + /* Calculate weights. */ + for (int i = 0; i < gps->totpoints; i++) { + MDeformVert *dvert = gps->dvert != NULL ? &gps->dvert[i] : NULL; + + /* Verify vertex group. */ + float weight = get_modifier_point_weight( + dvert, (mmd->flag & GP_SMOOTH_INVERT_VGROUP) != 0, def_nr); + + /* Custom curve to modulate value. */ + if (use_curve && weight > 0.0f) { + float value = (float)i / (gps->totpoints - 1); + weight *= BKE_curvemapping_evaluateF(mmd->curve_intensity, 0, value); } + + weights[i] = weight; } } + BKE_gpencil_stroke_smooth(gps, + mmd->factor, + mmd->step, + mmd->flag & GP_SMOOTH_MOD_LOCATION, + mmd->flag & GP_SMOOTH_MOD_STRENGTH, + mmd->flag & GP_SMOOTH_MOD_THICKNESS, + mmd->flag & GP_SMOOTH_MOD_UV, + mmd->flag & GP_SMOOTH_KEEP_SHAPE, + weights); + MEM_SAFE_FREE(weights); } static void bakeModifier(struct Main *UNUSED(bmain), @@ -161,7 +160,7 @@ static void foreachIDLink(GpencilModifierData *md, Object *ob, IDWalkFunc walk, static void panel_draw(const bContext *UNUSED(C), Panel *panel) { - uiLayout *row; + uiLayout *row, *col; uiLayout *layout = panel->layout; PointerRNA *ptr = gpencil_modifier_panel_get_property_pointers(panel, NULL); @@ -177,6 +176,10 @@ static void panel_draw(const bContext *UNUSED(C), Panel *panel) uiItemR(layout, ptr, "factor", 0, NULL, ICON_NONE); uiItemR(layout, ptr, "step", 0, IFACE_("Repeat"), ICON_NONE); + col = uiLayoutColumn(layout, false); + uiLayoutSetActive(col, RNA_boolean_get(ptr, "use_edit_position")); + uiItemR(col, ptr, "keep_shape", 0, NULL, ICON_NONE); + gpencil_modifier_panel_end(layout, ptr); } diff --git a/source/blender/makesdna/DNA_gpencil_modifier_defaults.h b/source/blender/makesdna/DNA_gpencil_modifier_defaults.h index a87e7a7c397..9d14ca039ac 100644 --- a/source/blender/makesdna/DNA_gpencil_modifier_defaults.h +++ b/source/blender/makesdna/DNA_gpencil_modifier_defaults.h @@ -193,7 +193,7 @@ .vgname = "", \ .pass_index = 0, \ .flag = GP_SMOOTH_MOD_LOCATION, \ - .factor = 0.5f, \ + .factor = 1.0f, \ .step = 1, \ .layer_pass = 0, \ .curve_intensity = NULL, \ diff --git a/source/blender/makesdna/DNA_gpencil_modifier_types.h b/source/blender/makesdna/DNA_gpencil_modifier_types.h index 1054c309ee5..4f655e87a09 100644 --- a/source/blender/makesdna/DNA_gpencil_modifier_types.h +++ b/source/blender/makesdna/DNA_gpencil_modifier_types.h @@ -750,6 +750,7 @@ typedef enum eSmoothGpencil_Flag { GP_SMOOTH_INVERT_LAYERPASS = (1 << 7), GP_SMOOTH_INVERT_MATERIAL = (1 << 4), GP_SMOOTH_CUSTOM_CURVE = (1 << 8), + GP_SMOOTH_KEEP_SHAPE = (1 << 9), } eSmoothGpencil_Flag; typedef struct ArmatureGpencilModifierData { diff --git a/source/blender/makesrna/intern/rna_brush.c b/source/blender/makesrna/intern/rna_brush.c index c86a852b0ea..b2b6bdbcffc 100644 --- a/source/blender/makesrna/intern/rna_brush.c +++ b/source/blender/makesrna/intern/rna_brush.c @@ -1355,7 +1355,8 @@ static void rna_def_gpencil_options(BlenderRNA *brna) /* Smoothing factor for new strokes */ prop = RNA_def_property(srna, "pen_smooth_factor", PROP_FLOAT, PROP_NONE); RNA_def_property_float_sdna(prop, NULL, "draw_smoothfac"); - RNA_def_property_range(prop, 0.0, 2.0f); + RNA_def_property_range(prop, 0.0, 2.0); + RNA_def_property_ui_range(prop, 0.0, 1.0, 10, 3); RNA_def_property_ui_text( prop, "Smooth", @@ -1366,7 +1367,7 @@ static void rna_def_gpencil_options(BlenderRNA *brna) /* Iterations of the Smoothing factor */ prop = RNA_def_property(srna, "pen_smooth_steps", PROP_INT, PROP_NONE); RNA_def_property_int_sdna(prop, NULL, "draw_smoothlvl"); - RNA_def_property_range(prop, 1, 3); + RNA_def_property_range(prop, 0, 100); RNA_def_property_ui_text(prop, "Iterations", "Number of times to smooth newly created strokes"); RNA_def_property_clear_flag(prop, PROP_ANIMATABLE); RNA_def_property_update(prop, NC_GPENCIL | ND_DATA, NULL); diff --git a/source/blender/makesrna/intern/rna_gpencil_modifier.c b/source/blender/makesrna/intern/rna_gpencil_modifier.c index edeb7443957..bf3f506251c 100644 --- a/source/blender/makesrna/intern/rna_gpencil_modifier.c +++ b/source/blender/makesrna/intern/rna_gpencil_modifier.c @@ -1026,11 +1026,16 @@ static void rna_def_modifier_gpencilsmooth(BlenderRNA *brna) prop = RNA_def_property(srna, "step", PROP_INT, PROP_NONE); RNA_def_property_int_sdna(prop, NULL, "step"); - RNA_def_property_range(prop, 1, 10); + RNA_def_property_range(prop, 1, 1000); RNA_def_property_ui_text( prop, "Step", "Number of times to apply smooth (high numbers can reduce fps)"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); + prop = RNA_def_property(srna, "keep_shape", PROP_BOOLEAN, PROP_NONE); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_SMOOTH_KEEP_SHAPE); + RNA_def_property_ui_text(prop, "Keep Shape", "Smooth the details, but keep the overall shape"); + RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); + prop = RNA_def_property(srna, "invert_layers", PROP_BOOLEAN, PROP_NONE); RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_SMOOTH_INVERT_LAYER); RNA_def_property_ui_text(prop, "Inverse Layers", "Inverse filter"); -- cgit v1.2.3 From 97f2210157dbd36bc0a44674c8470202fe808301 Mon Sep 17 00:00:00 2001 From: YimingWu Date: Fri, 25 Mar 2022 20:13:04 +0800 Subject: GPencil: Cyclic flag for dot dash modifier Cyclic option per segment, allows interesting "loop" visual effects. Reviewed by: Antonio Vazquez (antoniov) Differential Revision: https://developer.blender.org/D14439 --- source/blender/gpencil_modifiers/intern/MOD_gpencildash.c | 4 ++++ source/blender/makesdna/DNA_gpencil_modifier_types.h | 10 +++++++++- source/blender/makesrna/intern/rna_gpencil_modifier.c | 13 +++++++++---- 3 files changed, 22 insertions(+), 5 deletions(-) (limited to 'source') diff --git a/source/blender/gpencil_modifiers/intern/MOD_gpencildash.c b/source/blender/gpencil_modifiers/intern/MOD_gpencildash.c index 25c7fdca9f6..e57b9df03f5 100644 --- a/source/blender/gpencil_modifiers/intern/MOD_gpencildash.c +++ b/source/blender/gpencil_modifiers/intern/MOD_gpencildash.c @@ -148,6 +148,9 @@ static bool stroke_dash(const bGPDstroke *gps, bGPDstroke *stroke = BKE_gpencil_stroke_new( ds->mat_nr < 0 ? gps->mat_nr : ds->mat_nr, size, gps->thickness); + if (ds->flag & GP_DASH_USE_CYCLIC) { + stroke->flag |= GP_STROKE_CYCLIC; + } for (int is = 0; is < size; is++) { bGPDspoint *p = &gps->points[new_stroke_offset + is]; @@ -337,6 +340,7 @@ static void panel_draw(const bContext *C, Panel *panel) uiItemR(sub, &ds_ptr, "radius", 0, NULL, ICON_NONE); uiItemR(sub, &ds_ptr, "opacity", 0, NULL, ICON_NONE); uiItemR(sub, &ds_ptr, "material_index", 0, NULL, ICON_NONE); + uiItemR(sub, &ds_ptr, "use_cyclic", 0, NULL, ICON_NONE); } gpencil_modifier_panel_end(layout, ptr); diff --git a/source/blender/makesdna/DNA_gpencil_modifier_types.h b/source/blender/makesdna/DNA_gpencil_modifier_types.h index 4f655e87a09..7568dc5ff9a 100644 --- a/source/blender/makesdna/DNA_gpencil_modifier_types.h +++ b/source/blender/makesdna/DNA_gpencil_modifier_types.h @@ -522,7 +522,7 @@ typedef struct DashGpencilModifierSegment { float radius; float opacity; int mat_nr; - int _pad; + int flag; } DashGpencilModifierSegment; typedef struct DashGpencilModifierData { @@ -546,6 +546,14 @@ typedef struct DashGpencilModifierData { } DashGpencilModifierData; +typedef enum eDashGpencil_Flag { + GP_DASH_INVERT_LAYER = (1 << 0), + GP_DASH_INVERT_PASS = (1 << 1), + GP_DASH_INVERT_LAYERPASS = (1 << 2), + GP_DASH_INVERT_MATERIAL = (1 << 3), + GP_DASH_USE_CYCLIC = (1 << 7), +} eDashGpencil_Flag; + typedef struct MirrorGpencilModifierData { GpencilModifierData modifier; struct Object *object; diff --git a/source/blender/makesrna/intern/rna_gpencil_modifier.c b/source/blender/makesrna/intern/rna_gpencil_modifier.c index bf3f506251c..9e25ddaf790 100644 --- a/source/blender/makesrna/intern/rna_gpencil_modifier.c +++ b/source/blender/makesrna/intern/rna_gpencil_modifier.c @@ -3743,6 +3743,11 @@ static void rna_def_modifier_gpencildash(BlenderRNA *brna) "Use this index on generated segment. -1 means using the existing material"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); + prop = RNA_def_property(srna, "use_cyclic", PROP_BOOLEAN, PROP_NONE); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_DASH_USE_CYCLIC); + RNA_def_property_ui_text(prop, "Use Cyclic", "Enable cyclic on individual stroke dashes"); + RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); + srna = RNA_def_struct(brna, "DashGpencilModifierData", "GpencilModifier"); RNA_def_struct_ui_text(srna, "Dash Modifier", "Create dot-dash effect for strokes"); RNA_def_struct_sdna(srna, "DashGpencilModifierData"); @@ -3794,17 +3799,17 @@ static void rna_def_modifier_gpencildash(BlenderRNA *brna) RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); prop = RNA_def_property(srna, "invert_layers", PROP_BOOLEAN, PROP_NONE); - RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_LENGTH_INVERT_LAYER); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_DASH_INVERT_LAYER); RNA_def_property_ui_text(prop, "Inverse Layers", "Inverse filter"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); prop = RNA_def_property(srna, "invert_materials", PROP_BOOLEAN, PROP_NONE); - RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_LENGTH_INVERT_MATERIAL); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_DASH_INVERT_MATERIAL); RNA_def_property_ui_text(prop, "Inverse Materials", "Inverse filter"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); prop = RNA_def_property(srna, "invert_material_pass", PROP_BOOLEAN, PROP_NONE); - RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_LENGTH_INVERT_PASS); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_DASH_INVERT_PASS); RNA_def_property_ui_text(prop, "Inverse Pass", "Inverse filter"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); @@ -3815,7 +3820,7 @@ static void rna_def_modifier_gpencildash(BlenderRNA *brna) RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); prop = RNA_def_property(srna, "invert_layer_pass", PROP_BOOLEAN, PROP_NONE); - RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_LENGTH_INVERT_LAYERPASS); + RNA_def_property_boolean_sdna(prop, NULL, "flag", GP_DASH_INVERT_LAYERPASS); RNA_def_property_ui_text(prop, "Inverse Pass", "Inverse filter"); RNA_def_property_update(prop, 0, "rna_GpencilModifier_update"); -- cgit v1.2.3 From c0016d85b2a62858229d31bd97399fbe2608f99d Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 25 Mar 2022 09:00:30 -0500 Subject: Curves: Add a utility to count curves of each type This commit adds a utility that returns an array with the number of curves of every type. One use case for this is detecting whether to remove handle or NURBS attributes when changing curve types. It's best to avoid using this when it's not necessary, but sometimes it can't really be avoided, and having a utility at least makes using an optimized version simple. In the future, this information can be cached in the curves runtime. Differential Revision: https://developer.blender.org/D14448 --- source/blender/blenkernel/BKE_curves.hh | 2 ++ .../blender/blenkernel/intern/curves_geometry.cc | 34 ++++++++++++++++++++++ .../blenkernel/intern/curves_geometry_test.cc | 22 ++++++++++++++ source/blender/makesdna/DNA_curves_types.h | 1 + 4 files changed, 59 insertions(+) (limited to 'source') diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh index 0f2ae8a02a6..96963dcbd8d 100644 --- a/source/blender/blenkernel/BKE_curves.hh +++ b/source/blender/blenkernel/BKE_curves.hh @@ -146,6 +146,8 @@ class CurvesGeometry : public ::CurvesGeometry { MutableSpan curve_types(); bool has_curve_with_type(const CurveType type) const; + /** Return the number of curves with each type. */ + std::array count_curve_types() const; MutableSpan positions(); Span positions() const; diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc index 5bb6a97fa49..207d0d173ac 100644 --- a/source/blender/blenkernel/intern/curves_geometry.cc +++ b/source/blender/blenkernel/intern/curves_geometry.cc @@ -277,6 +277,40 @@ bool CurvesGeometry::has_curve_with_type(const CurveType type) const return false; } +std::array CurvesGeometry::count_curve_types() const +{ + using CountsType = std::array; + + CountsType identity; + identity.fill(0); + + const VArray types = this->curve_types(); + if (types.is_single()) { + identity[types.get_internal_single()] = this->curves_num(); + return identity; + } + + Span types_span = types.get_internal_span(); + return threading::parallel_reduce( + this->curves_range(), + 2048, + identity, + [&](const IndexRange curves_range, const CountsType &init) { + CountsType result = init; + for (const int curve_index : curves_range) { + result[types_span[curve_index]]++; + } + return result; + }, + [](const CountsType &a, const CountsType &b) { + CountsType result = a; + for (const int i : IndexRange(CURVE_TYPES_NUM)) { + result[i] += b[i]; + } + return result; + }); +} + MutableSpan CurvesGeometry::positions() { this->position = (float(*)[3])CustomData_duplicate_referenced_layer_named( diff --git a/source/blender/blenkernel/intern/curves_geometry_test.cc b/source/blender/blenkernel/intern/curves_geometry_test.cc index bc99785de1c..574f90f2e51 100644 --- a/source/blender/blenkernel/intern/curves_geometry_test.cc +++ b/source/blender/blenkernel/intern/curves_geometry_test.cc @@ -63,6 +63,28 @@ TEST(curves_geometry, Move) EXPECT_EQ(second_other.offsets().data(), offsets_data); } +TEST(curves_geometry, TypeCount) +{ + CurvesGeometry curves = create_basic_curves(100, 10); + curves.curve_types().copy_from({ + CURVE_TYPE_BEZIER, + CURVE_TYPE_NURBS, + CURVE_TYPE_NURBS, + CURVE_TYPE_NURBS, + CURVE_TYPE_CATMULL_ROM, + CURVE_TYPE_CATMULL_ROM, + CURVE_TYPE_CATMULL_ROM, + CURVE_TYPE_POLY, + CURVE_TYPE_POLY, + CURVE_TYPE_POLY, + }); + std::array counts = curves.count_curve_types(); + EXPECT_EQ(counts[CURVE_TYPE_CATMULL_ROM], 3); + EXPECT_EQ(counts[CURVE_TYPE_POLY], 3); + EXPECT_EQ(counts[CURVE_TYPE_BEZIER], 1); + EXPECT_EQ(counts[CURVE_TYPE_NURBS], 3); +} + TEST(curves_geometry, CatmullRomEvaluation) { CurvesGeometry curves(4, 1); diff --git a/source/blender/makesdna/DNA_curves_types.h b/source/blender/makesdna/DNA_curves_types.h index f1626781fc6..97cc588e639 100644 --- a/source/blender/makesdna/DNA_curves_types.h +++ b/source/blender/makesdna/DNA_curves_types.h @@ -28,6 +28,7 @@ typedef enum CurveType { CURVE_TYPE_BEZIER = 2, CURVE_TYPE_NURBS = 3, } CurveType; +#define CURVE_TYPES_NUM 4 typedef enum HandleType { /** The handle can be moved anywhere, and doesn't influence the point's other handle. */ -- cgit v1.2.3 From cea51c1bb500eb2cfca425e1cae5dcc419dda2ce Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 25 Mar 2022 09:03:35 -0500 Subject: Curves: Bezier and general interpolate to evaluated utility This commit implements generic evaluation for Bezier curves (which is really just linear interpolation, since attributes are not stored on Bezier handles). For complete parity with the old curve type, we would have to add options for this (RNA: `Spline.radius_interpolation`), but it's not clear that we want to do that. This also adds a generic `interpolate_to_evaluate` utility on curves that hides the implementation details. Though there is theoretically a performance cost to that, without some abstraction calling code would usually be too complex. Differential Revision: https://developer.blender.org/D14447 --- source/blender/blenkernel/BKE_curves.hh | 16 +++++ source/blender/blenkernel/intern/curve_bezier.cc | 40 ++++++++++++ .../blender/blenkernel/intern/curves_geometry.cc | 35 +++++++++++ .../blenkernel/intern/curves_geometry_test.cc | 73 ++++++++++++++++++++++ 4 files changed, 164 insertions(+) (limited to 'source') diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh index 96963dcbd8d..82f77d83bec 100644 --- a/source/blender/blenkernel/BKE_curves.hh +++ b/source/blender/blenkernel/BKE_curves.hh @@ -266,6 +266,15 @@ class CurvesGeometry : public ::CurvesGeometry { Span evaluated_positions() const; + /** + * Evaluate a generic data to the standard evaluated points of a specific curve, + * defined by the resolution attribute or other factors, depending on the curve type. + * + * \warning This function expects offsets to the evaluated points for each curve to be + * calculated. That can be ensured with #ensure_evaluated_offsets. + */ + void interpolate_to_evaluated(int curve_index, GSpan src, GMutableSpan dst) const; + private: /** * Make sure the basis weights for NURBS curve's evaluated points are calculated. @@ -381,6 +390,13 @@ void calculate_evaluated_positions(Span positions, Span evaluated_offsets, MutableSpan evaluated_positions); +/** + * Evaluate generic data to the evaluated points, with counts for each segment described by + * #evaluated_offsets. Unlike other curve types, for Bezier curves generic data and positions + * are treated separately, since attribute values aren't stored for the handle control points. + */ +void interpolate_to_evaluated(GSpan src, Span evaluated_offsets, GMutableSpan dst); + } // namespace bezier namespace catmull_rom { diff --git a/source/blender/blenkernel/intern/curve_bezier.cc b/source/blender/blenkernel/intern/curve_bezier.cc index c02555dcf6a..8efe7a17a35 100644 --- a/source/blender/blenkernel/intern/curve_bezier.cc +++ b/source/blender/blenkernel/intern/curve_bezier.cc @@ -134,6 +134,46 @@ void calculate_evaluated_positions(const Span positions, } } +template +static inline void linear_interpolation(const T &a, const T &b, MutableSpan dst) +{ + dst.first() = a; + const float step = 1.0f / dst.size(); + for (const int i : dst.index_range().drop_front(1)) { + dst[i] = attribute_math::mix2(i * step, a, b); + } +} + +template +static void interpolate_to_evaluated(const Span src, + const Span evaluated_offsets, + MutableSpan dst) +{ + linear_interpolation(src.first(), src[1], dst.take_front(evaluated_offsets.first())); + + threading::parallel_for( + src.index_range().drop_back(1).drop_front(1), 512, [&](IndexRange range) { + for (const int i : range) { + const IndexRange segment_points = offsets_to_range(evaluated_offsets, i - 1); + linear_interpolation(src[i], src[i + 1], dst.slice(segment_points)); + } + }); + + const IndexRange last_segment_points(evaluated_offsets.last(1), + evaluated_offsets.last() - evaluated_offsets.last(1)); + linear_interpolation(src.last(), src.first(), dst.slice(last_segment_points)); +} + +void interpolate_to_evaluated(const GSpan src, const Span evaluated_offsets, GMutableSpan dst) +{ + attribute_math::convert_to_static_type(src.type(), [&](auto dummy) { + using T = decltype(dummy); + if constexpr (!std::is_void_v>) { + interpolate_to_evaluated(src.typed(), evaluated_offsets, dst.typed()); + } + }); +} + /** \} */ } // namespace blender::bke::curves::bezier diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc index 207d0d173ac..1dfd95ebb5b 100644 --- a/source/blender/blenkernel/intern/curves_geometry.cc +++ b/source/blender/blenkernel/intern/curves_geometry.cc @@ -689,6 +689,41 @@ Span CurvesGeometry::evaluated_positions() const return this->runtime->evaluated_position_cache; } +void CurvesGeometry::interpolate_to_evaluated(const int curve_index, + const GSpan src, + GMutableSpan dst) const +{ + BLI_assert(!this->runtime->offsets_cache_dirty); + BLI_assert(!this->runtime->nurbs_basis_cache_dirty); + const IndexRange points = this->points_for_curve(curve_index); + BLI_assert(src.size() == points.size()); + BLI_assert(dst.size() == this->evaluated_points_for_curve(curve_index).size()); + switch (this->curve_types()[curve_index]) { + case CURVE_TYPE_CATMULL_ROM: + curves::catmull_rom::interpolate_to_evaluated( + src, this->cyclic()[curve_index], this->resolution()[curve_index], dst); + break; + case CURVE_TYPE_POLY: + dst.type().copy_assign_n(src.data(), dst.data(), src.size()); + break; + case CURVE_TYPE_BEZIER: + curves::bezier::interpolate_to_evaluated( + src, this->runtime->bezier_evaluated_offsets.as_span().slice(points), dst); + break; + case CURVE_TYPE_NURBS: + curves::nurbs::interpolate_to_evaluated(this->runtime->nurbs_basis_cache[curve_index], + this->nurbs_orders()[curve_index], + this->nurbs_weights().slice(points), + src, + dst); + break; + default: + BLI_assert_unreachable(); + break; + } +} + + /** \} */ /* -------------------------------------------------------------------- */ diff --git a/source/blender/blenkernel/intern/curves_geometry_test.cc b/source/blender/blenkernel/intern/curves_geometry_test.cc index 574f90f2e51..e4dc9eead60 100644 --- a/source/blender/blenkernel/intern/curves_geometry_test.cc +++ b/source/blender/blenkernel/intern/curves_geometry_test.cc @@ -405,4 +405,77 @@ TEST(curves_geometry, NURBSEvaluation) } } +TEST(curves_geometry, BezierGenericEvaluation) +{ + CurvesGeometry curves(3, 1); + curves.curve_types().fill(CURVE_TYPE_BEZIER); + curves.resolution().fill(8); + curves.offsets().last() = 3; + + MutableSpan handles_left = curves.handle_positions_left(); + MutableSpan handles_right = curves.handle_positions_right(); + MutableSpan positions = curves.positions(); + positions.first() = {-1, 0, 0}; + handles_right.first() = {-1, 1, 0}; + handles_left[1] = {0, 0, 0}; + positions[1] = {1, 0, 0}; + handles_right[1] = {2, 0, 0}; + handles_left.last() = {1, 1, 0}; + positions.last() = {2, 1, 0}; + + /* Dangling handles shouldn't be used in a non-cyclic curve. */ + handles_left.first() = {100, 100, 100}; + handles_right.last() = {100, 100, 100}; + + Span evaluated_positions = curves.evaluated_positions(); + static const Array result_1{{ + {-1.0f, 0.0f, 0.0f}, + {-0.955078f, 0.287109f, 0.0f}, + {-0.828125f, 0.421875f, 0.0f}, + {-0.630859f, 0.439453f, 0.0f}, + {-0.375f, 0.375f, 0.0f}, + {-0.0722656f, 0.263672f, 0.0f}, + {0.265625f, 0.140625f, 0.0f}, + {0.626953f, 0.0410156f, 0.0f}, + {1.0f, 0.0f, 0.0f}, + {1.28906f, 0.0429688f, 0.0f}, + {1.4375f, 0.15625f, 0.0f}, + {1.49219f, 0.316406f, 0.0f}, + {1.5f, 0.5f, 0.0f}, + {1.50781f, 0.683594f, 0.0f}, + {1.5625f, 0.84375f, 0.0f}, + {1.71094f, 0.957031f, 0.0f}, + {2.0f, 1.0f, 0.0f}, + }}; + for (const int i : evaluated_positions.index_range()) { + EXPECT_V3_NEAR(evaluated_positions[i], result_1[i], 1e-5f); + } + + Array radii{{0.0f, 1.0f, 2.0f}}; + Array evaluated_radii(17); + curves.interpolate_to_evaluated(0, radii.as_span(), evaluated_radii.as_mutable_span()); + static const Array result_2{{ + 0.0f, + 0.125f, + 0.25f, + 0.375f, + 0.5f, + 0.625f, + 0.75f, + 0.875f, + 1.0f, + 1.125f, + 1.25f, + 1.375f, + 1.5f, + 1.625f, + 1.75f, + 1.875f, + 2.0f, + }}; + for (const int i : evaluated_radii.index_range()) { + EXPECT_NEAR(evaluated_radii[i], result_2[i], 1e-6f); + } +} + } // namespace blender::bke::tests -- cgit v1.2.3 From 1243cb803e7d096d27e2b5dcdfa05bf0367e248c Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 25 Mar 2022 09:12:31 -0500 Subject: Cleanup: Add asserts, remove default case --- source/blender/blenkernel/intern/curve_bezier.cc | 4 ++++ source/blender/blenkernel/intern/curves_geometry.cc | 13 +++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/intern/curve_bezier.cc b/source/blender/blenkernel/intern/curve_bezier.cc index 8efe7a17a35..b11216983b2 100644 --- a/source/blender/blenkernel/intern/curve_bezier.cc +++ b/source/blender/blenkernel/intern/curve_bezier.cc @@ -149,6 +149,10 @@ static void interpolate_to_evaluated(const Span src, const Span evaluated_offsets, MutableSpan dst) { + BLI_assert(!src.is_empty()); + BLI_assert(dst.size() == src.size()); + BLI_assert(evaluated_offsets.last() == dst.size()); + linear_interpolation(src.first(), src[1], dst.take_front(evaluated_offsets.first())); threading::parallel_for( diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc index 1dfd95ebb5b..7ceaa8f0f37 100644 --- a/source/blender/blenkernel/intern/curves_geometry.cc +++ b/source/blender/blenkernel/intern/curves_geometry.cc @@ -702,28 +702,25 @@ void CurvesGeometry::interpolate_to_evaluated(const int curve_index, case CURVE_TYPE_CATMULL_ROM: curves::catmull_rom::interpolate_to_evaluated( src, this->cyclic()[curve_index], this->resolution()[curve_index], dst); - break; + return; case CURVE_TYPE_POLY: dst.type().copy_assign_n(src.data(), dst.data(), src.size()); - break; + return; case CURVE_TYPE_BEZIER: curves::bezier::interpolate_to_evaluated( src, this->runtime->bezier_evaluated_offsets.as_span().slice(points), dst); - break; + return; case CURVE_TYPE_NURBS: curves::nurbs::interpolate_to_evaluated(this->runtime->nurbs_basis_cache[curve_index], this->nurbs_orders()[curve_index], this->nurbs_weights().slice(points), src, dst); - break; - default: - BLI_assert_unreachable(); - break; + return; } + BLI_assert_unreachable(); } - /** \} */ /* -------------------------------------------------------------------- */ -- cgit v1.2.3 From 378022c7973db54f1ad8cf335a9359c3bf27ddb5 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 25 Mar 2022 09:57:10 -0500 Subject: BLI: Adjust interpolation to support integers, other tweaks In order to allow interpolation of integers with a float, add a separate template parameter for the factor and multiplication types. Also move some helper constexpr variables to the "base" header (reversing the dependency to "base" -> "vector"). This also adds a distance function for scalar types, which is helpful to allow sharing code between vectors and basic types. Differential Revision: https://developer.blender.org/D14446 --- source/blender/blenlib/BLI_math_base.hh | 22 +++++++++++++++++----- source/blender/blenlib/BLI_math_vec_types.hh | 11 +---------- source/blender/blenlib/BLI_math_vector.hh | 6 +++--- source/blender/blenlib/tests/BLI_math_base_test.cc | 5 +++++ .../blender/blenlib/tests/BLI_math_vector_test.cc | 20 ++++++++++++++++++++ 5 files changed, 46 insertions(+), 18 deletions(-) (limited to 'source') diff --git a/source/blender/blenlib/BLI_math_base.hh b/source/blender/blenlib/BLI_math_base.hh index 6a988eda8a9..83f414f853a 100644 --- a/source/blender/blenlib/BLI_math_base.hh +++ b/source/blender/blenlib/BLI_math_base.hh @@ -12,7 +12,6 @@ #include #include "BLI_math_base_safe.h" -#include "BLI_math_vec_types.hh" #include "BLI_utildefines.h" #ifdef WITH_GMP @@ -21,6 +20,15 @@ namespace blender::math { +template +inline constexpr bool is_math_float_type = (std::is_floating_point_v +#ifdef WITH_GMP + || std::is_same_v +#endif +); + +template inline constexpr bool is_math_integral_type = std::is_integral_v; + template inline bool is_zero(const T &a) { return a == T(0); @@ -84,19 +92,23 @@ template))> inline T ceil(const return std::ceil(a); } +template inline T distance(const T &a, const T &b) +{ + return std::abs(a - b); +} + template))> inline T fract(const T &a) { return a - std::floor(a); } -template))> -inline T interpolate(const T &a, const T &b, const T &t) +template))> +inline T interpolate(const T &a, const T &b, const FactorT &t) { return a * (1 - t) + b * t; } -template))> -inline T midpoint(const T &a, const T &b) +template inline T midpoint(const T &a, const T &b) { return (a + b) * T(0.5); } diff --git a/source/blender/blenlib/BLI_math_vec_types.hh b/source/blender/blenlib/BLI_math_vec_types.hh index 389307e331d..d9524eae746 100644 --- a/source/blender/blenlib/BLI_math_vec_types.hh +++ b/source/blender/blenlib/BLI_math_vec_types.hh @@ -321,7 +321,7 @@ template struct vec_base : public vec_struct_base BLI_VEC_OP_IMPL(ret, i, ret[i] = a[i] * b[i]); } - friend vec_base operator*(const vec_base &a, T b) + template friend vec_base operator*(const vec_base &a, FactorT b) { BLI_VEC_OP_IMPL(ret, i, ret[i] = a[i] * b); } @@ -579,13 +579,4 @@ using double2 = vec_base; using double3 = vec_base; using double4 = vec_base; -template -inline constexpr bool is_math_float_type = (std::is_floating_point_v -#ifdef WITH_GMP - || std::is_same_v -#endif -); - -template inline constexpr bool is_math_integral_type = std::is_integral_v; - } // namespace blender diff --git a/source/blender/blenlib/BLI_math_vector.hh b/source/blender/blenlib/BLI_math_vector.hh index b1a3242ae52..b9f0939674e 100644 --- a/source/blender/blenlib/BLI_math_vector.hh +++ b/source/blender/blenlib/BLI_math_vector.hh @@ -10,7 +10,7 @@ #include #include -#include "BLI_math_base_safe.h" +#include "BLI_math_base.hh" #include "BLI_math_vec_types.hh" #include "BLI_span.hh" #include "BLI_utildefines.h" @@ -339,10 +339,10 @@ inline vec_base cross_poly(Span> poly) return n; } -template))> +template))> inline vec_base interpolate(const vec_base &a, const vec_base &b, - const T &t) + const FactorT &t) { return a * (1 - t) + b * t; } diff --git a/source/blender/blenlib/tests/BLI_math_base_test.cc b/source/blender/blenlib/tests/BLI_math_base_test.cc index 62f2b2775d0..dd35deef4a8 100644 --- a/source/blender/blenlib/tests/BLI_math_base_test.cc +++ b/source/blender/blenlib/tests/BLI_math_base_test.cc @@ -151,4 +151,9 @@ TEST(math_base, Midpoint) EXPECT_NEAR(math::midpoint(100.0f, 200.0f), 150.0f, 1e-4f); } +TEST(math_base, InterpolateInt) +{ + EXPECT_EQ(math::interpolate(100, 200, 0.4f), 140); +} + } // namespace blender::tests diff --git a/source/blender/blenlib/tests/BLI_math_vector_test.cc b/source/blender/blenlib/tests/BLI_math_vector_test.cc index 8c310645d6d..282be5f1963 100644 --- a/source/blender/blenlib/tests/BLI_math_vector_test.cc +++ b/source/blender/blenlib/tests/BLI_math_vector_test.cc @@ -85,4 +85,24 @@ TEST(math_vector, Clamp) EXPECT_EQ(result_2.z, -50); } +TEST(math_vector, InterpolateInt) +{ + const int3 a(0, -100, 50); + const int3 b(0, 100, 100); + const int3 result = math::interpolate(a, b, 0.75); + EXPECT_EQ(result.x, 0); + EXPECT_EQ(result.y, 50); + EXPECT_EQ(result.z, 87); +} + +TEST(math_vector, InterpolateFloat) +{ + const float3 a(40.0f, -100.0f, 50.0f); + const float3 b(20.0f, 100.0f, 100.0f); + const float3 result = math::interpolate(a, b, 0.5); + EXPECT_FLOAT_EQ(result.x, 30.0f); + EXPECT_FLOAT_EQ(result.y, 0.0f); + EXPECT_FLOAT_EQ(result.z, 75.0f); +} + } // namespace blender::tests -- cgit v1.2.3 From c671a26637a7be3dc95cefdfb2cf566d43ef6627 Mon Sep 17 00:00:00 2001 From: Michael Kowalski Date: Fri, 25 Mar 2022 09:29:39 -0600 Subject: USD: Support building against USD 21.11+ For 3.2 USD will be bumped to a newer version with some slight API changes, however since we cannot simultaneously land the libs for all platforms as well as these code changes, we'll have to support both 21.02 and 21.11+ for at least a short period of time making the code slightly more messy than it could have been. Differential Revision: https://developer.blender.org/D14184 Reviewed by: sybren --- source/blender/io/usd/intern/usd_reader_light.cc | 21 ++++++---- source/blender/io/usd/intern/usd_reader_stage.cc | 17 +++++++- source/blender/io/usd/intern/usd_reader_xform.cc | 2 +- source/blender/io/usd/intern/usd_writer_light.cc | 48 +++++++++++++++++----- .../blender/io/usd/intern/usd_writer_material.cc | 12 +++--- 5 files changed, 73 insertions(+), 27 deletions(-) (limited to 'source') diff --git a/source/blender/io/usd/intern/usd_reader_light.cc b/source/blender/io/usd/intern/usd_reader_light.cc index 649ff45a6d0..55b9557dfb5 100644 --- a/source/blender/io/usd/intern/usd_reader_light.cc +++ b/source/blender/io/usd/intern/usd_reader_light.cc @@ -9,8 +9,6 @@ #include "DNA_light_types.h" #include "DNA_object_types.h" -#include - #include #include #include @@ -40,14 +38,17 @@ void USDLightReader::read_object_data(Main *bmain, const double motionSampleTime if (!prim_) { return; } +#if PXR_VERSION >= 2111 + pxr::UsdLuxLightAPI light_api(prim_); +#else + pxr::UsdLuxLight light_api(prim_); +#endif - pxr::UsdLuxLight light_prim(prim_); - - if (!light_prim) { + if (!light_api) { return; } - pxr::UsdLuxShapingAPI shaping_api(light_prim); + pxr::UsdLuxShapingAPI shaping_api; /* Set light type. */ @@ -63,6 +64,8 @@ void USDLightReader::read_object_data(Main *bmain, const double motionSampleTime else if (prim_.IsA()) { blight->type = LA_LOCAL; + shaping_api = pxr::UsdLuxShapingAPI(prim_); + if (shaping_api && shaping_api.GetShapingConeAngleAttr().IsAuthored()) { blight->type = LA_SPOT; } @@ -73,7 +76,7 @@ void USDLightReader::read_object_data(Main *bmain, const double motionSampleTime /* Set light values. */ - if (pxr::UsdAttribute intensity_attr = light_prim.GetIntensityAttr()) { + if (pxr::UsdAttribute intensity_attr = light_api.GetIntensityAttr()) { float intensity = 0.0f; if (intensity_attr.Get(&intensity, motionSampleTime)) { blight->energy = intensity * this->import_params_.light_intensity_scale; @@ -92,14 +95,14 @@ void USDLightReader::read_object_data(Main *bmain, const double motionSampleTime light_prim.GetDiffuseAttr().Get(&diffuse, motionSampleTime); #endif - if (pxr::UsdAttribute spec_attr = light_prim.GetSpecularAttr()) { + if (pxr::UsdAttribute spec_attr = light_api.GetSpecularAttr()) { float spec = 0.0f; if (spec_attr.Get(&spec, motionSampleTime)) { blight->spec_fac = spec; } } - if (pxr::UsdAttribute color_attr = light_prim.GetColorAttr()) { + if (pxr::UsdAttribute color_attr = light_api.GetColorAttr()) { pxr::GfVec3f color; if (color_attr.Get(&color, motionSampleTime)) { blight->r = color[0]; diff --git a/source/blender/io/usd/intern/usd_reader_stage.cc b/source/blender/io/usd/intern/usd_reader_stage.cc index 06f7d3ce3f5..583c58a1356 100644 --- a/source/blender/io/usd/intern/usd_reader_stage.cc +++ b/source/blender/io/usd/intern/usd_reader_stage.cc @@ -18,7 +18,13 @@ #include #include #include -#include + +#if PXR_VERSION >= 2111 +# include +# include +#else +# include +#endif #include @@ -55,7 +61,12 @@ USDPrimReader *USDStageReader::create_reader_if_allowed(const pxr::UsdPrim &prim if (params_.import_meshes && prim.IsA()) { return new USDMeshReader(prim, params_, settings_); } +#if PXR_VERSION >= 2111 + if (params_.import_lights && (prim.IsA() || + prim.IsA())) { +#else if (params_.import_lights && prim.IsA()) { +#endif return new USDLightReader(prim, params_, settings_); } if (params_.import_volumes && prim.IsA()) { @@ -82,7 +93,11 @@ USDPrimReader *USDStageReader::create_reader(const pxr::UsdPrim &prim) if (prim.IsA()) { return new USDMeshReader(prim, params_, settings_); } +#if PXR_VERSION >= 2111 + if (prim.IsA() || prim.IsA()) { +#else if (prim.IsA()) { +#endif return new USDLightReader(prim, params_, settings_); } if (prim.IsA()) { diff --git a/source/blender/io/usd/intern/usd_reader_xform.cc b/source/blender/io/usd/intern/usd_reader_xform.cc index 4cb7d4e1845..bc6e2dd5297 100644 --- a/source/blender/io/usd/intern/usd_reader_xform.cc +++ b/source/blender/io/usd/intern/usd_reader_xform.cc @@ -131,7 +131,7 @@ bool USDXformReader::is_root_xform_prim() const return false; } - if (prim_.IsInMaster()) { + if (prim_.IsInPrototype()) { /* We don't consider prototypes to be root prims, * because we never want to apply global scaling * or rotations to the prototypes themselves. */ diff --git a/source/blender/io/usd/intern/usd_writer_light.cc b/source/blender/io/usd/intern/usd_writer_light.cc index 282393bbcd2..982bc31d767 100644 --- a/source/blender/io/usd/intern/usd_writer_light.cc +++ b/source/blender/io/usd/intern/usd_writer_light.cc @@ -33,7 +33,12 @@ void USDLightWriter::do_write(HierarchyContext &context) pxr::UsdTimeCode timecode = get_export_time_code(); Light *light = static_cast(context.object->data); - pxr::UsdLuxLight usd_light; +#if PXR_VERSION >= 2111 + pxr::UsdLuxLightAPI usd_light_api; +#else + pxr::UsdLuxLight usd_light_api; + +#endif switch (light->type) { case LA_AREA: @@ -42,21 +47,33 @@ void USDLightWriter::do_write(HierarchyContext &context) case LA_AREA_ELLIPSE: { /* An ellipse light will deteriorate into a disk light. */ pxr::UsdLuxDiskLight disk_light = pxr::UsdLuxDiskLight::Define(stage, usd_path); disk_light.CreateRadiusAttr().Set(light->area_size, timecode); - usd_light = disk_light; +#if PXR_VERSION >= 2111 + usd_light_api = disk_light.LightAPI(); +#else + usd_light_api = disk_light; +#endif break; } case LA_AREA_RECT: { pxr::UsdLuxRectLight rect_light = pxr::UsdLuxRectLight::Define(stage, usd_path); rect_light.CreateWidthAttr().Set(light->area_size, timecode); rect_light.CreateHeightAttr().Set(light->area_sizey, timecode); - usd_light = rect_light; +#if PXR_VERSION >= 2111 + usd_light_api = rect_light.LightAPI(); +#else + usd_light_api = rect_light; +#endif break; } case LA_AREA_SQUARE: { pxr::UsdLuxRectLight rect_light = pxr::UsdLuxRectLight::Define(stage, usd_path); rect_light.CreateWidthAttr().Set(light->area_size, timecode); rect_light.CreateHeightAttr().Set(light->area_size, timecode); - usd_light = rect_light; +#if PXR_VERSION >= 2111 + usd_light_api = rect_light.LightAPI(); +#else + usd_light_api = rect_light; +#endif break; } } @@ -64,12 +81,23 @@ void USDLightWriter::do_write(HierarchyContext &context) case LA_LOCAL: { pxr::UsdLuxSphereLight sphere_light = pxr::UsdLuxSphereLight::Define(stage, usd_path); sphere_light.CreateRadiusAttr().Set(light->area_size, timecode); - usd_light = sphere_light; +#if PXR_VERSION >= 2111 + usd_light_api = sphere_light.LightAPI(); +#else + usd_light_api = sphere_light; +#endif break; } - case LA_SUN: - usd_light = pxr::UsdLuxDistantLight::Define(stage, usd_path); + case LA_SUN: { + pxr::UsdLuxDistantLight distant_light = pxr::UsdLuxDistantLight::Define(stage, usd_path); + /* TODO(makowalski): set angle attribute here. */ +#if PXR_VERSION >= 2111 + usd_light_api = distant_light.LightAPI(); +#else + usd_light_api = distant_light; +#endif break; + } default: BLI_assert_msg(0, "is_supported() returned true for unsupported light type"); } @@ -85,10 +113,10 @@ void USDLightWriter::do_write(HierarchyContext &context) else { usd_intensity = light->energy / 100.0f; } - usd_light.CreateIntensityAttr().Set(usd_intensity, timecode); + usd_light_api.CreateIntensityAttr().Set(usd_intensity, timecode); - usd_light.CreateColorAttr().Set(pxr::GfVec3f(light->r, light->g, light->b), timecode); - usd_light.CreateSpecularAttr().Set(light->spec_fac, timecode); + usd_light_api.CreateColorAttr().Set(pxr::GfVec3f(light->r, light->g, light->b), timecode); + usd_light_api.CreateSpecularAttr().Set(light->spec_fac, timecode); } } // namespace blender::io::usd diff --git a/source/blender/io/usd/intern/usd_writer_material.cc b/source/blender/io/usd/intern/usd_writer_material.cc index 29ab0479f6e..b548a666ef7 100644 --- a/source/blender/io/usd/intern/usd_writer_material.cc +++ b/source/blender/io/usd/intern/usd_writer_material.cc @@ -163,7 +163,7 @@ void create_usd_preview_surface_material(const USDExporterContext &usd_export_co created_shader = create_usd_preview_shader(usd_export_context, usd_material, input_node); preview_surface.CreateInput(input_spec.input_name, input_spec.input_type) - .ConnectToSource(created_shader, input_spec.source_name); + .ConnectToSource(created_shader.ConnectableAPI(), input_spec.source_name); } else if (input_spec.set_default_value) { /* Set hardcoded value. */ @@ -217,7 +217,7 @@ void create_usd_viewport_material(const USDExporterContext &usd_export_context, shader.CreateInput(usdtokens::metallic, pxr::SdfValueTypeNames->Float).Set(material->metallic); /* Connect the shader and the material together. */ - usd_material.CreateSurfaceOutput().ConnectToSource(shader, usdtokens::surface); + usd_material.CreateSurfaceOutput().ConnectToSource(shader.ConnectableAPI(), usdtokens::surface); } /* Return USD Preview Surface input map singleton. */ @@ -293,12 +293,12 @@ static void create_uvmap_shader(const USDExporterContext &usd_export_context, uv_shader.CreateInput(usdtokens::varname, pxr::SdfValueTypeNames->Token) .Set(pxr::TfToken(uv_set)); usd_tex_shader.CreateInput(usdtokens::st, pxr::SdfValueTypeNames->Float2) - .ConnectToSource(uv_shader, usdtokens::result); + .ConnectToSource(uv_shader.ConnectableAPI(), usdtokens::result); } else { uv_shader.CreateInput(usdtokens::varname, pxr::SdfValueTypeNames->Token).Set(default_uv); usd_tex_shader.CreateInput(usdtokens::st, pxr::SdfValueTypeNames->Float2) - .ConnectToSource(uv_shader, usdtokens::result); + .ConnectToSource(uv_shader.ConnectableAPI(), usdtokens::result); } } @@ -313,7 +313,7 @@ static void create_uvmap_shader(const USDExporterContext &usd_export_context, if (uv_shader.GetPrim().IsValid()) { uv_shader.CreateInput(usdtokens::varname, pxr::SdfValueTypeNames->Token).Set(default_uv); usd_tex_shader.CreateInput(usdtokens::st, pxr::SdfValueTypeNames->Float2) - .ConnectToSource(uv_shader, usdtokens::result); + .ConnectToSource(uv_shader.ConnectableAPI(), usdtokens::result); } } } @@ -488,7 +488,7 @@ static pxr::UsdShadeShader create_usd_preview_shader(const USDExporterContext &u case SH_NODE_BSDF_DIFFUSE: case SH_NODE_BSDF_PRINCIPLED: { shader.CreateIdAttr(pxr::VtValue(usdtokens::preview_surface)); - material.CreateSurfaceOutput().ConnectToSource(shader, usdtokens::surface); + material.CreateSurfaceOutput().ConnectToSource(shader.ConnectableAPI(), usdtokens::surface); break; } -- cgit v1.2.3 From 2fc77071b51a7c3ee16c50836d380ed339681992 Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Fri, 25 Mar 2022 16:30:10 +0100 Subject: Image editor: not updating after image operation. Fixes T96324, T96312, T96323 --- source/blender/editors/space_image/image_ops.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'source') diff --git a/source/blender/editors/space_image/image_ops.c b/source/blender/editors/space_image/image_ops.c index 1c4a1d7e8c9..aa77aab2283 100644 --- a/source/blender/editors/space_image/image_ops.c +++ b/source/blender/editors/space_image/image_ops.c @@ -2790,8 +2790,7 @@ static int image_flip_exec(bContext *C, wmOperator *op) ED_image_undo_push_end(); - /* force GPU re-upload, all image is invalid. */ - BKE_image_free_gputextures(ima); + BKE_image_partial_update_mark_full_update(ima); WM_event_add_notifier(C, NC_IMAGE | NA_EDITED, ima); @@ -2910,8 +2909,7 @@ static int image_invert_exec(bContext *C, wmOperator *op) ED_image_undo_push_end(); - /* Force GPU re-upload, all image is invalid. */ - BKE_image_free_gputextures(ima); + BKE_image_partial_update_mark_full_update(ima); WM_event_add_notifier(C, NC_IMAGE | NA_EDITED, ima); @@ -3001,8 +2999,7 @@ static int image_scale_exec(bContext *C, wmOperator *op) ED_image_undo_push_end(); - /* Force GPU re-upload, all image is invalid. */ - BKE_image_free_gputextures(ima); + BKE_image_partial_update_mark_full_update(ima); DEG_id_tag_update(&ima->id, 0); WM_event_add_notifier(C, NC_IMAGE | NA_EDITED, ima); -- cgit v1.2.3 From 19bcfba56fdaf11eab08a56deeec89e427a3fd0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 25 Mar 2022 16:52:14 +0100 Subject: USD I/O: explicitly set or clear the OPTYPE_UNDO flag Exporting USD cannot be undone, but importing should be undo'able. --- source/blender/editors/io/io_usd.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'source') diff --git a/source/blender/editors/io/io_usd.c b/source/blender/editors/io/io_usd.c index c4f1ae88d28..cf28c88edf0 100644 --- a/source/blender/editors/io/io_usd.c +++ b/source/blender/editors/io/io_usd.c @@ -201,6 +201,8 @@ void WM_OT_usd_export(struct wmOperatorType *ot) ot->poll = WM_operator_winactive; ot->ui = wm_usd_export_draw; + ot->flag = OPTYPE_REGISTER; /* No UNDO possible. */ + WM_operator_properties_filesel(ot, FILE_TYPE_FOLDER | FILE_TYPE_USD, FILE_BLENDER, @@ -459,6 +461,8 @@ void WM_OT_usd_import(struct wmOperatorType *ot) ot->poll = WM_operator_winactive; ot->ui = wm_usd_import_draw; + ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; + WM_operator_properties_filesel(ot, FILE_TYPE_FOLDER | FILE_TYPE_USD, FILE_BLENDER, -- cgit v1.2.3 From 2631b44e719b8b74dc9c166caf855491169b054f Mon Sep 17 00:00:00 2001 From: Ray Molenkamp Date: Fri, 25 Mar 2022 10:15:11 -0600 Subject: MSVC: Fix linker issue with USD USD requires to be linked with /WHOLEARCHIVE so the linker won't remove their static initializers. This strangely has never worked for MSVC since the flags were set on the LINK_FLAGS property which is only used to link .dll and .exe files, given this is a static lib, the flags were not used, nor did CMake propagate the link directive to the final targets that did link. Not quite sure how this has not lead to more problems in the past. Setting the link directive on the INTERFACE_LINK_OPTIONS makes cmake do the right thing. Differential Revision: https://developer.blender.org/D14394 Reviewed by: sybren --- source/blender/io/usd/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'source') diff --git a/source/blender/io/usd/CMakeLists.txt b/source/blender/io/usd/CMakeLists.txt index 01d0cfcd302..2b5ea39617e 100644 --- a/source/blender/io/usd/CMakeLists.txt +++ b/source/blender/io/usd/CMakeLists.txt @@ -107,10 +107,10 @@ list(APPEND LIB blender_add_lib(bf_usd "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") if(WIN32) - set_property(TARGET bf_usd APPEND_STRING PROPERTY LINK_FLAGS_DEBUG " /WHOLEARCHIVE:${USD_DEBUG_LIB}") - set_property(TARGET bf_usd APPEND_STRING PROPERTY LINK_FLAGS_RELEASE " /WHOLEARCHIVE:${USD_RELEASE_LIB}") - set_property(TARGET bf_usd APPEND_STRING PROPERTY LINK_FLAGS_RELWITHDEBINFO " /WHOLEARCHIVE:${USD_RELEASE_LIB}") - set_property(TARGET bf_usd APPEND_STRING PROPERTY LINK_FLAGS_MINSIZEREL " /WHOLEARCHIVE:${USD_RELEASE_LIB}") + set_property(TARGET bf_usd APPEND_STRING PROPERTY INTERFACE_LINK_OPTIONS "$<$:/WHOLEARCHIVE:${USD_DEBUG_LIB}>") + set_property(TARGET bf_usd APPEND_STRING PROPERTY INTERFACE_LINK_OPTIONS "$<$:/WHOLEARCHIVE:${USD_RELEASE_LIB}>") + set_property(TARGET bf_usd APPEND_STRING PROPERTY INTERFACE_LINK_OPTIONS "$<$:/WHOLEARCHIVE:${USD_RELEASE_LIB}>") + set_property(TARGET bf_usd APPEND_STRING PROPERTY INTERFACE_LINK_OPTIONS "$<$:/WHOLEARCHIVE:${USD_RELEASE_LIB}>") endif() # Source: https://github.com/PixarAnimationStudios/USD/blob/master/BUILDING.md#linking-whole-archives -- cgit v1.2.3 From ba49345705a3fc8ad8e5f4336ea960aec67f534e Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 25 Mar 2022 19:00:04 +0100 Subject: Fix Outliner highlighting multiple base elements in different libraries In the Blender File display mode of the Outliner, mouse hovering a "base" element (e.g. "Objects", "Materials", ...) would also highlight that same base element in other libraries linked into the scene. In fact operations like (un)collapsing would be applied to both too. Issue was that we'd always use the listbase containing the data-blocks from the current main as a way to identify the tree element. So for the same data-block types we'd use the same listbase pointers. Instead use the the library pointer + a per library index. --- source/blender/editors/space_outliner/tree/tree_display_libraries.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source') diff --git a/source/blender/editors/space_outliner/tree/tree_display_libraries.cc b/source/blender/editors/space_outliner/tree/tree_display_libraries.cc index 0023b7c7b62..476bbdb63ae 100644 --- a/source/blender/editors/space_outliner/tree/tree_display_libraries.cc +++ b/source/blender/editors/space_outliner/tree/tree_display_libraries.cc @@ -150,7 +150,7 @@ TreeElement *TreeDisplayLibraries::add_library_contents(Main &mainvar, ListBase } else { ten = outliner_add_element( - &space_outliner_, &tenlib->subtree, lbarray[a], nullptr, TSE_ID_BASE, 0); + &space_outliner_, &tenlib->subtree, lib, nullptr, TSE_ID_BASE, a); ten->directdata = lbarray[a]; ten->name = outliner_idcode_to_plural(GS(id->name)); } -- cgit v1.2.3