From 385bd0c4e97eb3961521fbd01c3358dbc468e477 Mon Sep 17 00:00:00 2001
From: Sergey Sharybin <sergey@blender.org>
Date: Mon, 9 Jan 2023 15:02:26 +0100
Subject: [PATCH] Fix T103685: Animation on objects that are disabled is
 ignored

Happens, for example, when the object has animation, and disabled for
render, and animation render is performed.

The regression has been uncovered by f12f7800c296 which made it so
the dependency graph relies on runtime visibility tracking and
updates (without updating relations).

The optimization from a while ago in the ff60dd8b18ed got in a way
of the visibilit updates because it removed relation between two
no-op nodes which belong to different IDs, which make the visibility
tracking impossible.

This change makes it so only relations which belong to the same
component are removed. This matches the expectations of the visibility
tracking (which, actually, also needed to happen at the moment of the
initial optimization commit). Technically, this change could introduce
some performance regression, but with the current design design of the
graph it is not really avoidable.

The idea to gain the best performance is to separate relations which
actually define the execution flow, and which are only needed to
define things like visibility dependencies.
---
 .../intern/builder/deg_builder_remove_noop.cc | 48 +++++++++++++++----
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc b/source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc
index 992620a6f03..89e590111a3 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc
@@ -30,9 +30,28 @@ static inline bool is_unused_noop(OperationNode *op_node)
   return op_node->is_noop() && op_node->outlinks.is_empty();
 }
 
+static inline bool is_removable_relation(const Relation *relation)
+{
+  if (relation->from->type != NodeType::OPERATION || relation->to->type != NodeType::OPERATION) {
+    return true;
+  }
+
+  const OperationNode *operation_from = static_cast<OperationNode *>(relation->from);
+  const OperationNode *operation_to = static_cast<OperationNode *>(relation->to);
+
+  /* If the relation connects two different IDs there is a high risk that the removal of the
+   * relation will make it so visibility flushing is not possible at runtime. This happens with
+   * relations like the DoF on camera of custom shape on bines: such relation do not lead to an
+   * actual depsgraph evaluation operation as they are handled on render engine level.
+   *
+   * The indirectly linked objects could have some of their components invisible as well, so
+   * also keep relations which connect different components of the same object so that visibility
+   * tracking happens correct in those cases as well. */
+  return operation_from->owner == operation_to->owner;
+}
+
 void deg_graph_remove_unused_noops(Depsgraph *graph)
 {
-  int num_removed_relations = 0;
   deque<OperationNode *> queue;
 
   for (OperationNode *node : graph->operations) {
@@ -41,18 +60,19 @@ void deg_graph_remove_unused_noops(Depsgraph *graph)
     }
   }
 
+  Vector<Relation *> relations_to_remove;
+
   while (!queue.empty()) {
     OperationNode *to_remove = queue.front();
     queue.pop_front();
 
-    while (!to_remove->inlinks.is_empty()) {
-      Relation *rel_in = to_remove->inlinks[0];
-      Node *dependency = rel_in->from;
+    for (Relation *rel_in : to_remove->inlinks) {
+      if (!is_removable_relation(rel_in)) {
+        continue;
+      }
 
-      /* Remove the relation. */
-      rel_in->unlink();
-      delete rel_in;
-      num_removed_relations++;
+      Node *dependency = rel_in->from;
+      relations_to_remove.append(rel_in);
 
       /* Queue parent no-op node that has now become unused. */
       OperationNode *operation = dependency->get_exit_operation();
@@ -64,8 +84,16 @@ void deg_graph_remove_unused_noops(Depsgraph *graph)
     /* TODO(Sybren): Remove the node itself. */
   }
 
-  DEG_DEBUG_PRINTF(
-      (::Depsgraph *)graph, BUILD, "Removed %d relations to no-op nodes\n", num_removed_relations);
+  /* Remove the relations. */
+  for (Relation *relation : relations_to_remove) {
+    relation->unlink();
+    delete relation;
+  }
+
+  DEG_DEBUG_PRINTF((::Depsgraph *)graph,
+                   BUILD,
+                   "Removed %d relations to no-op nodes\n",
+                   int(relations_to_remove.size()));
 }
 
 }  // namespace blender::deg