[Bf-blender-cvs] [1d4969905d9] temp-node-error-messages: Changes from review

Hans Goudey noreply at git.blender.org
Tue Feb 16 17:32:40 CET 2021


Commit: 1d4969905d9a7722857ec2d2305f0d78c399f659
Author: Hans Goudey
Date:   Tue Feb 16 10:32:13 2021 -0600
Branches: temp-node-error-messages
https://developer.blender.org/rB1d4969905d9a7722857ec2d2305f0d78c399f659

Changes from review

===================================================================

M	source/blender/blenkernel/BKE_node_ui_storage.hh
M	source/blender/blenkernel/intern/attribute_access.cc
M	source/blender/blenkernel/intern/node_ui_storage.cc
M	source/blender/editors/space_node/node_draw.cc
M	source/blender/modifiers/intern/MOD_nodes.cc
M	source/blender/nodes/NOD_geometry_exec.hh
M	source/blender/nodes/geometry/nodes/node_geo_attribute_mix.cc
M	source/blender/nodes/intern/node_geometry_exec.cc

===================================================================

diff --git a/source/blender/blenkernel/BKE_node_ui_storage.hh b/source/blender/blenkernel/BKE_node_ui_storage.hh
index b498e75f289..ac0c4dcfab0 100644
--- a/source/blender/blenkernel/BKE_node_ui_storage.hh
+++ b/source/blender/blenkernel/BKE_node_ui_storage.hh
@@ -32,6 +32,8 @@ struct ModifierData;
 struct NodeUIStorageContextModifier {
   std::string object_name;
   SessionUUID modifier_session_uuid;
+  /* TODO: The same node tree can be used multiple times in a parent node tree,
+   * so the tree path should be added to the context here. */
 
   NodeUIStorageContextModifier(const Object &object, const ModifierData &modifier)
   {
@@ -41,10 +43,9 @@ struct NodeUIStorageContextModifier {
 
   uint64_t hash() const
   {
-
     const uint64_t hash1 = blender::DefaultHash<std::string>{}(object_name);
     const uint64_t hash2 = BLI_session_uuid_hash_uint64(&modifier_session_uuid);
-    return hash1 ^ (hash2 * 33);
+    return hash1 ^ (hash2 * 33); /* Copied from DefaultHash pair hash function. */
   }
 
   bool operator==(const NodeUIStorageContextModifier &other) const
@@ -71,8 +72,6 @@ struct NodeUIStorage {
 
 struct NodeTreeUIStorage {
   blender::Map<std::string, blender::Map<NodeUIStorageContextModifier, NodeUIStorage>> node_map;
-
-  NodeTreeUIStorage() = default;
 };
 
 void BKE_nodetree_ui_storage_clear(struct bNodeTree &ntree);
@@ -83,4 +82,4 @@ void BKE_nodetree_error_message_add(struct bNodeTree &ntree,
                                     const NodeUIStorageContextModifier &context,
                                     const struct bNode &node,
                                     const NodeWarningType type,
-                                    const std::string &message);
+                                    std::string message);
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index ebe9d591dbd..f594ec54a7e 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -1662,10 +1662,7 @@ ReadAttributePtr GeometryComponent::attribute_get_for_read(const StringRef attri
   if (attribute) {
     return attribute;
   }
-  if (default_value != nullptr) {
-    return this->attribute_get_constant_for_read(domain, data_type, default_value);
-  }
-  return nullptr;
+  return this->attribute_get_constant_for_read(domain, data_type, default_value);
 }
 
 blender::bke::ReadAttributePtr GeometryComponent::attribute_get_constant_for_read(
@@ -1829,7 +1826,6 @@ void OutputAttributePtr::save()
 OutputAttributePtr::~OutputAttributePtr()
 {
   if (attribute_) {
-    /* TODO: Fix that this is printing all the time now, even after applying and saving. */
     CLOG_ERROR(&LOG, "Forgot to call #save or #apply_span_and_save.");
   }
 }
diff --git a/source/blender/blenkernel/intern/node_ui_storage.cc b/source/blender/blenkernel/intern/node_ui_storage.cc
index 5df1b5d34d2..696d228a425 100644
--- a/source/blender/blenkernel/intern/node_ui_storage.cc
+++ b/source/blender/blenkernel/intern/node_ui_storage.cc
@@ -79,11 +79,13 @@ static void node_error_message_log(bNodeTree &ntree,
   }
 }
 
+/* USE THE AS METHODS */
+
 void BKE_nodetree_error_message_add(bNodeTree &ntree,
                                     const NodeUIStorageContextModifier &context,
                                     const bNode &node,
                                     const NodeWarningType type,
-                                    const std::string &message)
+                                    std::string message)
 {
   BLI_assert(ntree.runtime != nullptr);
   NodeTreeUIStorage &node_tree_ui_storage = *ntree.runtime;
@@ -95,5 +97,5 @@ void BKE_nodetree_error_message_add(bNodeTree &ntree,
 
   NodeUIStorage &node_ui_storage = context_to_storage_map.lookup_or_add_default(context);
 
-  node_ui_storage.warnings.append({type, message});
+  node_ui_storage.warnings.append({type, std::move(message)});
 }
diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index ebd87ffdcbd..b71652ea58f 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -1727,7 +1727,7 @@ void node_set_cursor(wmWindow *win, SpaceNode *snode, float cursor[2])
     }
     else {
       /* check nodes front to back */
-      LISTBASE_FOREACH_BACKWARD (bNode *, node, &ntree->nodes) {
+      for (node = (bNode *)ntree->nodes.last; node; node = node->prev) {
         if (BLI_rctf_isect_pt(&node->totr, cursor[0], cursor[1])) {
           break; /* first hit on node stops */
         }
diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc
index 96255a9b6b4..210251142a1 100644
--- a/source/blender/modifiers/intern/MOD_nodes.cc
+++ b/source/blender/modifiers/intern/MOD_nodes.cc
@@ -274,10 +274,6 @@ class GeometryNodesEvaluator {
 
   Vector<GMutablePointer> execute()
   {
-    bNodeTree *original_ntree = (bNodeTree *)DEG_get_original_id(&(ID &)btree_);
-    BKE_nodetree_ui_storage_clear(*original_ntree);
-    BKE_nodetree_ui_storage_ensure(*original_ntree);
-
     Vector<GMutablePointer> results;
     for (const DInputSocket *group_output : group_outputs_) {
       Vector<GMutablePointer> result = this->get_input_values(*group_output);
@@ -1011,6 +1007,10 @@ static GeometrySet compute_geometry(const DerivedNodeTree &tree,
   group_outputs.append(&socket_to_compute);
 
   bNodeTree *ntree = tree.btree();
+  bNodeTree *original_ntree = (bNodeTree *)DEG_get_original_id((ID *)ntree);
+  BKE_nodetree_ui_storage_clear(*original_ntree);
+  BKE_nodetree_ui_storage_ensure(*original_ntree);
+
   GeometryNodesEvaluator evaluator{*ntree,
                                    group_inputs,
                                    group_outputs,
@@ -1019,6 +1019,7 @@ static GeometrySet compute_geometry(const DerivedNodeTree &tree,
                                    ctx->object,
                                    (ModifierData *)nmd,
                                    ctx->depsgraph};
+
   Vector<GMutablePointer> results = evaluator.execute();
   BLI_assert(results.size() == 1);
   GMutablePointer result = results[0];
diff --git a/source/blender/nodes/NOD_geometry_exec.hh b/source/blender/nodes/NOD_geometry_exec.hh
index f1f9a087eed..60fa41ad918 100644
--- a/source/blender/nodes/NOD_geometry_exec.hh
+++ b/source/blender/nodes/NOD_geometry_exec.hh
@@ -205,17 +205,14 @@ class GeoNodeExecParams {
   }
 
   /**
-   * Add an error message displayed at the bottom of the node when displaying the node tree,
+   * Add an error message displayed at the top of the node when displaying the node tree,
    * and potentially elsewhere in Blender.
    */
-  void error_message_add(const NodeWarningType type, const std::string &message) const;
+  void error_message_add(const NodeWarningType type, std::string message) const;
 
   /**
    * Creates a read-only attribute based on node inputs. The method automatically detects which
    * input socket with the given name is available.
-   *
-   * \note Can return null if no attribute with the provided name exists,
-   * in which case an error message is added for the UI.
    */
   ReadAttributePtr get_input_attribute(const StringRef name,
                                        const GeometryComponent &component,
diff --git a/source/blender/nodes/geometry/nodes/node_geo_attribute_mix.cc b/source/blender/nodes/geometry/nodes/node_geo_attribute_mix.cc
index 8f6492c9011..34503b8525e 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_attribute_mix.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_attribute_mix.cc
@@ -168,9 +168,6 @@ static void attribute_mix_calc(GeometryComponent &component, const GeoNodeExecPa
       "A", component, result_domain, result_type, nullptr);
   ReadAttributePtr attribute_b = params.get_input_attribute(
       "B", component, result_domain, result_type, nullptr);
-  if (!attribute_a || !attribute_b) {
-    return;
-  }
 
   do_mix_operation(result_type,
                    node_storage->blend_type,
diff --git a/source/blender/nodes/intern/node_geometry_exec.cc b/source/blender/nodes/intern/node_geometry_exec.cc
index eb0740b6f43..0e3f041ea24 100644
--- a/source/blender/nodes/intern/node_geometry_exec.cc
+++ b/source/blender/nodes/intern/node_geometry_exec.cc
@@ -27,14 +27,13 @@
 
 namespace blender::nodes {
 
-void GeoNodeExecParams::error_message_add(const NodeWarningType type,
-                                          const std::string &message) const
+void GeoNodeExecParams::error_message_add(const NodeWarningType type, std::string message) const
 {
   bNodeTree *original_ntree = (bNodeTree *)DEG_get_original_id(&(ID &)ntree_);
   if (original_ntree != nullptr) {
     const NodeUIStorageContextModifier context = NodeUIStorageContextModifier(*self_object_,
                                                                               *modifier_);
-    BKE_nodetree_error_message_add(*original_ntree, context, node_, type, message);
+    BKE_nodetree_error_message_add(*original_ntree, context, node_, type, std::move(message));
   }
 }
 
@@ -66,17 +65,17 @@ ReadAttributePtr GeoNodeExecParams::get_input_attribute(const StringRef name,
 
   if (found_socket->type == SOCK_STRING) {
     const std::string name = this->get_input<std::string>(found_socket->identifier);
-    if (name.empty()) {
-      return nullptr;
+    /* Try getting the attribute without the default value, if that doesn't work, use the default
+     * value and output an error message. */
+    ReadAttributePtr attribute = component.attribute_try_get_for_read(name, domain, type);
+    if (attribute) {
+      return attribute;
     }
-
-    ReadAttributePtr attribute = component.attribute_get_for_read(
-        name, domain, type, default_value);
-    if (!attribute) {
+    if (default_value == nullptr) {
       this->error_message_add(NodeWarningType::Error,
                               std::string("No attribute with name '") + name + "'.");
     }
-    return attribute;
+    return component.attribute_get_constant_for_read(domain, type, default_value);
   }
   if (found_socket->type == SOCK_FLOAT) {
     const float val

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list