Nodes: fix threading issues with node ui storage
Calling BKE_nodetree_attribute_hint_add from multiple threads still was not safe before.. One issue was that context_map embedded its values directly. So when context_map grows, all NodeUIStorage would move as well. I could patch around that by using std::unique_ptr in a few places, but that just becomes too complex for now. Instead I simplified the locking a bit by adding just locking a mutex in NodeTreeUIStorage all the time while an attribute hint is added. Differential Revision: https://developer.blender.org/D11399
This commit is contained in:
parent
afec66c024
commit
b67423f806
Notes:
blender-bot
2023-02-14 06:42:53 +01:00
Referenced by issue #88484, Compositor: adding viewer node with volume object causes crash in DebugInfo::graphviz Referenced by issue #87868, Plane with transparent scattered as particle generates artifact in the intersection with the emitter object Referenced by issue #86381, Rendering in Command line throws "Error: Cannot read file..." Referenced by issue #86023, Sculpting: Ctrl+S reverts recent progress. Referenced by issue #85529, Rigid body constraint hinge shift with animated object
|
@ -95,21 +95,13 @@ struct AvailableAttributeInfo {
|
|||
};
|
||||
|
||||
struct NodeUIStorage {
|
||||
std::mutex mutex;
|
||||
blender::Vector<NodeWarning> warnings;
|
||||
blender::Set<AvailableAttributeInfo> attribute_hints;
|
||||
|
||||
NodeUIStorage() = default;
|
||||
/* Needed because the mutex can't be moved or copied. */
|
||||
NodeUIStorage(NodeUIStorage &&other)
|
||||
: warnings(std::move(other.warnings)), attribute_hints(std::move(other.attribute_hints))
|
||||
{
|
||||
}
|
||||
};
|
||||
|
||||
struct NodeTreeUIStorage {
|
||||
std::mutex mutex;
|
||||
blender::Map<NodeTreeEvaluationContext, blender::Map<std::string, NodeUIStorage>> context_map;
|
||||
std::mutex context_map_mutex;
|
||||
|
||||
/**
|
||||
* Attribute search uses this to store the fake info for the string typed into a node, in order
|
||||
|
|
|
@ -39,7 +39,7 @@ using blender::Vector;
|
|||
* bNodeTree struct in DNA. This could change if the node tree had a runtime struct. */
|
||||
static std::mutex global_ui_storage_mutex;
|
||||
|
||||
static void ui_storage_ensure(bNodeTree &ntree)
|
||||
static NodeTreeUIStorage &ui_storage_ensure(bNodeTree &ntree)
|
||||
{
|
||||
/* As an optimization, only acquire a lock if the UI storage doesn't exist,
|
||||
* because it only needs to be allocated once for every node tree. */
|
||||
|
@ -50,6 +50,7 @@ static void ui_storage_ensure(bNodeTree &ntree)
|
|||
ntree.ui_storage = new NodeTreeUIStorage();
|
||||
}
|
||||
}
|
||||
return *ntree.ui_storage;
|
||||
}
|
||||
|
||||
const NodeUIStorage *BKE_node_tree_ui_storage_get_from_context(const bContext *C,
|
||||
|
@ -90,7 +91,7 @@ void BKE_nodetree_ui_storage_free_for_context(bNodeTree &ntree,
|
|||
{
|
||||
NodeTreeUIStorage *ui_storage = ntree.ui_storage;
|
||||
if (ui_storage != nullptr) {
|
||||
std::lock_guard<std::mutex> lock(ui_storage->context_map_mutex);
|
||||
std::lock_guard<std::mutex> lock(ui_storage->mutex);
|
||||
ui_storage->context_map.remove(context);
|
||||
}
|
||||
}
|
||||
|
@ -126,20 +127,14 @@ static void node_error_message_log(bNodeTree &ntree,
|
|||
}
|
||||
}
|
||||
|
||||
static NodeUIStorage &node_ui_storage_ensure(bNodeTree &ntree,
|
||||
static NodeUIStorage &node_ui_storage_ensure(NodeTreeUIStorage &locked_ui_storage,
|
||||
const NodeTreeEvaluationContext &context,
|
||||
const bNode &node)
|
||||
{
|
||||
ui_storage_ensure(ntree);
|
||||
NodeTreeUIStorage &ui_storage = *ntree.ui_storage;
|
||||
|
||||
std::lock_guard<std::mutex> lock(ui_storage.context_map_mutex);
|
||||
Map<std::string, NodeUIStorage> &node_tree_ui_storage =
|
||||
ui_storage.context_map.lookup_or_add_default(context);
|
||||
|
||||
locked_ui_storage.context_map.lookup_or_add_default(context);
|
||||
NodeUIStorage &node_ui_storage = node_tree_ui_storage.lookup_or_add_default_as(
|
||||
StringRef(node.name));
|
||||
|
||||
return node_ui_storage;
|
||||
}
|
||||
|
||||
|
@ -149,10 +144,12 @@ void BKE_nodetree_error_message_add(bNodeTree &ntree,
|
|||
const NodeWarningType type,
|
||||
std::string message)
|
||||
{
|
||||
NodeTreeUIStorage &ui_storage = ui_storage_ensure(ntree);
|
||||
std::lock_guard lock{ui_storage.mutex};
|
||||
|
||||
node_error_message_log(ntree, node, message, type);
|
||||
|
||||
NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ntree, context, node);
|
||||
std::lock_guard lock{node_ui_storage.mutex};
|
||||
NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ui_storage, context, node);
|
||||
node_ui_storage.warnings.append({type, std::move(message)});
|
||||
}
|
||||
|
||||
|
@ -163,8 +160,10 @@ void BKE_nodetree_attribute_hint_add(bNodeTree &ntree,
|
|||
const AttributeDomain domain,
|
||||
const CustomDataType data_type)
|
||||
{
|
||||
NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ntree, context, node);
|
||||
std::lock_guard lock{node_ui_storage.mutex};
|
||||
NodeTreeUIStorage &ui_storage = ui_storage_ensure(ntree);
|
||||
std::lock_guard lock{ui_storage.mutex};
|
||||
|
||||
NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ui_storage, context, node);
|
||||
node_ui_storage.attribute_hints.add_as(
|
||||
AvailableAttributeInfo{attribute_name, domain, data_type});
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue