Depsgraph: Fix wrong disabled bases deletion

Original optimization idea was wrong: it is possible that some other
ID would reference an object which is also used by a base.

Rolled back to a bit more fragile solution.

In the future would be nice to make it somewhat less duplicated with
the builder itself.

Fixes assert failure (and possibly crashes) when adding grease pencil
object and switching to a draw mode.

3 files changed, 25 insertions, 19 deletions

diff --git a/source/blender/depsgraph/intern/builder/deg_builder.cc b/source/blender/depsgraph/intern/builder/deg_builder.cc
index e7111bd5202..3365cdda53d 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder.cc
@@ -80,7 +80,7 @@ void visibility_animated_check_cb(ID * /*id*/, FCurve *fcu, void *user_data)
 	}
 }
 
-bool is_object_visibility_animated(Depsgraph *graph, Object *object)
+bool is_object_visibility_animated(const Depsgraph *graph, Object *object)
 {
 	AnimData* anim_data = BKE_animdata_from_id(&object->id);
 	if (anim_data == NULL) {
@@ -95,24 +95,29 @@ bool is_object_visibility_animated(Depsgraph *graph, Object *object)
 
 }  // namespace
 
-DepsgraphBuilder::DepsgraphBuilder(Main *bmain, Depsgraph *graph)
-        : bmain_(bmain),
-          graph_(graph) {
-}
-
-bool DepsgraphBuilder::need_pull_base_into_graph(struct Base *base)
+bool deg_check_base_available_for_build(const Depsgraph *graph, Base *base)
 {
-	const int base_flag = (graph_->mode == DAG_EVAL_VIEWPORT) ?
+	const int base_flag = (graph->mode == DAG_EVAL_VIEWPORT) ?
 	        BASE_ENABLED_VIEWPORT : BASE_ENABLED_RENDER;
 	if (base->flag & base_flag) {
 		return true;
 	}
-	if (is_object_visibility_animated(graph_, base->object)) {
+	if (is_object_visibility_animated(graph, base->object)) {
 		return true;
 	}
 	return false;
 }
 
+DepsgraphBuilder::DepsgraphBuilder(Main *bmain, Depsgraph *graph)
+        : bmain_(bmain),
+          graph_(graph) {
+}
+
+bool DepsgraphBuilder::need_pull_base_into_graph(struct Base *base)
+{
+	return deg_check_base_available_for_build(graph_, base);
+}
+
 /*******************************************************************************
  * Builder finalizer.
  */
diff --git a/source/blender/depsgraph/intern/builder/deg_builder.h b/source/blender/depsgraph/intern/builder/deg_builder.h
index 7c9f496ae7e..2461ca2211f 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.h
+++ b/source/blender/depsgraph/intern/builder/deg_builder.h
@@ -42,6 +42,8 @@ protected:
 	Depsgraph *graph_;
 };
 
+bool deg_check_base_available_for_build(const Depsgraph *graph,
+                                        Base *base);
 void deg_graph_build_finalize(struct Main *bmain, struct Depsgraph *graph);
 
 }  // namespace DEG
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
index b378aee6f97..dab403adf07 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
+++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
@@ -88,6 +88,7 @@ extern "C" {
 }
 
 #include "intern/depsgraph.h"
+#include "intern/builder/deg_builder.h"
 #include "intern/builder/deg_builder_nodes.h"
 #include "intern/node/deg_node.h"
 #include "intern/node/deg_node_id.h"
@@ -362,21 +363,19 @@ void scene_remove_unused_view_layers(const Depsgraph *depsgraph,
 
 /* Makes it so given view layer only has bases corresponding to enabled
  * objects. */
-void view_layer_remove_disabled_bases(const Depsgraph * /*depsgraph*/,
+void view_layer_remove_disabled_bases(const Depsgraph *depsgraph,
                                       ViewLayer *view_layer)
 {
 	ListBase enabled_bases = {NULL, NULL};
 	LISTBASE_FOREACH_MUTABLE (Base *, base, &view_layer->object_bases) {
-		const Object *object = base->object;
-		/* NOTE: The base will point to an original object if it's not pulled
-		 * into the dependency graph, and will point to a an ID which is already
-		 * tagged as COPIED_ON_WRITE.
-		 * This looks a bit dangerous to access original data from evaluation,
-		 * but this is not specific to this place and would need to be handled
-		 * in general by, probably, running copy-on-write update phase from a
-		 * locked state or so. */
+		/* TODO(sergey): Would be cool to optimize this somehow, or make it so
+		 * builder tags bases.
+		 *
+		 * NOTE: The idea of using id's tag and check whether its copied ot not
+		 * is not reliable, since object might be indirectly linked into the
+		 * graph. */
 		const bool is_object_enabled =
-		        (object->id.tag & LIB_TAG_COPIED_ON_WRITE);
+		        deg_check_base_available_for_build(depsgraph, base);
 		if (is_object_enabled) {
 			BLI_addtail(&enabled_bases, base);
 		}