Nodes: use modifier's persistent UID in context and viewer path

Previously, the modifier name was used to identify it in a compute context or
viewer path. Using `ModifierData.persistent_uid` (which was only introduced
later) has two main benefits: * It is stable even when the modifier name
changes. * It's cheaper and easier to work with since it's just an integer
instead of a string.

Note: Pinned viewer nodes will need to be re-pinned after the change.

Pull Request: https://projects.blender.org/blender/blender/pulls/138864
This commit is contained in:
Jacques Lucke
2025-05-14 15:18:36 +02:00
parent 153abc372e
commit c77b93f49d
12 changed files with 45 additions and 55 deletions

View File

@@ -86,7 +86,7 @@ class SPREADSHEET_HT_header(bpy.types.Header):
else:
layout.label(text="Invalid id")
elif ctx.type == 'MODIFIER':
layout.label(text=ctx.modifier_name, icon='MODIFIER')
layout.label(text=ctx.ui_name, icon='MODIFIER')
elif ctx.type == 'GROUP_NODE':
layout.label(text=ctx.ui_name, icon='NODE')
elif ctx.type == 'SIMULATION_ZONE':

View File

@@ -28,7 +28,7 @@ class ComputeContextCache {
/** The allocated computed contexts that need to be destructed in the end. */
Vector<destruct_ptr<ComputeContext>> cache_;
Map<std::pair<const ComputeContext *, StringRef>, const ModifierComputeContext *>
Map<std::pair<const ComputeContext *, int>, const ModifierComputeContext *>
modifier_contexts_cache_;
Map<const ComputeContext *, const OperatorComputeContext *> operator_contexts_cache_;
Map<std::pair<const ComputeContext *, int32_t>, const GroupNodeComputeContext *>
@@ -46,8 +46,7 @@ class ComputeContextCache {
public:
const ModifierComputeContext &for_modifier(const ComputeContext *parent,
const NodesModifierData &nmd);
const ModifierComputeContext &for_modifier(const ComputeContext *parent,
StringRef modifier_name);
const ModifierComputeContext &for_modifier(const ComputeContext *parent, int modifier_uid);
const OperatorComputeContext &for_operator(const ComputeContext *parent);
const OperatorComputeContext &for_operator(const ComputeContext *parent, const bNodeTree &tree);

View File

@@ -36,22 +36,18 @@ class ModifierComputeContext : public ComputeContext {
private:
static constexpr const char *s_static_type = "MODIFIER";
/**
* Use modifier name instead of something like `session_uid` for now because:
* - It's more obvious that the name matches between the original and evaluated object.
* - We might want that the context hash is consistent between sessions in the future.
*/
std::string modifier_name_;
/** #ModifierData.persistent_uid. */
int modifier_uid_;
/** The modifier data that this context is for. This may be null. */
const NodesModifierData *nmd_ = nullptr;
public:
ModifierComputeContext(const ComputeContext *parent, const NodesModifierData &nmd);
ModifierComputeContext(const ComputeContext *parent, std::string modifier_name);
ModifierComputeContext(const ComputeContext *parent, int modifier_uid);
StringRefNull modifier_name() const
int modifier_uid() const
{
return modifier_name_;
return modifier_uid_;
}
const NodesModifierData *nmd() const

View File

@@ -14,22 +14,24 @@ namespace blender::bke {
ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent,
const NodesModifierData &nmd)
: ModifierComputeContext(parent, nmd.modifier.name)
: ModifierComputeContext(parent, nmd.modifier.persistent_uid)
{
nmd_ = &nmd;
}
ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent,
std::string modifier_name)
: ComputeContext(s_static_type, parent), modifier_name_(std::move(modifier_name))
const int modifier_uid)
: ComputeContext(s_static_type, parent), modifier_uid_(std::move(modifier_uid))
{
hash_.mix_in(s_static_type, strlen(s_static_type));
hash_.mix_in(modifier_name_.data(), modifier_name_.size());
hash_.mix_in(&modifier_uid_, sizeof(modifier_uid_));
}
void ModifierComputeContext::print_current_in_line(std::ostream &stream) const
{
stream << "Modifier: " << modifier_name_;
if (nmd_) {
stream << "Modifier: " << nmd_->modifier.name;
}
}
GroupNodeComputeContext::GroupNodeComputeContext(
@@ -212,15 +214,15 @@ const ModifierComputeContext &ComputeContextCache::for_modifier(const ComputeCon
const NodesModifierData &nmd)
{
return *modifier_contexts_cache_.lookup_or_add_cb(
std::pair{parent, StringRef(nmd.modifier.name)},
std::pair{parent, nmd.modifier.persistent_uid},
[&]() { return &this->for_any_uncached<ModifierComputeContext>(parent, nmd); });
}
const ModifierComputeContext &ComputeContextCache::for_modifier(const ComputeContext *parent,
StringRef modifier_name)
const int modifier_uid)
{
return *modifier_contexts_cache_.lookup_or_add_cb(std::pair{parent, modifier_name}, [&]() {
return &this->for_any_uncached<ModifierComputeContext>(parent, modifier_name);
return *modifier_contexts_cache_.lookup_or_add_cb(std::pair{parent, modifier_uid}, [&]() {
return &this->for_any_uncached<ModifierComputeContext>(parent, modifier_uid);
});
}

View File

@@ -72,7 +72,6 @@ void BKE_viewer_path_blend_write(BlendWriter *writer, const ViewerPath *viewer_p
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
const auto *typed_elem = reinterpret_cast<ModifierViewerPathElem *>(elem);
BLO_write_struct(writer, ModifierViewerPathElem, typed_elem);
BLO_write_string(writer, typed_elem->modifier_name);
break;
}
case VIEWER_PATH_ELEM_TYPE_GROUP_NODE: {
@@ -123,14 +122,10 @@ void BKE_viewer_path_blend_read_data(BlendDataReader *reader, ViewerPath *viewer
case VIEWER_PATH_ELEM_TYPE_REPEAT_ZONE:
case VIEWER_PATH_ELEM_TYPE_FOREACH_GEOMETRY_ELEMENT_ZONE:
case VIEWER_PATH_ELEM_TYPE_EVALUATE_CLOSURE:
case VIEWER_PATH_ELEM_TYPE_MODIFIER:
case VIEWER_PATH_ELEM_TYPE_ID: {
break;
}
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
auto *typed_elem = reinterpret_cast<ModifierViewerPathElem *>(elem);
BLO_read_string(reader, &typed_elem->modifier_name);
break;
}
}
}
}
@@ -287,9 +282,7 @@ ViewerPathElem *BKE_viewer_path_elem_copy(const ViewerPathElem *src)
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
const auto *old_elem = reinterpret_cast<const ModifierViewerPathElem *>(src);
auto *new_elem = reinterpret_cast<ModifierViewerPathElem *>(dst);
if (old_elem->modifier_name != nullptr) {
new_elem->modifier_name = BLI_strdup(old_elem->modifier_name);
}
new_elem->modifier_uid = old_elem->modifier_uid;
break;
}
case VIEWER_PATH_ELEM_TYPE_GROUP_NODE: {
@@ -358,7 +351,7 @@ bool BKE_viewer_path_elem_equal(const ViewerPathElem *a,
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
const auto *a_elem = reinterpret_cast<const ModifierViewerPathElem *>(a);
const auto *b_elem = reinterpret_cast<const ModifierViewerPathElem *>(b);
return StringRef(a_elem->modifier_name) == StringRef(b_elem->modifier_name);
return a_elem->modifier_uid == b_elem->modifier_uid;
}
case VIEWER_PATH_ELEM_TYPE_GROUP_NODE: {
const auto *a_elem = reinterpret_cast<const GroupNodeViewerPathElem *>(a);
@@ -410,12 +403,8 @@ void BKE_viewer_path_elem_free(ViewerPathElem *elem)
case VIEWER_PATH_ELEM_TYPE_REPEAT_ZONE:
case VIEWER_PATH_ELEM_TYPE_FOREACH_GEOMETRY_ELEMENT_ZONE:
case VIEWER_PATH_ELEM_TYPE_EVALUATE_CLOSURE: {
break;
}
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
auto *typed_elem = reinterpret_cast<ModifierViewerPathElem *>(elem);
MEM_SAFE_FREE(typed_elem->modifier_name);
break;
case VIEWER_PATH_ELEM_TYPE_MODIFIER:
break;
}
}
if (elem->ui_name) {

View File

@@ -349,7 +349,7 @@ static Object *find_object_with_preview_geometry(const ViewerPath &viewer_path)
}
const ModifierViewerPathElem *modifier_elem = reinterpret_cast<const ModifierViewerPathElem *>(
elem->next);
ModifierData *md = BKE_modifiers_findby_name(object, modifier_elem->modifier_name);
ModifierData *md = BKE_modifiers_findby_persistent_uid(object, modifier_elem->modifier_uid);
if (md == nullptr) {
return nullptr;
}

View File

@@ -39,7 +39,8 @@ Object *parse_object_only(const ViewerPath &viewer_path);
*/
struct ViewerPathForGeometryNodesViewer {
Object *object;
blender::StringRefNull modifier_name;
/** #ModifierData.persistent_uid. */
int modifier_uid;
/** Contains only group node and simulation zone elements. */
blender::Vector<const ViewerPathElem *> node_path;
int32_t viewer_node_id;

View File

@@ -39,7 +39,10 @@ ViewerPathElem *viewer_path_elem_for_compute_context(const ComputeContext &compu
{
if (const auto *context = dynamic_cast<const bke::ModifierComputeContext *>(&compute_context)) {
ModifierViewerPathElem *elem = BKE_viewer_path_elem_new_modifier();
elem->modifier_name = BLI_strdup(context->modifier_name().c_str());
elem->modifier_uid = context->modifier_uid();
if (const NodesModifierData *nmd = context->nmd()) {
elem->base.ui_name = BLI_strdup(nmd->modifier.name);
}
return &elem->base;
}
if (const auto *context = dynamic_cast<const bke::GroupNodeComputeContext *>(&compute_context)) {
@@ -233,11 +236,9 @@ std::optional<ViewerPathForGeometryNodesViewer> parse_geometry_nodes_viewer(
if (modifier_elem.type != VIEWER_PATH_ELEM_TYPE_MODIFIER) {
return std::nullopt;
}
const char *modifier_name =
reinterpret_cast<const ModifierViewerPathElem &>(modifier_elem).modifier_name;
if (modifier_name == nullptr) {
return std::nullopt;
}
const int modifier_uid =
reinterpret_cast<const ModifierViewerPathElem &>(modifier_elem).modifier_uid;
remaining_elems = remaining_elems.drop_front(1);
Vector<const ViewerPathElem *> node_path;
for (const ViewerPathElem *elem : remaining_elems.drop_back(1)) {
@@ -258,7 +259,7 @@ std::optional<ViewerPathForGeometryNodesViewer> parse_geometry_nodes_viewer(
}
const int32_t viewer_node_id =
reinterpret_cast<const ViewerNodeViewerPathElem *>(last_elem)->node_id;
return ViewerPathForGeometryNodesViewer{root_ob, modifier_name, node_path, viewer_node_id};
return ViewerPathForGeometryNodesViewer{root_ob, modifier_uid, node_path, viewer_node_id};
}
bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed_viewer_path)
@@ -268,7 +269,7 @@ bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed
if (md->type != eModifierType_Nodes) {
continue;
}
if (md->name != parsed_viewer_path.modifier_name) {
if (md->persistent_uid != parsed_viewer_path.modifier_uid) {
continue;
}
modifier = reinterpret_cast<const NodesModifierData *>(md);
@@ -512,7 +513,7 @@ bNode *find_geometry_nodes_viewer(const ViewerPath &viewer_path, SpaceNode &snod
}
case VIEWER_PATH_ELEM_TYPE_MODIFIER: {
const auto &elem = reinterpret_cast<const ModifierViewerPathElem &>(elem_generic);
return &compute_context_cache.for_modifier(parent_compute_context, elem.modifier_name);
return &compute_context_cache.for_modifier(parent_compute_context, elem.modifier_uid);
}
case VIEWER_PATH_ELEM_TYPE_GROUP_NODE: {
const auto &elem = reinterpret_cast<const GroupNodeViewerPathElem &>(elem_generic);

View File

@@ -36,7 +36,9 @@ typedef struct IDViewerPathElem {
typedef struct ModifierViewerPathElem {
ViewerPathElem base;
char *modifier_name;
/** #ModifierData.persistent_uid. */
int modifier_uid;
char _pad[4];
} ModifierViewerPathElem;
typedef struct GroupNodeViewerPathElem {

View File

@@ -8602,8 +8602,8 @@ static void rna_def_modifier_viewer_path_elem(BlenderRNA *brna)
srna = RNA_def_struct(brna, "ModifierViewerPathElem", "ViewerPathElem");
prop = RNA_def_property(srna, "modifier_name", PROP_STRING, PROP_NONE);
RNA_def_property_ui_text(prop, "Modifier Name", "");
prop = RNA_def_property(srna, "modifier_uid", PROP_INT, PROP_NONE);
RNA_def_property_ui_text(prop, "Modifier UID", "The persistent UID of the modifier");
}
static void rna_def_group_node_viewer_path_elem(BlenderRNA *brna)

View File

@@ -498,7 +498,7 @@ static void try_add_side_effect_node(const ModifierEvalContext &ctx,
if (modifier_compute_context == nullptr) {
return;
}
if (modifier_compute_context->modifier_name() != nmd.modifier.name) {
if (modifier_compute_context->modifier_uid() != nmd.modifier.persistent_uid) {
return;
}
@@ -713,7 +713,7 @@ static void find_side_effect_nodes_for_viewer_path(
if (parsed_path->object != DEG_get_original(ctx.object)) {
return;
}
if (parsed_path->modifier_name != nmd.modifier.name) {
if (parsed_path->modifier_uid != nmd.modifier.persistent_uid) {
return;
}

View File

@@ -934,7 +934,7 @@ const ViewerNodeLog *GeoModifierLog::find_viewer_node_log_for_path(const ViewerP
const Object *object = parsed_path->object;
NodesModifierData *nmd = nullptr;
LISTBASE_FOREACH (ModifierData *, md, &object->modifiers) {
if (md->name == parsed_path->modifier_name) {
if (md->persistent_uid == parsed_path->modifier_uid) {
if (md->type == eModifierType_Nodes) {
nmd = reinterpret_cast<NodesModifierData *>(md);
}