[Bf-blender-cvs] [4e46ed37fc5] master: Fix T64618: Cycles crash with point density texture on Windows

[Brecht Van Lommel]

Commit: 4e46ed37fc5939d733001ba8ce17bd46e500839b
Author: Brecht Van Lommel
Date:   Wed May 15 00:42:51 2019 +0200
Branches: master
https://developer.blender.org/rB4e46ed37fc5939d733001ba8ce17bd46e500839b

Fix T64618: Cycles crash with point density texture on Windows

A better solution would be to not use the callback mechanism anymore for
cases like this where the dependency graph will free volume data, but
that would be a bigger refactor.

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

M	intern/cycles/blender/blender_session.cpp
M	intern/cycles/render/image.cpp
M	intern/cycles/render/image.h
M	intern/cycles/render/nodes.cpp

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

diff --git a/intern/cycles/blender/blender_session.cpp b/intern/cycles/blender/blender_session.cpp
index c50dbb6ba55..11b6a38c195 100644
--- a/intern/cycles/blender/blender_session.cpp
+++ b/intern/cycles/blender/blender_session.cpp
@@ -1440,7 +1440,12 @@ void BlenderSession::builtin_images_load()
 {
   /* Force builtin images to be loaded along with Blender data sync. This
    * is needed because we may be reading from depsgraph evaluated data which
-   * can be freed by Blender before Cycles reads it. */
+   * can be freed by Blender before Cycles reads it.
+   *
+   * TODO: the assumption that no further access to builtin image data will
+   * happen is really weak, and likely to break in the future. We should find
+   * a better solution to hand over the data directly to the image manager
+   * instead of through callbacks whose timing is difficult to control. */
   ImageManager *manager = session->scene->image_manager;
   Device *device = session->device;
   manager->device_load_builtin(device, session->scene, session->progress);
diff --git a/intern/cycles/render/image.cpp b/intern/cycles/render/image.cpp
index cf1c5fbb446..431e9230cb4 100644
--- a/intern/cycles/render/image.cpp
+++ b/intern/cycles/render/image.cpp
@@ -412,6 +412,17 @@ int ImageManager::add_image(const string &filename,
   return type_index_to_flattened_slot(slot, type);
 }
 
+void ImageManager::add_image_user(int flat_slot)
+{
+  ImageDataType type;
+  int slot = flattened_slot_to_type_index(flat_slot, &type);
+
+  Image *image = images[type][slot];
+  assert(image && image->users >= 1);
+
+  image->users++;
+}
+
 void ImageManager::remove_image(int flat_slot)
 {
   ImageDataType type;
diff --git a/intern/cycles/render/image.h b/intern/cycles/render/image.h
index e8ed657ee10..70d7fd3632d 100644
--- a/intern/cycles/render/image.h
+++ b/intern/cycles/render/image.h
@@ -86,6 +86,7 @@ class ImageManager {
                 bool use_alpha,
                 ustring colorspace,
                 ImageMetaData &metadata);
+  void add_image_user(int flat_slot);
   void remove_image(int flat_slot);
   void remove_image(const string &filename,
                     void *builtin_data,
diff --git a/intern/cycles/render/nodes.cpp b/intern/cycles/render/nodes.cpp
index 22b39701ef2..e4fa92fb1d7 100644
--- a/intern/cycles/render/nodes.cpp
+++ b/intern/cycles/render/nodes.cpp
@@ -263,13 +263,11 @@ ImageTextureNode::~ImageTextureNode()
 
 ShaderNode *ImageTextureNode::clone() const
 {
-  ImageTextureNode *node = new ImageTextureNode(*this);
-  node->image_manager = NULL;
-  node->slot = -1;
-  node->is_float = -1;
-  node->compress_as_srgb = false;
-  node->colorspace = colorspace;
-  return node;
+  /* Increase image user count for new node. */
+  if (slot != -1) {
+    image_manager->add_image_user(slot);
+  }
+  return new ImageTextureNode(*this);
 }
 
 void ImageTextureNode::attributes(Shader *shader, AttributeRequestSet *attributes)
@@ -448,13 +446,11 @@ EnvironmentTextureNode::~EnvironmentTextureNode()
 
 ShaderNode *EnvironmentTextureNode::clone() const
 {
-  EnvironmentTextureNode *node = new EnvironmentTextureNode(*this);
-  node->image_manager = NULL;
-  node->slot = -1;
-  node->is_float = -1;
-  node->compress_as_srgb = false;
-  node->colorspace = colorspace;
-  return node;
+  /* Increase image user count for new node. */
+  if (slot != -1) {
+    image_manager->add_image_user(slot);
+  }
+  return new EnvironmentTextureNode(*this);
 }
 
 void EnvironmentTextureNode::attributes(Shader *shader, AttributeRequestSet *attributes)
@@ -1481,10 +1477,13 @@ PointDensityTextureNode::~PointDensityTextureNode()
 
 ShaderNode *PointDensityTextureNode::clone() const
 {
-  PointDensityTextureNode *node = new PointDensityTextureNode(*this);
-  node->image_manager = NULL;
-  node->slot = -1;
-  return node;
+  /* Increase image user count for new node. We need to ensure to not call
+   * add_image again, to work around access of freed data on the Blender
+   * side. A better solution should be found to avoid this. */
+  if (slot != -1) {
+    image_manager->add_image_user(slot);
+  }
+  return new PointDensityTextureNode(*this);
 }
 
 void PointDensityTextureNode::attributes(Shader *shader, AttributeRequestSet *attributes)