Fix T83411: Crash when using a workspace/layout data path in a driver
Building IDs which are not covered by copy-on-write process was not implemented, which was causing parameters block not present, and, hence causing crashes in areas which expected parameters to present. First part of this change is related on making it so Copy-on-Write is optional for ID nodes in the dependency graph. Second part is related on using a generic builder for all ID types which were not covered by Copy-on-Write before. The final part is related on making it so build_id() is properly handling ParticleSettings and Grease Pencil Data. Before they were not covered there at all, and they need special handling because they do have own build functions. Not sure it worth trying to split those parts, as they are related to each other and are not really possible to be tested standalone. Open for a second opinion though. Possible nut-tightening is to re-organize build_id() function so that every branch does return and have an assert at the end, so that missing ID type in the switch statement is easier to spot even when using compilers which do not report missing switch cases. As for question "why not use default" the answer is: to make it more explicit and clear what is a decision when adding new ID types. We do not want to quietly fall-back to a non-copy-on-write case for a newly added ID types. Differential Revision: https://developer.blender.org/D10075
This commit is contained in:
@@ -388,7 +388,9 @@ void DepsgraphNodeBuilder::build_id(ID *id)
|
|||||||
if (id == nullptr) {
|
if (id == nullptr) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
switch (GS(id->name)) {
|
|
||||||
|
const ID_Type id_type = GS(id->name);
|
||||||
|
switch (id_type) {
|
||||||
case ID_AC:
|
case ID_AC:
|
||||||
build_action((bAction *)id);
|
build_action((bAction *)id);
|
||||||
break;
|
break;
|
||||||
@@ -478,13 +480,39 @@ void DepsgraphNodeBuilder::build_id(ID *id)
|
|||||||
case ID_SIM:
|
case ID_SIM:
|
||||||
build_simulation((Simulation *)id);
|
build_simulation((Simulation *)id);
|
||||||
break;
|
break;
|
||||||
default:
|
case ID_PA:
|
||||||
fprintf(stderr, "Unhandled ID %s\n", id->name);
|
build_particle_settings((ParticleSettings *)id);
|
||||||
BLI_assert(!"Should never happen");
|
break;
|
||||||
|
case ID_GD:
|
||||||
|
build_gpencil((bGPdata *)id);
|
||||||
|
break;
|
||||||
|
|
||||||
|
case ID_LI:
|
||||||
|
case ID_IP:
|
||||||
|
case ID_SCR:
|
||||||
|
case ID_VF:
|
||||||
|
case ID_BR:
|
||||||
|
case ID_WM:
|
||||||
|
case ID_PAL:
|
||||||
|
case ID_PC:
|
||||||
|
case ID_WS:
|
||||||
|
BLI_assert(!deg_copy_on_write_is_needed(id_type));
|
||||||
|
build_generic_id(id);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void DepsgraphNodeBuilder::build_generic_id(ID *id)
|
||||||
|
{
|
||||||
|
if (built_map_.checkIsBuiltAndTag(id)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
build_idproperties(id->properties);
|
||||||
|
build_animdata(id);
|
||||||
|
build_parameters(id);
|
||||||
|
}
|
||||||
|
|
||||||
static void build_idproperties_callback(IDProperty *id_property, void *user_data)
|
static void build_idproperties_callback(IDProperty *id_property, void *user_data)
|
||||||
{
|
{
|
||||||
DepsgraphNodeBuilder *builder = reinterpret_cast<DepsgraphNodeBuilder *>(user_data);
|
DepsgraphNodeBuilder *builder = reinterpret_cast<DepsgraphNodeBuilder *>(user_data);
|
||||||
|
@@ -152,6 +152,9 @@ class DepsgraphNodeBuilder : public DepsgraphBuilder {
|
|||||||
|
|
||||||
virtual void build_id(ID *id);
|
virtual void build_id(ID *id);
|
||||||
|
|
||||||
|
/* Build function for ID types that do not need their own build_xxx() function. */
|
||||||
|
virtual void build_generic_id(ID *id);
|
||||||
|
|
||||||
virtual void build_idproperties(IDProperty *id_property);
|
virtual void build_idproperties(IDProperty *id_property);
|
||||||
|
|
||||||
virtual void build_scene_render(Scene *scene, ViewLayer *view_layer);
|
virtual void build_scene_render(Scene *scene, ViewLayer *view_layer);
|
||||||
|
@@ -484,7 +484,9 @@ void DepsgraphRelationBuilder::build_id(ID *id)
|
|||||||
if (id == nullptr) {
|
if (id == nullptr) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
switch (GS(id->name)) {
|
|
||||||
|
const ID_Type id_type = GS(id->name);
|
||||||
|
switch (id_type) {
|
||||||
case ID_AC:
|
case ID_AC:
|
||||||
build_action((bAction *)id);
|
build_action((bAction *)id);
|
||||||
break;
|
break;
|
||||||
@@ -560,13 +562,40 @@ void DepsgraphRelationBuilder::build_id(ID *id)
|
|||||||
case ID_SIM:
|
case ID_SIM:
|
||||||
build_simulation((Simulation *)id);
|
build_simulation((Simulation *)id);
|
||||||
break;
|
break;
|
||||||
default:
|
case ID_PA:
|
||||||
fprintf(stderr, "Unhandled ID %s\n", id->name);
|
build_particle_settings((ParticleSettings *)id);
|
||||||
BLI_assert(!"Should never happen");
|
break;
|
||||||
|
case ID_GD:
|
||||||
|
build_gpencil((bGPdata *)id);
|
||||||
|
break;
|
||||||
|
|
||||||
|
case ID_LI:
|
||||||
|
case ID_IP:
|
||||||
|
case ID_SCR:
|
||||||
|
case ID_VF:
|
||||||
|
case ID_BR:
|
||||||
|
case ID_WM:
|
||||||
|
case ID_PAL:
|
||||||
|
case ID_PC:
|
||||||
|
case ID_WS:
|
||||||
|
BLI_assert(!deg_copy_on_write_is_needed(id_type));
|
||||||
|
build_generic_id(id);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void DepsgraphRelationBuilder::build_generic_id(ID *id)
|
||||||
|
{
|
||||||
|
|
||||||
|
if (built_map_.checkIsBuiltAndTag(id)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
build_idproperties(id->properties);
|
||||||
|
build_animdata(id);
|
||||||
|
build_parameters(id);
|
||||||
|
}
|
||||||
|
|
||||||
static void build_idproperties_callback(IDProperty *id_property, void *user_data)
|
static void build_idproperties_callback(IDProperty *id_property, void *user_data)
|
||||||
{
|
{
|
||||||
DepsgraphRelationBuilder *builder = reinterpret_cast<DepsgraphRelationBuilder *>(user_data);
|
DepsgraphRelationBuilder *builder = reinterpret_cast<DepsgraphRelationBuilder *>(user_data);
|
||||||
|
@@ -198,6 +198,9 @@ class DepsgraphRelationBuilder : public DepsgraphBuilder {
|
|||||||
|
|
||||||
virtual void build_id(ID *id);
|
virtual void build_id(ID *id);
|
||||||
|
|
||||||
|
/* Build function for ID types that do not need their own build_xxx() function. */
|
||||||
|
virtual void build_generic_id(ID *id);
|
||||||
|
|
||||||
virtual void build_idproperties(IDProperty *id_property);
|
virtual void build_idproperties(IDProperty *id_property);
|
||||||
|
|
||||||
virtual void build_scene_render(Scene *scene, ViewLayer *view_layer);
|
virtual void build_scene_render(Scene *scene, ViewLayer *view_layer);
|
||||||
|
@@ -142,6 +142,14 @@ static void clear_id_nodes_conditional(Depsgraph::IDDepsNodes *id_nodes, const F
|
|||||||
* datablock for her own dirty needs. */
|
* datablock for her own dirty needs. */
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
if (id_node->id_cow == id_node->id_orig) {
|
||||||
|
/* Copy-on-write version is not needed for this ID type.
|
||||||
|
*
|
||||||
|
* NOTE: Is important to not de-reference the original datablock here because it might be
|
||||||
|
* freed already (happens during main database free when some IDs are freed prior to a
|
||||||
|
* scene). */
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if (!deg_copy_on_write_is_expanded(id_node->id_cow)) {
|
if (!deg_copy_on_write_is_expanded(id_node->id_cow)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
@@ -265,6 +265,10 @@ void depsgraph_update_editors_tag(Main *bmain, Depsgraph *graph, ID *id)
|
|||||||
void depsgraph_id_tag_copy_on_write(Depsgraph *graph, IDNode *id_node, eUpdateSource update_source)
|
void depsgraph_id_tag_copy_on_write(Depsgraph *graph, IDNode *id_node, eUpdateSource update_source)
|
||||||
{
|
{
|
||||||
ComponentNode *cow_comp = id_node->find_component(NodeType::COPY_ON_WRITE);
|
ComponentNode *cow_comp = id_node->find_component(NodeType::COPY_ON_WRITE);
|
||||||
|
if (cow_comp == nullptr) {
|
||||||
|
BLI_assert(!deg_copy_on_write_is_needed(GS(id_node->id_orig->name)));
|
||||||
|
return;
|
||||||
|
}
|
||||||
cow_comp->tag_update(graph, update_source);
|
cow_comp->tag_update(graph, update_source);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@@ -526,7 +526,8 @@ typedef enum ID_Type {
|
|||||||
#define ID_IS_ASSET(_id) (((const ID *)(_id))->asset_data != NULL)
|
#define ID_IS_ASSET(_id) (((const ID *)(_id))->asset_data != NULL)
|
||||||
|
|
||||||
/* Check whether datablock type is covered by copy-on-write. */
|
/* Check whether datablock type is covered by copy-on-write. */
|
||||||
#define ID_TYPE_IS_COW(_id_type) (!ELEM(_id_type, ID_BR, ID_PAL, ID_IM))
|
#define ID_TYPE_IS_COW(_id_type) \
|
||||||
|
(!ELEM(_id_type, ID_LI, ID_IP, ID_SCR, ID_VF, ID_BR, ID_WM, ID_PAL, ID_PC, ID_WS, ID_IM))
|
||||||
|
|
||||||
#ifdef GS
|
#ifdef GS
|
||||||
# undef GS
|
# undef GS
|
||||||
|
Reference in New Issue
Block a user