Fix T58251: Cycles ignores linked meshes when rendering
authorSergey Sharybin <sergey.vfx@gmail.com>
Mon, 27 May 2019 09:45:33 +0000 (11:45 +0200)
committerSergey Sharybin <sergey.vfx@gmail.com>
Wed, 29 May 2019 08:44:11 +0000 (10:44 +0200)
The idea is to share a mesh data-block as a result across all objects
which are sharing same original mesh and have no effective modifiers.
This mesh is owned by an original copy-on-written version of object data.

Tricky part is to make sure it is only initialized once, and currently a
silly mutex lock is used. In practice it only locks if the mesh is not
already there.

As an extra bonus, even viewport memory is also lower after this change.

Reviewers: brecht, mont29

Reviewed By: brecht, mont29

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

source/blender/blenkernel/intern/DerivedMesh.c
source/blender/blenkernel/intern/mesh.c
source/blender/blenkernel/intern/mesh_runtime.c
source/blender/blenkernel/intern/object.c
source/blender/makesdna/DNA_mesh_types.h
source/blender/makesdna/DNA_object_types.h

index 615a68d..dce8f34 100644 (file)
@@ -1097,6 +1097,30 @@ static void mesh_calc_modifier_final_normals(const Mesh *mesh_input,
   }
 }
 
   }
 }
 
+/* Does final touches to the final evaluated mesh, making sure it is perfectly usable.
+ *
+ * This is needed because certain information is not passed along intermediate meshes allocated
+ * during stack evaluation.
+ */
+static void mesh_calc_finalize(const Mesh *mesh_input, Mesh *mesh_eval)
+{
+  /* Make sure the name is the same. This is because mesh allocation from template does not
+   * take care of naming. */
+  BLI_strncpy(mesh_eval->id.name, mesh_input->id.name, sizeof(mesh_eval->id.name));
+  /* Make sure materials are preserved from the input. */
+  if (mesh_eval->mat != NULL) {
+    MEM_freeN(mesh_eval->mat);
+  }
+  mesh_eval->mat = MEM_dupallocN(mesh_input->mat);
+  mesh_eval->totcol = mesh_input->totcol;
+  /* Make evaluated mesh to share same edit mesh pointer as original and copied meshes. */
+  mesh_eval->edit_mesh = mesh_input->edit_mesh;
+  /* Copy auth-smooth settings which are also not taken care about by mesh allocation from a
+   * template. */
+  mesh_eval->flag |= (mesh_input->flag & ME_AUTOSMOOTH);
+  mesh_eval->smoothresh = mesh_input->smoothresh;
+}
+
 static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
                                 Scene *scene,
                                 Object *ob,
 static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
                                 Scene *scene,
                                 Object *ob,
@@ -1514,7 +1538,12 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
    * need to apply these back onto the Mesh. If we have no
    * Mesh then we need to build one. */
   if (mesh_final == NULL) {
    * need to apply these back onto the Mesh. If we have no
    * Mesh then we need to build one. */
   if (mesh_final == NULL) {
-    mesh_final = BKE_mesh_copy_for_eval(mesh_input, true);
+    if (deformed_verts == NULL) {
+      mesh_final = mesh_input;
+    }
+    else {
+      mesh_final = BKE_mesh_copy_for_eval(mesh_input, true);
+    }
   }
   if (deformed_verts) {
     BKE_mesh_apply_vert_coords(mesh_final, deformed_verts);
   }
   if (deformed_verts) {
     BKE_mesh_apply_vert_coords(mesh_final, deformed_verts);
@@ -1522,9 +1551,17 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
     deformed_verts = NULL;
   }
 
     deformed_verts = NULL;
   }
 
+  /* Denotes whether the object which the modifier stack came from owns the mesh or whether the
+   * mesh is shared across multiple objects since there are no effective modifiers. */
+  const bool is_own_mesh = (mesh_final != mesh_input);
+
   /* Add orco coordinates to final and deformed mesh if requested. */
   if (dataMask->vmask & CD_MASK_ORCO) {
   /* Add orco coordinates to final and deformed mesh if requested. */
   if (dataMask->vmask & CD_MASK_ORCO) {
-    add_orco_mesh(ob, NULL, mesh_final, mesh_orco, CD_ORCO);
+    /* No need in ORCO layer if the mesh was not deformed or modified: undeformed mesh in this case
+     * matches input mesh. */
+    if (is_own_mesh) {
+      add_orco_mesh(ob, NULL, mesh_final, mesh_orco, CD_ORCO);
+    }
 
     if (mesh_deform) {
       add_orco_mesh(ob, NULL, mesh_deform, NULL, CD_ORCO);
 
     if (mesh_deform) {
       add_orco_mesh(ob, NULL, mesh_deform, NULL, CD_ORCO);
@@ -1539,7 +1576,28 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
   }
 
   /* Compute normals. */
   }
 
   /* Compute normals. */
-  mesh_calc_modifier_final_normals(mesh_input, dataMask, sculpt_dyntopo, mesh_final);
+  if (is_own_mesh) {
+    mesh_calc_modifier_final_normals(mesh_input, dataMask, sculpt_dyntopo, mesh_final);
+  }
+  else {
+    Mesh_Runtime *runtime = &mesh_input->runtime;
+    if (runtime->mesh_eval == NULL) {
+      BLI_assert(runtime->eval_mutex != NULL);
+      BLI_mutex_lock(runtime->eval_mutex);
+      if (runtime->mesh_eval == NULL) {
+        mesh_final = BKE_mesh_copy_for_eval(mesh_input, true);
+        mesh_calc_modifier_final_normals(mesh_input, dataMask, sculpt_dyntopo, mesh_final);
+        mesh_calc_finalize(mesh_input, mesh_final);
+        runtime->mesh_eval = mesh_final;
+      }
+      BLI_mutex_unlock(runtime->eval_mutex);
+    }
+    mesh_final = runtime->mesh_eval;
+  }
+
+  if (is_own_mesh) {
+    mesh_calc_finalize(mesh_input, mesh_final);
+  }
 
   /* Return final mesh */
   *r_final = mesh_final;
 
   /* Return final mesh */
   *r_final = mesh_final;
@@ -1915,40 +1973,29 @@ static void editbmesh_calc_modifiers(struct Depsgraph *depsgraph,
   }
 }
 
   }
 }
 
-static void mesh_finalize_eval(Object *object)
+static void assign_object_mesh_eval(Object *object)
 {
 {
+  BLI_assert(object->id.tag & LIB_TAG_COPIED_ON_WRITE);
+
   Mesh *mesh = (Mesh *)object->data;
   Mesh *mesh_eval = object->runtime.mesh_eval;
   Mesh *mesh = (Mesh *)object->data;
   Mesh *mesh_eval = object->runtime.mesh_eval;
-  /* Special Tweaks for cases when evaluated mesh came from
-   * BKE_mesh_new_nomain_from_template().
-   */
-  BLI_strncpy(mesh_eval->id.name, mesh->id.name, sizeof(mesh_eval->id.name));
-  if (mesh_eval->mat != NULL) {
-    MEM_freeN(mesh_eval->mat);
+
+  /* The modifier stack evaluation is storing result in mesh->runtime.mesh_eval, but this result
+   * is not guaranteed to be owned by object.
+   *
+   * Check ownership now, since later on we can not go to a mesh owned by someone else via object's
+   * runtime: this could cause access freed data on depsgraph destruction (mesh who owns the final
+   * result might be freed prior to object). */
+  if (mesh_eval == mesh->runtime.mesh_eval) {
+    object->runtime.is_mesh_eval_owned = false;
+  }
+  else {
+    mesh_eval->id.tag |= LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT;
+    object->runtime.is_mesh_eval_owned = true;
   }
   }
-  /* Set flag which makes it easier to see what's going on in a debugger. */
-  mesh_eval->id.tag |= LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT;
-  mesh_eval->mat = MEM_dupallocN(mesh->mat);
-  mesh_eval->totcol = mesh->totcol;
-  /* Make evaluated mesh to share same edit mesh pointer as original
-   * and copied meshes.
-   */
-  mesh_eval->edit_mesh = mesh->edit_mesh;
-  /* Copy autosmooth settings from original mesh.
-   * This is not done by BKE_mesh_new_nomain_from_template(), so need to take
-   * extra care here.
-   */
-  mesh_eval->flag |= (mesh->flag & ME_AUTOSMOOTH);
-  mesh_eval->smoothresh = mesh->smoothresh;
-  /* Replace evaluated object's data with fully evaluated mesh. */
-  /* TODO(sergey): There was statement done by Sybren and Mai that this
-   * caused modifiers to be applied twice. which is weirtd and shouldn't
-   * really happen. But since there is no reference to the report, can not
-   * do much about this.
-   */
 
 
-  /* Object is sometimes not evaluated!
-   * TODO(sergey): BAD TEMPORARY HACK FOR UNTIL WE ARE SMARTER */
+  /* NOTE: We are not supposed to invoke evaluation for original object, but some areas are still
+   * under process of being ported, so we play safe here. */
   if (object->id.tag & LIB_TAG_COPIED_ON_WRITE) {
     object->data = mesh_eval;
   }
   if (object->id.tag & LIB_TAG_COPIED_ON_WRITE) {
     object->data = mesh_eval;
   }
@@ -2016,7 +2063,7 @@ static void mesh_build_data(struct Depsgraph *depsgraph,
     BKE_mesh_texspace_copy_from_object(ob->runtime.mesh_eval, ob);
   }
 
     BKE_mesh_texspace_copy_from_object(ob->runtime.mesh_eval, ob);
   }
 
-  mesh_finalize_eval(ob);
+  assign_object_mesh_eval(ob);
 
   ob->runtime.last_data_mask = *dataMask;
   ob->runtime.last_need_mapping = need_mapping;
 
   ob->runtime.last_data_mask = *dataMask;
   ob->runtime.last_need_mapping = need_mapping;
@@ -2032,7 +2079,9 @@ static void mesh_build_data(struct Depsgraph *depsgraph,
 #endif
   }
 
 #endif
   }
 
-  mesh_runtime_check_normals_valid(ob->runtime.mesh_eval);
+  if (ob->runtime.mesh_eval != NULL) {
+    mesh_runtime_check_normals_valid(ob->runtime.mesh_eval);
+  }
   mesh_build_extra_data(depsgraph, ob);
 }
 
   mesh_build_extra_data(depsgraph, ob);
 }
 
index 56832c1..aa13950 100644 (file)
@@ -522,6 +522,8 @@ void BKE_mesh_init(Mesh *me)
   CustomData_reset(&me->fdata);
   CustomData_reset(&me->pdata);
   CustomData_reset(&me->ldata);
   CustomData_reset(&me->fdata);
   CustomData_reset(&me->pdata);
   CustomData_reset(&me->ldata);
+
+  BKE_mesh_runtime_reset(me);
 }
 
 Mesh *BKE_mesh_add(Main *bmain, const char *name)
 }
 
 Mesh *BKE_mesh_add(Main *bmain, const char *name)
index 06abd80..4c3761c 100644 (file)
@@ -33,6 +33,7 @@
 #include "BLI_threads.h"
 
 #include "BKE_bvhutils.h"
 #include "BLI_threads.h"
 
 #include "BKE_bvhutils.h"
+#include "BKE_library.h"
 #include "BKE_mesh.h"
 #include "BKE_mesh_runtime.h"
 #include "BKE_subdiv_ccg.h"
 #include "BKE_mesh.h"
 #include "BKE_mesh_runtime.h"
 #include "BKE_subdiv_ccg.h"
@@ -50,6 +51,8 @@ static ThreadRWMutex loops_cache_lock = PTHREAD_RWLOCK_INITIALIZER;
 void BKE_mesh_runtime_reset(Mesh *mesh)
 {
   memset(&mesh->runtime, 0, sizeof(mesh->runtime));
 void BKE_mesh_runtime_reset(Mesh *mesh)
 {
   memset(&mesh->runtime, 0, sizeof(mesh->runtime));
+  mesh->runtime.eval_mutex = MEM_mallocN(sizeof(ThreadMutex), "mesh runtime eval_mutex");
+  BLI_mutex_init(mesh->runtime.eval_mutex);
 }
 
 /* Clear all pointers which we don't want to be shared on copying the datablock.
 }
 
 /* Clear all pointers which we don't want to be shared on copying the datablock.
@@ -59,16 +62,30 @@ void BKE_mesh_runtime_reset_on_copy(Mesh *mesh, const int UNUSED(flag))
 {
   Mesh_Runtime *runtime = &mesh->runtime;
 
 {
   Mesh_Runtime *runtime = &mesh->runtime;
 
+  runtime->mesh_eval = NULL;
   runtime->edit_data = NULL;
   runtime->batch_cache = NULL;
   runtime->subdiv_ccg = NULL;
   memset(&runtime->looptris, 0, sizeof(runtime->looptris));
   runtime->bvh_cache = NULL;
   runtime->shrinkwrap_data = NULL;
   runtime->edit_data = NULL;
   runtime->batch_cache = NULL;
   runtime->subdiv_ccg = NULL;
   memset(&runtime->looptris, 0, sizeof(runtime->looptris));
   runtime->bvh_cache = NULL;
   runtime->shrinkwrap_data = NULL;
+
+  mesh->runtime.eval_mutex = MEM_mallocN(sizeof(ThreadMutex), "mesh runtime eval_mutex");
+  BLI_mutex_init(mesh->runtime.eval_mutex);
 }
 
 void BKE_mesh_runtime_clear_cache(Mesh *mesh)
 {
 }
 
 void BKE_mesh_runtime_clear_cache(Mesh *mesh)
 {
+  if (mesh->runtime.eval_mutex != NULL) {
+    BLI_mutex_end(mesh->runtime.eval_mutex);
+    MEM_freeN(mesh->runtime.eval_mutex);
+    mesh->runtime.eval_mutex = NULL;
+  }
+  if (mesh->runtime.mesh_eval != NULL) {
+    mesh->runtime.mesh_eval->edit_mesh = NULL;
+    BKE_id_free(NULL, mesh->runtime.mesh_eval);
+    mesh->runtime.mesh_eval = NULL;
+  }
   BKE_mesh_runtime_clear_geometry(mesh);
   BKE_mesh_batch_cache_free(mesh);
   BKE_mesh_runtime_clear_edit_data(mesh);
   BKE_mesh_runtime_clear_geometry(mesh);
   BKE_mesh_batch_cache_free(mesh);
   BKE_mesh_runtime_clear_edit_data(mesh);
index a4dbbb5..6752392 100644 (file)
@@ -361,6 +361,13 @@ static void object_update_from_subsurf_ccg(Object *object)
   if (object->type != OB_MESH) {
     return;
   }
   if (object->type != OB_MESH) {
     return;
   }
+  /* If object does not own evaluated mesh we can not access it since it might be freed already
+   * (happens on dependency graph free where order of CoW-ed IDs free is undefined).
+   *
+   * Good news is: such mesh does not have modifiers applied, so no need to worry about CCG. */
+  if (!object->runtime.is_mesh_eval_owned) {
+    return;
+  }
   /* Object was never evaluated, so can not have CCG subdivision surface. */
   Mesh *mesh_eval = object->runtime.mesh_eval;
   if (mesh_eval == NULL) {
   /* Object was never evaluated, so can not have CCG subdivision surface. */
   Mesh *mesh_eval = object->runtime.mesh_eval;
   if (mesh_eval == NULL) {
@@ -448,12 +455,13 @@ void BKE_object_free_derived_caches(Object *ob)
   object_update_from_subsurf_ccg(ob);
   BKE_object_free_derived_mesh_caches(ob);
 
   object_update_from_subsurf_ccg(ob);
   BKE_object_free_derived_mesh_caches(ob);
 
-  if (ob->runtime.mesh_eval != NULL) {
+  /* Restore initial pointer. */
+  if (ob->runtime.mesh_orig != NULL) {
+    ob->data = ob->runtime.mesh_orig;
+  }
+
+  if ((ob->runtime.mesh_eval != NULL && ob->runtime.is_mesh_eval_owned)) {
     Mesh *mesh_eval = ob->runtime.mesh_eval;
     Mesh *mesh_eval = ob->runtime.mesh_eval;
-    /* Restore initial pointer. */
-    if (ob->data == mesh_eval) {
-      ob->data = ob->runtime.mesh_orig;
-    }
     /* Evaluated mesh points to edit mesh, but does not own it. */
     mesh_eval->edit_mesh = NULL;
     BKE_mesh_free(mesh_eval);
     /* Evaluated mesh points to edit mesh, but does not own it. */
     mesh_eval->edit_mesh = NULL;
     BKE_mesh_free(mesh_eval);
index f54c178..8a9a69a 100644 (file)
@@ -75,6 +75,12 @@ struct MLoopTri_Store {
 
 /* not saved in file! */
 typedef struct Mesh_Runtime {
 
 /* not saved in file! */
 typedef struct Mesh_Runtime {
+  /* Evaluated mesh for objects which do not have effective modifiers. This mesh is sued as a
+   * result of modifier stack evaluation.
+   * Since modifier stack evaluation is threaded on object level we need some synchronization. */
+  struct Mesh *mesh_eval;
+  void *eval_mutex;
+
   struct EditMeshData *edit_data;
   void *batch_cache;
 
   struct EditMeshData *edit_data;
   void *batch_cache;
 
index 9bbb6f1..e248d60 100644 (file)
@@ -155,6 +155,10 @@ typedef struct Object_Runtime {
    * It has all modifiers applied.
    */
   struct Mesh *mesh_eval;
    * It has all modifiers applied.
    */
   struct Mesh *mesh_eval;
+  /* Denotes whether the evaluated mesh is ownbed by this object or is referenced and owned by
+   * somebody else. */
+  int is_mesh_eval_owned;
+  int _pad3[3];
   /**
    * Mesh structure created during object evaluation.
    * It has deforemation only modifiers applied on it.
   /**
    * Mesh structure created during object evaluation.
    * It has deforemation only modifiers applied on it.