[Bf-blender-cvs] [2e1ba1ee20d] temp-asset-representation: Free asset representations when no longer displayed

[Julian Eisel]

Commit: 2e1ba1ee20dd28292fbacca123e33dac243e9ff8
Author: Julian Eisel
Date:   Tue Nov 8 16:29:58 2022 +0100
Branches: temp-asset-representation
https://developer.blender.org/rB2e1ba1ee20dd28292fbacca123e33dac243e9ff8

Free asset representations when no longer displayed

The lifetime management of assets should still be improved, but
meanwhile each part of the UI loads its own representation of an asset
into memory. This is still done via the file browser backend, so this
should also remove the representations once it frees its file-list.

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

M	source/blender/blenkernel/BKE_asset_library.hh
M	source/blender/blenkernel/intern/asset_library.cc
M	source/blender/editors/space_file/filelist.cc

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

diff --git a/source/blender/blenkernel/BKE_asset_library.hh b/source/blender/blenkernel/BKE_asset_library.hh
index d0140b2b3f7..6402799b3a1 100644
--- a/source/blender/blenkernel/BKE_asset_library.hh
+++ b/source/blender/blenkernel/BKE_asset_library.hh
@@ -53,8 +53,18 @@ struct AssetLibrary {
   /** Load catalogs that have changed on disk. */
   void refresh();
 
+  /**
+   * Create a representation of an asset to be considered part of this library. Once the
+   * representation is not needed anymore, it must be freed using #remove_asset(), or there will be
+   * leaking that's only cleared when the library storage is destructed (typically on exit or
+   * loading a different file).
+   */
   AssetRepresentation &add_external_asset(std::unique_ptr<AssetMetaData> metadata);
   AssetRepresentation &add_local_id_asset(const ID &id);
+  /** Remove an asset from the library that was added using #add_external_asset() or
+   * #add_local_id_asset().
+   * \return True on success, false if the asset couldn't be found inside the library. */
+  bool remove_asset(AssetRepresentation &asset);
 
   /**
    * Update `catalog_simple_name` by looking up the asset's catalog by its ID.
@@ -72,13 +82,22 @@ struct AssetLibrary {
  private:
   bCallbackFuncStore on_save_callback_store_{};
 
-  /** Container to store asset representations. Assets are not automatically loaded into this when
-   * loading an asset library. Assets have to be loaded externally and added to this storage via
-   * #add_external_asset() or #add_local_id_asset().
-   * So this really is arbitrary storage as far as #AssetLibrary is concerned (allowing the API
-   * user to manage partial library storage and partial loading, so only relevant parts of a
-   * library are kept in memory). */
+  /** Storage for assets (better said their representations) that are considered to be part of this
+   * library. Assets are not automatically loaded into this when loading an asset library. Assets
+   * have to be loaded externally and added to this storage via #add_external_asset() or
+   * #add_local_id_asset(). So this really is arbitrary storage as far as #AssetLibrary is
+   * concerned (allowing the API user to manage partial library storage and partial loading, so
+   * only relevant parts of a library are kept in memory).
+   *
+   * For now, multiple parts of Blender just keep adding their own assets to this storage. E.g.
+   * multiple asset browsers might load multiple representations for the same asset into this.
+   * Currently there is just no way to properly identify assets, or keep track of which assets are
+   * already in memory and which not. Neither do we keep track of how many parts of Blender are
+   * using an asset or an asset library, which is needed to know when assets can be freed.
+   */
   Vector<std::unique_ptr<AssetRepresentation>> asset_storage_;
+
+  std::optional<int> find_asset_index(const AssetRepresentation &asset);
 };
 
 Vector<AssetLibraryReference> all_valid_asset_library_refs();
diff --git a/source/blender/blenkernel/intern/asset_library.cc b/source/blender/blenkernel/intern/asset_library.cc
index 4efa2c33891..edf804f70bc 100644
--- a/source/blender/blenkernel/intern/asset_library.cc
+++ b/source/blender/blenkernel/intern/asset_library.cc
@@ -136,6 +136,31 @@ AssetRepresentation &AssetLibrary::add_local_id_asset(const ID &id)
   return *asset_storage_.last();
 }
 
+std::optional<int> AssetLibrary::find_asset_index(const AssetRepresentation &asset)
+{
+  int index = 0;
+  /* Find index of asset. */
+  for (auto &asset_uptr : asset_storage_) {
+    if (&asset == asset_uptr.get()) {
+      return index;
+    }
+    index++;
+  }
+
+  return {};
+}
+
+bool AssetLibrary::remove_asset(AssetRepresentation &asset)
+{
+  std::optional<int> asset_index = find_asset_index(asset);
+  if (!asset_index) {
+    return false;
+  }
+
+  asset_storage_.remove_and_reorder(*asset_index);
+  return true;
+}
+
 namespace {
 void asset_library_on_save_post(struct Main *main,
                                 struct PointerRNA **pointers,
diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc
index 221701c9bc0..af72cad9668 100644
--- a/source/blender/editors/space_file/filelist.cc
+++ b/source/blender/editors/space_file/filelist.cc
@@ -1404,8 +1404,13 @@ static void filelist_direntryarr_free(FileDirEntryArr *array)
   array->entries_filtered_num = FILEDIR_NBR_ENTRIES_UNSET;
 }
 
-static void filelist_intern_entry_free(FileListInternEntry *entry)
+static void filelist_intern_entry_free(FileList *filelist, FileListInternEntry *entry)
 {
+  if (entry->asset) {
+    BLI_assert(filelist->asset_library);
+    filelist->asset_library->remove_asset(*entry->asset);
+  }
+
   if (entry->relpath) {
     MEM_freeN(entry->relpath);
   }
@@ -1418,10 +1423,11 @@ static void filelist_intern_entry_free(FileListInternEntry *entry)
   MEM_freeN(entry);
 }
 
-static void filelist_intern_free(FileListIntern *filelist_intern)
+static void filelist_intern_free(FileList *filelist)
 {
+  FileListIntern *filelist_intern = &filelist->filelist_intern;
   LISTBASE_FOREACH_MUTABLE (FileListInternEntry *, entry, &filelist_intern->entries) {
-    filelist_intern_entry_free(entry);
+    filelist_intern_entry_free(filelist, entry);
   }
   BLI_listbase_clear(&filelist_intern->entries);
 
@@ -1431,8 +1437,9 @@ static void filelist_intern_free(FileListIntern *filelist_intern)
 /**
  * \return the number of main files removed.
  */
-static int filelist_intern_free_main_files(FileListIntern *filelist_intern)
+static int filelist_intern_free_main_files(FileList *filelist)
 {
+  FileListIntern *filelist_intern = &filelist->filelist_intern;
   int removed_counter = 0;
   LISTBASE_FOREACH_MUTABLE (FileListInternEntry *, entry, &filelist_intern->entries) {
     if (!filelist_intern_entry_is_main_file(entry)) {
@@ -1440,7 +1447,7 @@ static int filelist_intern_free_main_files(FileListIntern *filelist_intern)
     }
 
     BLI_remlink(&filelist_intern->entries, entry);
-    filelist_intern_entry_free(entry);
+    filelist_intern_entry_free(filelist, entry);
     removed_counter++;
   }
 
@@ -1795,7 +1802,7 @@ void filelist_clear_ex(struct FileList *filelist,
     filelist_cache_clear(&filelist->filelist_cache, filelist->filelist_cache.size);
   }
 
-  filelist_intern_free(&filelist->filelist_intern);
+  filelist_intern_free(filelist);
 
   filelist_direntryarr_free(&filelist->filelist);
 
@@ -1823,7 +1830,7 @@ static void filelist_clear_main_files(FileList *filelist,
     filelist_cache_clear(&filelist->filelist_cache, filelist->filelist_cache.size);
   }
 
-  const int removed_files = filelist_intern_free_main_files(&filelist->filelist_intern);
+  const int removed_files = filelist_intern_free_main_files(filelist);
 
   filelist->filelist.entries_num -= removed_files;
   filelist->filelist.entries_filtered_num = FILEDIR_NBR_ENTRIES_UNSET;