Fix #120058: Undoing a rename while in edit mode crashes Blender.

The root of the issue, as identified by Jake-Faulkner in his PR
(!120099), was that after changes in commit 195bb4f8f5,
`BKE_libblock_ensure_unique_name` would not always correctly set
`bmain->is_memfile_undo_written` to false when an ID name was actually
modified.

However, after analyzing a bit more the code in ID renaming, it appeared
that `BKE_libblock_ensure_unique_name` was not needed and only made
things more confusing. Some ID renaming code (from RNA, the Outliner,
and some do_version areas) would then do some manual ID renaming
operations and then call it, instead of simply using the existing 'all
in one' `BKE_libblock_rename` function.

This commit removes `BKE_libblock_ensure_unique_name` and all of its
usages, and simplify all code previously using it by calling
`BKE_libblock_rename` instead.

NOTE: The only non-trivial (not-so-nice) aspect of this commit is the
changes needed in the Outliner renaming code, since here the name of the
ID is directly edited, before calling the rename function, so this edit
needs to be undone to allow calling the generic ID rename API.

Pull Request: https://projects.blender.org/blender/blender/pulls/120196
This commit is contained in:
Bastien Montagne 2024-04-03 15:43:25 +09:00 committed by Gitea
parent 800e470ced
commit cb66cc3028
6 changed files with 32 additions and 45 deletions

@ -248,11 +248,6 @@ void *BKE_libblock_copy(Main *bmain, const ID *id) ATTR_WARN_UNUSED_RESULT ATTR_
* Sets the name of a block to name, suitably adjusted for uniqueness.
*/
void BKE_libblock_rename(Main *bmain, ID *id, const char *name) ATTR_NONNULL();
/**
* Use after setting the ID's name
* When name exists: call 'new_id'
*/
void BKE_libblock_ensure_unique_name(Main *bmain, ID *id) ATTR_NONNULL();
ID *BKE_libblock_find_name(Main *bmain, short type, const char *name) ATTR_WARN_UNUSED_RESULT
ATTR_NONNULL();
@ -582,16 +577,20 @@ void id_sort_by_name(ListBase *lb, ID *id, ID *id_sorting_hint);
void BKE_lib_id_expand_local(Main *bmain, ID *id, int flags);
/**
* Ensures given ID has a unique name in given listbase.
* Optionally set the given ID's name from given parameter, and ensure that the ID has a unique
* name in given listbase.
*
* Uniqueness is only ensured within the ID's library (nullptr for local ones), libraries act as
* some kind of namespace for IDs.
*
* \param name: The new name of the given ID, if NULL the current given ID name is used instead.
* \param do_linked_data: if true, also ensure a unique name in case the given \a id is linked
* \param name: The new name of the given ID, if `nullptr` the current given ID name is used
* instead. If the given ID has no name (or the given name is an empty string), the default
* matching data name is used as fallback.
* \param do_linked_data: if true, also ensure a unique name in case the given ID is linked
* (otherwise, just ensure that it is properly sorted).
*
* \return true if a new name had to be created.
* \return true if the ID's name has been modified (either from given `name` parameter, or because
* its current name was colliding with another existing ID).
*/
bool BKE_id_new_name_validate(Main *bmain,
ListBase *lb,

@ -2121,24 +2121,12 @@ void BKE_library_make_local(Main *bmain,
#endif
}
void BKE_libblock_ensure_unique_name(Main *bmain, ID *id)
{
ListBase *lb;
lb = which_libbase(bmain, GS(id->name));
if (lb == nullptr) {
return;
}
/* BKE_id_new_name_validate also takes care of sorting. */
if (!ID_IS_LINKED(id) && BKE_id_new_name_validate(bmain, lb, id, nullptr, false)) {
bmain->is_memfile_undo_written = false;
}
}
void BKE_libblock_rename(Main *bmain, ID *id, const char *name)
{
BLI_assert(!ID_IS_LINKED(id));
if (STREQ(id->name + 2, name)) {
return;
}
BKE_main_namemap_remove_name(bmain, id, id->name + 2);
ListBase *lb = which_libbase(bmain, GS(id->name));
if (BKE_id_new_name_validate(bmain, lb, id, name, false)) {

@ -101,10 +101,7 @@ ID *do_versions_rename_id(Main *bmain,
}
}
if (id != nullptr) {
BKE_main_namemap_remove_name(bmain, id, id->name + 2);
BLI_strncpy(id->name + 2, name_dst, sizeof(id->name) - 2);
/* We know it's unique, this just sorts. */
BKE_libblock_ensure_unique_name(bmain, id);
BKE_libblock_rename(bmain, id, name_dst);
}
return id;
}

@ -581,9 +581,7 @@ void BLO_update_defaults_startup_blend(Main *bmain, const char *app_template)
if (layout->screen) {
bScreen *screen = layout->screen;
if (!STREQ(screen->id.name + 2, workspace->id.name + 2)) {
BKE_main_namemap_remove_name(bmain, &screen->id, screen->id.name + 2);
BLI_strncpy(screen->id.name + 2, workspace->id.name + 2, sizeof(screen->id.name) - 2);
BKE_libblock_ensure_unique_name(bmain, &screen->id);
BKE_libblock_rename(bmain, &screen->id, workspace->id.name + 2);
}
}

@ -679,12 +679,22 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname)
BLI_mempool *ts = space_outliner->treestore;
TreeStoreElem *tselem = static_cast<TreeStoreElem *>(tsep);
/* Unfortunately at this point, the name of the ID has already been set to its new value. Revert
* it to its old name, to be able to use the generic 'rename' function for IDs.
*
* NOTE: While utterly inelegant, performances are not really a concern here, so this is an
* acceptable solution for now. */
auto id_rename_helper = [bmain, tselem, oldname] {
std::string new_name = tselem->id->name + 2;
BLI_strncpy(tselem->id->name + 2, oldname, sizeof(tselem->id->name) - 2);
BKE_libblock_rename(bmain, tselem->id, new_name.c_str());
};
if (ts && tselem) {
TreeElement *te = outliner_find_tree_element(&space_outliner->tree, tselem);
if (tselem->type == TSE_SOME_ID) {
BKE_main_namemap_remove_name(bmain, tselem->id, oldname);
BKE_libblock_ensure_unique_name(bmain, tselem->id);
id_rename_helper();
WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name);
@ -750,10 +760,9 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname)
break;
}
case TSE_NLA_ACTION: {
bAction *act = (bAction *)tselem->id;
BKE_main_namemap_remove_name(bmain, &act->id, oldname);
BKE_libblock_ensure_unique_name(bmain, &act->id);
WM_msg_publish_rna_prop(mbus, &act->id, &act->id, ID, name);
/* The #tselem->id is a #bAction. */
id_rename_helper();
WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name);
DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL);
break;
}
@ -871,11 +880,9 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname)
break;
}
case TSE_LAYER_COLLECTION: {
/* The ID is a #Collection, not a #LayerCollection */
Collection *collection = (Collection *)tselem->id;
BKE_main_namemap_remove_name(bmain, &collection->id, oldname);
BKE_libblock_ensure_unique_name(bmain, &collection->id);
WM_msg_publish_rna_prop(mbus, &collection->id, &collection->id, ID, name);
/* The #tselem->id is a #Collection, not a #LayerCollection */
id_rename_helper();
WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name);
WM_event_add_notifier(C, NC_ID | NA_RENAME, nullptr);
DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL);
break;

@ -289,9 +289,7 @@ void rna_ID_name_set(PointerRNA *ptr, const char *value)
BLI_assert(BKE_id_is_in_global_main(id));
BLI_assert(!ID_IS_LINKED(id));
BKE_main_namemap_remove_name(G_MAIN, id, id->name + 2);
BLI_strncpy_utf8(id->name + 2, value, sizeof(id->name) - 2);
BKE_libblock_ensure_unique_name(G_MAIN, id);
BKE_libblock_rename(G_MAIN, id, value);
if (GS(id->name) == ID_OB) {
Object *ob = (Object *)id;