Refactor: Lazy create & update asset catalog trees

Rebuilding the tree immediately after changes could cause the tree to be
rebuilt multiple times. More importantly, it made it harder to reason
about thread safety, since we would touch the tree within a whole bunch
of API functions. Now tree building is simplified and managed in a
single place, so making the tree building thread safe can be made
trivially in a follow-up.

Note, this means the initial catalog tree building doesn't happen in a
background thread together with loading the asset library and catalogs
anymore. But we would already do all further rebuilds on the main thread
anyway, this shouldn't have any notable impact.

source/blender/asset_system/AS_asset_catalog.hh

@@ -41,7 +41,11 @@ using OwningAssetCatalogMap = Map<CatalogID, std::unique_ptr<AssetCatalog>>;
  * directory hierarchy). */
 class AssetCatalogService {
   std::unique_ptr<AssetCatalogCollection> catalog_collection_;
-  std::unique_ptr<AssetCatalogTree> catalog_tree_;
+  /**
+   * Cached catalog tree storage. Lazy-created by #AssetCatalogService::catalog_tree() (hence
+   * mutable).
+   */
+  mutable std::unique_ptr<AssetCatalogTree> catalog_tree_;
   CatalogFilePath asset_library_root_;
 
   Vector<std::unique_ptr<AssetCatalogCollection>> undo_snapshots_;
@@ -130,15 +134,6 @@ class AssetCatalogService {
    */
   void reload_catalogs();
 
-  /**
-   * Make sure the tree is updated to the latest collection of catalogs stored in this service.
-   * Does not depend on a CDF file being available so this can be called on a service that stores
-   * catalogs that are not stored in a CDF.
-   * Most API functions that modify catalog data will trigger this, unless otherwise specified (for
-   * batch operations).
-   */
-  void rebuild_tree();
-
   /** Return catalog with the given ID. Return nullptr if not found. */
   AssetCatalog *find_catalog(CatalogID catalog_id) const;
 
@@ -183,7 +178,7 @@ class AssetCatalogService {
    */
   void update_catalog_path(CatalogID catalog_id, const AssetCatalogPath &new_catalog_path);
 
-  const AssetCatalogTree *get_catalog_tree() const;
+  const AssetCatalogTree &catalog_tree() const;
 
   /** Return true only if there are no catalogs known. */
   bool is_empty() const;
@@ -256,6 +251,9 @@ class AssetCatalogService {
       const CatalogFilePath &blend_file_path);
 
   std::unique_ptr<AssetCatalogTree> read_into_tree() const;
+  /** Ensure a #catalog_tree() will update the tree. Must be called whenever the contained user
+   * visible catalogs change. */
+  void invalidate_catalog_tree();
 
   /**
    * For every catalog, ensure that its parent path also has a known catalog.

source/blender/asset_system/intern/asset_catalog.cc

@@ -43,8 +43,7 @@ const std::string AssetCatalogDefinitionFile::HEADER =
     "# Other lines are of the format \"UUID:catalog/path/for/assets:simple catalog name\"\n";
 
 AssetCatalogService::AssetCatalogService()
-    : catalog_collection_(std::make_unique<AssetCatalogCollection>()),
-      catalog_tree_(std::make_unique<AssetCatalogTree>())
+    : catalog_collection_(std::make_unique<AssetCatalogCollection>())
 {
 }
 
@@ -237,7 +236,7 @@ void AssetCatalogService::prune_catalogs_by_path(const AssetCatalogPath &path)
     this->delete_catalog_by_id_soft(cat_id);
   }
 
-  this->rebuild_tree();
+  this->invalidate_catalog_tree();
   AssetLibraryService::get()->tag_all_library_catalogs_dirty();
 }
 
@@ -272,7 +271,8 @@ void AssetCatalogService::update_catalog_path(const CatalogID catalog_id,
      * blend file, and update the catalog simple name stored there. */
   }
 
-  this->rebuild_tree();
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
   AssetLibraryService::get()->tag_all_library_catalogs_dirty();
 }
 
@@ -297,8 +297,8 @@ AssetCatalog *AssetCatalogService::create_catalog(const AssetCatalogPath &catalo
     catalog_collection_->catalog_definition_file_->add_new(catalog_ptr);
   }
 
-  BLI_assert_msg(catalog_tree_, "An Asset Catalog tree should always exist.");
-  catalog_tree_->insert_item(*catalog_ptr);
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
   AssetLibraryService::get()->tag_all_library_catalogs_dirty();
 
   return catalog_ptr;
@@ -340,7 +340,8 @@ void AssetCatalogService::load_from_disk(const CatalogFilePath &file_or_director
 
   /* TODO: Should there be a sanitize step? E.g. to remove catalogs with identical paths? */
 
-  this->rebuild_tree();
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
 }
 
 void AssetCatalogService::add_from_existing(
@@ -439,7 +440,8 @@ void AssetCatalogService::reload_catalogs()
 
   cdf->parse_catalog_file(cdf->file_path, catalog_parsed_callback);
   this->purge_catalogs_not_listed(cats_in_file);
-  this->rebuild_tree();
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
 }
 
 void AssetCatalogService::purge_catalogs_not_listed(const Set<CatalogID> &catalogs_to_keep)
@@ -488,7 +490,7 @@ bool AssetCatalogService::write_to_disk(const CatalogFilePath &blend_file_path)
   }
 
   this->untag_has_unsaved_changes();
-  this->rebuild_tree();
+  this->invalidate_catalog_tree();
   return true;
 }
 
@@ -574,11 +576,6 @@ std::unique_ptr<AssetCatalogDefinitionFile> AssetCatalogService::construct_cdf_i
   return cdf;
 }
 
-const AssetCatalogTree *AssetCatalogService::get_catalog_tree() const
-{
-  return catalog_tree_.get();
-}
-
 std::unique_ptr<AssetCatalogTree> AssetCatalogService::read_into_tree() const
 {
   auto tree = std::make_unique<AssetCatalogTree>();
@@ -591,10 +588,17 @@ std::unique_ptr<AssetCatalogTree> AssetCatalogService::read_into_tree() const
   return tree;
 }
 
-void AssetCatalogService::rebuild_tree()
+void AssetCatalogService::invalidate_catalog_tree()
 {
-  this->create_missing_catalogs();
-  this->catalog_tree_ = this->read_into_tree();
+  this->catalog_tree_ = nullptr;
+}
+
+const AssetCatalogTree &AssetCatalogService::catalog_tree() const
+{
+  if (!catalog_tree_) {
+    catalog_tree_ = read_into_tree();
+  }
+  return *catalog_tree_;
 }
 
 void AssetCatalogService::create_missing_catalogs()
@@ -653,7 +657,8 @@ void AssetCatalogService::undo()
 
   redo_snapshots_.append(std::move(catalog_collection_));
   catalog_collection_ = undo_snapshots_.pop_last();
-  this->rebuild_tree();
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
   AssetLibraryService::get()->tag_all_library_catalogs_dirty();
 }
 
@@ -664,7 +669,8 @@ void AssetCatalogService::redo()
 
   undo_snapshots_.append(std::move(catalog_collection_));
   catalog_collection_ = redo_snapshots_.pop_last();
-  this->rebuild_tree();
+  this->create_missing_catalogs();
+  this->invalidate_catalog_tree();
   AssetLibraryService::get()->tag_all_library_catalogs_dirty();
 }
 

source/blender/asset_system/intern/asset_library_all.cc

@@ -56,8 +56,6 @@ void AllAssetLibrary::rebuild_catalogs_from_nested(const bool reload_nested_cata
       },
       false);
 
-  new_catalog_service->rebuild_tree();
-
   this->catalog_service = std::move(new_catalog_service);
   catalogs_dirty_ = false;
 }

source/blender/asset_system/tests/asset_catalog_test.cc

@@ -566,8 +566,8 @@ TEST_F(AssetCatalogTest, delete_catalog_leaf)
       "path/without/simplename",
   };
 
-  const AssetCatalogTree *tree = service.get_catalog_tree();
-  AssetCatalogTreeTestFunctions::expect_tree_items(*tree, expected_paths);
+  const AssetCatalogTree &tree = service.catalog_tree();
+  AssetCatalogTreeTestFunctions::expect_tree_items(tree, expected_paths);
 }
 
 TEST_F(AssetCatalogTest, delete_catalog_parent_by_id)
@@ -620,8 +620,8 @@ TEST_F(AssetCatalogTest, delete_catalog_parent_by_path)
       "path/without/simplename",
   };
 
-  const AssetCatalogTree *tree = service.get_catalog_tree();
-  AssetCatalogTreeTestFunctions::expect_tree_items(*tree, expected_paths);
+  const AssetCatalogTree &tree = service.catalog_tree();
+  AssetCatalogTreeTestFunctions::expect_tree_items(tree, expected_paths);
 }
 
 TEST_F(AssetCatalogTest, delete_catalog_write_to_disk)

source/blender/asset_system/tests/asset_catalog_tree_test.cc

@@ -117,8 +117,8 @@ TEST_F(AssetCatalogTreeTest, load_single_file_into_tree)
       "path/without/simplename", /* From CDF. */
   };
 
-  const AssetCatalogTree *tree = service.get_catalog_tree();
-  expect_tree_items(*tree, expected_paths);
+  const AssetCatalogTree &tree = service.catalog_tree();
+  expect_tree_items(tree, expected_paths);
 }
 
 TEST_F(AssetCatalogTreeTest, foreach_in_tree)
@@ -141,8 +141,8 @@ TEST_F(AssetCatalogTreeTest, foreach_in_tree)
   service.load_from_disk(asset_library_root_ + SEP_STR + "blender_assets.cats.txt");
 
   std::vector<AssetCatalogPath> expected_root_items{{"character", "path"}};
-  const AssetCatalogTree *tree = service.get_catalog_tree();
-  expect_tree_root_items(*tree, expected_root_items);
+  const AssetCatalogTree &tree = service.catalog_tree();
+  expect_tree_root_items(tree, expected_root_items);
 
   /* Test if the direct children of the root item are what's expected. */
   std::vector<std::vector<AssetCatalogPath>> expected_root_child_items = {
@@ -152,7 +152,7 @@ TEST_F(AssetCatalogTreeTest, foreach_in_tree)
       {"path/without"},
   };
   int i = 0;
-  tree->foreach_root_item([&expected_root_child_items, &i](const AssetCatalogTreeItem &item) {
+  tree.foreach_root_item([&expected_root_child_items, &i](const AssetCatalogTreeItem &item) {
     expect_tree_item_child_items(item, expected_root_child_items[i]);
     i++;
   });

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

@@ -75,7 +75,7 @@ asset_system::AssetCatalogTree build_filtered_catalog_tree(
 
   /* Build catalog tree. */
   asset_system::AssetCatalogTree filtered_tree;
-  const asset_system::AssetCatalogTree &full_tree = *library.catalog_service->get_catalog_tree();
+  const asset_system::AssetCatalogTree &full_tree = library.catalog_service->catalog_tree();
   full_tree.foreach_item([&](const asset_system::AssetCatalogTreeItem &item) {
     if (!known_paths.contains(item.catalog_path().str())) {
       return;
@@ -135,8 +135,7 @@ AssetItemTree build_filtered_all_catalog_tree(
   });
 
   asset_system::AssetCatalogTree catalogs_with_node_assets;
-  const asset_system::AssetCatalogTree &catalog_tree =
-      *library->catalog_service->get_catalog_tree();
+  const asset_system::AssetCatalogTree &catalog_tree = library->catalog_service->catalog_tree();
   catalog_tree.foreach_item([&](const asset_system::AssetCatalogTreeItem &item) {
     if (assets_per_path.lookup(item.catalog_path()).is_empty()) {
       return;

source/blender/editors/space_file/asset_catalog_tree_view.cc

@@ -184,7 +184,7 @@ AssetCatalogTreeView::AssetCatalogTreeView(asset_system::AssetLibrary *library,
     : asset_library_(library), params_(params), space_file_(space_file)
 {
   if (library && library->catalog_service) {
-    catalog_tree_ = library->catalog_service->get_catalog_tree();
+    catalog_tree_ = &library->catalog_service->catalog_tree();
   }
   else {
     catalog_tree_ = nullptr;