Alembic: integrate cache file into the dependency graph

* The cache file datablock is now evaluated as part of the dependency graph,
  creating/freeing the Alembic file handle matching the current frame.
  Modifiers and constraints depend on this evaluation.
* Cache file handles and readers now only exist on COW datablocks, never the
  original ones.
* Object data paths are flushed back to the original for the user interface.
* The cache file keeps a list of all readers associated with its handle, and
  automatically frees them when the handle is freed. This kind of sharing of
  data across datablocks is weak but we have no better mechanism for it.

Fix T62720: Alembic sequences not working and crashing

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

4 files changed, 157 insertions, 150 deletions

diff --git a/source/blender/blenkernel/BKE_cachefile.h b/source/blender/blenkernel/BKE_cachefile.h
index b991ac24284..257975e3c17 100644
--- a/source/blender/blenkernel/BKE_cachefile.h
+++ b/source/blender/blenkernel/BKE_cachefile.h
@@ -29,8 +29,10 @@ extern "C" {
 #endif
 
 struct CacheFile;
+struct CacheReader;
 struct Depsgraph;
 struct Main;
+struct Object;
 struct Scene;
 
 void BKE_cachefiles_init(void);
@@ -52,24 +54,27 @@ void BKE_cachefile_make_local(struct Main *bmain,
                               struct CacheFile *cache_file,
                               const bool lib_local);
 
-void BKE_cachefile_reload(const struct Main *bmain, struct CacheFile *cache_file);
+void BKE_cachefile_reload(struct Depsgraph *depsgraph, struct CacheFile *cache_file);
 
-void BKE_cachefile_ensure_handle(const struct Main *bmain, struct CacheFile *cache_file);
-
-void BKE_cachefile_update_frame(struct Main *bmain,
-                                struct Depsgraph *depsgraph,
-                                struct Scene *scene,
-                                const float ctime,
-                                const float fps);
+void BKE_cachefile_eval(struct Main *bmain,
+                        struct Depsgraph *depsgraph,
+                        struct CacheFile *cache_file);
 
 bool BKE_cachefile_filepath_get(const struct Main *bmain,
+                                const struct Depsgraph *depsgrah,
                                 const struct CacheFile *cache_file,
-                                float frame,
                                 char r_filename[1024]);
 
-float BKE_cachefile_time_offset(struct CacheFile *cache_file, const float time, const float fps);
+float BKE_cachefile_time_offset(const struct CacheFile *cache_file,
+                                const float time,
+                                const float fps);
 
-void BKE_cachefile_clean(struct Main *bmain, struct CacheFile *cache_file);
+/* Modifiers and constraints open and free readers through these. */
+void BKE_cachefile_reader_open(struct CacheFile *cache_file,
+                               struct CacheReader **reader,
+                               struct Object *object,
+                               const char *object_path);
+void BKE_cachefile_reader_free(struct CacheFile *cache_file, struct CacheReader **reader);
 
 #ifdef __cplusplus
 }
diff --git a/source/blender/blenkernel/intern/cachefile.c b/source/blender/blenkernel/intern/cachefile.c
index e4fc4706e66..a60b840344c 100644
--- a/source/blender/blenkernel/intern/cachefile.c
+++ b/source/blender/blenkernel/intern/cachefile.c
@@ -21,18 +21,21 @@
  * \ingroup bke
  */
 
+#include <string.h>
+
 #include "DNA_anim_types.h"
 #include "DNA_cachefile_types.h"
 #include "DNA_constraint_types.h"
 #include "DNA_object_types.h"
 #include "DNA_scene_types.h"
 
+#include "BLI_utildefines.h"
 #include "BLI_fileops.h"
+#include "BLI_ghash.h"
 #include "BLI_listbase.h"
 #include "BLI_path_util.h"
 #include "BLI_string.h"
 #include "BLI_threads.h"
-#include "BLI_utildefines.h"
 
 #include "BKE_animsys.h"
 #include "BKE_cachefile.h"
@@ -41,10 +44,13 @@
 #include "BKE_modifier.h"
 #include "BKE_scene.h"
 
+#include "DEG_depsgraph_query.h"
+
 #ifdef WITH_ALEMBIC
 #  include "ABC_alembic.h"
 #endif
 
+/* TODO: make this per cache file to avoid global locks. */
 static SpinLock spin;
 
 void BKE_cachefiles_init(void)
@@ -57,6 +63,94 @@ void BKE_cachefiles_exit(void)
   BLI_spin_end(&spin);
 }
 
+void BKE_cachefile_reader_open(CacheFile *cache_file,
+                               struct CacheReader **reader,
+                               Object *object,
+                               const char *object_path)
+{
+#ifdef WITH_ALEMBIC
+  BLI_assert(cache_file->id.tag & LIB_TAG_COPIED_ON_WRITE);
+
+  if (cache_file->handle == NULL) {
+    return;
+  }
+
+  /* Open Alembic cache reader. */
+  *reader = CacheReader_open_alembic_object(cache_file->handle, *reader, object, object_path);
+
+  /* Multiple modifiers and constraints can call this function concurrently. */
+  BLI_spin_lock(&spin);
+  if (*reader) {
+    /* Register in set so we can free it when the cache file changes. */
+    if (cache_file->handle_readers == NULL) {
+      cache_file->handle_readers = BLI_gset_ptr_new("CacheFile.handle_readers");
+    }
+    BLI_gset_reinsert(cache_file->handle_readers, reader, NULL);
+  }
+  else if (cache_file->handle_readers) {
+    /* Remove in case CacheReader_open_alembic_object free the existing reader. */
+    BLI_gset_remove(cache_file->handle_readers, reader, NULL);
+  }
+  BLI_spin_unlock(&spin);
+#else
+  UNUSED_VARS(reader, object, object_path);
+#endif
+}
+
+void BKE_cachefile_reader_free(CacheFile *cache_file, struct CacheReader **reader)
+{
+#ifdef WITH_ALEMBIC
+  BLI_assert(cache_file->id.tag & LIB_TAG_COPIED_ON_WRITE);
+
+  if (*reader != NULL) {
+    CacheReader_free(*reader);
+    *reader = NULL;
+
+    /* Multiple modifiers and constraints can call this function concurrently. */
+    BLI_spin_lock(&spin);
+    if (cache_file->handle_readers) {
+      BLI_gset_remove(cache_file->handle_readers, reader, NULL);
+    }
+    BLI_spin_unlock(&spin);
+  }
+#else
+  UNUSED_VARS(cache_file, reader);
+#endif
+}
+
+static void cachefile_handle_free(CacheFile *cache_file)
+{
+#ifdef WITH_ALEMBIC
+  /* Free readers in all modifiers and constraints that use the handle, before
+   * we free the handle itself. */
+  BLI_spin_lock(&spin);
+  if (cache_file->handle_readers) {
+    GSetIterator gs_iter;
+    GSET_ITER (gs_iter, cache_file->handle_readers) {
+      struct CacheReader **reader = BLI_gsetIterator_getKey(&gs_iter);
+      if (*reader != NULL) {
+        CacheReader_free(*reader);
+        *reader = NULL;
+      }
+    }
+
+    BLI_gset_free(cache_file->handle_readers, NULL);
+    cache_file->handle_readers = NULL;
+  }
+  BLI_spin_unlock(&spin);
+
+  /* Free handle. */
+  if (cache_file->handle) {
+    ABC_free_handle(cache_file->handle);
+    cache_file->handle = NULL;
+  }
+
+  cache_file->handle_filepath[0] = '\0';
+#else
+  UNUSED_VARS(cache_file);
+#endif
+}
+
 void *BKE_cachefile_add(Main *bmain, const char *name)
 {
   CacheFile *cache_file = BKE_libblock_alloc(bmain, ID_CF, name, 0);
@@ -68,37 +162,23 @@ void *BKE_cachefile_add(Main *bmain, const char *name)
 
 void BKE_cachefile_init(CacheFile *cache_file)
 {
-  cache_file->handle = NULL;
   cache_file->filepath[0] = '\0';
   cache_file->override_frame = false;
   cache_file->frame = 0.0f;
   cache_file->is_sequence = false;
   cache_file->scale = 1.0f;
-  cache_file->handle_mutex = BLI_mutex_alloc();
   BLI_listbase_clear(&cache_file->object_paths);
+
+  cache_file->handle = NULL;
+  cache_file->handle_filepath[0] = '\0';
+  cache_file->handle_readers = NULL;
 }
 
 /** Free (or release) any data used by this cachefile (does not free the cachefile itself). */
 void BKE_cachefile_free(CacheFile *cache_file)
 {
   BKE_animdata_free((ID *)cache_file, false);
-
-  if (cache_file->id.tag & LIB_TAG_NO_MAIN) {
-    /* CoW/no-main copies reuse the existing ArchiveReader and mutex */
-    return;
-  }
-
-  if (cache_file->handle) {
-#ifdef WITH_ALEMBIC
-    ABC_free_handle(cache_file->handle);
-#endif
-    cache_file->handle = NULL;
-  }
-  if (cache_file->handle_mutex) {
-    BLI_mutex_free(cache_file->handle_mutex);
-    cache_file->handle_mutex = NULL;
-  }
-
+  cachefile_handle_free(cache_file);
   BLI_freelistN(&cache_file->object_paths);
 }
 
@@ -117,13 +197,8 @@ void BKE_cachefile_copy_data(Main *UNUSED(bmain),
                              const CacheFile *UNUSED(cache_file_src),
                              const int UNUSED(flag))
 {
-  if (cache_file_dst->id.tag & LIB_TAG_NO_MAIN) {
-    /* CoW/no-main copies reuse the existing ArchiveReader and mutex */
-    return;
-  }
-
   cache_file_dst->handle = NULL;
-  cache_file_dst->handle_mutex = NULL;
+  cache_file_dst->handle_readers = NULL;
   BLI_duplicatelist(&cache_file_dst->object_paths, &cache_file_dst->object_paths);
 }
 
@@ -139,72 +214,51 @@ void BKE_cachefile_make_local(Main *bmain, CacheFile *cache_file, const bool lib
   BKE_id_make_local_generic(bmain, &cache_file->id, true, lib_local);
 }
 
-void BKE_cachefile_reload(const Main *bmain, CacheFile *cache_file)
+void BKE_cachefile_reload(Depsgraph *depsgraph, CacheFile *cache_file)
 {
-  char filepath[FILE_MAX];
-
-  BLI_strncpy(filepath, cache_file->filepath, sizeof(filepath));
-  BLI_path_abs(filepath, ID_BLEND_PATH(bmain, &cache_file->id));
-
-#ifdef WITH_ALEMBIC
-  if (cache_file->handle) {
-    ABC_free_handle(cache_file->handle);
+  /* To force reload, free the handle and tag depsgraph to load it again. */
+  CacheFile *cache_file_eval = (CacheFile *)DEG_get_evaluated_id(depsgraph, &cache_file->id);
+  if (cache_file_eval) {
+    cachefile_handle_free(cache_file_eval);
   }
 
-  cache_file->handle = ABC_create_handle(filepath, &cache_file->object_paths);
-#endif
+  DEG_id_tag_update(&cache_file->id, ID_RECALC_COPY_ON_WRITE);
 }
 
-void BKE_cachefile_ensure_handle(const Main *bmain, CacheFile *cache_file)
+void BKE_cachefile_eval(Main *bmain, Depsgraph *depsgraph, CacheFile *cache_file)
 {
-  BLI_spin_lock(&spin);
-  if (cache_file->handle_mutex == NULL) {
-    cache_file->handle_mutex = BLI_mutex_alloc();
-  }
-  BLI_spin_unlock(&spin);
-
-  BLI_mutex_lock(cache_file->handle_mutex);
+  BLI_assert(cache_file->id.tag & LIB_TAG_COPIED_ON_WRITE);
 
-  if (cache_file->handle == NULL) {
-    /* Assigning to a CoW copy is a bad idea; assign to the original instead. */
-    BLI_assert((cache_file->id.tag & LIB_TAG_COPIED_ON_WRITE) == 0);
-    BKE_cachefile_reload(bmain, cache_file);
+  /* Compute filepath. */
+  char filepath[FILE_MAX];
+  if (!BKE_cachefile_filepath_get(bmain, depsgraph, cache_file, filepath)) {
+    return;
   }
 
-  BLI_mutex_unlock(cache_file->handle_mutex);
-}
-
-void BKE_cachefile_update_frame(
-    Main *bmain, struct Depsgraph *depsgraph, Scene *scene, const float ctime, const float fps)
-{
-  CacheFile *cache_file;
-  char filename[FILE_MAX];
-
-  for (cache_file = bmain->cachefiles.first; cache_file; cache_file = cache_file->id.next) {
-    /* TODO: dependency graph should be updated to do drivers on cachefile.
-     * Execute drivers only, as animation has already been done. */
-    BKE_animsys_evaluate_animdata(
-        depsgraph, scene, &cache_file->id, cache_file->adt, ctime, ADT_RECALC_DRIVERS);
-
-    if (!cache_file->is_sequence) {
-      continue;
-    }
+  /* Test if filepath change or if we can keep the existing handle. */
+  if (STREQ(cache_file->handle_filepath, filepath)) {
+    return;
+  }
 
-    const float time = BKE_cachefile_time_offset(cache_file, ctime, fps);
+  cachefile_handle_free(cache_file);
+  BLI_freelistN(&cache_file->object_paths);
 
-    if (BKE_cachefile_filepath_get(bmain, cache_file, time, filename)) {
-      BKE_cachefile_clean(bmain, cache_file);
 #ifdef WITH_ALEMBIC
-      ABC_free_handle(cache_file->handle);
-      cache_file->handle = ABC_create_handle(filename, NULL);
+  cache_file->handle = ABC_create_handle(filepath, &cache_file->object_paths);
+  BLI_strncpy(cache_file->handle_filepath, filepath, FILE_MAX);
 #endif
-    }
+
+  if (DEG_is_active(depsgraph)) {
+    /* Flush object paths back to original datablock for UI. */
+    CacheFile *cache_file_orig = (CacheFile *)DEG_get_original_id(&cache_file->id);
+    BLI_freelistN(&cache_file_orig->object_paths);
+    BLI_duplicatelist(&cache_file_orig->object_paths, &cache_file->object_paths);
   }
 }
 
 bool BKE_cachefile_filepath_get(const Main *bmain,
+                                const Depsgraph *depsgraph,
                                 const CacheFile *cache_file,
-                                float frame,
                                 char r_filepath[FILE_MAX])
 {
   BLI_strncpy(r_filepath, cache_file->filepath, FILE_MAX);
@@ -214,6 +268,11 @@ bool BKE_cachefile_filepath_get(const Main *bmain,
   int frame_len;
 
   if (cache_file->is_sequence && BLI_path_frame_get(r_filepath, &fframe, &frame_len)) {
+    Scene *scene = DEG_get_evaluated_scene(depsgraph);
+    const float ctime = BKE_scene_frame_get(scene);
+    const float fps = (((double)scene->r.frs_sec) / (double)scene->r.frs_sec_base);
+    const float frame = BKE_cachefile_time_offset(cache_file, ctime, fps);
+
     char ext[32];
     BLI_path_frame_strip(r_filepath, ext);
     BLI_path_frame(r_filepath, frame, frame_len);
@@ -226,47 +285,9 @@ bool BKE_cachefile_filepath_get(const Main *bmain,
   return true;
 }
 
-float BKE_cachefile_time_offset(CacheFile *cache_file, const float time, const float fps)
+float BKE_cachefile_time_offset(const CacheFile *cache_file, const float time, const float fps)
 {
   const float time_offset = cache_file->frame_offset / fps;
   const float frame = (cache_file->override_frame ? cache_file->frame : time);
   return cache_file->is_sequence ? frame : frame / fps - time_offset;
 }
-
-/* TODO(kevin): replace this with some depsgraph mechanism, or something similar. */
-void BKE_cachefile_clean(struct Main *bmain, CacheFile *cache_file)
-{
-  for (Object *ob = bmain->objects.first; ob; ob = ob->id.next) {
-    ModifierData *md = modifiers_findByType(ob, eModifierType_MeshSequenceCache);
-
-    if (md) {
-      MeshSeqCacheModifierData *mcmd = (MeshSeqCacheModifierData *)md;
-
-      if (cache_file == mcmd->cache_file) {
-#ifdef WITH_ALEMBIC
-        if (mcmd->reader != NULL) {
-          CacheReader_free(mcmd->reader);
-        }
-#endif
-        mcmd->reader = NULL;
-      }
-    }
-
-    for (bConstraint *con = ob->constraints.first; con; con = con->next) {
-      if (con->type != CONSTRAINT_TYPE_TRANSFORM_CACHE) {
-        continue;
-      }
-
-      bTransformCacheConstraint *data = con->data;
-
-      if (cache_file == data->cache_file) {
-#ifdef WITH_ALEMBIC
-        if (data->reader != NULL) {
-          CacheReader_free(data->reader);
-        }
-#endif
-        data->reader = NULL;
-      }
-    }
-  }
-}
diff --git a/source/blender/blenkernel/intern/constraint.c b/source/blender/blenkernel/intern/constraint.c
index a475a16dd57..0e29f165992 100644
--- a/source/blender/blenkernel/intern/constraint.c
+++ b/source/blender/blenkernel/intern/constraint.c
@@ -4835,13 +4835,9 @@ static void transformcache_evaluate(bConstraint *con, bConstraintOb *cob, ListBa
   const float frame = DEG_get_ctime(cob->depsgraph);
   const float time = BKE_cachefile_time_offset(cache_file, frame, FPS);
 
-  /* Must always load ABC handle on original. */
-  CacheFile *cache_file_orig = (CacheFile *)DEG_get_original_id(&cache_file->id);
-  BKE_cachefile_ensure_handle(G.main, cache_file_orig);
-
-  if (!data->reader) {
-    data->reader = CacheReader_open_alembic_object(
-        cache_file_orig->handle, data->reader, cob->ob, data->object_path);
+  if (!data->reader || !STREQ(data->reader_object_path, data->object_path)) {
+    STRNCPY(data->reader_object_path, data->object_path);
+    BKE_cachefile_reader_open(cache_file, &data->reader, cob->ob, data->object_path);
   }
 
   ABC_get_transform(data->reader, cob->matrix, time, cache_file->scale);
@@ -4859,12 +4855,8 @@ static void transformcache_copy(bConstraint *con, bConstraint *srccon)
 
   BLI_strncpy(dst->object_path, src->object_path, sizeof(dst->object_path));
   dst->cache_file = src->cache_file;
-
-#ifdef WITH_ALEMBIC
-  if (dst->reader) {
-    CacheReader_incref(dst->reader);
-  }
-#endif
+  dst->reader = NULL;
+  dst->reader_object_path[0] = '\0';
 }
 
 static void transformcache_free(bConstraint *con)
@@ -4872,10 +4864,8 @@ static void transformcache_free(bConstraint *con)
   bTransformCacheConstraint *data = con->data;
 
   if (data->reader) {
-#ifdef WITH_ALEMBIC
-    CacheReader_free(data->reader);
-#endif
-    data->reader = NULL;
+    BKE_cachefile_reader_free(data->cache_file, &data->reader);
+    data->reader_object_path[0] = '\0';
   }
 }
 
diff --git a/source/blender/blenkernel/intern/scene.c b/source/blender/blenkernel/intern/scene.c
index e0bf62402d2..58b36aed24f 100644
--- a/source/blender/blenkernel/intern/scene.c
+++ b/source/blender/blenkernel/intern/scene.c
@@ -1565,15 +1565,6 @@ void BKE_scene_graph_update_for_newframe(Depsgraph *depsgraph, Main *bmain)
   BKE_image_editors_update_frame(bmain, scene->r.cfra);
   BKE_sound_set_cfra(scene->r.cfra);
   DEG_graph_relations_update(depsgraph, bmain, scene, view_layer);
-  /* Update animated cache files for modifiers.
-   *
-   * TODO(sergey): Make this a depsgraph node?
-   */
-  BKE_cachefile_update_frame(bmain,
-                             depsgraph,
-                             scene,
-                             ctime,
-                             (((double)scene->r.frs_sec) / (double)scene->r.frs_sec_base));
 #ifdef POSE_ANIMATION_WORKAROUND
   scene_armature_depsgraph_workaround(bmain, depsgraph);
 #endif