Commit: fcf1a9ff71e20e4af56815c4db1816d786298c3f Author: Bastien Montagne Date: Thu Jul 7 16:20:27 2022 +0200 Branches: master https://developer.blender.org/rBfcf1a9ff71e20e4af56815c4db1816d786298c3f
Fix T99256: Regression: Meta balls segfaulting copy-to-selected. Revealed by rB43167a2c251b, but actuall issue is the `rna_MetaBall_update_data` function expecting a never-NULL `scene` pointer, which is not guaranteed. This lead to refactoring the duo `rna_MetaBall_update_data`/`BKE_mball_properties_copy`, since it was doing a very sub-optimal O(n^2) process, new code is O(2n) now (n being the number of Objects). Not sure why the objects were processed through the existing bases of the existing scene in the first place, this could miss a lot of affected objects (e.g. in case said objects are in an excluded collection, etc.). Also noticed that both old and new implementation can fail to fully propagate changes to all affected meta-balls in some specific corner cases, added a comment about it in the code. Reviewed By: sergey Maniphest Tasks: T99256 Differential Revision: https://developer.blender.org/D15338 =================================================================== M source/blender/blenkernel/BKE_mball.h M source/blender/blenkernel/intern/mball.c M source/blender/makesrna/intern/rna_access.c M source/blender/makesrna/intern/rna_meta.c =================================================================== diff --git a/source/blender/blenkernel/BKE_mball.h b/source/blender/blenkernel/BKE_mball.h index f40d0bb3004..5a4988c7a5f 100644 --- a/source/blender/blenkernel/BKE_mball.h +++ b/source/blender/blenkernel/BKE_mball.h @@ -24,14 +24,25 @@ struct MetaBall *BKE_mball_add(struct Main *bmain, const char *name); bool BKE_mball_is_any_selected(const struct MetaBall *mb); bool BKE_mball_is_any_selected_multi(struct Base **bases, int bases_len); bool BKE_mball_is_any_unselected(const struct MetaBall *mb); -bool BKE_mball_is_basis_for(struct Object *ob1, struct Object *ob2); + +/** + * Return `true` if `ob1` and `ob2` are part of the same metaBall group. + * + * \note Currently checks whether their two base names (without numerical suffix) is the same. + */ +bool BKE_mball_is_same_group(const struct Object *ob1, const struct Object *ob2); +/** + * Return `true` if `ob1` and `ob2` are part of the same metaBall group, and `ob1` is its + * basis. + */ +bool BKE_mball_is_basis_for(const struct Object *ob1, const struct Object *ob2); /** * Test, if \a ob is a basis meta-ball. * * It test last character of Object ID name. * If last character is digit it return 0, else it return 1. */ -bool BKE_mball_is_basis(struct Object *ob); +bool BKE_mball_is_basis(const struct Object *ob); /** * This function finds the basis meta-ball. * @@ -58,13 +69,14 @@ struct BoundBox *BKE_mball_boundbox_get(struct Object *ob); float *BKE_mball_make_orco(struct Object *ob, struct ListBase *dispbase); /** - * Copy some properties from object to other meta-ball object with same base name. + * Copy some properties from a meta-ball obdata to all other meta-ball obdata belonging to the same + * family (i.e. object sharing the same name basis). * * When some properties (wire-size, threshold, update flags) of meta-ball are changed, then this * properties are copied to all meta-balls in same "group" (meta-balls with same base name: * `MBall`, `MBall.001`, `MBall.002`, etc). The most important is to copy properties to the base - * meta-ball, because this meta-ball influence polygonization of meta-balls. */ -void BKE_mball_properties_copy(struct Scene *scene, struct Object *active_object); + * meta-ball, because this meta-ball influences polygonization of meta-balls. */ +void BKE_mball_properties_copy(struct Main *bmain, struct MetaBall *active_metaball); bool BKE_mball_minmax_ex( const struct MetaBall *mb, float min[3], float max[3], const float obmat[4][4], short flag); diff --git a/source/blender/blenkernel/intern/mball.c b/source/blender/blenkernel/intern/mball.c index 1340e53f06e..819bbde6556 100644 --- a/source/blender/blenkernel/intern/mball.c +++ b/source/blender/blenkernel/intern/mball.c @@ -347,7 +347,7 @@ float *BKE_mball_make_orco(Object *ob, ListBase *dispbase) return orcodata; } -bool BKE_mball_is_basis(Object *ob) +bool BKE_mball_is_basis(const Object *ob) { /* Meta-Ball Basis Notes from Blender-2.5x * ======================================= @@ -370,7 +370,7 @@ bool BKE_mball_is_basis(Object *ob) return (!isdigit(ob->id.name[len - 1])); } -bool BKE_mball_is_basis_for(Object *ob1, Object *ob2) +bool BKE_mball_is_same_group(const Object *ob1, const Object *ob2) { int basis1nr, basis2nr; char basis1name[MAX_ID_NAME], basis2name[MAX_ID_NAME]; @@ -383,11 +383,12 @@ bool BKE_mball_is_basis_for(Object *ob1, Object *ob2) BLI_split_name_num(basis1name, &basis1nr, ob1->id.name + 2, '.'); BLI_split_name_num(basis2name, &basis2nr, ob2->id.name + 2, '.'); - if (STREQ(basis1name, basis2name)) { - return BKE_mball_is_basis(ob1); - } + return STREQ(basis1name, basis2name); +} - return false; +bool BKE_mball_is_basis_for(const Object *ob1, const Object *ob2) +{ + return BKE_mball_is_same_group(ob1, ob2) && BKE_mball_is_basis(ob1); } bool BKE_mball_is_any_selected(const MetaBall *mb) @@ -422,41 +423,82 @@ bool BKE_mball_is_any_unselected(const MetaBall *mb) return false; } -void BKE_mball_properties_copy(Scene *scene, Object *active_object) +static void mball_data_properties_copy(MetaBall *mb_dst, MetaBall *mb_src) { - Scene *sce_iter = scene; - Base *base; - Object *ob; - MetaBall *active_mball = (MetaBall *)active_object->data; - int basisnr, obnr; - char basisname[MAX_ID_NAME], obname[MAX_ID_NAME]; - SceneBaseIter iter; - - BLI_split_name_num(basisname, &basisnr, active_object->id.name + 2, '.'); - - /* Pass depsgraph as NULL, which means we will not expand into - * duplis unlike when we generate the meta-ball. Expanding duplis - * would not be compatible when editing multiple view layers. */ - BKE_scene_base_iter_next(NULL, &iter, &sce_iter, 0, NULL, NULL); - while (BKE_scene_base_iter_next(NULL, &iter, &sce_iter, 1, &base, &ob)) { - if (ob->type == OB_MBALL) { - if (ob != active_object) { - BLI_split_name_num(obname, &obnr, ob->id.name + 2, '.'); - - /* Object ob has to be in same "group" ... it means, that it has to have - * same base of its name */ - if (STREQ(obname, basisname)) { - MetaBall *mb = ob->data; - - /* Copy properties from selected/edited metaball */ - mb->wiresize = active_mball->wiresize; - mb->rendersize = active_mball->rendersize; - mb->thresh = active_mball->thresh; - mb->flag = active_mball->flag; - DEG_id_tag_update(&mb->id, 0); - } + mb_dst->wiresize = mb_src->wiresize; + mb_dst->rendersize = mb_src->rendersize; + mb_dst->thresh = mb_src->thresh; + mb_dst->flag = mb_src->flag; + DEG_id_tag_update(&mb_dst->id, 0); +} + +void BKE_mball_properties_copy(Main *bmain, MetaBall *metaball_src) +{ + /* WARNING: This code does not cover all potential corner-cases. E.g. if: + * | Object | ObData | + * | ---------- | ---------- | + * | Meta_A | Meta_A | + * | Meta_A.001 | Meta_A.001 | + * | Meta_B | Meta_A | + * | Meta_B.001 | Meta_B.001 | + * + * Calling this function with `metaball_src` being `Meta_A.001` will update `Meta_A`, but NOT + * `Meta_B.001`. So in the 'Meta_B' family, the two metaballs will have unmatching settings now. + * + * Solving this case would drastically increase the complexity of this code though, so don't + * think it would be worth it. + */ + for (Object *ob_src = bmain->objects.first; ob_src != NULL && !ID_IS_LINKED(ob_src);) { + if (ob_src->data != metaball_src) { + ob_src = ob_src->id.next; + continue; + } + + /* In this code we take advantage of two facts: + * - MetaBalls of the same family have the same basis name, + * - IDs are sorted by name in their Main listbase. + * So, all MetaBall objects of the same family are contiguous in bmain list (potentially mixed + * with non-meta-ball objects with same basis names). + * + * Using this, it is possible to process the whole set of meta-balls with a single loop on the + * whole list of Objects, though additionally going backward on part of the list in some cases. + */ + Object *ob_iter = NULL; + int obactive_nr, ob_nr; + char obactive_name[MAX_ID_NAME], ob_name[MAX_ID_NAME]; + BLI_split_name_num(obactive_name, &obactive_nr, ob_src->id.name + 2, '.'); + + for (ob_iter = ob_src->id.prev; ob_iter != NULL; ob_iter = ob_iter->id.prev) { + if (ob_iter->id.name[2] != obactive_name[0]) { + break; + } + if (ob_iter->type != OB_MBALL || ob_iter->data == metaball_src) { + continue; + } + BLI_split_name_num(ob_name, &ob_nr, ob_iter->id.name + 2, '.'); + if (!STREQ(obactive_name, ob_name)) { + break; + } + + mball_data_properties_copy(ob_iter->data, metaball_src); + } + + for (ob_iter = ob_src->id.next; ob_iter != NULL; ob_iter = ob_iter->id.next) { + if (ob_iter->id.name[2] != obactive_name[0] || ID_IS_LINKED(ob_iter)) { + break; + } + if (ob_iter->type != OB_MBALL || ob_iter->data == metaball_src) { + continue; } + BLI_split_name_num(ob_name, &ob_nr, ob_iter->id.name + 2, '.'); + if (!STREQ(obactive_name, ob_name)) { + break; + } + + mball_data_properties_copy(ob_iter->data, metaball_src); } + + ob_src = ob_iter; } } diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c index a0b25cf60b2..cf4182243b9 100644 --- a/source/blender/makesrna/intern/rna_access.c +++ b/source/blender/makesrna/intern/rna_access.c @@ -2141,6 +2141,7 @@ void RNA_property_update(bContext *C, PointerRNA *ptr, PropertyRNA *prop) void RNA_property_update_main(Main *bmain, Scene *scene, PointerRNA *ptr, PropertyRNA *prop) { + BLI_assert(bmain != NULL); rna_property_update(NULL, bmain, scene, ptr, prop); } diff --git a/source/blender/makesrna/intern/rna_meta.c b/source/blender/makesrna/intern/rna_meta.c index e6cf743e167..898208e0ba1 100644 --- a/source/blender/makesrna/intern/rna_meta.c +++ b/source/blender/makesrna/intern/rna_meta.c @@ -81,18 +81,17 @@ static void rna_MetaBall_redraw_data(Main *UNUSED(bmain), Scene *UNUSED(scene), WM_main_add_notifier(NC_GEOM | ND_DATA, id); } -static void rna_MetaBall_update_data(Main *bmain, Scene *scene, PointerRNA *ptr) +static void rna_MetaBall_update_data(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr) { MetaBall *mb = (MetaBall *)ptr->owner_id; Object *ob; - /* cheating way for importers to avoid slow updates */ + /* NOTE: The check on the number of users allows to avoid many repetitive (slow) updates in some + * cases, like e.g. importers. Calling `BKE_mball_properties_copy` on an obdata with no users + * would be meaningless anyway, as by definition it would not be used by any object, so not part + * of any meta-ball group. */ if (mb->id.us > 0) { - for (ob = bmain->objects.first; ob; ob = ob->id.next) { - if (ob->data == mb) { - BKE_mball_properties_copy(scene, ob); - } - } + BKE_mball_properties_copy(bmain, mb); DEG_id_tag_update(&mb->id, 0); WM_main_add_notifier(NC_GEOM | ND_DATA, mb); _______________________________________________ Bf-blender-cvs mailing list Bf-blender-cvs@blender.org List details, subscription details or unsubscribe: https://lists.blender.org/mailman/listinfo/bf-blender-cvs
