[Bf-blender-cvs] SVN commit: /data/svn/bf-blender [49120] trunk/blender/source/blender/ blenkernel/intern: Bugfix [#32017] Infinite recursion in depsgraph material /node driver handling

Joshua Leung aligorith at gmail.com
Sun Jul 22 18:14:58 CEST 2012


Revision: 49120
          http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=49120
Author:   aligorith
Date:     2012-07-22 16:14:57 +0000 (Sun, 22 Jul 2012)
Log Message:
-----------
Bugfix [#32017] Infinite recursion in depsgraph material/node driver handling

When initially coding this functionality, I was aware of the potential for
infinite recursion here, just not how frequently such setups are actually
used/created out in the wild (nodetree.ma_node -> ma -> ma.nodetree is all too
common, and often even with several levels of indirection!).

However, the best fix for these problems was not immediately clear. Alternatives
considered included...
 1) checking for common recursive cases.  This was the solution employed for one
of the early patches committed to try and get around this. However, it's all too
easy to defeat these measures (with all the possible combinations of indirection
node groups bring).
 2) arbitrarily restricting recursion to only go down 2/3 levels? Has the risk
of missing some deeply chained/nested drivers, but at least we're guaranteed to
not get too bad. (Plus, who creates such setups anyway ;)
*3) using the generic LIB_DOIT flag (check for tagged items and not recurse down
there). Not as future-proof if some new code suddenly decides to start adding
these tags to materials along the way, but is easiest to add, and should be
flexible enough to catch most cases, since we only care that at some point those
drivers will be evaluated if they're attached to stuff we're interested in.
 4)  introducing a separate flag for Materials indicating they've been checked
already. Similar to 3) and solves the future-proofing, but this leads to...
 5) why bother with remembering to clear flags before traversing for drivers to
evaluate, when they should be tagged for evaluation like everything else?
Downside - requires depsgraph refactor so that we can actually track the fact
that there are dependencies to/from the material datablock, and not just to the
object using said material. (i.e. Currently infeasible)

Modified Paths:
--------------
    trunk/blender/source/blender/blenkernel/intern/depsgraph.c
    trunk/blender/source/blender/blenkernel/intern/material.c
    trunk/blender/source/blender/blenkernel/intern/scene.c

Modified: trunk/blender/source/blender/blenkernel/intern/depsgraph.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/depsgraph.c	2012-07-22 16:10:06 UTC (rev 49119)
+++ trunk/blender/source/blender/blenkernel/intern/depsgraph.c	2012-07-22 16:14:57 UTC (rev 49120)
@@ -352,10 +352,9 @@
 static void dag_add_material_driver_relations(DagForest *dag, DagNode *node, Material *ma);
 
 /* recursive handling for material nodetree drivers */
-static void dag_add_material_nodetree_driver_relations(DagForest *dag, DagNode *node, bNodeTree *ntree, Material *rootma)
+static void dag_add_material_nodetree_driver_relations(DagForest *dag, DagNode *node, bNodeTree *ntree)
 {
 	bNode *n;
-	Material *ma;
 
 	/* nodetree itself */
 	if (ntree->adt) {
@@ -364,21 +363,29 @@
 	
 	/* nodetree's nodes... */
 	for (n = ntree->nodes.first; n; n = n->next) {
-		if (n->id && GS(n->id->name) == ID_MA) {
-			ma = (Material *)n->id;
-			if (ma != rootma) {
-				dag_add_material_driver_relations(dag, node, ma);
+		if (n->id) {
+			if (GS(n->id->name) == ID_MA) {
+				dag_add_material_driver_relations(dag, node, (Material *)n->id);
 			}
+			else if (n->type == NODE_GROUP) {
+				dag_add_material_nodetree_driver_relations(dag, node, (bNodeTree *)n->id);
+			}
 		}
-		else if (n->type == NODE_GROUP && n->id) {
-			dag_add_material_nodetree_driver_relations(dag, node, (bNodeTree *)n->id, rootma);
-		}
 	}
 }
 
 /* recursive handling for material drivers */
 static void dag_add_material_driver_relations(DagForest *dag, DagNode *node, Material *ma)
 {
+	/* Prevent infinite recursion by checking (and tagging the material) as having been visited 
+	 * already (see build_dag()). This assumes ma->id.flag & LIB_DOIT isn't set by anything else
+	 * in the meantime... [#32017]
+	 */
+	if (ma->id.flag & LIB_DOIT)
+		return;
+	else
+		ma->id.flag |= LIB_DOIT;
+	
 	/* material itself */
 	if (ma->adt) {
 		dag_add_driver_relation(ma->adt, dag, node, 1);
@@ -390,7 +397,7 @@
 
 	/* material's nodetree */
 	if (ma->nodetree) {
-		dag_add_material_nodetree_driver_relations(dag, node, ma->nodetree, ma);
+		dag_add_material_nodetree_driver_relations(dag, node, ma->nodetree);
 	}
 }
 
@@ -804,6 +811,9 @@
 		sce->theDag = dag;
 	}
 	
+	/* clear "LIB_DOIT" flag from all materials, to prevent infinite recursion problems later [#32017] */
+	tag_main_idcode(bmain, ID_MA, FALSE);
+	
 	/* add base node for scene. scene is always the first node in DAG */
 	scenenode = dag_add_node(dag, sce);	
 	

Modified: trunk/blender/source/blender/blenkernel/intern/material.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/material.c	2012-07-22 16:10:06 UTC (rev 49119)
+++ trunk/blender/source/blender/blenkernel/intern/material.c	2012-07-22 16:14:57 UTC (rev 49120)
@@ -1056,29 +1056,25 @@
 /* ****************** */
 
 /* Update drivers for materials in a nodetree */
-static void material_node_drivers_update(Scene *scene, bNodeTree *ntree, float ctime, Material *rootma)
+static void material_node_drivers_update(Scene *scene, bNodeTree *ntree, float ctime)
 {
 	bNode *node;
-	Material *ma;
 
 	/* nodetree itself */
 	if (ntree->adt && ntree->adt->drivers.first) {
 		BKE_animsys_evaluate_animdata(scene, &ntree->id, ntree->adt, ctime, ADT_RECALC_DRIVERS);
 	}
 	
-	/* nodes... */
+	/* nodes */
 	for (node = ntree->nodes.first; node; node = node->next) {
-		if (node->id && GS(node->id->name) == ID_MA) {
-			/* TODO: prevent infinite recursion here... */
-            ma = (Material *)node->id;
-            if (ma != rootma) {
-                material_drivers_update(scene, ma, ctime);
-            }
+		if (node->id) {
+			if (GS(node->id->name) == ID_MA) {
+				material_drivers_update(scene, (Material *)node->id, ctime);
+			}
+			else if (node->type == NODE_GROUP) {
+				material_node_drivers_update(scene, (bNodeTree *)node->id, ctime);
+			}
 		}
-		else if (node->type == NODE_GROUP && node->id) {
-			material_node_drivers_update(scene, (bNodeTree *)node->id,
-                                         ctime, rootma);
-		}
 	}
 }
 
@@ -1092,6 +1088,15 @@
 	//if (G.f & G_DEBUG)
 	//	printf("material_drivers_update(%s, %s)\n", scene->id.name, ma->id.name);
 	
+	/* Prevent infinite recursion by checking (and tagging the material) as having been visited already
+	 * (see BKE_scene_update_tagged()). This assumes ma->id.flag & LIB_DOIT isn't set by anything else
+	 * in the meantime... [#32017]
+	 */
+	if (ma->id.flag & LIB_DOIT)
+		return;
+	else
+		ma->id.flag |= LIB_DOIT;
+	
 	/* material itself */
 	if (ma->adt && ma->adt->drivers.first) {
 		BKE_animsys_evaluate_animdata(scene, &ma->id, ma->adt, ctime, ADT_RECALC_DRIVERS);
@@ -1099,7 +1104,7 @@
 	
 	/* nodes */
 	if (ma->nodetree) {
-		material_node_drivers_update(scene, ma->nodetree, ctime, ma);
+		material_node_drivers_update(scene, ma->nodetree, ctime);
 	}
 }
 	

Modified: trunk/blender/source/blender/blenkernel/intern/scene.c
===================================================================
--- trunk/blender/source/blender/blenkernel/intern/scene.c	2012-07-22 16:10:06 UTC (rev 49119)
+++ trunk/blender/source/blender/blenkernel/intern/scene.c	2012-07-22 16:14:57 UTC (rev 49120)
@@ -1018,6 +1018,11 @@
 	DAG_ids_flush_tagged(bmain);
 
 	scene->physics_settings.quick_cache_step = 0;
+	
+	/* clear "LIB_DOIT" flag from all materials, to prevent infinite recursion problems later 
+	 * when trying to find materials with drivers that need evaluating [#32017] 
+	 */
+	tag_main_idcode(bmain, ID_MA, FALSE);
 
 	/* update all objects: drivers, matrices, displists, etc. flags set
 	 * by depgraph or manual, no layer check here, gets correct flushed




More information about the Bf-blender-cvs mailing list