[Bf-blender-cvs] [429afe0c626] master: Cycles: introduce an ownership system to protect nodes from unwanted deletions.

Kévin Dietrich noreply at git.blender.org
Sun Aug 30 23:56:36 CEST 2020


Commit: 429afe0c626a6d608385c6bc3a348b3ac8cfa8c0
Author: Kévin Dietrich
Date:   Sun Aug 30 23:20:51 2020 +0200
Branches: master
https://developer.blender.org/rB429afe0c626a6d608385c6bc3a348b3ac8cfa8c0

Cycles: introduce an ownership system to protect nodes from unwanted deletions.

Problem: the Blender synchronization process creates and tags nodes for usage. It does
this by directly adding and removing nodes from the scene data. If some node is not tagged
as used at the end of a synchronization, it then deletes the node from the scene. This poses
a problem when it comes to supporting procedural nodes who can create other nodes not known
by the Blender synchonization system, which will remove them.

Nodes now have a NodeOwner, which is set after creation. Those owners for now are the Scene
for scene level nodes and ShaderGraph for shader nodes. Instead of creating and deleting
nodes using `new` and `delete` explicitely, we now use `create_node` and `delete_node` methods
found on the owners. `delete_node` will assert that the owner is the right one.

Whenever a scene level node is created or deleted, the appropriate node manager is tagged for
an update, freeing this responsability from BlenderSync or other software exporters.

Concerning BlenderSync, the `id_maps` do not explicitely manipulate scene data anymore, they
only keep track of which nodes are used, employing the scene to create and delete them. To
achieve this, the ParticleSystem is now a Node, although it does not have any sockets.

This is part of T79131.

Reviewed By: #cycles, brecht

Maniphest Tasks: T79131

Differential Revision: https://developer.blender.org/D8540

===================================================================

M	intern/cycles/blender/blender_geometry.cpp
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_shader.cpp
M	intern/cycles/blender/blender_sync.cpp
M	intern/cycles/graph/node.cpp
M	intern/cycles/graph/node.h
M	intern/cycles/render/graph.cpp
M	intern/cycles/render/graph.h
M	intern/cycles/render/nodes.cpp
M	intern/cycles/render/nodes.h
M	intern/cycles/render/osl.cpp
M	intern/cycles/render/osl.h
M	intern/cycles/render/particles.cpp
M	intern/cycles/render/particles.h
M	intern/cycles/render/scene.cpp
M	intern/cycles/render/scene.h
M	intern/cycles/render/shader.cpp

===================================================================

diff --git a/intern/cycles/blender/blender_geometry.cpp b/intern/cycles/blender/blender_geometry.cpp
index 9a82c5e9371..002f5e0fdb7 100644
--- a/intern/cycles/blender/blender_geometry.cpp
+++ b/intern/cycles/blender/blender_geometry.cpp
@@ -83,13 +83,13 @@ Geometry *BlenderSync::sync_geometry(BL::Depsgraph &b_depsgraph,
   if (geom == NULL) {
     /* Add new geometry if it did not exist yet. */
     if (geom_type == Geometry::HAIR) {
-      geom = new Hair();
+      geom = scene->create_node<Hair>();
     }
     else if (geom_type == Geometry::VOLUME) {
-      geom = new Volume();
+      geom = scene->create_node<Volume>();
     }
     else {
-      geom = new Mesh();
+      geom = scene->create_node<Mesh>();
     }
     geometry_map.add(key, geom);
   }
diff --git a/intern/cycles/blender/blender_id_map.h b/intern/cycles/blender/blender_id_map.h
index b5f6aaa67a8..f9f201c2e4e 100644
--- a/intern/cycles/blender/blender_id_map.h
+++ b/intern/cycles/blender/blender_id_map.h
@@ -19,6 +19,8 @@
 
 #include <string.h>
 
+#include "render/scene.h"
+
 #include "util/util_map.h"
 #include "util/util_set.h"
 #include "util/util_vector.h"
@@ -32,9 +34,8 @@ CCL_NAMESPACE_BEGIN
 
 template<typename K, typename T> class id_map {
  public:
-  id_map(vector<T *> *scene_data_)
+  id_map()
   {
-    scene_data = scene_data_;
   }
 
   T *find(const BL::ID &id)
@@ -76,7 +77,6 @@ template<typename K, typename T> class id_map {
   void add(const K &key, T *data)
   {
     assert(find(key) == NULL);
-    scene_data->push_back(data);
     b_map[key] = data;
     used(data);
   }
@@ -97,22 +97,23 @@ template<typename K, typename T> class id_map {
   }
 
   /* Combined add and update as needed. */
-  bool add_or_update(T **r_data, const BL::ID &id)
+  bool add_or_update(Scene *scene, T **r_data, const BL::ID &id)
   {
-    return add_or_update(r_data, id, id, id.ptr.owner_id);
+    return add_or_update(scene, r_data, id, id, id.ptr.owner_id);
   }
-  bool add_or_update(T **r_data, const BL::ID &id, const K &key)
+  bool add_or_update(Scene *scene, T **r_data, const BL::ID &id, const K &key)
   {
-    return add_or_update(r_data, id, id, key);
+    return add_or_update(scene, r_data, id, id, key);
   }
-  bool add_or_update(T **r_data, const BL::ID &id, const BL::ID &parent, const K &key)
+  bool add_or_update(
+      Scene *scene, T **r_data, const BL::ID &id, const BL::ID &parent, const K &key)
   {
     T *data = find(key);
     bool recalc;
 
     if (!data) {
       /* Add data if it didn't exist yet. */
-      data = new T();
+      data = scene->create_node<T>();
       add(key, data);
       recalc = true;
     }
@@ -144,27 +145,8 @@ template<typename K, typename T> class id_map {
     b_map[NULL] = data;
   }
 
-  bool post_sync(bool do_delete = true)
+  void post_sync(Scene *scene, bool do_delete = true)
   {
-    /* remove unused data */
-    vector<T *> new_scene_data;
-    typename vector<T *>::iterator it;
-    bool deleted = false;
-
-    for (it = scene_data->begin(); it != scene_data->end(); it++) {
-      T *data = *it;
-
-      if (do_delete && used_set.find(data) == used_set.end()) {
-        delete data;
-        deleted = true;
-      }
-      else
-        new_scene_data.push_back(data);
-    }
-
-    *scene_data = new_scene_data;
-
-    /* update mapping */
     map<K, T *> new_map;
     typedef pair<const K, T *> TMapPair;
     typename map<K, T *>::iterator jt;
@@ -172,15 +154,17 @@ template<typename K, typename T> class id_map {
     for (jt = b_map.begin(); jt != b_map.end(); jt++) {
       TMapPair &pair = *jt;
 
-      if (used_set.find(pair.second) != used_set.end())
+      if (do_delete && used_set.find(pair.second) == used_set.end()) {
+        scene->delete_node(pair.second);
+      }
+      else {
         new_map[pair.first] = pair.second;
+      }
     }
 
     used_set.clear();
     b_recalc.clear();
     b_map = new_map;
-
-    return deleted;
   }
 
   const map<K, T *> &key_to_scene_data()
@@ -189,7 +173,6 @@ template<typename K, typename T> class id_map {
   }
 
  protected:
-  vector<T *> *scene_data;
   map<K, T *> b_map;
   set<T *> used_set;
   set<void *> b_recalc;
diff --git a/intern/cycles/blender/blender_light.cpp b/intern/cycles/blender/blender_light.cpp
index 6f95821e31e..117e9214e5a 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(&light, b_ob, b_parent, key)) {
+  if (!light_map.add_or_update(scene, &light, b_ob, b_parent, key)) {
     Shader *shader;
-    if (!shader_map.add_or_update(&shader, b_light)) {
+    if (!shader_map.add_or_update(scene, &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(&light, b_world, b_world, key) || world_recalc ||
+      if (light_map.add_or_update(scene, &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 e0792962b01..212b9cbe103 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(&object, b_ob, b_parent, key))
+  if (object_map.add_or_update(scene, &object, b_ob, b_parent, key))
     object_updated = true;
 
   /* mesh sync */
@@ -405,14 +405,10 @@ void BlenderSync::sync_objects(BL::Depsgraph &b_depsgraph,
     sync_background_light(b_v3d, use_portal);
 
     /* handle removed data and modified pointers */
-    if (light_map.post_sync())
-      scene->light_manager->tag_update(scene);
-    if (geometry_map.post_sync())
-      scene->geometry_manager->tag_update(scene);
-    if (object_map.post_sync())
-      scene->object_manager->tag_update(scene);
-    if (particle_system_map.post_sync())
-      scene->particle_system_manager->tag_update(scene);
+    light_map.post_sync(scene);
+    geometry_map.post_sync(scene);
+    object_map.post_sync(scene);
+    particle_system_map.post_sync(scene);
   }
 
   if (motion)
diff --git a/intern/cycles/blender/blender_particles.cpp b/intern/cycles/blender/blender_particles.cpp
index e5eab1ae62b..ee14217988b 100644
--- a/intern/cycles/blender/blender_particles.cpp
+++ b/intern/cycles/blender/blender_particles.cpp
@@ -53,7 +53,8 @@ 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(&psys, b_ob, b_instance.object(), key);
+  bool need_update = particle_system_map.add_or_update(
+      scene, &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_shader.cpp b/intern/cycles/blender/blender_shader.cpp
index ae681432a43..03ed88ea9ec 100644
--- a/intern/cycles/blender/blender_shader.cpp
+++ b/intern/cycles/blender/blender_shader.cpp
@@ -224,7 +224,7 @@ static ShaderNode *add_node(Scene *scene,
   if (b_node.is_a(&RNA_ShaderNodeRGBCurve)) {
     BL::ShaderNodeRGBCurve b_curve_node(b_node);
     BL::CurveMapping mapping(b_curve_node.mapping());
-    RGBCurvesNode *curves = new RGBCurvesNode();
+    RGBCurvesNode *curves = graph->create_node<RGBCurvesNode>();
     curvemapping_color_to_array(mapping, curves->curves, RAMP_TABLE_SIZE, true);
     curvemapping_minmax(mapping, true, &curves->min_x, &curves->max_x);
     node = curves;
@@ -232,13 +232,13 @@ static ShaderNode *add_node(Scene *scene,
   if (b_node.is_a(&RNA_ShaderNodeVectorCurve)) {
     BL::ShaderNodeVectorCurve b_curve_node(b_node);
     BL::CurveMapping mapping(b_curve_node.mapping());
-    VectorCurvesNode *curves = new VectorCurvesNode();
+    VectorCurvesNode *curves = graph->create_node<VectorCurvesNode>();
     curvemapping_color_to_array(mapping, curves->curves, RAMP_TABLE_SIZE, false);
     curvemapping_minmax(mapping, false, &curves->min_x, &curves->max_x);
     node = curves;
   }
   else if (b_node.is_a(&RNA_ShaderNodeValToRGB)) {
-    RGBRampNode *ramp = new RGBRampNode();
+    RGBRampNode *ramp = graph->create_node<RGBRampNode>();
     BL::ShaderNodeValToRGB b_ramp_node(b_node);
     BL::ColorRamp b_color_ramp(b_ramp_node.color_ramp());
     colorramp_to_array(b_color_ramp, ramp->ramp, ramp->ramp_alpha, RAMP_TABLE_SIZE);
@@ -246,94 +246,94 @@ static ShaderNode *add_node(Scene *scene,
     node = ramp;
   }
   else if (b_node.is_a(&RNA_ShaderNodeRGB)) {
-    ColorNode *color = new ColorNode();
+    ColorNode *color = graph->create_node<ColorNode>();
     color->value = get_node_output_rgba(b_node, "Color");
     node = color;
   }
   else if (b_node.is_a(&RNA_ShaderNodeValue)) {
-    ValueNode *value = new ValueNode();
+    ValueNode *value = graph->create_node<ValueNode>();
     value->value = get_node_output_value(b_node, "Value");
     node = value;
   }
   else if (b_node.is_a(&RNA_ShaderNodeCameraData)) {
-    node = new CameraNode();
+    node = graph->create_node<CameraNode>();
   }
   else if (b_node.is_a(&RNA_ShaderNodeInvert)) {
-    node = new InvertNode();
+    node = graph->create_node<InvertNode>();
   }
   else if (b_node.is_a(&RNA_ShaderNodeGamma)) {
-    node = new GammaNode();
+    node = graph->create_node<GammaNode>();
   }
   else if (b_node.is_a(&RNA_ShaderNodeBrightContrast)) {
-    node = new BrightContrastNode();
+    node = graph->create_node<BrightContrastNode>();
   }
   else if (b_node.is_a(&RNA_ShaderNodeMixRGB)) {
     BL::ShaderNodeMixRGB b_mix_node(b_node);
-    MixNode *mix = new MixNode();
+    MixNode *mix = graph->cr

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list