[Bf-blender-cvs] [fb2303fb739] temp-asset-library-all: Avoid ugly nested library storage

Julian Eisel noreply at git.blender.org
Fri Dec 2 19:21:55 CET 2022


Commit: fb2303fb739e72e263289d9aeee10031b10a3ec6
Author: Julian Eisel
Date:   Fri Dec 2 16:58:47 2022 +0100
Branches: temp-asset-library-all
https://developer.blender.org/rBfb2303fb739e72e263289d9aeee10031b10a3ec6

Avoid ugly nested library storage

We actually don't have to do this, since we can just iterate over all
loaded libraries after calling the loading for the "All" asset library.

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

M	source/blender/asset_system/AS_asset_library.hh
M	source/blender/asset_system/intern/asset_library.cc
M	source/blender/asset_system/intern/asset_library_service.cc
M	source/blender/asset_system/intern/asset_library_service.hh
M	source/blender/editors/space_file/filelist.cc

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

diff --git a/source/blender/asset_system/AS_asset_library.hh b/source/blender/asset_system/AS_asset_library.hh
index a9ed5ff0594..d377f2334b8 100644
--- a/source/blender/asset_system/AS_asset_library.hh
+++ b/source/blender/asset_system/AS_asset_library.hh
@@ -56,10 +56,6 @@ class AssetLibrary {
    */
   std::unique_ptr<AssetStorage> asset_storage_;
 
-  /** In some cases an asset library is a combination of multiple other ones, these are then
-   * referenced here. "All" asset library only currently. */
-  Vector<AssetLibrary *> nested_libs_; /* Non-owning pointers. */
-
   bCallbackFuncStore on_save_callback_store_{};
 
  public:
@@ -78,6 +74,17 @@ class AssetLibrary {
   AssetLibrary(StringRef root_path = "");
   ~AssetLibrary();
 
+  /**
+   * Execute \a fn for every asset library that is loaded. The asset library is passed to the
+   * \a fn call.
+   *
+   * \param skip_all_library: When true, the \a fn will also be executed for the "All" asset
+   *                          library. This is just a combination of the other ones, so usually
+   *                          iterating over it is redundant.
+   */
+  static void foreach_loaded(FunctionRef<void(AssetLibrary &)> fn,
+                             bool include_all_library = true);
+
   void load_catalogs();
 
   /** Load catalogs that have changed on disk. */
@@ -101,8 +108,6 @@ class AssetLibrary {
   /** Remove an asset from the library that was added using #add_external_asset() or
    * #add_local_id_asset(). Can usually be expected to be constant time complexity (worst case may
    * differ).
-   * Can also be called when this asset library is just a merged library containing multiple nested
-   * ones ("All" library). Will then check if it exists in a nested library and remove it.
    *
    * \note This is save to call if \a asset is freed (dangling reference), will not perform any
    *       change then.
@@ -110,11 +115,6 @@ class AssetLibrary {
    *         case when the reference is dangling). */
   bool remove_asset(AssetRepresentation &asset);
 
-  /** In some cases an asset library is a combination of multiple other ones ("All" asset library
-   * only currently). Iterate over the contained asset libraries, executing \a fn for each of them.
-   */
-  void foreach_nested(FunctionRef<void(AssetLibrary &nested_library)> fn);
-
   /**
    * Remap ID pointers for local ID assets, see #BKE_lib_remap.h. When an ID pointer would be
    * mapped to null (typically when an ID gets removed), the asset is removed, because we don't
@@ -142,9 +142,6 @@ class AssetLibrary {
   AssetIdentifier asset_identifier_from_library(StringRef relative_asset_path);
 
   StringRefNull root_path() const;
-
- private:
-  std::optional<int> find_asset_index(const AssetRepresentation &asset);
 };
 
 Vector<AssetLibraryReference> all_valid_asset_library_refs();
@@ -152,6 +149,13 @@ Vector<AssetLibraryReference> all_valid_asset_library_refs();
 }  // namespace blender::asset_system
 
 /**
+ * Load the data for an asset library, but not the asset representations themselves (loading these
+ * is currently not done in the asset system).
+ *
+ * For the "All" asset library (#ASSET_LIBRARY_ALL), every other known asset library will be
+ * loaded as well. So a call to #AssetLibrary::foreach_loaded() can be expected to iterate over all
+ * libraries.
+ *
  * \warning Catalogs are reloaded, invalidating catalog pointers. Do not store catalog pointers,
  *          store CatalogIDs instead and lookup the catalog where needed.
  */
diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc
index 1fe477707a1..b763822c1ac 100644
--- a/source/blender/asset_system/intern/asset_library.cc
+++ b/source/blender/asset_system/intern/asset_library.cc
@@ -121,9 +121,11 @@ void AS_asset_library_refresh_catalog_simplename(struct ::AssetLibrary *asset_li
 void AS_asset_library_remap_ids(const IDRemapper *mappings)
 {
   AssetLibraryService *service = AssetLibraryService::get();
-  service->foreach_loaded_asset_library([mappings](asset_system::AssetLibrary &library) {
-    library.remap_ids_and_remove_invalid(*mappings);
-  });
+  service->foreach_loaded_asset_library(
+      [mappings](asset_system::AssetLibrary &library) {
+        library.remap_ids_and_remove_invalid(*mappings);
+      },
+      true);
 }
 
 namespace blender::asset_system {
@@ -142,6 +144,13 @@ AssetLibrary::~AssetLibrary()
   }
 }
 
+void AssetLibrary::foreach_loaded(FunctionRef<void(AssetLibrary &)> fn,
+                                  const bool include_all_library)
+{
+  AssetLibraryService *service = AssetLibraryService::get();
+  service->foreach_loaded_asset_library(fn, include_all_library);
+}
+
 void AssetLibrary::load_catalogs()
 {
   auto catalog_service = std::make_unique<AssetCatalogService>(root_path());
@@ -170,36 +179,7 @@ AssetRepresentation &AssetLibrary::add_local_id_asset(StringRef relative_asset_p
 
 bool AssetLibrary::remove_asset(AssetRepresentation &asset)
 {
-  /* Usual case, only the "All" library differs and uses nested libraries (see below). */
-  if (asset_storage_->remove_asset(asset)) {
-    return true;
-  }
-
-  /* If asset is not stored in this library, check nested ones (for "All" library). */
-  for (AssetLibrary *library : nested_libs_) {
-    if (!library) {
-      BLI_assert_unreachable();
-      continue;
-    }
-
-    if (asset_storage_->remove_asset(asset)) {
-      return true;
-    }
-  }
-
-  return false;
-}
-
-void AssetLibrary::foreach_nested(FunctionRef<void(AssetLibrary &)> fn)
-{
-  for (AssetLibrary *library : nested_libs_) {
-    if (!library) {
-      BLI_assert_unreachable();
-      continue;
-    }
-
-    fn(*library);
-  }
+  return asset_storage_->remove_asset(asset);
 }
 
 void AssetLibrary::remap_ids_and_remove_invalid(const IDRemapper &mappings)
diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc
index 8a00ebe2a08..7f44143fef8 100644
--- a/source/blender/asset_system/intern/asset_library_service.cc
+++ b/source/blender/asset_system/intern/asset_library_service.cc
@@ -132,7 +132,6 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain)
 {
   if (all_library_) {
     CLOG_INFO(&LOG, 2, "get all lib (cached)");
-    all_library_->nested_libs_.clear();
   }
   else {
     CLOG_INFO(&LOG, 2, "get all lib (loaded)");
@@ -145,8 +144,8 @@ AssetLibrary *AssetLibraryService::get_asset_library_all(const Main *bmain)
       continue;
     }
 
-    AssetLibrary *nested_lib = get_asset_library(bmain, library_ref);
-    all_library_->nested_libs_.append(nested_lib);
+    /* Ensure all asset libraries are loaded. */
+    get_asset_library(bmain, library_ref);
   }
 
   return all_library_.get();
@@ -214,21 +213,25 @@ void AssetLibraryService::app_handler_unregister()
 
 bool AssetLibraryService::has_any_unsaved_catalogs() const
 {
-  if (current_file_library_ && current_file_library_->catalog_service->has_unsaved_changes()) {
-    return true;
-  }
-
-  for (const auto &asset_lib_uptr : on_disk_libraries_.values()) {
-    if (asset_lib_uptr->catalog_service->has_unsaved_changes()) {
-      return true;
-    }
-  }
-
-  return false;
+  bool has_unsaved_changes = false;
+
+  foreach_loaded_asset_library(
+      [&has_unsaved_changes](AssetLibrary &library) {
+        if (library.catalog_service->has_unsaved_changes()) {
+          has_unsaved_changes = true;
+        }
+      },
+      true);
+  return has_unsaved_changes;
 }
 
-void AssetLibraryService::foreach_loaded_asset_library(FunctionRef<void(AssetLibrary &)> fn) const
+void AssetLibraryService::foreach_loaded_asset_library(FunctionRef<void(AssetLibrary &)> fn,
+                                                       const bool include_all_library) const
 {
+  if (include_all_library && all_library_) {
+    fn(*all_library_);
+  }
+
   if (current_file_library_) {
     fn(*current_file_library_);
   }
diff --git a/source/blender/asset_system/intern/asset_library_service.hh b/source/blender/asset_system/intern/asset_library_service.hh
index 478465d25ed..1dd9bda7aca 100644
--- a/source/blender/asset_system/intern/asset_library_service.hh
+++ b/source/blender/asset_system/intern/asset_library_service.hh
@@ -70,13 +70,15 @@ class AssetLibraryService {
   /** Get the "Current File" asset library. */
   AssetLibrary *get_asset_library_current_file();
 
-  /** Get the "All" asset library, merging all others into one. */
+  /** Get the "All" asset library, which loads all others and merges them into one. */
   AssetLibrary *get_asset_library_all(const Main *bmain);
 
   /** Returns whether there are any known asset libraries with unsaved catalog edits. */
   bool has_any_unsaved_catalogs() const;
 
-  void foreach_loaded_asset_library(FunctionRef<void(AssetLibrary &)> fn) const;
+  /** See AssetLibrary::foreach_loaded(). */
+  void foreach_loaded_asset_library(FunctionRef<void(AssetLibrary &)> fn,
+                                    bool include_all_library) const;
 
  protected:
   /** Allocate a new instance of the service and assign it to `instance_`. */
diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc
index ca83ff6d51b..60c34ceda5f 100644
--- a/source/blender/editors/space_file/filelist.cc
+++ b/source/blender/editors/space_file/filelist.cc
@@ -3125,7 +3125,7 @@ static void filelist_readjob_list_lib_add_datablock(FileListReadJob *job_params,
     if (datablock_info->asset_data) {
       entry->typeflag |= FILE_TYPE_ASSET;
 
-      if (job_params->asset_library) {
+      if (job_params->load_asset_library) {
         /* Take ownership over the asset data (shallow copies into unique_ptr managed memory) to
          * pass it on to the asset system. */
         std::unique_ptr metadata = std::make_unique<AssetMetaData>(*datablock_info->asset_data);
@@ -3905,8 +3905,9 @@ static void filelist_readjob_all_asset_library(FileListReadJob *job_params,
 
   BLI_assert(filelist->asset_library != nullptr);
 
-  /* Add assets from asset libraries on disk. */
-  filelist->asset_library->foreach_nested([&](asset_system::AssetLibrary &nested_library) {
+

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list