[Bf-blender-cvs] [e3b8495] soc-2013-depsgraph_mt: Address some of the codereview comments
Sergey Sharybin
noreply at git.blender.org
Thu Nov 21 13:41:08 CET 2013
Commit: e3b849565e32896fe38ce07d8a0aabd4cfb87914
Author: Sergey Sharybin
Date: Thu Nov 21 00:17:52 2013 +0600
http://developer.blender.org/rBe3b849565e32896fe38ce07d8a0aabd4cfb87914
Address some of the codereview comments
- Call it just DAG_init/DAG_exit
- Move EvaluationContext to a better place (DAG header).
- Rename valency to num_pending_parents.
- Added a comment about why we might want to not
use threads for the scene update/
- Update comment for screen switch.
===================================================================
M source/blender/blenkernel/BKE_depsgraph.h
M source/blender/blenkernel/BKE_scene.h
M source/blender/blenkernel/depsgraph_private.h
M source/blender/blenkernel/intern/blender.c
M source/blender/blenkernel/intern/depsgraph.c
M source/blender/blenkernel/intern/group.c
M source/blender/blenkernel/intern/library.c
M source/blender/blenkernel/intern/scene.c
M source/blender/blenkernel/intern/sequencer.c
M source/blender/editors/screen/screen_edit.c
M source/blender/render/intern/source/pipeline.c
M source/blender/windowmanager/intern/wm_playanim.c
M source/creator/creator.c
M source/gameengine/GamePlayer/ghost/GPG_ghost.cpp
===================================================================
diff --git a/source/blender/blenkernel/BKE_depsgraph.h b/source/blender/blenkernel/BKE_depsgraph.h
index 5a30ee0..592baa7 100644
--- a/source/blender/blenkernel/BKE_depsgraph.h
+++ b/source/blender/blenkernel/BKE_depsgraph.h
@@ -50,6 +50,16 @@ struct Object;
struct Scene;
struct ListBase;
+/* Dependency graph evaluation context
+ *
+ * This structure stores all the local dependency graph data,
+ * which is needed for it's evaluation,
+ */
+typedef struct EvaluationContext {
+ bool for_render; /* Set to true if evaluation shall be performed for render purposes,
+ keep at false if update shall happen for the viewport. */
+} EvaluationContext;
+
/* Build and Update
*
* DAG_scene_relations_update will rebuild the dependency graph for a given
@@ -119,8 +129,8 @@ void DAG_editors_update_cb(void (*id_func)(struct Main *bmain, struct ID *id),
/* ** Threaded update ** */
/* Global initialization/deinitialization */
-void DAG_threaded_init(void);
-void DAG_threaded_exit(void);
+void DAG_init(void);
+void DAG_exit(void);
/* Initialize the DAG for threaded update. */
void DAG_threaded_update_begin(struct Scene *scene,
diff --git a/source/blender/blenkernel/BKE_scene.h b/source/blender/blenkernel/BKE_scene.h
index d7e1cb3..191fd6f 100644
--- a/source/blender/blenkernel/BKE_scene.h
+++ b/source/blender/blenkernel/BKE_scene.h
@@ -114,12 +114,6 @@ void BKE_scene_frame_set(struct Scene *scene, double cfra);
/* ** Scene evaluation ** */
-/* TODO(sergey): Find a better place for this. */
-typedef struct EvaluationContext {
- bool for_render; /* Set to true if evaluation shall be performed for render purposes,
- keep at false if update shall happen for the viewport. */
-} EvaluationContext;
-
void BKE_scene_update_tagged_ex(struct EvaluationContext *evaluation_context, struct Main *bmain, struct Scene *scene, bool use_threads);
void BKE_scene_update_tagged(struct EvaluationContext *evaluation_context, struct Main *bmain, struct Scene *sce);
diff --git a/source/blender/blenkernel/depsgraph_private.h b/source/blender/blenkernel/depsgraph_private.h
index b97890c..f4a6e44 100644
--- a/source/blender/blenkernel/depsgraph_private.h
+++ b/source/blender/blenkernel/depsgraph_private.h
@@ -94,11 +94,11 @@ typedef struct DagNode {
struct DagNode *next;
/* Threaded evaluation routines */
- uint32_t valency; /* valency of the node is a number of parents which are not updated yet
- * this node has got.
- * Used by threaded update for faster detect whether node could be
- * updated aready.
- */
+ uint32_t num_pending_parents; /* number of parents which are not updated yet
+ * this node has got.
+ * Used by threaded update for faster detect whether node could be
+ * updated aready.
+ */
bool tag, scheduled;
} DagNode;
diff --git a/source/blender/blenkernel/intern/blender.c b/source/blender/blenkernel/intern/blender.c
index c4b2573..aade3da 100644
--- a/source/blender/blenkernel/intern/blender.c
+++ b/source/blender/blenkernel/intern/blender.c
@@ -120,7 +120,7 @@ void free_blender(void)
IMB_exit();
BKE_images_exit();
- DAG_threaded_exit();
+ DAG_exit();
BKE_brush_system_exit();
diff --git a/source/blender/blenkernel/intern/depsgraph.c b/source/blender/blenkernel/intern/depsgraph.c
index 1330e58..91fe928 100644
--- a/source/blender/blenkernel/intern/depsgraph.c
+++ b/source/blender/blenkernel/intern/depsgraph.c
@@ -2711,12 +2711,12 @@ void DAG_pose_sort(Object *ob)
static SpinLock threaded_update_lock;
-void DAG_threaded_init(void)
+void DAG_init(void)
{
BLI_spin_init(&threaded_update_lock);
}
-void DAG_threaded_exit(void)
+void DAG_exit(void)
{
BLI_spin_end(&threaded_update_lock);
}
@@ -2726,7 +2726,7 @@ void DAG_threaded_exit(void)
* the root node to the queue.
*
* This will mark DAG nodes as object/non-object and will calculate
- * "valency" of nodes (which is how many non-updated parents node
+ * num_pending_parents of nodes (which is how many non-updated parents node
* have, which helps a lot checking whether node could be scheduled
* already or not).
*/
@@ -2736,21 +2736,21 @@ void DAG_threaded_update_begin(Scene *scene,
{
DagNode *node, *root_node;
- /* We reset valency to zero first... */
+ /* We reset num_pending_parents to zero first and tag node as not scheduled yet... */
for (node = scene->theDag->DagNode.first; node; node = node->next) {
- node->valency = 0;
+ node->num_pending_parents = 0;
node->scheduled = false;
}
/* ... and then iterate over all the nodes and
- * increase valency for node childs.
+ * increase num_pending_parents for node childs.
*/
for (node = scene->theDag->DagNode.first; node; node = node->next) {
DagAdjList *itA;
for (itA = node->child; itA; itA = itA->next) {
if (itA->node != node) {
- itA->node->valency++;
+ itA->node->num_pending_parents++;
}
}
}
@@ -2763,7 +2763,7 @@ void DAG_threaded_update_begin(Scene *scene,
/* This function is called when handling node is done.
*
- * This function updates valency for all childs and
+ * This function updates num_pending_parents for all childs and
* schedules them if they're ready.
*/
void DAG_threaded_update_handle_node_updated(void *node_v,
@@ -2776,9 +2776,9 @@ void DAG_threaded_update_handle_node_updated(void *node_v,
for (itA = node->child; itA; itA = itA->next) {
DagNode *child_node = itA->node;
if (child_node != node) {
- atomic_sub_uint32(&child_node->valency, 1);
+ atomic_sub_uint32(&child_node->num_pending_parents, 1);
- if (child_node->valency == 0) {
+ if (child_node->num_pending_parents == 0) {
bool need_schedule;
BLI_spin_lock(&threaded_update_lock);
diff --git a/source/blender/blenkernel/intern/group.c b/source/blender/blenkernel/intern/group.c
index d662c79..59c5cee 100644
--- a/source/blender/blenkernel/intern/group.c
+++ b/source/blender/blenkernel/intern/group.c
@@ -47,6 +47,7 @@
#include "BLI_utildefines.h"
+#include "BKE_depsgraph.h"
#include "BKE_global.h"
#include "BKE_group.h"
#include "BKE_library.h"
diff --git a/source/blender/blenkernel/intern/library.c b/source/blender/blenkernel/intern/library.c
index 4344112..bf0cda8 100644
--- a/source/blender/blenkernel/intern/library.c
+++ b/source/blender/blenkernel/intern/library.c
@@ -82,6 +82,7 @@
#include "BKE_camera.h"
#include "BKE_context.h"
#include "BKE_curve.h"
+#include "BKE_depsgraph.h"
#include "BKE_fcurve.h"
#include "BKE_font.h"
#include "BKE_global.h"
diff --git a/source/blender/blenkernel/intern/scene.c b/source/blender/blenkernel/intern/scene.c
index 558b81f..e1879737 100644
--- a/source/blender/blenkernel/intern/scene.c
+++ b/source/blender/blenkernel/intern/scene.c
@@ -1394,6 +1394,18 @@ static void scene_update_objects(EvaluationContext *evaluation_context, Scene *s
}
/* this is called in main loop, doing tagged updates before redraw */
+/* NOTE: We don't use threads when scene update was called from python
+ * via scene.update() call.
+ *
+ * This is so because this call will set python's GIL and if any of
+ * the objects is using drivers we'll end up being in a dead-lock
+ * because driver evaluation will also try to set GIL.
+ *
+ * We might try using Py_BEGIN_ALLOW_THREADS in RNA callback, but
+ * not sure how it's gonna to fit into design and what would be the
+ * rules there and whether it'll help even.
+ * - sergey -
+ */
void BKE_scene_update_tagged_ex(EvaluationContext *evaluation_context, Main *bmain, Scene *scene, bool use_threads)
{
Scene *sce_iter;
diff --git a/source/blender/blenkernel/intern/sequencer.c b/source/blender/blenkernel/intern/sequencer.c
index 5825ae0..9c3f2d6 100644
--- a/source/blender/blenkernel/intern/sequencer.c
+++ b/source/blender/blenkernel/intern/sequencer.c
@@ -57,6 +57,7 @@
#include "BLF_translation.h"
#include "BKE_animsys.h"
+#include "BKE_depsgraph.h"
#include "BKE_global.h"
#include "BKE_image.h"
#include "BKE_main.h"
diff --git a/source/blender/editors/screen/screen_edit.c b/source/blender/editors/screen/screen_edit.c
index 2273dc7..d6e80bd 100644
--- a/source/blender/editors/screen/screen_edit.c
+++ b/source/blender/editors/screen/screen_edit.c
@@ -1428,15 +1428,18 @@ void ED_screen_set(bContext *C, bScreen *sc)
/* makes button hilites work */
WM_event_add_mousemove(C);
- /* TODO(sergey): Needed to make sure all the derivedMeshes are
- * up-to-date before viewport starts acquiring this.
+ /* Needed to make sure all the derivedMeshes are
+ * up-to-date before viewport starts acquiring this.
*
- * This is needed in cases when, for example, boolean
- * modifier uses operant from invisible layer.
- * Without this trick boolean wouldn't apply correct.
+ * This is needed in cases when, for example, boolean
+ * modifier uses operant from invisible layer.
+ * Without this trick boolean wouldn't apply correct.
*
- * Quite the same happens when setting screen's scene,
- * so perhaps this is in fact correct thing to do.
+ * Quite the same happens when setting screen's scene,
+ * so perhaps this is in fact correct thing to do.
+ */
+ /* TODO(sergey): Check whether DAG is actually need to
+ * be rebuilt.
*/
BKE_scene_set_background(bmain, sc->scene);
DAG_on_visible_update(bmain, FALSE);
diff --git a/source/blender/render/intern/source/pipeline.c b/source/blender/render/intern/source/pipeline.c
index 29d51b3..f1b5c34 100644
--- a/source/blender/render/intern/source/pipeline.c
+++ b/source/blender/render/intern/source/pipeline.c
@@ -59,6 +59,7 @@
#include "BKE_animsys.h" /* <------ should this be here?, needed for sequencer update */
#include "BKE_camera.h"
+#include "BKE_depsgraph.h"
#include "BKE_global.h"
#include "BKE_image.h"
#include "BKE_main.h"
diff --git a/source/blender/windowmanager/intern/wm_playanim.c b/source/blender/windowma
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list