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.
This commit is contained in:
Sergey Sharybin 2019-02-28 18:47:07 +01:00
parent f864cbd705
commit 95b150ba87
3 changed files with 26 additions and 20 deletions

@ -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,6 +95,19 @@ bool is_object_visibility_animated(Depsgraph *graph, Object *object)
} // namespace
bool deg_check_base_available_for_build(const Depsgraph *graph, Base *base)
{
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)) {
return true;
}
return false;
}
DepsgraphBuilder::DepsgraphBuilder(Main *bmain, Depsgraph *graph)
: bmain_(bmain),
graph_(graph) {
@ -102,15 +115,7 @@ DepsgraphBuilder::DepsgraphBuilder(Main *bmain, Depsgraph *graph)
bool DepsgraphBuilder::need_pull_base_into_graph(struct Base *base)
{
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)) {
return true;
}
return false;
return deg_check_base_available_for_build(graph_, base);
}
/*******************************************************************************

@ -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

@ -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);
}