[Bf-blender-cvs] [0dcee6a3866] master: Fix T99733: Objects with driven visibility are evaluated when not needed
Sergey Sharybin
noreply at git.blender.org
Thu Jul 21 09:53:59 CEST 2022
Commit: 0dcee6a386645bb1e976d11aa2d1ae45b01f968a
Author: Sergey Sharybin
Date: Wed Jul 20 10:04:02 2022 +0200
Branches: master
https://developer.blender.org/rB0dcee6a386645bb1e976d11aa2d1ae45b01f968a
Fix T99733: Objects with driven visibility are evaluated when not needed
The issue was caused by the fact that objects with driven or animated
visibility were considered visible by the dependency graph evaluation.
This change makes it so the dependency graph evaluation is aware of
visibility which might be changing. This is achieved by evaluating the
path of the graph which affects objects visibility and adjusts to it
before evaluating the rest of the graph.
There is some time penalty to this, but there does not seem to be a
way to fully avoid this penalty.
With the production shot from the heist project the FPS drops by a
tenth of a frame (~9.4 vs ~9.3 fps) when adding a driver to an object
which keeps it visible. Note that this is a bit hard to measure since
the FPS fluctuates quite a bit throughout the playback. On the other
hand, having a driver on a visibility of a heavy object from character
and setting visibility to false gives big speedup.
Also worth noting that there is no penalty at all when there are no
animated visibilities in the scene.
Differential Revision: https://developer.blender.org/D15498
===================================================================
M source/blender/depsgraph/CMakeLists.txt
M source/blender/depsgraph/intern/builder/deg_builder.cc
M source/blender/depsgraph/intern/builder/deg_builder.h
M source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
M source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
M source/blender/depsgraph/intern/builder/deg_builder_relations.cc
M source/blender/depsgraph/intern/depsgraph.cc
M source/blender/depsgraph/intern/depsgraph.h
M source/blender/depsgraph/intern/depsgraph_query_iter.cc
M source/blender/depsgraph/intern/eval/deg_eval.cc
A source/blender/depsgraph/intern/eval/deg_eval_visibility.cc
A source/blender/depsgraph/intern/eval/deg_eval_visibility.h
M source/blender/depsgraph/intern/node/deg_node_component.cc
M source/blender/depsgraph/intern/node/deg_node_component.h
M source/blender/depsgraph/intern/node/deg_node_id.cc
M source/blender/depsgraph/intern/node/deg_node_id.h
M source/blender/depsgraph/intern/node/deg_node_operation.h
===================================================================
diff --git a/source/blender/depsgraph/CMakeLists.txt b/source/blender/depsgraph/CMakeLists.txt
index 3d539018cef..e799c5ce32a 100644
--- a/source/blender/depsgraph/CMakeLists.txt
+++ b/source/blender/depsgraph/CMakeLists.txt
@@ -67,6 +67,8 @@ set(SRC
intern/eval/deg_eval_runtime_backup_sound.cc
intern/eval/deg_eval_runtime_backup_volume.cc
intern/eval/deg_eval_stats.cc
+ intern/eval/deg_eval_visibility.cc
+ intern/eval/deg_eval_visibility.h
intern/node/deg_node.cc
intern/node/deg_node_component.cc
intern/node/deg_node_factory.cc
diff --git a/source/blender/depsgraph/intern/builder/deg_builder.cc b/source/blender/depsgraph/intern/builder/deg_builder.cc
index 1fec1fb3219..5353f71685c 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder.cc
@@ -29,6 +29,7 @@
#include "intern/depsgraph_tag.h"
#include "intern/depsgraph_type.h"
#include "intern/eval/deg_eval_copy_on_write.h"
+#include "intern/eval/deg_eval_visibility.h"
#include "intern/node/deg_node.h"
#include "intern/node/deg_node_component.h"
#include "intern/node/deg_node_id.h"
@@ -71,10 +72,15 @@ bool DepsgraphBuilder::need_pull_base_into_graph(const Base *base)
if (base->flag & base_flag) {
return true;
}
+
/* More involved check: since we don't support dynamic changes in dependency graph topology and
* all visible objects are to be part of dependency graph, we pull all objects which has animated
* visibility. */
- const Object *object = base->object;
+ return is_object_visibility_animated(base->object);
+}
+
+bool DepsgraphBuilder::is_object_visibility_animated(const Object *object)
+{
AnimatedPropertyID property_id;
if (graph_->mode == DAG_EVAL_VIEWPORT) {
property_id = AnimatedPropertyID(&object->id, &RNA_Object, "hide_viewport");
@@ -127,84 +133,9 @@ bool DepsgraphBuilder::check_pchan_has_bbone_segments(const Object *object, cons
/** \name Builder Finalizer.
* \{ */
-namespace {
-
-void deg_graph_build_flush_visibility(Depsgraph *graph)
-{
- enum {
- DEG_NODE_VISITED = (1 << 0),
- };
-
- BLI_Stack *stack = BLI_stack_new(sizeof(OperationNode *), "DEG flush layers stack");
- for (IDNode *id_node : graph->id_nodes) {
- for (ComponentNode *comp_node : id_node->components.values()) {
- comp_node->affects_directly_visible |= id_node->is_directly_visible;
- }
- }
-
- for (OperationNode *op_node : graph->operations) {
- op_node->custom_flags = 0;
- op_node->num_links_pending = 0;
- for (Relation *rel : op_node->outlinks) {
- if ((rel->from->type == NodeType::OPERATION) && (rel->flag & RELATION_FLAG_CYCLIC) == 0) {
- ++op_node->num_links_pending;
- }
- }
- if (op_node->num_links_pending == 0) {
- BLI_stack_push(stack, &op_node);
- op_node->custom_flags |= DEG_NODE_VISITED;
- }
- }
-
- while (!BLI_stack_is_empty(stack)) {
- OperationNode *op_node;
- BLI_stack_pop(stack, &op_node);
- /* Flush layers to parents. */
- for (Relation *rel : op_node->inlinks) {
- if (rel->from->type == NodeType::OPERATION) {
- OperationNode *op_from = (OperationNode *)rel->from;
- 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. */
- for (Relation *rel : op_node->inlinks) {
- if (rel->from->type == NodeType::OPERATION) {
- OperationNode *op_from = (OperationNode *)rel->from;
- if ((rel->flag & RELATION_FLAG_CYCLIC) == 0) {
- BLI_assert(op_from->num_links_pending > 0);
- --op_from->num_links_pending;
- }
- if ((op_from->num_links_pending == 0) && (op_from->custom_flags & DEG_NODE_VISITED) == 0) {
- BLI_stack_push(stack, &op_from);
- op_from->custom_flags |= DEG_NODE_VISITED;
- }
- }
- }
- }
- BLI_stack_free(stack);
-}
-
-} // namespace
-
void deg_graph_build_finalize(Main *bmain, Depsgraph *graph)
{
- /* Make sure dependencies of visible ID data-blocks are visible. */
- deg_graph_build_flush_visibility(graph);
+ deg_graph_flush_visibility_flags(graph);
deg_graph_remove_unused_noops(graph);
/* Re-tag IDs for update if it was tagged before the relations
diff --git a/source/blender/depsgraph/intern/builder/deg_builder.h b/source/blender/depsgraph/intern/builder/deg_builder.h
index 950ebfb35ba..c44e5fd5f4d 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.h
+++ b/source/blender/depsgraph/intern/builder/deg_builder.h
@@ -24,6 +24,8 @@ class DepsgraphBuilder {
virtual bool need_pull_base_into_graph(const Base *base);
+ virtual bool is_object_visibility_animated(const Object *object);
+
virtual bool check_pchan_has_bbone(const Object *object, const bPoseChannel *pchan);
virtual bool check_pchan_has_bbone_segments(const Object *object, const bPoseChannel *pchan);
virtual bool check_pchan_has_bbone_segments(const Object *object, const char *bone_name);
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index 473dda2290c..c65c4beeed6 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -107,6 +107,7 @@
#include "intern/depsgraph_tag.h"
#include "intern/depsgraph_type.h"
#include "intern/eval/deg_eval_copy_on_write.h"
+#include "intern/eval/deg_eval_visibility.h"
#include "intern/node/deg_node.h"
#include "intern/node/deg_node_component.h"
#include "intern/node/deg_node_id.h"
@@ -179,11 +180,27 @@ IDNode *DepsgraphNodeBuilder::add_id_node(ID *id)
}
ComponentNode *visibility_component = id_node->add_component(NodeType::VISIBILITY);
- OperationNode *visibility_operation = visibility_component->add_operation(
- nullptr, OperationCode::VISIBILITY);
+ OperationNode *visibility_operation;
+
+ /* Optimization: currently only objects need a special visibility evaluation. For the rest ID
+ * types keep the node as a NO-OP so that relations can still be routed, but without penalty
+ * during the graph evaluation. */
+ if (id_type == ID_OB) {
+ visibility_operation = visibility_component->add_operation(
+ [id_node](::Depsgraph *depsgraph) {
+ deg_evaluate_object_node_visibility(depsgraph, id_node);
+ },
+ OperationCode::VISIBILITY);
+ }
+ else {
+ visibility_operation = visibility_component->add_operation(nullptr,
+ OperationCode::VISIBILITY);
+ }
+
/* 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. */
- visibility_operation->flag |= OperationFlag::DEPSOP_FLAG_PINNED;
+ visibility_operation->flag |= (OperationFlag::DEPSOP_FLAG_PINNED |
+ OperationFlag::DEPSOP_FLAG_AFFECTS_VISIBILITY);
graph_->operations.append(visibility_operation);
}
return id_node;
@@ -652,7 +669,7 @@ void DepsgraphNodeBuilder::build_collection(LayerCollection *from_layer_collecti
IDNode *id_node;
if (built_map_.checkIsBuiltAndTag(collection)) {
id_node = find_id_node(&collection->id);
- if (is_collection_visible && id_node->is_directly_visible == false &&
+ if (is_collection_visible && id_node->is_visible_on_build == false &&
id_node->is_collection_fully_expanded == true) {
/* Collection became visible, make sure nested collections and
* objects are poked with the new visibility flag, since they
@@ -669,7 +686,7 @@ void DepsgraphNodeBuilder::build_collection(LayerCollection *from_layer_collecti
else {
/* Collection itself. */
id_node = add_id_node(&collection->id);
- id_node->is_directly_visible = is_collection_visible;
+ id_node->is_visible_on_build = is_collection_visible;
build_idproperties(collection->id.properties);
add_operation_node(&collection->id, NodeType::GEOMETRY, OperationCode::GEOMETRY_EVAL_DONE);
@@ -716,7 +733,7 @@ void DepsgraphNodeBuilder::build_object(int base_index,
build_object_flags(base_index, object, linked_state);
}
id_node->linked_state = max(id_node->linked_state, linked_state);
- id_node->is_directly_visible |= is_visible;
+ id_node->is_visible_on_build |= is_visible;
id_node->has_base |= (base_index != -1);
/* There is no relation path which will connect current object with all the ones which come
@@ -735,10 +752,10 @@ void DepsgraphNodeBuilder::build_object(int base_index,
* Probably need to assign that to something non-nullptr, but then the logic here will still be
* somewhat weird. */
if (scene_ != nullptr && object == scene_->camera) {
- id_node->is_directly_visible = true;
+ id_node->is_visible_on_build = true;
}
else {
- id_node->is_directly_visible = is_visible;
+ id_node->is_visible_on_build = is_visible;
}
id_node->has_base |= (base_index != -1);
/* Various flags, flushing from bases/collections. */
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
index 32ec94211a0..5af9e7d4fe9 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
@@ -92,14 +92,20 @@ void DepsgraphNodeBuilder::build_view_layer(Scene *scene,
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list