Refactor: Geometry Nodes: store tree identifier in tree logger

The main goal here is to add `GeoTreeLogger.tree_orig_session_uid`. Previously,
it was always possible to derive this information in `ensure_node_warnings`.
However, with closures that's not possible in general anymore, because the
Evaluate Closure node does not know statically which node tree the closure zone
is from that it evaluates. Therefore, this information has to be logged as well.

This patch initializes `tree_orig_session_uid` the same way it initializes
`parent_node_id`, by scanning the compute context when creating the tree logger.
To make this work properly, some extra contextual data had to be stored in some
compute contexts.

This is just a refactor with no expected functional changes. Node warnings for
closures are still not properly logged, because that requires storing
source-location data in closures, which will be implemented separately.

Pull Request: https://projects.blender.org/blender/blender/pulls/137208
This commit is contained in:
Jacques Lucke
2025-04-10 08:56:02 +02:00
parent 07a9306bb4
commit dcc8d28859
11 changed files with 182 additions and 49 deletions

View File

@@ -10,6 +10,10 @@
/**
* This file implements some specific compute contexts for concepts in Blender.
*
* All compute contexts have to store the data that's required to uniquely identify them and to
* compute its hash. Some compute contexts contain some optional additional data that provides more
* information to code that uses the contexts.
*/
#include <optional>
@@ -18,6 +22,7 @@
struct bNode;
struct bNodeTree;
struct NodesModifierData;
namespace blender::bke {
@@ -31,8 +36,11 @@ class ModifierComputeContext : public ComputeContext {
* - We might want that the context hash is consistent between sessions in the future.
*/
std::string modifier_name_;
/** 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);
StringRefNull modifier_name() const
@@ -40,6 +48,11 @@ class ModifierComputeContext : public ComputeContext {
return modifier_name_;
}
const NodesModifierData *nmd() const
{
return nmd_;
}
private:
void print_current_in_line(std::ostream &stream) const override;
};
@@ -61,8 +74,9 @@ class GroupNodeComputeContext : public ComputeContext {
int32_t node_id,
const std::optional<ComputeContextHash> &cached_hash = {});
GroupNodeComputeContext(const ComputeContext *parent,
const bNode &node,
const bNodeTree &caller_tree);
const bNode &caller_group_node,
const bNodeTree &caller_tree,
const std::optional<ComputeContextHash> &cached_hash = {});
int32_t node_id() const
{
@@ -188,9 +202,18 @@ class OperatorComputeContext : public ComputeContext {
private:
static constexpr const char *s_static_type = "OPERATOR";
/** The tree that is executed. May be null. */
const bNodeTree *tree_ = nullptr;
public:
OperatorComputeContext();
OperatorComputeContext(const ComputeContext *parent);
OperatorComputeContext(const ComputeContext *parent, const bNodeTree &tree);
const bNodeTree *tree() const
{
return tree_;
}
private:
void print_current_in_line(std::ostream &stream) const override;

View File

@@ -2,6 +2,7 @@
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "DNA_modifier_types.h"
#include "DNA_node_types.h"
#include "BKE_compute_contexts.hh"
@@ -10,6 +11,13 @@
namespace blender::bke {
ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent,
const NodesModifierData &nmd)
: ModifierComputeContext(parent, nmd.modifier.name)
{
nmd_ = &nmd;
}
ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent,
std::string modifier_name)
: ComputeContext(s_static_type, parent), modifier_name_(std::move(modifier_name))
@@ -45,12 +53,14 @@ GroupNodeComputeContext::GroupNodeComputeContext(
}
}
GroupNodeComputeContext::GroupNodeComputeContext(const ComputeContext *parent,
const bNode &node,
const bNodeTree &caller_tree)
: GroupNodeComputeContext(parent, node.identifier)
GroupNodeComputeContext::GroupNodeComputeContext(
const ComputeContext *parent,
const bNode &caller_group_node,
const bNodeTree &caller_tree,
const std::optional<ComputeContextHash> &cached_hash)
: GroupNodeComputeContext(parent, caller_group_node.identifier, cached_hash)
{
caller_group_node_ = &node;
caller_group_node_ = &caller_group_node;
caller_tree_ = &caller_tree;
}
@@ -182,6 +192,12 @@ OperatorComputeContext::OperatorComputeContext(const ComputeContext *parent)
hash_.mix_in(s_static_type, strlen(s_static_type));
}
OperatorComputeContext::OperatorComputeContext(const ComputeContext *parent, const bNodeTree &tree)
: OperatorComputeContext(parent)
{
tree_ = &tree;
}
void OperatorComputeContext::print_current_in_line(std::ostream &stream) const
{
stream << "Operator";

View File

@@ -638,7 +638,7 @@ static wmOperatorStatus run_node_group_exec(bContext *C, wmOperator *op)
}
geo_log::GeoTreeLog &tree_log = eval_log.log->get_tree_log(compute_context.hash());
tree_log.ensure_node_warnings(node_tree);
tree_log.ensure_node_warnings(*bmain);
for (const geo_log::NodeWarning &warning : tree_log.all_warnings) {
if (warning.type == geo_log::NodeWarningType::Info) {
BKE_report(op->reports, RPT_INFO, warning.message.c_str());

View File

@@ -4367,8 +4367,7 @@ static Set<const bNodeSocket *> find_sockets_on_active_gizmo_paths(const bContex
std::optional<ComputeContextHash> current_compute_context_hash =
[&]() -> std::optional<ComputeContextHash> {
ComputeContextBuilder compute_context_builder;
compute_context_builder.push<bke::ModifierComputeContext>(
object_and_modifier->nmd->modifier.name);
compute_context_builder.push<bke::ModifierComputeContext>(*object_and_modifier->nmd);
if (!ed::space_node::push_compute_context_for_tree_path(snode, compute_context_builder)) {
return std::nullopt;
}
@@ -4996,7 +4995,7 @@ static void draw_nodetree(const bContext &C,
if (ntree.type == NTREE_GEOMETRY) {
tree_draw_ctx.tree_logs = geo_log::GeoModifierLog::get_contextual_tree_logs(*snode);
tree_draw_ctx.tree_logs.foreach_tree_log([&](geo_log::GeoTreeLog &log) {
log.ensure_node_warnings(&ntree);
log.ensure_node_warnings(*tree_draw_ctx.bmain);
log.ensure_execution_times();
});
const WorkSpace *workspace = CTX_wm_workspace(&C);

View File

@@ -1959,7 +1959,7 @@ static blender::nodes::geo_eval_log::GeoTreeLog *get_nodes_modifier_log(NodesMod
if (!nmd.runtime->eval_log) {
return nullptr;
}
blender::bke::ModifierComputeContext compute_context{nullptr, nmd.modifier.name};
blender::bke::ModifierComputeContext compute_context{nullptr, nmd};
return &nmd.runtime->eval_log->get_tree_log(compute_context.hash());
}
@@ -1967,7 +1967,7 @@ static blender::Span<blender::nodes::geo_eval_log::NodeWarning> get_node_modifie
NodesModifierData &nmd)
{
if (auto *log = get_nodes_modifier_log(nmd)) {
log->ensure_node_warnings(nmd.node_group);
log->ensure_node_warnings(nmd);
return log->all_warnings;
}
return {};

View File

@@ -672,7 +672,7 @@ static void find_side_effect_nodes_for_viewer_path(
}
ComputeContextBuilder compute_context_builder;
compute_context_builder.push<bke::ModifierComputeContext>(parsed_path->modifier_name);
compute_context_builder.push<bke::ModifierComputeContext>(nmd);
for (const ViewerPathElem *elem : parsed_path->node_path) {
if (!ed::viewer_path::add_compute_context_for_viewer_path_elem(*elem, compute_context_builder))
@@ -691,7 +691,7 @@ static void find_side_effect_nodes_for_nested_node(
nodes::GeoNodesSideEffectNodes &r_side_effect_nodes)
{
ComputeContextBuilder compute_context_builder;
compute_context_builder.push<bke::ModifierComputeContext>(nmd.modifier.name);
compute_context_builder.push<bke::ModifierComputeContext>(nmd);
int nested_node_id = root_nested_node_id;
const bNodeTree *tree = nmd.node_group;
@@ -835,8 +835,7 @@ static void find_socket_log_contexts(const NodesModifierData &nmd,
continue;
}
const Map<const bke::bNodeTreeZone *, ComputeContextHash> hash_by_zone =
geo_log::GeoModifierLog::get_context_hash_by_zone_for_node_editor(snode,
nmd.modifier.name);
geo_log::GeoModifierLog::get_context_hash_by_zone_for_node_editor(snode, nmd);
for (const ComputeContextHash &hash : hash_by_zone.values()) {
r_socket_log_contexts.add(hash);
}
@@ -1834,7 +1833,7 @@ static void modifyGeometry(ModifierData *md,
find_side_effect_nodes(*nmd, *ctx, side_effect_nodes, socket_log_contexts);
call_data.side_effect_nodes = &side_effect_nodes;
bke::ModifierComputeContext modifier_compute_context{nullptr, nmd->modifier.name};
bke::ModifierComputeContext modifier_compute_context{nullptr, *nmd};
geometry_set = nodes::execute_geometry_nodes_on_geometry(
tree, properties, modifier_compute_context, call_data, std::move(geometry_set));
@@ -1931,7 +1930,7 @@ static geo_log::GeoTreeLog *get_root_tree_log(const NodesModifierData &nmd)
if (!nmd.runtime->eval_log) {
return nullptr;
}
bke::ModifierComputeContext compute_context{nullptr, nmd.modifier.name};
bke::ModifierComputeContext compute_context{nullptr, nmd};
return &nmd.runtime->eval_log->get_tree_log(compute_context.hash());
}
@@ -2518,7 +2517,7 @@ static void draw_warnings(const bContext *C,
if (!tree_log) {
return;
}
tree_log->ensure_node_warnings(nmd.node_group);
tree_log->ensure_node_warnings(*CTX_data_main(C));
const int warnings_num = tree_log->all_warnings.size();
if (warnings_num == 0) {
return;

View File

@@ -34,20 +34,20 @@
#include "BLI_enumerable_thread_specific.hh"
#include "BLI_generic_pointer.hh"
#include "BLI_linear_allocator_chunked_list.hh"
#include "BLI_multi_value_map.hh"
#include "BKE_geometry_set.hh"
#include "BKE_node.hh"
#include "BKE_node_tree_zones.hh"
#include "BKE_volume_grid_fwd.hh"
#include "NOD_geometry_nodes_bundle.hh"
#include "NOD_socket_interface_key.hh"
#include "FN_field.hh"
#include "DNA_node_types.h"
struct SpaceNode;
struct NodesModifierData;
namespace blender::nodes::geo_eval_log {
@@ -233,6 +233,11 @@ class GeoTreeLogger {
std::optional<ComputeContextHash> parent_hash;
std::optional<int32_t> parent_node_id;
Vector<ComputeContextHash> children_hashes;
/**
* The #ID.session_uid of the tree that this logger is for. It's an optional value because under
* some circumstances it's not possible to know this exactly currently (e.g. for closures).
*/
std::optional<uint32_t> tree_orig_session_uid;
/** The time spend in the compute context that this logger corresponds to. */
std::chrono::nanoseconds execution_time{};
@@ -347,7 +352,15 @@ class GeoTreeLog {
GeoTreeLog(GeoModifierLog *modifier_log, Vector<GeoTreeLogger *> tree_loggers);
~GeoTreeLog();
void ensure_node_warnings(const bNodeTree *tree);
/**
* Propagate node warnings. This needs access to the node group pointers, because propagation
* settings are stored on the nodes. However, the log can only store weak pointers (in the form
* of e.g. session ids) to original data to avoid dangling pointers.
*/
void ensure_node_warnings(const NodesModifierData &nmd);
void ensure_node_warnings(const Main &bmain);
void ensure_node_warnings(const Map<uint32_t, const bNodeTree *> &orig_tree_by_session_uid);
void ensure_execution_times();
void ensure_socket_values();
void ensure_viewer_node_logs();
@@ -439,7 +452,7 @@ class GeoModifierLog {
* Utility accessor to logged data.
*/
static Map<const bke::bNodeTreeZone *, ComputeContextHash>
get_context_hash_by_zone_for_node_editor(const SpaceNode &snode, StringRefNull modifier_name);
get_context_hash_by_zone_for_node_editor(const SpaceNode &snode, const NodesModifierData &nmd);
static Map<const bke::bNodeTreeZone *, ComputeContextHash>
get_context_hash_by_zone_for_node_editor(const SpaceNode &snode,
ComputeContextBuilder &compute_context_builder);

View File

@@ -265,7 +265,7 @@ static void foreach_active_gizmo_in_open_node_editor(
}
const ComputeContext *prev_compute_context = compute_context_builder.current();
compute_context_builder.push<bke::ModifierComputeContext>(nmd.modifier.name);
compute_context_builder.push<bke::ModifierComputeContext>(nmd);
BLI_SCOPED_DEFER([&]() { compute_context_builder.pop_until(prev_compute_context); });
if (!ed::space_node::push_compute_context_for_tree_path(snode, compute_context_builder)) {
@@ -369,7 +369,7 @@ static void foreach_active_gizmo_exposed_to_modifier(
if (!tree.runtime->gizmo_propagation) {
return;
}
compute_context_builder.push<bke::ModifierComputeContext>(nmd.modifier.name);
compute_context_builder.push<bke::ModifierComputeContext>(nmd);
BLI_SCOPED_DEFER([&]() { compute_context_builder.pop(); });
for (auto &&item : tree.runtime->gizmo_propagation->gizmo_inputs_by_group_inputs.items()) {

View File

@@ -1175,8 +1175,10 @@ class LazyFunctionForGroupNode : public LazyFunction {
Storage *storage = static_cast<Storage *>(context.storage);
/* The compute context changes when entering a node group. */
bke::GroupNodeComputeContext compute_context{
user_data->compute_context, group_node_.identifier, storage->context_hash_cache};
bke::GroupNodeComputeContext compute_context{user_data->compute_context,
group_node_,
group_node_.owner_tree(),
storage->context_hash_cache};
storage->context_hash_cache = compute_context.hash();
GeoNodesLFUserData group_user_data = *user_data;

View File

@@ -14,6 +14,7 @@
#include "BKE_compute_contexts.hh"
#include "BKE_curves.hh"
#include "BKE_geometry_nodes_gizmos_transforms.hh"
#include "BKE_lib_query.hh"
#include "BKE_node_legacy_types.hh"
#include "BKE_node_runtime.hh"
#include "BKE_node_socket_value.hh"
@@ -34,6 +35,8 @@
#include "UI_resources.hh"
#include "DEG_depsgraph_query.hh"
namespace blender::nodes::geo_eval_log {
using bke::bNodeTreeZone;
@@ -343,11 +346,58 @@ static bool warning_is_propagated(const NodeWarningPropagation propagation,
return true;
}
void GeoTreeLog::ensure_node_warnings(const bNodeTree *tree)
void GeoTreeLog::ensure_node_warnings(const NodesModifierData &nmd)
{
if (reduced_node_warnings_) {
return;
}
if (!nmd.node_group) {
reduced_node_warnings_ = true;
return;
}
Map<uint32_t, const bNodeTree *> map;
BKE_library_foreach_ID_link(
nullptr,
&nmd.node_group->id,
[&](LibraryIDLinkCallbackData *cb_data) {
if (ID *id = *cb_data->id_pointer) {
if (GS(id->name) == ID_NT) {
const bNodeTree *tree = reinterpret_cast<const bNodeTree *>(id);
map.add(id->session_uid, tree);
}
}
return IDWALK_RET_NOP;
},
nullptr,
IDWALK_READONLY | IDWALK_RECURSE);
this->ensure_node_warnings(map);
}
void GeoTreeLog::ensure_node_warnings(const Main &bmain)
{
if (reduced_node_warnings_) {
return;
}
Map<uint32_t, const bNodeTree *> map;
FOREACH_NODETREE_BEGIN (const_cast<Main *>(&bmain), tree, id) {
map.add_new(tree->id.session_uid, tree);
}
FOREACH_NODETREE_END;
this->ensure_node_warnings(map);
}
void GeoTreeLog::ensure_node_warnings(
const Map<uint32_t, const bNodeTree *> &orig_tree_by_session_uid)
{
if (reduced_node_warnings_) {
return;
}
if (tree_loggers_.is_empty()) {
return;
}
const std::optional<uint32_t> tree_uid = tree_loggers_[0]->tree_orig_session_uid;
const bNodeTree *tree = tree_uid ? orig_tree_by_session_uid.lookup_default(*tree_uid, nullptr) :
nullptr;
for (GeoTreeLogger *tree_logger : tree_loggers_) {
for (const GeoTreeLogger::WarningWithNode &warning : tree_logger->node_warnings) {
@@ -368,23 +418,17 @@ void GeoTreeLog::ensure_node_warnings(const bNodeTree *tree)
if (child_log.tree_loggers_.is_empty()) {
continue;
}
const GeoTreeLogger &first_child_logger = *child_log.tree_loggers_[0];
NodeWarningPropagation propagation = NODE_WARNING_PROPAGATION_ALL;
const bNodeTree *child_tree = nullptr;
const std::optional<int32_t> &parent_node_id = child_log.tree_loggers_[0]->parent_node_id;
if (tree && parent_node_id) {
if (const bNode *node = tree->node_by_id(*parent_node_id)) {
propagation = NodeWarningPropagation(node->warning_propagation);
if (node->is_group() && node->id) {
child_tree = reinterpret_cast<const bNodeTree *>(node->id);
}
else if (bke::all_zone_output_node_types().contains(node->type_legacy)) {
child_tree = tree;
}
const std::optional<int32_t> &caller_node_id = first_child_logger.parent_node_id;
if (tree && caller_node_id) {
if (const bNode *caller_node = tree->node_by_id(*caller_node_id)) {
propagation = NodeWarningPropagation(caller_node->warning_propagation);
}
}
child_log.ensure_node_warnings(child_tree);
if (parent_node_id.has_value()) {
this->nodes.lookup_or_add_default(*parent_node_id)
child_log.ensure_node_warnings(orig_tree_by_session_uid);
if (caller_node_id.has_value()) {
this->nodes.lookup_or_add_default(*caller_node_id)
.warnings.add_multiple(child_log.all_warnings);
}
for (const NodeWarning &warning : child_log.all_warnings) {
@@ -626,6 +670,20 @@ bool GeoTreeLog::try_convert_primitive_socket_value(const GenericValueLog &value
return true;
}
static std::optional<uint32_t> get_original_session_uid(const ID *id)
{
if (!id) {
return {};
}
if (DEG_is_original_id(id)) {
return id->session_uid;
}
if (const ID *id_orig = DEG_get_original_id(id)) {
return id_orig->session_uid;
}
return {};
}
GeoTreeLogger &GeoModifierLog::get_local_tree_logger(const ComputeContext &compute_context)
{
LocalData &local_data = data_per_thread_.local();
@@ -640,35 +698,58 @@ GeoTreeLogger &GeoModifierLog::get_local_tree_logger(const ComputeContext &compu
GeoTreeLogger &tree_logger = *tree_logger_ptr;
tree_logger.allocator = &local_data.allocator;
const ComputeContext *parent_compute_context = compute_context.parent();
std::optional<uint32_t> parent_tree_session_uid;
if (parent_compute_context != nullptr) {
tree_logger.parent_hash = parent_compute_context->hash();
GeoTreeLogger &parent_logger = this->get_local_tree_logger(*parent_compute_context);
parent_logger.children_hashes.append(compute_context.hash());
parent_tree_session_uid = parent_logger.tree_orig_session_uid;
}
if (const auto *context = dynamic_cast<const bke::GroupNodeComputeContext *>(&compute_context)) {
tree_logger.parent_node_id.emplace(context->node_id());
if (const bNode *caller_node = context->caller_group_node()) {
tree_logger.tree_orig_session_uid = get_original_session_uid(caller_node->id);
}
}
else if (const auto *context = dynamic_cast<const bke::RepeatZoneComputeContext *>(
&compute_context))
{
tree_logger.parent_node_id.emplace(context->output_node_id());
tree_logger.tree_orig_session_uid = parent_tree_session_uid;
}
else if (const auto *context =
dynamic_cast<const bke::ForeachGeometryElementZoneComputeContext *>(
&compute_context))
{
tree_logger.parent_node_id.emplace(context->output_node_id());
tree_logger.tree_orig_session_uid = parent_tree_session_uid;
}
else if (const auto *context = dynamic_cast<const bke::SimulationZoneComputeContext *>(
&compute_context))
{
tree_logger.parent_node_id.emplace(context->output_node_id());
tree_logger.tree_orig_session_uid = parent_tree_session_uid;
}
else if (const auto *context = dynamic_cast<const bke::EvaluateClosureComputeContext *>(
&compute_context))
{
tree_logger.parent_node_id.emplace(context->node_id());
}
else if (const auto *context = dynamic_cast<const bke::ModifierComputeContext *>(
&compute_context))
{
if (const NodesModifierData *nmd = context->nmd()) {
tree_logger.tree_orig_session_uid = get_original_session_uid(
reinterpret_cast<const ID *>(nmd->node_group));
}
}
else if (const auto *context = dynamic_cast<const bke::OperatorComputeContext *>(
&compute_context))
{
if (const bNodeTree *tree = context->tree()) {
tree_logger.tree_orig_session_uid = tree->id.session_uid;
}
}
return tree_logger;
}
@@ -746,10 +827,10 @@ Map<const bNodeTreeZone *, ComputeContextHash> GeoModifierLog::
}
Map<const bNodeTreeZone *, ComputeContextHash> GeoModifierLog::
get_context_hash_by_zone_for_node_editor(const SpaceNode &snode, StringRefNull modifier_name)
get_context_hash_by_zone_for_node_editor(const SpaceNode &snode, const NodesModifierData &nmd)
{
ComputeContextBuilder compute_context_builder;
compute_context_builder.push<bke::ModifierComputeContext>(modifier_name);
compute_context_builder.push<bke::ModifierComputeContext>(nmd);
return get_context_hash_by_zone_for_node_editor(snode, compute_context_builder);
}
@@ -767,8 +848,8 @@ ContextualGeoTreeLogs GeoModifierLog::get_contextual_tree_logs(const SpaceNode &
return {};
}
const Map<const bNodeTreeZone *, ComputeContextHash> hash_by_zone =
GeoModifierLog::get_context_hash_by_zone_for_node_editor(
snode, object_and_modifier->nmd->modifier.name);
GeoModifierLog::get_context_hash_by_zone_for_node_editor(snode,
*object_and_modifier->nmd);
Map<const bke::bNodeTreeZone *, GeoTreeLog *> tree_logs_by_zone;
for (const auto item : hash_by_zone.items()) {
GeoTreeLog &tree_log = modifier_log->get_tree_log(item.value);
@@ -823,7 +904,7 @@ const ViewerNodeLog *GeoModifierLog::find_viewer_node_log_for_path(const ViewerP
nodes::geo_eval_log::GeoModifierLog *modifier_log = nmd->runtime->eval_log.get();
ComputeContextBuilder compute_context_builder;
compute_context_builder.push<bke::ModifierComputeContext>(parsed_path->modifier_name);
compute_context_builder.push<bke::ModifierComputeContext>(*nmd);
for (const ViewerPathElem *elem : parsed_path->node_path) {
if (!ed::viewer_path::add_compute_context_for_viewer_path_elem(*elem, compute_context_builder))
{

View File

@@ -773,7 +773,7 @@ bool backpropagate_socket_values(bContext &C,
}
}
/* Set new values for modifier inputs. */
const bke::ModifierComputeContext modifier_context{nullptr, nmd.modifier.name};
const bke::ModifierComputeContext modifier_context{nullptr, nmd};
for (const bNode *group_input_node : nmd.node_group->group_input_nodes()) {
for (const bNodeSocket *socket : group_input_node->output_sockets().drop_back(1)) {
if (const SocketValueVariant *value = value_by_socket.lookup_ptr(