Fix T64618: Cycles crash with point density texture on Windows
authorBrecht Van Lommel <brechtvanlommel@gmail.com>
Tue, 14 May 2019 22:42:51 +0000 (00:42 +0200)
committerBrecht Van Lommel <brechtvanlommel@gmail.com>
Tue, 14 May 2019 22:59:31 +0000 (00:59 +0200)
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.

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

index c50dbb6ba558b18c86ceec47cfc0de00b552b761..11b6a38c195600affe48bd5675937d01c3924020 100644 (file)
@@ -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);
index cf1c5fbb446a782224a89a3bb0e64d556c491c85..431e9230cb42bba77a8148ee7af4df56ceeb2148 100644 (file)
@@ -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;
index e8ed657ee10a89c68a7c438bfca2bc7e91951afc..70d7fd3632d94813f424ded144b34c89582c89a4 100644 (file)
@@ -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,
index 22b39701ef2b9e4654a3bd04c006aad19c9df72d..e4fa92fb1d75618804d28c9ca4046102a2bb0e86 100644 (file)
@@ -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)