[Bf-blender-cvs] [8c3d42bd0f0] blender-v2.91-release: Fix T82129: Cycles "Persistent Images" incorrectly retains scene data
Kévin Dietrich
noreply at git.blender.org
Thu Oct 29 17:40:00 CET 2020
Commit: 8c3d42bd0f0160ba58c6a932c0e4c228ce6b0426
Author: Kévin Dietrich
Date: Thu Oct 29 14:40:29 2020 +0100
Branches: blender-v2.91-release
https://developer.blender.org/rB8c3d42bd0f0160ba58c6a932c0e4c228ce6b0426
Fix T82129: Cycles "Persistent Images" incorrectly retains scene data
The issue stems from the fact that scene arrays are not cleared when rendering is done. This was not really an issue before the introduction of the ownership system (rB429afe0c626a) as the id_map would recreate scene data arrays based on their new content. However, now that the id_maps do not have access to the scene data anymore the arrays are never created.
Another related issue is that the BlenderSync instance is never freed when the persistent data option is activated.
To fix this, we delete nodes created by the id_maps in their destructors, and delete the BlenderSync instance before creating a new one, so the id_maps destructors are actually called.
Reviewed By: brecht
Maniphest Tasks: T82129
Differential Revision: https://developer.blender.org/D9378
===================================================================
M intern/cycles/blender/blender_id_map.h
M intern/cycles/blender/blender_light.cpp
M intern/cycles/blender/blender_object.cpp
M intern/cycles/blender/blender_particles.cpp
M intern/cycles/blender/blender_session.cpp
M intern/cycles/blender/blender_shader.cpp
M intern/cycles/blender/blender_sync.cpp
M intern/cycles/render/scene.cpp
M intern/cycles/render/scene.h
===================================================================
diff --git a/intern/cycles/blender/blender_id_map.h b/intern/cycles/blender/blender_id_map.h
index 8ce1d23665d..198cfb4b29a 100644
--- a/intern/cycles/blender/blender_id_map.h
+++ b/intern/cycles/blender/blender_id_map.h
@@ -35,10 +35,22 @@ CCL_NAMESPACE_BEGIN
template<typename K, typename T> class id_map {
public:
- id_map()
+ id_map(Scene *scene_) : scene(scene_)
{
}
+ ~id_map()
+ {
+ set<T *> nodes;
+
+ typename map<K, T *>::iterator jt;
+ for (jt = b_map.begin(); jt != b_map.end(); jt++) {
+ nodes.insert(jt->second);
+ }
+
+ scene->delete_nodes(nodes);
+ }
+
T *find(const BL::ID &id)
{
return find(id.ptr.owner_id);
@@ -98,16 +110,15 @@ template<typename K, typename T> class id_map {
}
/* Combined add and update as needed. */
- bool add_or_update(Scene *scene, T **r_data, const BL::ID &id)
+ bool add_or_update(T **r_data, const BL::ID &id)
{
- return add_or_update(scene, r_data, id, id, id.ptr.owner_id);
+ return add_or_update(r_data, id, id, id.ptr.owner_id);
}
- bool add_or_update(Scene *scene, T **r_data, const BL::ID &id, const K &key)
+ bool add_or_update(T **r_data, const BL::ID &id, const K &key)
{
- return add_or_update(scene, r_data, id, id, key);
+ return add_or_update(r_data, id, id, key);
}
- bool add_or_update(
- Scene *scene, T **r_data, const BL::ID &id, const BL::ID &parent, const K &key)
+ bool add_or_update(T **r_data, const BL::ID &id, const BL::ID &parent, const K &key)
{
T *data = find(key);
bool recalc;
@@ -146,7 +157,7 @@ template<typename K, typename T> class id_map {
b_map[NULL] = data;
}
- void post_sync(Scene *scene, bool do_delete = true)
+ void post_sync(bool do_delete = true)
{
map<K, T *> new_map;
typedef pair<const K, T *> TMapPair;
@@ -177,6 +188,7 @@ template<typename K, typename T> class id_map {
map<K, T *> b_map;
set<T *> used_set;
set<void *> b_recalc;
+ Scene *scene;
};
/* Object Key
diff --git a/intern/cycles/blender/blender_light.cpp b/intern/cycles/blender/blender_light.cpp
index 117e9214e5a..6f95821e31e 100644
--- a/intern/cycles/blender/blender_light.cpp
+++ b/intern/cycles/blender/blender_light.cpp
@@ -39,9 +39,9 @@ void BlenderSync::sync_light(BL::Object &b_parent,
BL::Light b_light(b_ob.data());
/* Update if either object or light data changed. */
- if (!light_map.add_or_update(scene, &light, b_ob, b_parent, key)) {
+ if (!light_map.add_or_update(&light, b_ob, b_parent, key)) {
Shader *shader;
- if (!shader_map.add_or_update(scene, &shader, b_light)) {
+ if (!shader_map.add_or_update(&shader, b_light)) {
if (light->is_portal)
*use_portal = true;
return;
@@ -176,7 +176,7 @@ void BlenderSync::sync_background_light(BL::SpaceView3D &b_v3d, bool use_portal)
Light *light;
ObjectKey key(b_world, 0, b_world, false);
- if (light_map.add_or_update(scene, &light, b_world, b_world, key) || world_recalc ||
+ if (light_map.add_or_update(&light, b_world, b_world, key) || world_recalc ||
b_world.ptr.data != world_map) {
light->type = LIGHT_BACKGROUND;
if (sampling_method == SAMPLING_MANUAL) {
diff --git a/intern/cycles/blender/blender_object.cpp b/intern/cycles/blender/blender_object.cpp
index 212b9cbe103..4146d87ad2e 100644
--- a/intern/cycles/blender/blender_object.cpp
+++ b/intern/cycles/blender/blender_object.cpp
@@ -207,7 +207,7 @@ Object *BlenderSync::sync_object(BL::Depsgraph &b_depsgraph,
/* test if we need to sync */
bool object_updated = false;
- if (object_map.add_or_update(scene, &object, b_ob, b_parent, key))
+ if (object_map.add_or_update(&object, b_ob, b_parent, key))
object_updated = true;
/* mesh sync */
@@ -405,10 +405,10 @@ void BlenderSync::sync_objects(BL::Depsgraph &b_depsgraph,
sync_background_light(b_v3d, use_portal);
/* handle removed data and modified pointers */
- light_map.post_sync(scene);
- geometry_map.post_sync(scene);
- object_map.post_sync(scene);
- particle_system_map.post_sync(scene);
+ light_map.post_sync();
+ geometry_map.post_sync();
+ object_map.post_sync();
+ particle_system_map.post_sync();
}
if (motion)
diff --git a/intern/cycles/blender/blender_particles.cpp b/intern/cycles/blender/blender_particles.cpp
index ee14217988b..e5eab1ae62b 100644
--- a/intern/cycles/blender/blender_particles.cpp
+++ b/intern/cycles/blender/blender_particles.cpp
@@ -53,8 +53,7 @@ bool BlenderSync::sync_dupli_particle(BL::Object &b_ob,
ParticleSystem *psys;
bool first_use = !particle_system_map.is_used(key);
- bool need_update = particle_system_map.add_or_update(
- scene, &psys, b_ob, b_instance.object(), key);
+ bool need_update = particle_system_map.add_or_update(&psys, b_ob, b_instance.object(), key);
/* no update needed? */
if (!need_update && !object->geometry->need_update && !scene->object_manager->need_update)
diff --git a/intern/cycles/blender/blender_session.cpp b/intern/cycles/blender/blender_session.cpp
index c2bdec0e53d..513cac1e0e9 100644
--- a/intern/cycles/blender/blender_session.cpp
+++ b/intern/cycles/blender/blender_session.cpp
@@ -238,6 +238,7 @@ void BlenderSession::reset_session(BL::BlendData &b_data, BL::Depsgraph &b_depsg
* See note on create_session().
*/
/* sync object should be re-created */
+ delete sync;
sync = new BlenderSync(b_engine, b_data, b_scene, scene, !background, session->progress);
BL::SpaceView3D b_null_space_view3d(PointerRNA_NULL);
diff --git a/intern/cycles/blender/blender_shader.cpp b/intern/cycles/blender/blender_shader.cpp
index 03ed88ea9ec..c171982b29d 100644
--- a/intern/cycles/blender/blender_shader.cpp
+++ b/intern/cycles/blender/blender_shader.cpp
@@ -1241,7 +1241,7 @@ void BlenderSync::sync_materials(BL::Depsgraph &b_depsgraph, bool update_all)
Shader *shader;
/* test if we need to sync */
- if (shader_map.add_or_update(scene, &shader, b_mat) || update_all) {
+ if (shader_map.add_or_update(&shader, b_mat) || update_all) {
ShaderGraph *graph = new ShaderGraph();
shader->name = b_mat.name().c_str();
@@ -1467,7 +1467,7 @@ void BlenderSync::sync_lights(BL::Depsgraph &b_depsgraph, bool update_all)
Shader *shader;
/* test if we need to sync */
- if (shader_map.add_or_update(scene, &shader, b_light) || update_all) {
+ if (shader_map.add_or_update(&shader, b_light) || update_all) {
ShaderGraph *graph = new ShaderGraph();
/* create nodes */
diff --git a/intern/cycles/blender/blender_sync.cpp b/intern/cycles/blender/blender_sync.cpp
index d2760d55f2d..0139afb711d 100644
--- a/intern/cycles/blender/blender_sync.cpp
+++ b/intern/cycles/blender/blender_sync.cpp
@@ -56,11 +56,11 @@ BlenderSync::BlenderSync(BL::RenderEngine &b_engine,
: b_engine(b_engine),
b_data(b_data),
b_scene(b_scene),
- shader_map(),
- object_map(),
- geometry_map(),
- light_map(),
- particle_system_map(),
+ shader_map(scene),
+ object_map(scene),
+ geometry_map(scene),
+ light_map(scene),
+ particle_system_map(scene),
world_map(NULL),
world_recalc(false),
scene(scene),
@@ -247,7 +247,7 @@ void BlenderSync::sync_data(BL::RenderSettings &b_render,
/* Shader sync done at the end, since object sync uses it.
* false = don't delete unused shaders, not supported. */
- shader_map.post_sync(scene, false);
+ shader_map.post_sync(false);
free_data_after_sync(b_depsgraph);
diff --git a/intern/cycles/render/scene.cpp b/intern/cycles/render/scene.cpp
index e98b2c76e88..3fb6c9620e6 100644
--- a/intern/cycles/render/scene.cpp
+++ b/intern/cycles/render/scene.cpp
@@ -707,4 +707,58 @@ template<> void Scene::delete_node_impl(Shader * /*node*/)
/* don't delete unused shaders, not supported */
}
+template<typename T>
+static void remove_nodes_in_set(const set<T *> &nodes_set,
+ vector<T *> &nodes_array,
+ const NodeOwner *owner)
+{
+ size_t new_size = nodes_array.size();
+
+ for (size_t i = 0; i < new_size; ++i) {
+ T *node = nodes_array[i];
+
+ if (nodes_set.find(node) != nodes_set.end()) {
+ std::swap(nodes_array[i], nodes_array[new_size - 1]);
+
+ assert(node->get_owner() == owner);
+ delete node;
+
+ i -= 1;
+ new_size -= 1;
+ }
+ }
+
+ nodes_array.resize(new_size);
+ (void)owner;
+}
+
+template<> void Scene::delete_nodes(const set<Light *> &nodes, const NodeOwner *owner)
+{
+ remove_nodes_in_set(nodes, lights, owner);
+ light_manager->tag_update(this);
+}
+
+template<> void Scene::delete_nodes(const set<Geometry *> &nodes, const NodeOwner *owner)
+{
+ remove_nodes_in_set(nodes, geometry, owner);
+ geometry_manager->tag_update(this);
+}
+
+template<> void Scene::delete_nodes(const set<Object *> &nodes, const NodeOwner *owner)
+{
+ remove_nodes_in_set(nodes, objects, owner);
+ object_manager->tag_update(this);
+}
+
+template<> void Scene::delete_nodes(const set<ParticleSystem *> &nodes, const NodeOwner *owner)
+{
+ remove_nodes_in_set(nodes, particle_systems, owner);
+ particle_system_manager->tag_update(this);
+}
+
+template<> void Scene::delete_nodes(const set<Shader *> & /*nodes*/, const NodeOwner * /*owner*/)
+{
+ /* don't delete unused shaders, not supported */
+}
+
CCL_NAMESPACE_END
diff --git a/intern/cycles/render/scene.h b/intern/cycles/render/scene.h
index cc2ef8da589..6686327dc49 100644
--- a/intern/cycles/render/scene.h
+++ b/intern/cycles/render/scene.h
@@ -322,6 +322,18 @@ class Scene : public NodeOwner {
(void)owner;
}
+ /* Remove all nodes in the set from the appropriate data arrays, and tag the
+ * specific managers for an update. This assumes that the scene owns the nodes.
+ */
+ template<typename T> void delete_nodes(const set<T *> &nodes)
+ {
+ delete_nodes(nodes, this);
+ }
+
+ /* Same as above, but specify the actual owner of all the nodes in the set.
+ */
+ template<typename T> void delete_nodes(const set<T *> &nodes, const NodeOwner *owner);
+
protected:
/* Check if some heavy data worth logging was updated.
* Mai
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list