Fix modifier freeing code re. ID refcounting.

Free code should not handle ID refcounting at all. This has to be done
at higher level, since in some case we want to free (temp) data that
actually did not refcount at all its IDs.

This change seems to be working OK, but as usual in that area, only
lots of testing in real-case situation will say whether there are some
hidden bugs or not.
This commit is contained in:
Bastien Montagne 2018-04-04 14:56:32 +02:00
parent 0c7ec58966
commit d59c2d12b1
12 changed files with 39 additions and 55 deletions

View File

@ -346,6 +346,7 @@ const ModifierTypeInfo *modifierType_getInfo(ModifierType type);
* default values if pointer is optional.
*/
struct ModifierData *modifier_new(int type);
void modifier_free_ex(struct ModifierData *md, const int flag);
void modifier_free(struct ModifierData *md);
bool modifier_unique_name(struct ListBase *modifiers, struct ModifierData *md);

View File

@ -73,7 +73,7 @@ void BKE_object_modifier_hook_reset(struct Object *ob, struct HookModifierData *
bool BKE_object_support_modifier_type_check(struct Object *ob, int modifier_type);
void BKE_object_link_modifiers(struct Object *ob_dst, const struct Object *ob_src);
void BKE_object_free_modifiers(struct Object *ob);
void BKE_object_free_modifiers(struct Object *ob, const int flag);
void BKE_object_make_proxy(struct Object *ob, struct Object *target, struct Object *gob);
void BKE_object_copy_proxy_drivers(struct Object *ob, struct Object *target);

View File

@ -2429,7 +2429,7 @@ Mesh *BKE_mesh_new_from_object(
/* if getting the original caged mesh, delete object modifiers */
if (cage)
BKE_object_free_modifiers(tmpobj);
BKE_object_free_modifiers(tmpobj, 0);
/* copies the data */
copycu = tmpobj->data = BKE_curve_copy(bmain, (Curve *) ob->data);

View File

@ -136,16 +136,38 @@ ModifierData *modifier_new(int type)
return md;
}
void modifier_free(ModifierData *md)
static void modifier_free_data_id_us_cb(void *UNUSED(userData), Object *UNUSED(ob), ID **idpoin, int cb_flag)
{
ID *id = *idpoin;
if (id != NULL && (cb_flag & IDWALK_CB_USER) != 0) {
id_us_min(id);
}
}
void modifier_free_ex(ModifierData *md, const int flag)
{
const ModifierTypeInfo *mti = modifierType_getInfo(md->type);
if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) {
if (mti->foreachIDLink) {
mti->foreachIDLink(md, NULL, modifier_free_data_id_us_cb, NULL);
}
else if (mti->foreachObjectLink) {
mti->foreachObjectLink(md, NULL, (ObjectWalkFunc)modifier_free_data_id_us_cb, NULL);
}
}
if (mti->freeData) mti->freeData(md);
if (md->error) MEM_freeN(md->error);
MEM_freeN(md);
}
void modifier_free(ModifierData *md)
{
modifier_free_ex(md, 0);
}
bool modifier_unique_name(ListBase *modifiers, ModifierData *md)
{
if (modifiers && md) {

View File

@ -201,12 +201,12 @@ void BKE_object_free_curve_cache(Object *ob)
}
}
void BKE_object_free_modifiers(Object *ob)
void BKE_object_free_modifiers(Object *ob, const int flag)
{
ModifierData *md;
while ((md = BLI_pophead(&ob->modifiers))) {
modifier_free(md);
modifier_free_ex(md, flag);
}
/* particle modifiers were freed, so free the particlesystems as well */
@ -268,7 +268,7 @@ bool BKE_object_support_modifier_type_check(Object *ob, int modifier_type)
void BKE_object_link_modifiers(struct Object *ob_dst, const struct Object *ob_src)
{
ModifierData *md;
BKE_object_free_modifiers(ob_dst);
BKE_object_free_modifiers(ob_dst, 0);
if (!ELEM(ob_dst->type, OB_MESH, OB_CURVE, OB_SURF, OB_FONT, OB_LATTICE)) {
/* only objects listed above can have modifiers and linking them to objects
@ -410,7 +410,8 @@ void BKE_object_free(Object *ob)
{
BKE_animdata_free((ID *)ob, false);
BKE_object_free_modifiers(ob);
/* BKE_<id>_free shall never touch to ID->us. Never ever. */
BKE_object_free_modifiers(ob, LIB_ID_CREATE_NO_USER_REFCOUNT);
MEM_SAFE_FREE(ob->mat);
MEM_SAFE_FREE(ob->matbits);

View File

@ -1573,7 +1573,7 @@ static void curvetomesh(Main *bmain, Scene *scene, Object *ob)
BKE_mesh_from_nurbs(ob); /* also does users */
if (ob->type == OB_MESH) {
BKE_object_free_modifiers(ob);
BKE_object_free_modifiers(ob, 0);
/* Game engine defaults for mesh objects */
ob->body_type = OB_BODY_TYPE_STATIC;
@ -1702,7 +1702,7 @@ static int convert_exec(bContext *C, wmOperator *op)
/* When 2 objects with linked data are selected, converting both
* would keep modifiers on all but the converted object [#26003] */
if (ob->type == OB_MESH) {
BKE_object_free_modifiers(ob); /* after derivedmesh calls! */
BKE_object_free_modifiers(ob, 0); /* after derivedmesh calls! */
}
}
}
@ -1727,7 +1727,7 @@ static int convert_exec(bContext *C, wmOperator *op)
BKE_mesh_to_curve(scene, newob);
if (newob->type == OB_CURVE) {
BKE_object_free_modifiers(newob); /* after derivedmesh calls! */
BKE_object_free_modifiers(newob, 0); /* after derivedmesh calls! */
ED_rigidbody_object_remove(bmain, scene, newob);
}
}
@ -1760,7 +1760,7 @@ static int convert_exec(bContext *C, wmOperator *op)
/* re-tessellation is called by DM_to_mesh */
BKE_object_free_modifiers(newob); /* after derivedmesh calls! */
BKE_object_free_modifiers(newob, 0); /* after derivedmesh calls! */
}
else if (ob->type == OB_FONT) {
ob->flag |= OB_DONE;

View File

@ -81,14 +81,6 @@ static void copyData(ModifierData *md, ModifierData *target)
modifier_copyData_generic(md, target);
}
static void freeData(ModifierData *md)
{
DisplaceModifierData *dmd = (DisplaceModifierData *) md;
if (dmd->texture) {
id_us_min(&dmd->texture->id);
}
}
static CustomDataMask requiredDataMask(Object *UNUSED(ob), ModifierData *md)
{
DisplaceModifierData *dmd = (DisplaceModifierData *)md;
@ -444,7 +436,7 @@ ModifierTypeInfo modifierType_Displace = {
/* applyModifierEM */ NULL,
/* initData */ initData,
/* requiredDataMask */ requiredDataMask,
/* freeData */ freeData,
/* freeData */ NULL,
/* isDisabled */ isDisabled,
/* updateDepgraph */ updateDepgraph,
/* updateDepsgraph */ updateDepsgraph,

View File

@ -75,10 +75,6 @@ static void freeData(ModifierData *md)
{
MeshSeqCacheModifierData *mcmd = (MeshSeqCacheModifierData *) md;
if (mcmd->cache_file) {
id_us_min(&mcmd->cache_file->id);
}
if (mcmd->reader) {
#ifdef WITH_ALEMBIC
CacheReader_free(mcmd->reader);

View File

@ -78,14 +78,6 @@ static void initData(ModifierData *md)
wmd->defgrp_name[0] = 0;
}
static void freeData(ModifierData *md)
{
WaveModifierData *wmd = (WaveModifierData *) md;
if (wmd->texture) {
id_us_min(&wmd->texture->id);
}
}
static void copyData(ModifierData *md, ModifierData *target)
{
#if 0
@ -383,7 +375,7 @@ ModifierTypeInfo modifierType_Wave = {
/* applyModifierEM */ NULL,
/* initData */ initData,
/* requiredDataMask */ requiredDataMask,
/* freeData */ freeData,
/* freeData */ NULL,
/* isDisabled */ NULL,
/* updateDepgraph */ updateDepgraph,
/* updateDepsgraph */ updateDepsgraph,

View File

@ -79,10 +79,6 @@ static void freeData(ModifierData *md)
{
WeightVGEditModifierData *wmd = (WeightVGEditModifierData *) md;
curvemapping_free(wmd->cmap_curve);
if (wmd->mask_texture) {
id_us_min(&wmd->mask_texture->id);
}
}
static void copyData(ModifierData *md, ModifierData *target)

View File

@ -126,14 +126,6 @@ static void initData(ModifierData *md)
wmd->mask_tex_mapping = MOD_DISP_MAP_LOCAL;
}
static void freeData(ModifierData *md)
{
WeightVGMixModifierData *wmd = (WeightVGMixModifierData *) md;
if (wmd->mask_texture) {
id_us_min(&wmd->mask_texture->id);
}
}
static void copyData(ModifierData *md, ModifierData *target)
{
#if 0
@ -427,7 +419,7 @@ ModifierTypeInfo modifierType_WeightVGMix = {
/* applyModifierEM */ NULL,
/* initData */ initData,
/* requiredDataMask */ requiredDataMask,
/* freeData */ freeData,
/* freeData */ NULL,
/* isDisabled */ isDisabled,
/* updateDepgraph */ updateDepgraph,
/* updateDepsgraph */ updateDepsgraph,

View File

@ -286,14 +286,6 @@ static void initData(ModifierData *md)
wmd->max_dist = 1.0f; /* vert arbitrary distance, but don't use 0 */
}
static void freeData(ModifierData *md)
{
WeightVGProximityModifierData *wmd = (WeightVGProximityModifierData *) md;
if (wmd->mask_texture) {
id_us_min(&wmd->mask_texture->id);
}
}
static void copyData(ModifierData *md, ModifierData *target)
{
#if 0
@ -616,7 +608,7 @@ ModifierTypeInfo modifierType_WeightVGProximity = {
/* applyModifierEM */ NULL,
/* initData */ initData,
/* requiredDataMask */ requiredDataMask,
/* freeData */ freeData,
/* freeData */ NULL,
/* isDisabled */ isDisabled,
/* updateDepgraph */ updateDepgraph,
/* updateDepsgraph */ updateDepsgraph,