blender
/
blender
136
406
Fork 589
Code Issues 6.2k Pull Requests 750 Projects 19 Wiki Activity

Fix part of T89313: Attribute search crash during animation playback 

During animation playback, data-blocks are reallocated, so storing
pointers to the resulting data is not okay. Instead, the data should
be retrieved from the context. This works when the applied search
item is the "dummy" item added for non-matches. However, it still
crashes for every other item, because the memory is owned by the
modifier value log, which has been freed by the time the exec function
runs.

The next part of the solution is to allow uiSearchItems
to own memory for the search items.
This commit is contained in:
Hans Goudey 2021-11-05 11:19:12 -05:00
parent 594ee5f160
commit c473b2ce8b
Notes: blender-bot 2023-02-14 05:43:04 +01:00 
Referenced by commit 762d3a48e8, Cleanup: Avoid storing pointers for attribute search callback
Referenced by commit 22ffd69a91, Fix T89313: Attribute search crash with animation playback
3 changed files with 79 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/BKE_lib_id.h
View File

@ -163,7 +163,7 @@ void BLI_libblock_ensure_unique_name(struct Main *bmain, const char *name) ATTR_
struct ID *BKE_libblock_find_name(struct Main *bmain,
const short type,
const char *name) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL();
struct ID *BKE_libblock_find_session_uuid(struct Main *bmain, short type, uint32_t session_uuid);
/**
* Duplicate (a.k.a. deep copy) common processing options.
* See also eDupli_ID_Flags for options controlling what kind of IDs to duplicate.

14
source/blender/blenkernel/intern/lib_id.c
View File

@ -1375,6 +1375,20 @@ ID *BKE_libblock_find_name(struct Main *bmain, const short type, const char *nam
return BLI_findstring(lb, name, offsetof(ID, name) + 2);
}
struct ID *BKE_libblock_find_session_uuid(Main *bmain,
const short type,
const uint32_t session_uuid)
{
ListBase *lb = which_libbase(bmain, type);
BLI_assert(lb != NULL);
LISTBASE_FOREACH (ID *, id, lb) {
if (id->session_uuid == session_uuid) {
return id;
}
}
return NULL;
}
/**
* Sort given \a id into given \a lb list, using case-insensitive comparison of the id names.
*

96
source/blender/modifiers/intern/MOD_nodes.cc
View File

@ -55,6 +55,7 @@
#include "BKE_geometry_set_instances.hh"
#include "BKE_global.h"
#include "BKE_idprop.h"
#include "BKE_lib_id.h"
#include "BKE_lib_query.h"
#include "BKE_main.h"
#include "BKE_mesh.h"
@ -86,6 +87,7 @@
#include "MOD_nodes_evaluator.hh"
#include "MOD_ui_common.h"
#include "ED_object.h"
#include "ED_spreadsheet.h"
#include "ED_undo.h"
@ -1122,25 +1124,45 @@ static void modifyGeometrySet(ModifierData *md,
}
struct AttributeSearchData {
const geo_log::ModifierLog &modifier_log;
IDProperty &name_property;
uint32_t object_session_uid;
char modifier_name[MAX_NAME];
char socket_identifier[MAX_NAME];
bool is_output;
};
/* This class must not have a destructor, since it is used by buttons and freed with #MEM_freeN. */
BLI_STATIC_ASSERT(std::is_trivially_destructible_v<AttributeSearchData>, "");
static void attribute_search_update_fn(const bContext *UNUSED(C),
void *arg,
const char *str,
uiSearchItems *items,
const bool is_first)
static NodesModifierData *get_modifier_data(Main &bmain, const AttributeSearchData &data)
{
AttributeSearchData *data = static_cast<AttributeSearchData *>(arg);
const Object *object = (Object *)BKE_libblock_find_session_uuid(
&bmain, ID_OB, data.object_session_uid);
if (object == nullptr) {
return nullptr;
}
ModifierData *md = BKE_modifiers_findby_name(object, data.modifier_name);
if (md == nullptr) {
return nullptr;
}
BLI_assert(md->type == eModifierType_Nodes);
return reinterpret_cast<NodesModifierData *>(md);
}
const geo_log::GeometryValueLog *geometry_log = data->is_output ?
data->modifier_log.output_geometry_log() :
data->modifier_log.input_geometry_log();
static void attribute_search_update_fn(
const bContext *C, void *arg, const char *str, uiSearchItems *items, const bool is_first)
{
AttributeSearchData &data = *static_cast<AttributeSearchData *>(arg);
const NodesModifierData *nmd = get_modifier_data(*CTX_data_main(C), data);
if (nmd == nullptr) {
return;
}
const geo_log::ModifierLog *modifier_log = static_cast<const geo_log::ModifierLog *>(
nmd->runtime_eval_log);
if (modifier_log == nullptr) {
return;
}
const geo_log::GeometryValueLog *geometry_log = data.is_output ?
modifier_log->output_geometry_log() :
modifier_log->input_geometry_log();
if (geometry_log == nullptr) {
return;
}
@ -1153,7 +1175,7 @@ static void attribute_search_update_fn(const bContext *UNUSED(C),
info_ptrs[i] = &infos[i];
}
blender::ui::attribute_search_add_items(
str, data->is_output, info_ptrs.as_span(), items, is_first);
str, data.is_output, info_ptrs.as_span(), items, is_first);
}
static void attribute_search_exec_fn(bContext *C, void *data_v, void *item_v)
@ -1163,15 +1185,21 @@ static void attribute_search_exec_fn(bContext *C, void *data_v, void *item_v)
}
AttributeSearchData &data = *static_cast<AttributeSearchData *>(data_v);
const GeometryAttributeInfo &item = *static_cast<const GeometryAttributeInfo *>(item_v);
const NodesModifierData *nmd = get_modifier_data(*CTX_data_main(C), data);
if (nmd == nullptr) {
return;
}
IDProperty &name_property = data.name_property;
BLI_assert(name_property.type == IDP_STRING);
const std::string attribute_prop_name = data.socket_identifier + attribute_name_suffix;
IDProperty &name_property = *IDP_GetPropertyFromGroup(nmd->settings.properties,
attribute_prop_name.c_str());
IDP_AssignString(&name_property, item.name.c_str(), 0);
ED_undo_push(C, "Assign Attribute Name");
}
static void add_attribute_search_button(uiLayout *layout,
static void add_attribute_search_button(const bContext &C,
uiLayout *layout,
const NodesModifierData &nmd,
PointerRNA *md_ptr,
const StringRefNull rna_path_attribute_name,
@ -1203,16 +1231,17 @@ static void add_attribute_search_button(uiLayout *layout,
0.0f,
"");
const std::string use_attribute_prop_name = socket.identifier + attribute_name_suffix;
IDProperty *property = IDP_GetPropertyFromGroup(nmd.settings.properties,
use_attribute_prop_name.c_str());
BLI_assert(property != nullptr);
if (property == nullptr) {
const Object *object = ED_object_context(&C);
BLI_assert(object != nullptr);
if (object == nullptr) {
return;
}
AttributeSearchData *data = OBJECT_GUARDED_NEW(AttributeSearchData,
{*log, *property, is_output});
AttributeSearchData *data = OBJECT_GUARDED_NEW(AttributeSearchData);
data->object_session_uid = object->id.session_uuid;
STRNCPY(data->modifier_name, nmd.modifier.name);
STRNCPY(data->socket_identifier, socket.identifier);
data->is_output = is_output;
UI_but_func_search_set_results_are_suggestions(but, true);
UI_but_func_search_set_sep_string(but, UI_MENU_ARROW_SEP);
@ -1226,7 +1255,8 @@ static void add_attribute_search_button(uiLayout *layout,
nullptr);
}
static void add_attribute_search_or_value_buttons(uiLayout *layout,
static void add_attribute_search_or_value_buttons(const bContext &C,
uiLayout *layout,
const NodesModifierData &nmd,
PointerRNA *md_ptr,
const bNodeSocket &socket)
@ -1260,7 +1290,7 @@ static void add_attribute_search_or_value_buttons(uiLayout *layout,
const int use_attribute = RNA_int_get(md_ptr, rna_path_use_attribute.c_str()) != 0;
if (use_attribute) {
add_attribute_search_button(row, nmd, md_ptr, rna_path_attribute_name, socket, false);
add_attribute_search_button(C, row, nmd, md_ptr, rna_path_attribute_name, socket, false);
uiItemL(row, "", ICON_BLANK1);
}
else {
@ -1272,7 +1302,8 @@ static void add_attribute_search_or_value_buttons(uiLayout *layout,
/* Drawing the properties manually with #uiItemR instead of #uiDefAutoButsRNA allows using
* the node socket identifier for the property names, since they are unique, but also having
* the correct label displayed in the UI. */
static void draw_property_for_socket(uiLayout *layout,
static void draw_property_for_socket(const bContext &C,
uiLayout *layout,
NodesModifierData *nmd,
PointerRNA *bmain_ptr,
PointerRNA *md_ptr,
@ -1327,7 +1358,7 @@ static void draw_property_for_socket(uiLayout *layout,
}
default: {
if (input_has_attribute_toggle(*nmd->node_group, socket_index)) {
add_attribute_search_or_value_buttons(layout, *nmd, md_ptr, socket);
add_attribute_search_or_value_buttons(C, layout, *nmd, md_ptr, socket);
}
else {
uiLayout *row = uiLayoutRow(layout, false);
@ -1338,7 +1369,8 @@ static void draw_property_for_socket(uiLayout *layout,
}
}
static void draw_property_for_output_socket(uiLayout *layout,
static void draw_property_for_output_socket(const bContext &C,
uiLayout *layout,
const NodesModifierData &nmd,
PointerRNA *md_ptr,
const bNodeSocket &socket)
@ -1354,7 +1386,7 @@ static void draw_property_for_output_socket(uiLayout *layout,
uiItemL(name_row, socket.name, ICON_NONE);
uiLayout *row = uiLayoutRow(split, true);
add_attribute_search_button(row, nmd, md_ptr, rna_path_attribute_name, socket, true);
add_attribute_search_button(C, row, nmd, md_ptr, rna_path_attribute_name, socket, true);
}
static void panel_draw(const bContext *C, Panel *panel)
@ -1387,7 +1419,7 @@ static void panel_draw(const bContext *C, Panel *panel)
int socket_index;
LISTBASE_FOREACH_INDEX (bNodeSocket *, socket, &nmd->node_group->inputs, socket_index) {
draw_property_for_socket(layout, nmd, &bmain_ptr, ptr, *socket, socket_index);
draw_property_for_socket(*C, layout, nmd, &bmain_ptr, ptr, *socket, socket_index);
}
}
@ -1418,7 +1450,7 @@ static void panel_draw(const bContext *C, Panel *panel)
modifier_panel_end(layout, ptr);
}
static void output_attribute_panel_draw(const bContext *UNUSED(C), Panel *panel)
static void output_attribute_panel_draw(const bContext *C, Panel *panel)
{
uiLayout *layout = panel->layout;
@ -1433,7 +1465,7 @@ static void output_attribute_panel_draw(const bContext *UNUSED(C), Panel *panel)
LISTBASE_FOREACH (bNodeSocket *, socket, &nmd->node_group->outputs) {
if (socket_type_has_attribute_toggle(*socket)) {
has_output_attribute = true;
draw_property_for_output_socket(layout, *nmd, ptr, *socket);
draw_property_for_output_socket(*C, layout, *nmd, ptr, *socket);
}
}
}