[Bf-blender-cvs] [aa7051c8f21] blender-v3.0-release: Fix T93439: Armature widgets from hidden collections are invisible

Sergey Sharybin noreply at git.blender.org
Mon Nov 29 17:09:41 CET 2021


Commit: aa7051c8f21a6b7e2b413b40317502e69764fa05
Author: Sergey Sharybin
Date:   Mon Nov 29 11:57:31 2021 +0100
Branches: blender-v3.0-release
https://developer.blender.org/rBaa7051c8f21a6b7e2b413b40317502e69764fa05

Fix T93439: Armature widgets from hidden collections are invisible

The are few things in the dependency graph which lead to the issue:
- IDs are only built once.
- Object-data level (Armature, i,e,) builder dependent on the object
  visibility.

This caused issues when an armature is first built as not directly
visible (via driver, i.e.) and then was built as a directly visible.
This did not update visibility flag on the node for the custom shape
object.

The idea behind the fix is to go away form passing object visibility
flag to the geometry-level builders and instead rely on the common
visibility flush post-processing to make sure certain objects are
fully visible when needed.

This is the safest minimal part of the change for 3.0 release which
acts as an additional way to ensure visibility. This means that it
might not be a complete fix (if some configuration was overseen) but
it should not make currently working cases to not work.

The fix should also make modifiers used on rigify widgets to work.

The more complete fix will have `is_object_visible` argument removed
from the geometry-level builder functions.

Differential Revision: https://developer.blender.org/D13404

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

M	source/blender/depsgraph/intern/builder/deg_builder.cc
M	source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
M	source/blender/depsgraph/intern/builder/deg_builder_relations.cc
M	source/blender/depsgraph/intern/builder/deg_builder_relations.h
M	source/blender/depsgraph/intern/builder/deg_builder_relations_rig.cc
M	source/blender/depsgraph/intern/debug/deg_debug_relations_graphviz.cc
M	source/blender/depsgraph/intern/depsgraph_query_foreach.cc
M	source/blender/depsgraph/intern/node/deg_node.cc
M	source/blender/depsgraph/intern/node/deg_node.h
M	source/blender/depsgraph/intern/node/deg_node_component.cc
M	source/blender/depsgraph/intern/node/deg_node_component.h

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

diff --git a/source/blender/depsgraph/intern/builder/deg_builder.cc b/source/blender/depsgraph/intern/builder/deg_builder.cc
index 41512168f57..b2e136c3d3b 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder.cc
@@ -176,7 +176,22 @@ void deg_graph_build_flush_visibility(Depsgraph *graph)
     for (Relation *rel : op_node->inlinks) {
       if (rel->from->type == NodeType::OPERATION) {
         OperationNode *op_from = (OperationNode *)rel->from;
-        op_from->owner->affects_directly_visible |= op_node->owner->affects_directly_visible;
+        ComponentNode *comp_from = op_from->owner;
+        const bool target_directly_visible = op_node->owner->affects_directly_visible;
+
+        /* Visibility component forces all components of the current ID to be considered as
+         * affecting directly visible. */
+        if (comp_from->type == NodeType::VISIBILITY) {
+          if (target_directly_visible) {
+            IDNode *id_node_from = comp_from->owner;
+            for (ComponentNode *comp_node : id_node_from->components.values()) {
+              comp_node->affects_directly_visible |= target_directly_visible;
+            }
+          }
+        }
+        else {
+          comp_from->affects_directly_visible |= target_directly_visible;
+        }
       }
     }
     /* Schedule parent nodes. */
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index a09f79ffa39..6c37ba8bfb3 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -179,15 +179,30 @@ IDNode *DepsgraphNodeBuilder::add_id_node(ID *id)
   id_node->previously_visible_components_mask = previously_visible_components_mask;
   id_node->previous_eval_flags = previous_eval_flags;
   id_node->previous_customdata_masks = previous_customdata_masks;
+
   /* NOTE: Zero number of components indicates that ID node was just created. */
-  if (id_node->components.is_empty() && deg_copy_on_write_is_needed(id_type)) {
-    ComponentNode *comp_cow = id_node->add_component(NodeType::COPY_ON_WRITE);
-    OperationNode *op_cow = comp_cow->add_operation(
-        [id_node](::Depsgraph *depsgraph) { deg_evaluate_copy_on_write(depsgraph, id_node); },
-        OperationCode::COPY_ON_WRITE,
-        "",
-        -1);
-    graph_->operations.append(op_cow);
+  const bool is_newly_created = id_node->components.is_empty();
+
+  if (is_newly_created) {
+    if (deg_copy_on_write_is_needed(id_type)) {
+      ComponentNode *comp_cow = id_node->add_component(NodeType::COPY_ON_WRITE);
+      OperationNode *op_cow = comp_cow->add_operation(
+          [id_node](::Depsgraph *depsgraph) { deg_evaluate_copy_on_write(depsgraph, id_node); },
+          OperationCode::COPY_ON_WRITE,
+          "",
+          -1);
+      graph_->operations.append(op_cow);
+    }
+
+    ComponentNode *visibility_component = id_node->add_component(NodeType::VISIBILITY);
+    OperationNode *visibility_operation = visibility_component->add_operation(
+        nullptr, OperationCode::OPERATION, "", -1);
+    /* Pin the node so that it and its relations are preserved by the unused nodes/relations
+     * deletion. This is mainly to make it easier to debug visibility. */
+    /* NOTE: Keep un-pinned for the 3.0 release. This way we are more sure that side effects of the
+     * change is minimal outside of the dependency graph area. */
+    // visibility_operation->flag |= OperationFlag::DEPSOP_FLAG_PINNED;
+    graph_->operations.append(visibility_operation);
   }
   return id_node;
 }
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index 55e8c5ed033..3bc5385ae6f 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -368,6 +368,13 @@ Relation *DepsgraphRelationBuilder::add_time_relation(TimeSourceNode *timesrc,
   return nullptr;
 }
 
+void DepsgraphRelationBuilder::add_visibility_relation(ID *id_from, ID *id_to)
+{
+  ComponentKey from_key(id_from, NodeType::VISIBILITY);
+  ComponentKey to_key(id_to, NodeType::VISIBILITY);
+  add_relation(from_key, to_key, "visibility");
+}
+
 Relation *DepsgraphRelationBuilder::add_operation_relation(OperationNode *node_from,
                                                            OperationNode *node_to,
                                                            const char *description,
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.h b/source/blender/depsgraph/intern/builder/deg_builder_relations.h
index f0393544511..09003de3ce4 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.h
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.h
@@ -336,6 +336,11 @@ class DepsgraphRelationBuilder : public DepsgraphBuilder {
                               Node *node_to,
                               const char *description,
                               int flags = 0);
+
+  /* Add relation which ensures visibility of `id_from` when `id_to` is visible.
+   * For the more detailed explanation see comment for `NodeType::VISIBILITY`. */
+  void add_visibility_relation(ID *id_from, ID *id_to);
+
   Relation *add_operation_relation(OperationNode *node_from,
                                    OperationNode *node_to,
                                    const char *description,
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations_rig.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations_rig.cc
index 4754749e2e5..3039eebe857 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations_rig.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations_rig.cc
@@ -450,6 +450,7 @@ void DepsgraphRelationBuilder::build_rig(Object *object)
     /* Custom shape. */
     if (pchan->custom != nullptr) {
       build_object(pchan->custom);
+      add_visibility_relation(&pchan->custom->id, &armature->id);
     }
   }
 }
@@ -506,6 +507,12 @@ void DepsgraphRelationBuilder::build_proxy_rig(Object *object)
           &proxy_from->id, NodeType::PARAMETERS, OperationCode::PARAMETERS_EVAL, pchan->name);
       add_relation(from_bone_parameters, bone_parameters, "Proxy Bone Parameters");
     }
+
+    /* Custom shape. */
+    if (pchan->custom != nullptr) {
+      build_object(pchan->custom);
+      add_visibility_relation(&pchan->custom->id, &armature->id);
+    }
   }
 }
 
diff --git a/source/blender/depsgraph/intern/debug/deg_debug_relations_graphviz.cc b/source/blender/depsgraph/intern/debug/deg_debug_relations_graphviz.cc
index 57c6f062611..63f3d4f91f4 100644
--- a/source/blender/depsgraph/intern/debug/deg_debug_relations_graphviz.cc
+++ b/source/blender/depsgraph/intern/debug/deg_debug_relations_graphviz.cc
@@ -426,6 +426,7 @@ static void deg_debug_graphviz_node(DotExportContext &ctx,
     case NodeType::AUDIO:
     case NodeType::ARMATURE:
     case NodeType::GENERIC_DATABLOCK:
+    case NodeType::VISIBILITY:
     case NodeType::SIMULATION: {
       ComponentNode *comp_node = (ComponentNode *)node;
       if (comp_node->operations.is_empty()) {
diff --git a/source/blender/depsgraph/intern/depsgraph_query_foreach.cc b/source/blender/depsgraph/intern/depsgraph_query_foreach.cc
index 6ce7cc0837b..65a21323258 100644
--- a/source/blender/depsgraph/intern/depsgraph_query_foreach.cc
+++ b/source/blender/depsgraph/intern/depsgraph_query_foreach.cc
@@ -77,6 +77,12 @@ void deg_foreach_dependent_operation(const Depsgraph *UNUSED(graph),
   TraversalQueue queue;
   Set<OperationNode *> scheduled;
   for (ComponentNode *comp_node : target_id_node->components.values()) {
+    if (comp_node->type == NodeType::VISIBILITY) {
+      /* Visibility component is only used internally. It is not to be reporting dependencies to
+       * the outer world. */
+      continue;
+    }
+
     if (source_component_type != DEG_OB_COMP_ANY &&
         nodeTypeToObjectComponent(comp_node->type) != source_component_type) {
       continue;
diff --git a/source/blender/depsgraph/intern/node/deg_node.cc b/source/blender/depsgraph/intern/node/deg_node.cc
index 16089ba27dd..650ce7757b9 100644
--- a/source/blender/depsgraph/intern/node/deg_node.cc
+++ b/source/blender/depsgraph/intern/node/deg_node.cc
@@ -114,6 +114,8 @@ const char *nodeTypeAsString(NodeType type)
       return "ARMATURE";
     case NodeType::GENERIC_DATABLOCK:
       return "GENERIC_DATABLOCK";
+    case NodeType::VISIBILITY:
+      return "VISIBILITY";
     case NodeType::SIMULATION:
       return "SIMULATION";
 
@@ -176,6 +178,10 @@ eDepsSceneComponentType nodeTypeToSceneComponent(NodeType type)
     case NodeType::PROXY:
     case NodeType::SIMULATION:
       return DEG_SCENE_COMP_PARAMETERS;
+
+    case NodeType::VISIBILITY:
+      BLI_assert_msg(0, "Visibility component is supposed to be only used internally.");
+      return DEG_SCENE_COMP_PARAMETERS;
   }
   BLI_assert_msg(0, "Unhandled node type, not supposed to happen.");
   return DEG_SCENE_COMP_PARAMETERS;
@@ -252,6 +258,10 @@ eDepsObjectComponentType nodeTypeToObjectComponent(NodeType type)
     case NodeType::UNDEFINED:
     case NodeType::NUM_TYPES:
       return DEG_OB_COMP_PARAMETERS;
+
+    case NodeType::VISIBILITY:
+      BLI_assert_msg(0, "Visibility component is supposed to be only used internally.");
+      return DEG_OB_COMP_PARAMETERS;
   }
   BLI_assert_msg(0, "Unhandled node type, not suppsed to happen.");
   return DEG_OB_COMP_PARAMETERS;
diff --git a/source/blender/depsgraph/intern/node/deg_node.h b/source/blender/depsgraph/intern/node/deg_node.h
index e1469b68b0e..6274066ba25 100644
--- a/source/blender/depsgraph/intern/node/deg_node.h
+++ b/source/blender/depsgraph/intern/node/deg_node.h
@@ -103,6 +103,22 @@ enum class NodeType {
    * not have very distinctive update procedure. */
   GENERIC_DATABLOCK,
 
+  /* Component which is used to define visibility relation between IDs, on the ID level.
+   *
+   * Consider two ID nodes NodeA and NodeB, with the relation b

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list