Refactor: Assets: Improve asset system APIs for asset operators

Brush and pose asset operators were doing some avoidable roundtrips
through asset types, lookups and rather low level operations. This
indicates that the asset system APIs need some improvements. Moving
lower-level logic there can help avoiding errors, since implementation
details are kept inside the corresponding module.

- Avoid lookups for asset library reference in operator code, make asset
  library itself construct it.
- Avoid redundant lookups for asset library definition (was looking up
  the asset library definition in the Preferences from the library
  reference type, just to turn it into the reference type again).
- Add utility for refreshing loading asset libraries
- Add utility for saving asset catalogs for an asset
- Remove unused function

source/blender/asset_system/AS_asset_library.hh

@@ -10,6 +10,7 @@
 
 #include <memory>
 #include <mutex>
+#include <optional>
 
 #include "AS_asset_catalog.hh"
 
@@ -120,6 +121,13 @@ class AssetLibrary {
    */
   static void foreach_loaded(FunctionRef<void(AssetLibrary &)> fn, bool include_all_library);
 
+  /**
+   * Get the #AssetLibraryReference referencing this library. This can fail for custom libraries,
+   * which have too look up their #bUserAssetLibrary. It will not return a value for values that
+   * were loaded directly through a path.
+   */
+  virtual std::optional<AssetLibraryReference> library_reference() const = 0;
+
   void load_catalogs();
 
   AssetCatalogService &catalog_service() const;

source/blender/asset_system/AS_asset_representation.hh

@@ -34,7 +34,7 @@ class AssetLibrary;
 
 class AssetRepresentation : NonCopyable, NonMovable {
   /** Pointer back to the asset library that owns this asset representation. */
-  const AssetLibrary &owner_asset_library_;
+  AssetLibrary &owner_asset_library_;
   /**
    * Uniquely identifies the asset within the asset library. Currently this is always a path (path
    * within the asset library).
@@ -57,14 +57,12 @@ class AssetRepresentation : NonCopyable, NonMovable {
                       StringRef name,
                       int id_type,
                       std::unique_ptr<AssetMetaData> metadata,
-                      const AssetLibrary &owner_asset_library);
+                      AssetLibrary &owner_asset_library);
   /**
    * Constructs an asset representation for an ID stored in the current file. This makes the asset
    * local and fully editable.
    */
-  AssetRepresentation(StringRef relative_asset_path,
-                      ID &id,
-                      const AssetLibrary &owner_asset_library);
+  AssetRepresentation(StringRef relative_asset_path, ID &id, AssetLibrary &owner_asset_library);
   ~AssetRepresentation();
 
   /**
@@ -119,7 +117,7 @@ class AssetRepresentation : NonCopyable, NonMovable {
   ID *local_id() const;
   /** Returns if this asset is stored inside this current file, and as such fully editable. */
   bool is_local_id() const;
-  const AssetLibrary &owner_asset_library() const;
+  AssetLibrary &owner_asset_library() const;
 };
 
 }  // namespace blender::asset_system

source/blender/asset_system/intern/asset_representation.cc

@@ -28,7 +28,7 @@ AssetRepresentation::AssetRepresentation(StringRef relative_asset_path,
                                          StringRef name,
                                          const int id_type,
                                          std::unique_ptr<AssetMetaData> metadata,
-                                         const AssetLibrary &owner_asset_library)
+                                         AssetLibrary &owner_asset_library)
     : owner_asset_library_(owner_asset_library),
       relative_identifier_(relative_asset_path),
       asset_(AssetRepresentation::ExternalAsset{name, id_type, std::move(metadata), nullptr})
@@ -37,7 +37,7 @@ AssetRepresentation::AssetRepresentation(StringRef relative_asset_path,
 
 AssetRepresentation::AssetRepresentation(StringRef relative_asset_path,
                                          ID &id,
-                                         const AssetLibrary &owner_asset_library)
+                                         AssetLibrary &owner_asset_library)
     : owner_asset_library_(owner_asset_library),
       relative_identifier_(relative_asset_path),
       asset_(&id)
@@ -168,7 +168,7 @@ bool AssetRepresentation::is_local_id() const
   return std::holds_alternative<ID *>(asset_);
 }
 
-const AssetLibrary &AssetRepresentation::owner_asset_library() const
+AssetLibrary &AssetRepresentation::owner_asset_library() const
 {
   return owner_asset_library_;
 }

source/blender/asset_system/intern/library_types/all_library.cc

@@ -22,6 +22,11 @@ namespace blender::asset_system {
 
 AllAssetLibrary::AllAssetLibrary() : AssetLibrary(ASSET_LIBRARY_ALL) {}
 
+std::optional<AssetLibraryReference> AllAssetLibrary::library_reference() const
+{
+  return all_library_reference();
+}
+
 void AllAssetLibrary::rebuild_catalogs_from_nested(const bool reload_nested_catalogs)
 {
   /* Start with empty catalog storage. Don't do this directly in #this.catalog_service to avoid

source/blender/asset_system/intern/library_types/all_library.hh

@@ -20,6 +20,7 @@ class AllAssetLibrary : public AssetLibrary {
  public:
   AllAssetLibrary();
 
+  std::optional<AssetLibraryReference> library_reference() const override;
   void refresh_catalogs() override;
 
   /**

source/blender/asset_system/intern/library_types/essentials_library.cc

@@ -23,6 +23,14 @@ EssentialsAssetLibrary::EssentialsAssetLibrary()
   import_method_ = ASSET_IMPORT_APPEND_REUSE;
 }
 
+std::optional<AssetLibraryReference> EssentialsAssetLibrary::library_reference() const
+{
+  AssetLibraryReference library_ref{};
+  library_ref.custom_library_index = -1;
+  library_ref.type = ASSET_LIBRARY_ESSENTIALS;
+  return library_ref;
+}
+
 StringRefNull essentials_directory_path()
 {
   static std::string path = []() {

source/blender/asset_system/intern/library_types/essentials_library.hh

@@ -15,6 +15,8 @@ namespace blender::asset_system {
 class EssentialsAssetLibrary : public OnDiskAssetLibrary {
  public:
   EssentialsAssetLibrary();
+
+  std::optional<AssetLibraryReference> library_reference() const override;
 };
 
 }  // namespace blender::asset_system

source/blender/asset_system/intern/library_types/on_disk_library.cc

@@ -18,6 +18,14 @@ OnDiskAssetLibrary::OnDiskAssetLibrary(eAssetLibraryType library_type,
   this->on_blend_save_handler_register();
 }
 
+std::optional<AssetLibraryReference> OnDiskAssetLibrary::library_reference() const
+{
+  BLI_assert_msg(false,
+                 "Library references are only available for built-in libraries and libraries "
+                 "configured in the Preferences");
+  return {};
+}
+
 void OnDiskAssetLibrary::refresh_catalogs()
 {
   this->catalog_service().reload_catalogs();

source/blender/asset_system/intern/library_types/on_disk_library.hh

@@ -18,6 +18,7 @@ class OnDiskAssetLibrary : public AssetLibrary {
                      StringRef name = "",
                      StringRef root_path = "");
 
+  std::optional<AssetLibraryReference> library_reference() const override;
   void refresh_catalogs() override;
 };
 

source/blender/asset_system/intern/library_types/preferences_on_disk_library.cc

@@ -6,6 +6,12 @@
  * \ingroup asset_system
  */
 
+#include "BLI_fileops.h"
+#include "BLI_listbase.h"
+#include "BLI_path_utils.hh"
+
+#include "DNA_userdef_types.h"
+
 #include "preferences_on_disk_library.hh"
 
 namespace blender::asset_system {
@@ -15,4 +21,23 @@ PreferencesOnDiskAssetLibrary::PreferencesOnDiskAssetLibrary(StringRef name, Str
 {
 }
 
+std::optional<AssetLibraryReference> PreferencesOnDiskAssetLibrary::library_reference() const
+{
+  int i;
+  LISTBASE_FOREACH_INDEX (const bUserAssetLibrary *, asset_library, &U.asset_libraries, i) {
+    if (!BLI_is_dir(asset_library->dirpath)) {
+      continue;
+    }
+
+    if (BLI_path_cmp_normalized(asset_library->dirpath, this->root_path().c_str()) == 0) {
+      AssetLibraryReference library_ref{};
+      library_ref.type = ASSET_LIBRARY_CUSTOM;
+      library_ref.custom_library_index = i;
+      return library_ref;
+    }
+  }
+
+  return {};
+}
+
 }  // namespace blender::asset_system

source/blender/asset_system/intern/library_types/preferences_on_disk_library.hh

@@ -15,6 +15,8 @@ namespace blender::asset_system {
 class PreferencesOnDiskAssetLibrary : public OnDiskAssetLibrary {
  public:
   PreferencesOnDiskAssetLibrary(StringRef name = "", StringRef root_path = "");
+
+  std::optional<AssetLibraryReference> library_reference() const override;
 };
 
 }  // namespace blender::asset_system

source/blender/asset_system/intern/library_types/runtime_library.cc

@@ -15,4 +15,12 @@ RuntimeAssetLibrary::RuntimeAssetLibrary() : AssetLibrary(ASSET_LIBRARY_LOCAL)
   this->on_blend_save_handler_register();
 }
 
+std::optional<AssetLibraryReference> RuntimeAssetLibrary::library_reference() const
+{
+  AssetLibraryReference library_ref{};
+  library_ref.type = ASSET_LIBRARY_LOCAL;
+  library_ref.custom_library_index = -1;
+  return library_ref;
+}
+
 }  // namespace blender::asset_system

source/blender/asset_system/intern/library_types/runtime_library.hh

@@ -18,6 +18,8 @@ namespace blender::asset_system {
 class RuntimeAssetLibrary : public AssetLibrary {
  public:
   RuntimeAssetLibrary();
+
+  std::optional<AssetLibraryReference> library_reference() const override;
 };
 
 }  // namespace blender::asset_system

source/blender/editors/animation/anim_asset_ops.cc

@@ -627,15 +627,6 @@ static void update_pose_action_from_scene(Main *bmain,
   }
 }
 
-static void refresh_asset_library(bContext *C)
-{
-  const blender::asset_system::AssetRepresentation *asset = CTX_wm_asset(C);
-  AssetWeakReference asset_reference = asset->make_weak_reference();
-  bUserAssetLibrary *library = BKE_preferences_asset_library_find_by_name(
-      &U, asset_reference.asset_library_identifier);
-  asset::refresh_asset_library(C, *library);
-}
-
 static int pose_asset_modify_exec(bContext *C, wmOperator *op)
 {
   bAction *action = get_action_of_selected_asset(C);
@@ -657,8 +648,7 @@ static int pose_asset_modify_exec(bContext *C, wmOperator *op)
     bke::asset_edit_id_save(*bmain, action->id, *op->reports);
   }
 
-  refresh_asset_library(C);
-
+  asset::refresh_asset_library_from_asset(C, *CTX_wm_asset(C));
   WM_main_add_notifier(NC_ASSET | ND_ASSET_LIST | NA_EDITED, nullptr);
 
   return OPERATOR_FINISHED;
@@ -755,11 +745,6 @@ static int pose_asset_delete_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
-  const blender::asset_system::AssetRepresentation *asset = CTX_wm_asset(C);
-  AssetWeakReference asset_reference = asset->make_weak_reference();
-  bUserAssetLibrary *library = BKE_preferences_asset_library_find_by_name(
-      &U, asset_reference.asset_library_identifier);
-
   if (ID_IS_LINKED(action)) {
     bke::asset_edit_id_delete(*CTX_data_main(C), action->id, *op->reports);
   }
@@ -767,7 +752,8 @@ static int pose_asset_delete_exec(bContext *C, wmOperator *op)
     asset::clear_id(&action->id);
   }
 
-  asset::refresh_asset_library(C, *library);
+  const blender::asset_system::AssetRepresentation *asset = CTX_wm_asset(C);
+  asset::refresh_asset_library_from_asset(C, *asset);
   WM_main_add_notifier(NC_ASSET | ND_ASSET_LIST | NA_REMOVED, nullptr);
 
   return OPERATOR_FINISHED;

source/blender/editors/asset/ED_asset_catalog.hh

@@ -23,6 +23,7 @@
 
 #include "BLI_string_ref.hh"
 
+struct AssetWeakReference;
 struct Main;
 
 namespace blender::asset_system {
@@ -32,6 +33,8 @@ class AssetLibrary;
 namespace blender::ed::asset {
 
 void catalogs_save_from_main_path(asset_system::AssetLibrary *library, const Main *bmain);
+void catalogs_save_from_asset_reference(asset_system::AssetLibrary &library,
+                                        const AssetWeakReference &reference);
 
 /**
  * Saving catalog edits when the file is saved is a global option shared for each asset library,

source/blender/editors/asset/ED_asset_library.hh

@@ -21,6 +21,7 @@ struct StringPropertySearchVisitParams;
 namespace blender::asset_system {
 class AssetCatalog;
 class AssetCatalogPath;
+class AssetRepresentation;
 }  // namespace blender::asset_system
 
 namespace blender::ed::asset {
@@ -61,10 +62,6 @@ blender::asset_system::AssetCatalog &library_ensure_catalogs_in_path(
     blender::asset_system::AssetLibrary &library,
     const blender::asset_system::AssetCatalogPath &path);
 
-/**
- * May return a nullptr if the given AssetLibraryReference is not a user library.
- */
-const bUserAssetLibrary *library_ref_to_user_library(const AssetLibraryReference &library_ref);
 AssetLibraryReference user_library_to_library_ref(const bUserAssetLibrary &user_library);
 
 /**
@@ -72,5 +69,7 @@ AssetLibraryReference user_library_to_library_ref(const bUserAssetLibrary &user_
  */
 void refresh_asset_library(const bContext *C, const AssetLibraryReference &library_ref);
 void refresh_asset_library(const bContext *C, const bUserAssetLibrary &user_library);
+void refresh_asset_library_from_asset(const bContext *C,
+                                      const blender::asset_system::AssetRepresentation &asset);
 
 }  // namespace blender::ed::asset

source/blender/editors/asset/intern/asset_catalog.cc

@@ -170,6 +170,28 @@ void catalogs_save_from_main_path(AssetLibrary *library, const Main *bmain)
   catalog_service.write_to_disk(bmain->filepath);
 }
 
+void catalogs_save_from_asset_reference(AssetLibrary &library, const AssetWeakReference &reference)
+{
+  asset_system::AssetCatalogService &catalog_service = library.catalog_service();
+  if (catalog_service.is_read_only()) {
+    return;
+  }
+
+  char asset_full_path_buffer[1024 + MAX_ID_NAME /*FILE_MAX_LIBEXTRA*/];
+  char *file_path = nullptr;
+  AS_asset_full_path_explode_from_weak_ref(
+      &reference, asset_full_path_buffer, &file_path, nullptr, nullptr);
+  if (!file_path) {
+    BLI_assert_unreachable();
+    return;
+  }
+
+  /* Since writing to disk also means loading any on-disk changes, it may be a good idea to store
+   * an undo step. */
+  catalog_service.undo_push();
+  catalog_service.write_to_disk(file_path);
+}
+
 void catalogs_set_save_catalogs_when_file_is_saved(const bool should_save)
 {
   asset_system::AssetLibrary::save_catalogs_when_file_is_saved = should_save;

source/blender/editors/asset/intern/asset_library_utils.cc

@@ -6,6 +6,8 @@
  * \ingroup edasset
  */
 
+#include "AS_asset_representation.hh"
+
 #include "BKE_context.hh"
 
 #include "ED_asset_library.hh"
@@ -65,15 +67,6 @@ AssetLibraryReference user_library_to_library_ref(const bUserAssetLibrary &user_
   return library_ref;
 }
 
-const bUserAssetLibrary *library_ref_to_user_library(const AssetLibraryReference &library_ref)
-{
-  if (library_ref.type != ASSET_LIBRARY_CUSTOM) {
-    return nullptr;
-  }
-  return static_cast<const bUserAssetLibrary *>(
-      BLI_findlink(&U.asset_libraries, library_ref.custom_library_index));
-}
-
 void refresh_asset_library(const bContext *C, const AssetLibraryReference &library_ref)
 {
   asset::list::clear(&library_ref, C);
@@ -87,4 +80,14 @@ void refresh_asset_library(const bContext *C, const bUserAssetLibrary &user_libr
   refresh_asset_library(C, user_library_to_library_ref(user_library));
 }
 
+void refresh_asset_library_from_asset(const bContext *C,
+                                      const asset_system::AssetRepresentation &asset)
+{
+  if (std::optional<AssetLibraryReference> library_ref =
+          asset.owner_asset_library().library_reference())
+  {
+    refresh_asset_library(C, *library_ref);
+  }
+}
+
 }  // namespace blender::ed::asset

source/blender/editors/asset/intern/asset_ui_utils.cc

@@ -69,6 +69,12 @@ const bUserAssetLibrary *get_asset_library_from_opptr(PointerRNA &ptr)
   return BKE_preferences_asset_library_find_index(&U, lib_ref.custom_library_index);
 }
 
+AssetLibraryReference get_asset_library_ref_from_opptr(PointerRNA &ptr)
+{
+  const int enum_value = RNA_enum_get(&ptr, "asset_library_reference");
+  return asset::library_reference_from_enum_value(enum_value);
+}
+
 void visit_library_catalogs_catalog_for_search(
     const Main &bmain,
     const AssetLibraryReference lib,

source/blender/editors/include/ED_asset.hh

@@ -44,6 +44,10 @@ void operatortypes_asset();
  * The PointerRNA is expected to have an enum called "asset_library_reference".
  */
 const bUserAssetLibrary *get_asset_library_from_opptr(PointerRNA &ptr);
+/**
+ * The PointerRNA is expected to have an enum called "asset_library_reference".
+ */
+AssetLibraryReference get_asset_library_ref_from_opptr(PointerRNA &ptr);
 
 /**
  * For each catalog of the given bUserAssetLibrary call `visit_fn`.

source/blender/editors/sculpt_paint/brush_asset_ops.cc

@@ -90,20 +90,6 @@ void BRUSH_OT_asset_activate(wmOperatorType *ot)
   asset::operator_asset_reference_props_register(*ot->srna);
 }
 
-static std::optional<AssetLibraryReference> library_to_library_ref(
-    const asset_system::AssetLibrary &library)
-{
-  for (const AssetLibraryReference &ref : asset_system::all_valid_asset_library_refs()) {
-    const std::string root_path = AS_asset_library_root_path_from_library_ref(ref);
-    /* Use #BLI_path_cmp_normalized because `library.root_path()` ends with a slash while
-     * `root_path` doesn't. */
-    if (BLI_path_cmp_normalized(root_path.c_str(), library.root_path().c_str()) == 0) {
-      return ref;
-    }
-  }
-  return std::nullopt;
-}
-
 static bool brush_asset_save_as_poll(bContext *C)
 {
   Paint *paint = BKE_paint_get_active_from_context(C);
@@ -215,7 +201,7 @@ static int brush_asset_save_as_invoke(bContext *C, wmOperator *op, const wmEvent
     return OPERATOR_CANCELLED;
   }
   const asset_system::AssetLibrary &library = asset->owner_asset_library();
-  const std::optional<AssetLibraryReference> library_ref = library_to_library_ref(library);
+  const std::optional<AssetLibraryReference> library_ref = library.library_reference();
   if (!library_ref) {
     BLI_assert_unreachable();
     return OPERATOR_CANCELLED;
@@ -278,10 +264,8 @@ static void visit_library_prop_catalogs_catalog_for_search_fn(
     FunctionRef<void(StringPropertySearchVisitParams)> visit_fn)
 {
   /* NOTE: Using the all library would also be a valid choice. */
-  if (const bUserAssetLibrary *user_library = asset::get_asset_library_from_opptr(*ptr)) {
-    asset::visit_library_catalogs_catalog_for_search(
-        *CTX_data_main(C), asset::user_library_to_library_ref(*user_library), edit_text, visit_fn);
-  }
+  asset::visit_library_catalogs_catalog_for_search(
+      *CTX_data_main(C), asset::get_asset_library_ref_from_opptr(*ptr), edit_text, visit_fn);
 }
 
 void BRUSH_OT_asset_save_as(wmOperatorType *ot)
@@ -321,9 +305,7 @@ static int brush_asset_edit_metadata_exec(bContext *C, wmOperator *op)
   if (!asset) {
     return OPERATOR_CANCELLED;
   }
-  const asset_system::AssetLibrary &library_const = asset->owner_asset_library();
-  const AssetLibraryReference library_ref = *library_to_library_ref(library_const);
-  asset_system::AssetLibrary *library = AS_asset_library_load(bmain, library_ref);
+  asset_system::AssetLibrary &library = asset->owner_asset_library();
 
   char catalog_path[MAX_NAME];
   RNA_string_get(op->ptr, "catalog_path", catalog_path);
@@ -336,7 +318,7 @@ static int brush_asset_edit_metadata_exec(bContext *C, wmOperator *op)
 
   if (catalog_path[0]) {
     const asset_system::AssetCatalog &catalog = asset::library_ensure_catalogs_in_path(
-        *library, catalog_path);
+        library, catalog_path);
     BKE_asset_metadata_catalog_id_set(&meta_data, catalog.catalog_id, catalog.simple_name.c_str());
   }
 
@@ -344,18 +326,9 @@ static int brush_asset_edit_metadata_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
-  char asset_full_path_buffer[FILE_MAX_LIBEXTRA];
-  char *file_path = nullptr;
-  AS_asset_full_path_explode_from_weak_ref(
-      &brush_weak_ref, asset_full_path_buffer, &file_path, nullptr, nullptr);
-  if (!file_path) {
-    BLI_assert_unreachable();
-    return OPERATOR_CANCELLED;
-  }
+  asset::catalogs_save_from_asset_reference(library, brush_weak_ref);
 
-  library->catalog_service().write_to_disk(file_path);
-
-  asset::refresh_asset_library(C, library_ref);
+  asset::refresh_asset_library_from_asset(C, *asset);
   WM_main_add_notifier(NC_ASSET | ND_ASSET_LIST | NA_EDITED, nullptr);
 
   return OPERATOR_FINISHED;
@@ -403,11 +376,12 @@ static void visit_active_library_catalogs_catalog_for_search_fn(
   if (!asset) {
     return;
   }
+
   const asset_system::AssetLibrary &library = asset->owner_asset_library();
 
   /* NOTE: Using the all library would also be a valid choice. */
   asset::visit_library_catalogs_catalog_for_search(
-      *CTX_data_main(C), *library_to_library_ref(library), edit_text, visit_fn);
+      *CTX_data_main(C), *library.library_reference(), edit_text, visit_fn);
 }
 
 static bool brush_asset_edit_metadata_poll(bContext *C)
@@ -432,8 +406,8 @@ static bool brush_asset_edit_metadata_poll(bContext *C)
     /* May happen if library loading hasn't finished. */
     return false;
   }
-  const std::optional<AssetLibraryReference> library_ref = library_to_library_ref(
-      asset->owner_asset_library());
+  const std::optional<AssetLibraryReference> library_ref =
+      asset->owner_asset_library().library_reference();
   if (!library_ref) {
     BLI_assert_unreachable();
     return false;
@@ -479,7 +453,6 @@ static int brush_asset_load_preview_exec(bContext *C, wmOperator *op)
   if (!asset) {
     return OPERATOR_CANCELLED;
   }
-  const AssetLibraryReference library_ref = *library_to_library_ref(asset->owner_asset_library());
 
   char filepath[FILE_MAX];
   RNA_string_get(op->ptr, "filepath", filepath);
@@ -494,7 +467,7 @@ static int brush_asset_load_preview_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
-  asset::refresh_asset_library(C, library_ref);
+  asset::refresh_asset_library_from_asset(C, *asset);
   WM_main_add_notifier(NC_ASSET | ND_ASSET_LIST | NA_EDITED, nullptr);
 
   return OPERATOR_FINISHED;