From 1155fc94fd1826620c2901e6b36a4020ca88ff69 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Aug 2017 12:52:28 +0200 Subject: [PATCH 1/5] Fix T52454: Crash in DEG_graph_on_visible_update when activating scene layer Most likely needs in 2.79 final release. --- source/blender/depsgraph/intern/depsgraph_tag.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source/blender/depsgraph/intern/depsgraph_tag.cc b/source/blender/depsgraph/intern/depsgraph_tag.cc index 31b4bbc7950..b30110732a2 100644 --- a/source/blender/depsgraph/intern/depsgraph_tag.cc +++ b/source/blender/depsgraph/intern/depsgraph_tag.cc @@ -347,6 +347,12 @@ void DEG_graph_on_visible_update(Main *bmain, Scene *scene) GHASH_FOREACH_END(); } scene->lay_updated |= graph->layers; + /* If graph is tagged for update, we don't need to bother with updates here, + * nodes will be re-created. + */ + if (graph->need_update) { + return; + } /* Special trick to get local view to work. */ LINKLIST_FOREACH (Base *, base, &scene->base) { Object *object = base->object; From 46997992882097f79ee2fd48e250f8fbed857645 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Aug 2017 15:50:05 +0200 Subject: [PATCH 2/5] Fix threading conflict when doing Cycles background render It is possible to have same image used multiple times at different frames, which means we can not free it's buffers without any guard. From quick tests this seems to be doing what it is supposed to. Need more testing and port this to 2.79. --- source/blender/blenkernel/BKE_image.h | 1 + source/blender/blenkernel/intern/image.c | 14 +++++++++++++- source/blender/makesrna/intern/rna_image_api.c | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/source/blender/blenkernel/BKE_image.h b/source/blender/blenkernel/BKE_image.h index 3f8be511212..3c716f39dd0 100644 --- a/source/blender/blenkernel/BKE_image.h +++ b/source/blender/blenkernel/BKE_image.h @@ -58,6 +58,7 @@ void BKE_images_exit(void); void BKE_image_free_packedfiles(struct Image *image); void BKE_image_free_views(struct Image *image); void BKE_image_free_buffers(struct Image *image); +void BKE_image_free_buffers_ex(struct Image *image, bool do_lock); /* call from library */ void BKE_image_free(struct Image *image); diff --git a/source/blender/blenkernel/intern/image.c b/source/blender/blenkernel/intern/image.c index bbe6814f6b7..902076c2d14 100644 --- a/source/blender/blenkernel/intern/image.c +++ b/source/blender/blenkernel/intern/image.c @@ -303,8 +303,11 @@ static void image_free_anims(Image *ima) * Simply free the image data from memory, * on display the image can load again (except for render buffers). */ -void BKE_image_free_buffers(Image *ima) +void BKE_image_free_buffers_ex(Image *ima, bool do_lock) { + if (do_lock) { + BLI_spin_lock(&image_spin); + } image_free_cached_frames(ima); image_free_anims(ima); @@ -323,6 +326,15 @@ void BKE_image_free_buffers(Image *ima) } ima->ok = IMA_OK; + + if (do_lock) { + BLI_spin_unlock(&image_spin); + } +} + +void BKE_image_free_buffers(Image *ima) +{ + BKE_image_free_buffers_ex(ima, false); } /** Free (or release) any data used by this image (does not free the image itself). */ diff --git a/source/blender/makesrna/intern/rna_image_api.c b/source/blender/makesrna/intern/rna_image_api.c index 344c1781b46..5c706d9d8db 100644 --- a/source/blender/makesrna/intern/rna_image_api.c +++ b/source/blender/makesrna/intern/rna_image_api.c @@ -291,7 +291,7 @@ static void rna_Image_filepath_from_user(Image *image, ImageUser *image_user, ch static void rna_Image_buffers_free(Image *image) { - BKE_image_free_buffers(image); + BKE_image_free_buffers_ex(image, true); } #else From c80ab62aee62ec1807955657b94da600715b2067 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Aug 2017 16:10:52 +0200 Subject: [PATCH 3/5] Depsgraph: Remove placeholder for path evaluation Wasn't used in years, if it really needs to be dedicated operation it needs to be revisited anyway. --- source/blender/blenkernel/BKE_curve.h | 3 --- source/blender/blenkernel/intern/curve.c | 11 ----------- .../depsgraph/intern/builder/deg_builder_nodes.cc | 11 ----------- .../depsgraph/intern/depsgraph_type_defines.cc | 1 - source/blender/depsgraph/intern/depsgraph_types.h | 3 --- 5 files changed, 29 deletions(-) diff --git a/source/blender/blenkernel/BKE_curve.h b/source/blender/blenkernel/BKE_curve.h index be05f7d4136..a900ba43443 100644 --- a/source/blender/blenkernel/BKE_curve.h +++ b/source/blender/blenkernel/BKE_curve.h @@ -218,7 +218,4 @@ struct EvaluationContext; void BKE_curve_eval_geometry(struct EvaluationContext *eval_ctx, struct Curve *curve); -void BKE_curve_eval_path(struct EvaluationContext *eval_ctx, - struct Curve *curve); - #endif /* __BKE_CURVE_H__ */ diff --git a/source/blender/blenkernel/intern/curve.c b/source/blender/blenkernel/intern/curve.c index ece33786940..795feb58bf6 100644 --- a/source/blender/blenkernel/intern/curve.c +++ b/source/blender/blenkernel/intern/curve.c @@ -4680,14 +4680,3 @@ void BKE_curve_eval_geometry(EvaluationContext *UNUSED(eval_ctx), BKE_curve_texspace_calc(curve); } } - -void BKE_curve_eval_path(EvaluationContext *UNUSED(eval_ctx), - Curve *curve) -{ - /* TODO(sergey): This will probably need to be a part of - * the modifier stack still. - */ - if (G.debug & G_DEBUG_DEPSGRAPH) { - printf("%s on %s\n", __func__, curve->id.name); - } -} diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc index a90f8ff02b6..46ef4841639 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc @@ -848,17 +848,6 @@ void DepsgraphNodeBuilder::build_obdata_geom(Scene *scene, Object *ob) "Geometry Eval"); op_node->set_as_entry(); - /* Calculate curve path - this is used by constraints, etc. */ - if (ELEM(ob->type, OB_CURVE, OB_FONT)) { - add_operation_node(obdata, - DEG_NODE_TYPE_GEOMETRY, - function_bind(BKE_curve_eval_path, - _1, - (Curve *)obdata), - DEG_OPCODE_GEOMETRY_PATH, - "Path"); - } - /* Make sure objects used for bevel.taper are in the graph. * NOTE: This objects might be not linked to the scene. */ diff --git a/source/blender/depsgraph/intern/depsgraph_type_defines.cc b/source/blender/depsgraph/intern/depsgraph_type_defines.cc index e5033affe2f..05a144900f9 100644 --- a/source/blender/depsgraph/intern/depsgraph_type_defines.cc +++ b/source/blender/depsgraph/intern/depsgraph_type_defines.cc @@ -108,7 +108,6 @@ static const char *stringify_opcode(eDepsOperation_Code opcode) STRINGIFY_OPCODE(TRANSFORM_FINAL); STRINGIFY_OPCODE(OBJECT_UBEREVAL); STRINGIFY_OPCODE(GEOMETRY_UBEREVAL); - STRINGIFY_OPCODE(GEOMETRY_PATH); STRINGIFY_OPCODE(POSE_INIT); STRINGIFY_OPCODE(POSE_DONE); STRINGIFY_OPCODE(POSE_IK_SOLVER); diff --git a/source/blender/depsgraph/intern/depsgraph_types.h b/source/blender/depsgraph/intern/depsgraph_types.h index f05f82caa3d..6c0e3839b39 100644 --- a/source/blender/depsgraph/intern/depsgraph_types.h +++ b/source/blender/depsgraph/intern/depsgraph_types.h @@ -176,9 +176,6 @@ typedef enum eDepsOperation_Code { /* XXX: Placeholder - UberEval */ DEG_OPCODE_GEOMETRY_UBEREVAL, - /* Curve Objects - Path Calculation (used for path-following tools, */ - DEG_OPCODE_GEOMETRY_PATH, - /* Pose -------------------------------------------- */ /* Init IK Trees, etc. */ From f3e02eb32ef68a6ba91ca783056fe8fc1a35af2c Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Aug 2017 16:24:58 +0200 Subject: [PATCH 4/5] Depsgraph: Cleanup, make code friendlier to be edited in columns --- .../intern/builder/deg_builder_relations.cc | 167 +++++++++++------- 1 file changed, 104 insertions(+), 63 deletions(-) diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc index bb8e2a710ef..0bbefe3a954 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc @@ -661,33 +661,46 @@ void DepsgraphRelationBuilder::build_object_parent(Object *ob) } } -void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, eDepsNode_Type component_type, const char *component_subdata, - ListBase *constraints, RootPChanMap *root_map) +void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, + eDepsNode_Type component_type, + const char *component_subdata, + ListBase *constraints, + RootPChanMap *root_map) { - OperationKey constraint_op_key(id, component_type, component_subdata, - (component_type == DEG_NODE_TYPE_BONE) ? DEG_OPCODE_BONE_CONSTRAINTS : DEG_OPCODE_TRANSFORM_CONSTRAINTS); - - /* add dependencies for each constraint in turn */ + OperationKey constraint_op_key( + id, + component_type, + component_subdata, + (component_type == DEG_NODE_TYPE_BONE) + ? DEG_OPCODE_BONE_CONSTRAINTS + : DEG_OPCODE_TRANSFORM_CONSTRAINTS); + /* Add dependencies for each constraint in turn. */ for (bConstraint *con = (bConstraint *)constraints->first; con; con = con->next) { const bConstraintTypeInfo *cti = BKE_constraint_typeinfo_get(con); - - /* invalid constraint type... */ - if (cti == NULL) + /* Invalid constraint type. */ + if (cti == NULL) { continue; - - /* special case for camera tracking -- it doesn't use targets to define relations */ - // TODO: we can now represent dependencies in a much richer manner, so review how this is done... - if (ELEM(cti->type, CONSTRAINT_TYPE_FOLLOWTRACK, CONSTRAINT_TYPE_CAMERASOLVER, CONSTRAINT_TYPE_OBJECTSOLVER)) { + } + /* Special case for camera tracking -- it doesn't use targets to + * define relations. + */ + /* TODO: we can now represent dependencies in a much richer manner, + * so review how this is done. + */ + if (ELEM(cti->type, + CONSTRAINT_TYPE_FOLLOWTRACK, + CONSTRAINT_TYPE_CAMERASOLVER, + CONSTRAINT_TYPE_OBJECTSOLVER)) + { bool depends_on_camera = false; - if (cti->type == CONSTRAINT_TYPE_FOLLOWTRACK) { bFollowTrackConstraint *data = (bFollowTrackConstraint *)con->data; - - if (((data->clip) || (data->flag & FOLLOWTRACK_ACTIVECLIP)) && data->track[0]) + if (((data->clip) || + (data->flag & FOLLOWTRACK_ACTIVECLIP)) && data->track[0]) + { depends_on_camera = true; - + } if (data->depth_ob) { - // DAG_RL_DATA_OB | DAG_RL_OB_OB ComponentKey depth_key(&data->depth_ob->id, DEG_NODE_TYPE_TRANSFORM); add_relation(depth_key, constraint_op_key, cti->name); } @@ -695,24 +708,23 @@ void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, eDepsNode else if (cti->type == CONSTRAINT_TYPE_OBJECTSOLVER) { depends_on_camera = true; } - if (depends_on_camera && scene->camera) { - // DAG_RL_DATA_OB | DAG_RL_OB_OB ComponentKey camera_key(&scene->camera->id, DEG_NODE_TYPE_TRANSFORM); add_relation(camera_key, constraint_op_key, cti->name); } - - /* TODO(sergey): This is more a TimeSource -> MovieClip -> Constraint dependency chain. */ + /* TODO(sergey): This is more a TimeSource -> MovieClip -> + * Constraint dependency chain. + */ TimeSourceKey time_src_key; add_relation(time_src_key, constraint_op_key, "[TimeSrc -> Animation]"); } else if (cti->type == CONSTRAINT_TYPE_TRANSFORM_CACHE) { - /* TODO(kevin): This is more a TimeSource -> CacheFile -> Constraint dependency chain. */ + /* TODO(kevin): This is more a TimeSource -> CacheFile -> Constraint + * dependency chain. + */ TimeSourceKey time_src_key; add_relation(time_src_key, constraint_op_key, "[TimeSrc -> Animation]"); - bTransformCacheConstraint *data = (bTransformCacheConstraint *)con->data; - if (data->cache_file) { ComponentKey cache_key(&data->cache_file->id, DEG_NODE_TYPE_CACHE); add_relation(cache_key, constraint_op_key, cti->name); @@ -721,52 +733,70 @@ void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, eDepsNode else if (cti->get_constraint_targets) { ListBase targets = {NULL, NULL}; cti->get_constraint_targets(con, &targets); - LINKLIST_FOREACH (bConstraintTarget *, ct, &targets) { if (ct->tar == NULL) { continue; } - - if (ELEM(con->type, CONSTRAINT_TYPE_KINEMATIC, CONSTRAINT_TYPE_SPLINEIK)) { - /* ignore IK constraints - these are handled separately (on pose level) */ + if (ELEM(con->type, + CONSTRAINT_TYPE_KINEMATIC, + CONSTRAINT_TYPE_SPLINEIK)) + { + /* Ignore IK constraints - these are handled separately + * (on pose level). + */ } - else if (ELEM(con->type, CONSTRAINT_TYPE_FOLLOWPATH, CONSTRAINT_TYPE_CLAMPTO)) { - /* these constraints require path geometry data... */ + else if (ELEM(con->type, + CONSTRAINT_TYPE_FOLLOWPATH, + CONSTRAINT_TYPE_CLAMPTO)) + { + /* These constraints require path geometry data. */ ComponentKey target_key(&ct->tar->id, DEG_NODE_TYPE_GEOMETRY); - add_relation(target_key, constraint_op_key, cti->name); // XXX: type = geom_transform - // TODO: path dependency + add_relation(target_key, constraint_op_key, cti->name); } else if ((ct->tar->type == OB_ARMATURE) && (ct->subtarget[0])) { /* bone */ if (&ct->tar->id == id) { /* same armature */ eDepsOperation_Code target_key_opcode; - - /* Using "done" here breaks in-chain deps, while using "ready" here breaks most production rigs instead... - * So, we do a compromise here, and only do this when an IK chain conflict may occur + /* Using "done" here breaks in-chain deps, while using + * "ready" here breaks most production rigs instead. + * So, we do a compromise here, and only do this when an + * IK chain conflict may occur. */ - if (root_map->has_common_root(component_subdata, ct->subtarget)) { + if (root_map->has_common_root(component_subdata, + ct->subtarget)) + { target_key_opcode = DEG_OPCODE_BONE_READY; } else { target_key_opcode = DEG_OPCODE_BONE_DONE; } - - OperationKey target_key(&ct->tar->id, DEG_NODE_TYPE_BONE, ct->subtarget, target_key_opcode); + OperationKey target_key(&ct->tar->id, + DEG_NODE_TYPE_BONE, + ct->subtarget, + target_key_opcode); add_relation(target_key, constraint_op_key, cti->name); } else { - /* different armature - we can safely use the result of that */ - OperationKey target_key(&ct->tar->id, DEG_NODE_TYPE_BONE, ct->subtarget, DEG_OPCODE_BONE_DONE); + /* Different armature - we can safely use the result + * of that. + */ + OperationKey target_key(&ct->tar->id, + DEG_NODE_TYPE_BONE, + ct->subtarget, + DEG_OPCODE_BONE_DONE); add_relation(target_key, constraint_op_key, cti->name); } } - else if (ELEM(ct->tar->type, OB_MESH, OB_LATTICE) && (ct->subtarget[0])) { - /* vertex group */ - /* NOTE: for now, we don't need to represent vertex groups separately... */ + else if (ELEM(ct->tar->type, OB_MESH, OB_LATTICE) && + (ct->subtarget[0])) + { + /* Vertex group. */ + /* NOTE: for now, we don't need to represent vertex groups + * separately. + */ ComponentKey target_key(&ct->tar->id, DEG_NODE_TYPE_GEOMETRY); add_relation(target_key, constraint_op_key, cti->name); - if (ct->tar->type == OB_MESH) { OperationDepsNode *node2 = find_operation_node(target_key); if (node2 != NULL) { @@ -778,37 +808,48 @@ void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, eDepsNode /* Constraints which requires the target object surface. */ ComponentKey target_key(&ct->tar->id, DEG_NODE_TYPE_GEOMETRY); add_relation(target_key, constraint_op_key, cti->name); - - /* NOTE: obdata eval now doesn't necessarily depend on the object's transform... */ - ComponentKey target_transform_key(&ct->tar->id, DEG_NODE_TYPE_TRANSFORM); + /* NOTE: obdata eval now doesn't necessarily depend on the + * object's transform. + */ + ComponentKey target_transform_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM); add_relation(target_transform_key, constraint_op_key, cti->name); } else { - /* standard object relation */ + /* Standard object relation. */ // TODO: loc vs rot vs scale? if (&ct->tar->id == id) { /* Constraint targetting own object: - * - This case is fine IFF we're dealing with a bone constraint pointing to - * its own armature. In that case, it's just transform -> bone. - * - If however it is a real self targetting case, just make it depend on the - * previous constraint (or the pre-constraint state)... + * - This case is fine IFF we're dealing with a bone + * constraint pointing to its own armature. In that + * case, it's just transform -> bone. + * - If however it is a real self targetting case, just + * make it depend on the previous constraint (or the + * pre-constraint state). */ - if ((ct->tar->type == OB_ARMATURE) && (component_type == DEG_NODE_TYPE_BONE)) { - OperationKey target_key(&ct->tar->id, DEG_NODE_TYPE_TRANSFORM, DEG_OPCODE_TRANSFORM_FINAL); + if ((ct->tar->type == OB_ARMATURE) && + (component_type == DEG_NODE_TYPE_BONE)) + { + OperationKey target_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM, + DEG_OPCODE_TRANSFORM_FINAL); add_relation(target_key, constraint_op_key, cti->name); } else { - OperationKey target_key(&ct->tar->id, DEG_NODE_TYPE_TRANSFORM, DEG_OPCODE_TRANSFORM_LOCAL); + OperationKey target_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM, + DEG_OPCODE_TRANSFORM_LOCAL); add_relation(target_key, constraint_op_key, cti->name); } } else { - /* normal object dependency */ - OperationKey target_key(&ct->tar->id, DEG_NODE_TYPE_TRANSFORM, DEG_OPCODE_TRANSFORM_FINAL); + /* Normal object dependency. */ + OperationKey target_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM, + DEG_OPCODE_TRANSFORM_FINAL); add_relation(target_key, constraint_op_key, cti->name); } } - /* Constraints which needs world's matrix for transform. * TODO(sergey): More constraints here? */ @@ -819,14 +860,14 @@ void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, eDepsNode CONSTRAINT_TYPE_TRANSLIKE)) { /* TODO(sergey): Add used space check. */ - ComponentKey target_transform_key(&ct->tar->id, DEG_NODE_TYPE_TRANSFORM); + ComponentKey target_transform_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM); add_relation(target_transform_key, constraint_op_key, cti->name); } - } - - if (cti->flush_constraint_targets) + if (cti->flush_constraint_targets) { cti->flush_constraint_targets(con, &targets, 1); + } } } } From 9f40153094bc72c0c4372893f20f9d2aace17049 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 22 Aug 2017 16:27:33 +0200 Subject: [PATCH 5/5] Fix T52209: New Depsgraph - animated follow curve constraint sometimes freaks out when the curve has a parent --- .../blender/depsgraph/intern/builder/deg_builder_relations.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc index 0bbefe3a954..ce5ecf79948 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc @@ -752,6 +752,9 @@ void DepsgraphRelationBuilder::build_constraints(Scene *scene, ID *id, /* These constraints require path geometry data. */ ComponentKey target_key(&ct->tar->id, DEG_NODE_TYPE_GEOMETRY); add_relation(target_key, constraint_op_key, cti->name); + ComponentKey target_transform_key(&ct->tar->id, + DEG_NODE_TYPE_TRANSFORM); + add_relation(target_transform_key, constraint_op_key, cti->name); } else if ((ct->tar->type == OB_ARMATURE) && (ct->subtarget[0])) { /* bone */