[Bf-blender-cvs] [090be2775e3] master: Geometry Nodes: fix force-computing multiple non-output sockets

Jacques Lucke noreply at git.blender.org
Thu Oct 21 15:50:22 CEST 2021


Commit: 090be2775e3d7c0dc3f7956b05c7fb9a72c4bdfc
Author: Jacques Lucke
Date:   Thu Oct 21 15:49:29 2021 +0200
Branches: master
https://developer.blender.org/rB090be2775e3d7c0dc3f7956b05c7fb9a72c4bdfc

Geometry Nodes: fix force-computing multiple non-output sockets

There were some issues when multiple inputs of the same node
were forced to be computed (e.g. for the spreadsheet), but none
of the node outputs (if existant) were used. Essentially the node
was marked as "finished" too early in this case.

This fix is necessary for the improved viewer node (T92167).

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

M	source/blender/modifiers/intern/MOD_nodes.cc
M	source/blender/modifiers/intern/MOD_nodes_evaluator.cc
M	source/blender/nodes/NOD_geometry_nodes_eval_log.hh
M	source/blender/nodes/intern/geometry_nodes_eval_log.cc

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

diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc
index b5ca5f3545f..562589e2610 100644
--- a/source/blender/modifiers/intern/MOD_nodes.cc
+++ b/source/blender/modifiers/intern/MOD_nodes.cc
@@ -758,32 +758,33 @@ static Vector<SpaceSpreadsheet *> find_spreadsheet_editors(Main *bmain)
   return spreadsheets;
 }
 
-static DSocket try_get_socket_to_preview_for_spreadsheet(SpaceSpreadsheet *sspreadsheet,
-                                                         NodesModifierData *nmd,
-                                                         const ModifierEvalContext *ctx,
-                                                         const DerivedNodeTree &tree)
+static void find_sockets_to_preview_for_spreadsheet(SpaceSpreadsheet *sspreadsheet,
+                                                    NodesModifierData *nmd,
+                                                    const ModifierEvalContext *ctx,
+                                                    const DerivedNodeTree &tree,
+                                                    Set<DSocket> &r_sockets_to_preview)
 {
   Vector<SpreadsheetContext *> context_path = sspreadsheet->context_path;
   if (context_path.size() < 3) {
-    return {};
+    return;
   }
   if (context_path[0]->type != SPREADSHEET_CONTEXT_OBJECT) {
-    return {};
+    return;
   }
   if (context_path[1]->type != SPREADSHEET_CONTEXT_MODIFIER) {
-    return {};
+    return;
   }
   SpreadsheetContextObject *object_context = (SpreadsheetContextObject *)context_path[0];
   if (object_context->object != DEG_get_original_object(ctx->object)) {
-    return {};
+    return;
   }
   SpreadsheetContextModifier *modifier_context = (SpreadsheetContextModifier *)context_path[1];
   if (StringRef(modifier_context->modifier_name) != nmd->modifier.name) {
-    return {};
+    return;
   }
   for (SpreadsheetContext *context : context_path.as_span().drop_front(2)) {
     if (context->type != SPREADSHEET_CONTEXT_NODE) {
-      return {};
+      return;
     }
   }
 
@@ -802,11 +803,11 @@ static DSocket try_get_socket_to_preview_for_spreadsheet(SpaceSpreadsheet *sspre
       }
     }
     if (found_node == nullptr) {
-      return {};
+      return;
     }
     context = context->child_context(*found_node);
     if (context == nullptr) {
-      return {};
+      return;
     }
   }
 
@@ -814,10 +815,13 @@ static DSocket try_get_socket_to_preview_for_spreadsheet(SpaceSpreadsheet *sspre
   for (const NodeRef *node_ref : tree_ref.nodes_by_type("GeometryNodeViewer")) {
     if (node_ref->name() == last_context->node_name) {
       const DNode viewer_node{context, node_ref};
-      return viewer_node.input(0);
+      for (const InputSocketRef *input_socket : node_ref->inputs()) {
+        if (input_socket->is_available() && input_socket->is_logically_linked()) {
+          r_sockets_to_preview.add(DSocket{context, input_socket});
+        }
+      }
     }
   }
-  return {};
 }
 
 static void find_sockets_to_preview(NodesModifierData *nmd,
@@ -831,10 +835,7 @@ static void find_sockets_to_preview(NodesModifierData *nmd,
    * intermediate geometries cached for display. */
   Vector<SpaceSpreadsheet *> spreadsheets = find_spreadsheet_editors(bmain);
   for (SpaceSpreadsheet *sspreadsheet : spreadsheets) {
-    const DSocket socket = try_get_socket_to_preview_for_spreadsheet(sspreadsheet, nmd, ctx, tree);
-    if (socket) {
-      r_sockets_to_preview.add(socket);
-    }
+    find_sockets_to_preview_for_spreadsheet(sspreadsheet, nmd, ctx, tree, r_sockets_to_preview);
   }
 }
 
diff --git a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
index 51dd210c77a..69132f6c177 100644
--- a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
+++ b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
@@ -126,6 +126,12 @@ struct InputState {
    * changed by others anymore.
    */
   bool was_ready_for_execution = false;
+
+  /**
+   * True when this input has to be computed for logging/debugging purposes, regardless of whether
+   * it is needed for some output.
+   */
+  bool force_compute = false;
 };
 
 struct OutputState {
@@ -497,6 +503,14 @@ class GeometryNodesEvaluator {
             this->initialize_node_state(item.node, *item.state, allocator);
           }
         });
+
+    /* Mark input sockets that have to be computed. */
+    for (const DSocket &socket : params_.force_compute_sockets) {
+      NodeState &node_state = *node_states_.lookup_key_as(socket.node()).state;
+      if (socket->is_input()) {
+        node_state.inputs[socket->index()].force_compute = true;
+      }
+    }
   }
 
   void initialize_node_state(const DNode node, NodeState &node_state, LinearAllocator<> &allocator)
@@ -743,7 +757,8 @@ class GeometryNodesEvaluator {
     return do_execute_node;
   }
 
-  /* A node is finished when it has computed all outputs that may be used. */
+  /* A node is finished when it has computed all outputs that may be used have been computed and
+   * when no input is still forced to be computed. */
   bool finish_node_if_possible(LockedNode &locked_node)
   {
     if (locked_node.node_state.node_has_finished) {
@@ -752,35 +767,41 @@ class GeometryNodesEvaluator {
     }
 
     /* Check if there is any output that might be used but has not been computed yet. */
-    bool has_remaining_output = false;
     for (OutputState &output_state : locked_node.node_state.outputs) {
       if (output_state.has_been_computed) {
         continue;
       }
       if (output_state.output_usage != ValueUsage::Unused) {
-        has_remaining_output = true;
-        break;
+        return false;
       }
     }
-    if (!has_remaining_output) {
-      /* If there are no remaining outputs, all the inputs can be destructed and/or can become
-       * unused. This can also trigger a chain reaction where nodes to the left become finished
-       * too. */
-      for (const int i : locked_node.node->inputs().index_range()) {
-        const DInputSocket socket = locked_node.node.input(i);
-        InputState &input_state = locked_node.node_state.inputs[i];
-        if (input_state.usage == ValueUsage::Maybe) {
-          this->set_input_unused(locked_node, socket);
-        }
-        else if (input_state.usage == ValueUsage::Required) {
-          /* The value was required, so it cannot become unused. However, we can destruct the
-           * value. */
-          this->destruct_input_value_if_exists(locked_node, socket);
+
+    /* Check if there is an input that still has to be computed. */
+    for (InputState &input_state : locked_node.node_state.inputs) {
+      if (input_state.force_compute) {
+        if (!input_state.was_ready_for_execution) {
+          return false;
         }
       }
-      locked_node.node_state.node_has_finished = true;
     }
-    return locked_node.node_state.node_has_finished;
+
+    /* If there are no remaining outputs, all the inputs can be destructed and/or can become
+     * unused. This can also trigger a chain reaction where nodes to the left become finished
+     * too. */
+    for (const int i : locked_node.node->inputs().index_range()) {
+      const DInputSocket socket = locked_node.node.input(i);
+      InputState &input_state = locked_node.node_state.inputs[i];
+      if (input_state.usage == ValueUsage::Maybe) {
+        this->set_input_unused(locked_node, socket);
+      }
+      else if (input_state.usage == ValueUsage::Required) {
+        /* The value was required, so it cannot become unused. However, we can destruct the
+         * value. */
+        this->destruct_input_value_if_exists(locked_node, socket);
+      }
+    }
+    locked_node.node_state.node_has_finished = true;
+    return true;
   }
 
   bool prepare_node_outputs_for_execution(LockedNode &locked_node)
diff --git a/source/blender/nodes/NOD_geometry_nodes_eval_log.hh b/source/blender/nodes/NOD_geometry_nodes_eval_log.hh
index ff8e137e341..ffe2a0d63c3 100644
--- a/source/blender/nodes/NOD_geometry_nodes_eval_log.hh
+++ b/source/blender/nodes/NOD_geometry_nodes_eval_log.hh
@@ -181,17 +181,19 @@ class LocalGeoLogger {
 /** The root logger class. */
 class GeoLogger {
  private:
-  /** The entire geometry of sockets in this set should be cached, because e.g. the spreadsheet
-   * displays the data. We don't log the entire geometry at all places, because that would require
-   * way too much memory. */
-  Set<DSocket> log_full_geometry_sockets_;
+  /**
+   * Log the entire value for these sockets, because they may be inspected afterwards.
+   * We don't log everything, because that would take up too much memory and cause significant
+   * slowdowns.
+   */
+  Set<DSocket> log_full_sockets_;
   threading::EnumerableThreadSpecific<LocalGeoLogger> threadlocals_;
 
   friend LocalGeoLogger;
 
  public:
-  GeoLogger(Set<DSocket> log_full_geometry_sockets)
-      : log_full_geometry_sockets_(std::move(log_full_geometry_sockets)),
+  GeoLogger(Set<DSocket> log_full_sockets)
+      : log_full_sockets_(std::move(log_full_sockets)),
         threadlocals_([this]() { return LocalGeoLogger(*this); })
   {
   }
diff --git a/source/blender/nodes/intern/geometry_nodes_eval_log.cc b/source/blender/nodes/intern/geometry_nodes_eval_log.cc
index fa9bf09d8d9..d39f8367686 100644
--- a/source/blender/nodes/intern/geometry_nodes_eval_log.cc
+++ b/source/blender/nodes/intern/geometry_nodes_eval_log.cc
@@ -354,7 +354,7 @@ void LocalGeoLogger::log_value_for_sockets(Span<DSocket> sockets, GPointer value
   if (type.is<GeometrySet>()) {
     bool log_full_geometry = false;
     for (const DSocket &socket : sockets) {
-      if (main_logger_->log_full_geometry_sockets_.contains(socket)) {
+      if (main_logger_->log_full_sockets_.contains(socket)) {
         log_full_geometry = true;
         break;
       }



More information about the Bf-blender-cvs mailing list