diff options
author | Julian Eisel <julian@blender.org> | 2021-09-27 18:45:02 +0300 |
---|---|---|
committer | Julian Eisel <julian@blender.org> | 2021-09-27 18:49:31 +0300 |
commit | 824733ea47d13030c18faa618c1dc31a08dfa43d (patch) | |
tree | 4d2c9ee4da79e51d5f02bfd9091c9d5bcee71753 /source/blender | |
parent | 5bea5e25d52fe26b53928781ec0b1f5d4ddf5ad0 (diff) |
Assets: Additions/fixes to the catalog system in preparation for the UI
* Fixes missing update of the catalog tree when adding catalogs.
* Adds iterators for the catalogs, needed for UI code.
* Store catalog ID in the catalog tree items, needed for UI code.
* Other smaller API additions for the UI.
* Improve comments and smaller cleanups.
New functions are covered with unit tests.
Differential Revision: https://developer.blender.org/D12618
Reviewed by: Sybren Stüvel
Diffstat (limited to 'source/blender')
-rw-r--r-- | source/blender/blenkernel/BKE_asset_catalog.hh | 46 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog.cc | 147 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/asset_catalog_test.cc | 200 |
3 files changed, 343 insertions, 50 deletions
diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh index cf9d581b148..07373caf701 100644 --- a/source/blender/blenkernel/BKE_asset_catalog.hh +++ b/source/blender/blenkernel/BKE_asset_catalog.hh @@ -86,6 +86,10 @@ class AssetCatalogService { /** Return catalog with the given ID. Return nullptr if not found. */ AssetCatalog *find_catalog(CatalogID catalog_id); + /** Return first catalog with the given path. Return nullptr if not found. This is not an + * efficient call as it's just a linear search over the catalogs. */ + AssetCatalog *find_catalog_by_path(const CatalogPath &path) const; + /** Create a catalog with some sensible auto-generated catalog ID. * The catalog will be saved to the default catalog file.*/ AssetCatalog *create_catalog(const CatalogPath &catalog_path); @@ -124,48 +128,74 @@ class AssetCatalogService { void rebuild_tree(); }; +/** + * Representation of a catalog path in the #AssetCatalogTree. + */ class AssetCatalogTreeItem { - friend class AssetCatalogService; + friend class AssetCatalogTree; public: + /** Container for child items. Uses a #std::map to keep items ordered by their name (i.e. their + * last catalog component). */ using ChildMap = std::map<std::string, AssetCatalogTreeItem>; - using ItemIterFn = FunctionRef<void(const AssetCatalogTreeItem &)>; + using ItemIterFn = FunctionRef<void(AssetCatalogTreeItem &)>; - AssetCatalogTreeItem(StringRef name, const AssetCatalogTreeItem *parent = nullptr); + AssetCatalogTreeItem(StringRef name, + CatalogID catalog_id, + const AssetCatalogTreeItem *parent = nullptr); + CatalogID get_catalog_id() const; StringRef get_name() const; /** Return the full catalog path, defined as the name of this catalog prefixed by the full * catalog path of its parent and a separator. */ CatalogPath catalog_path() const; int count_parents() const; + bool has_children() const; - static void foreach_item_recursive(const ChildMap &children_, const ItemIterFn callback); + /** Iterate over children calling \a callback for each of them, but do not recurse into their + * children. */ + void foreach_child(const ItemIterFn callback); protected: /** Child tree items, ordered by their names. */ ChildMap children_; /** The user visible name of this component. */ CatalogPathComponent name_; + CatalogID catalog_id_; /** Pointer back to the parent item. Used to reconstruct the hierarchy from an item (e.g. to * build a path). */ const AssetCatalogTreeItem *parent_ = nullptr; + + private: + static void foreach_item_recursive(ChildMap &children_, ItemIterFn callback); }; /** * A representation of the catalog paths as tree structure. Each component of the catalog tree is - * represented by a #AssetCatalogTreeItem. + * represented by an #AssetCatalogTreeItem. The last path component of an item is used as its name, + * which may also be shown to the user. + * An item can not have multiple children with the same name. That means the name uniquely + * identifies an item within its parent. + * * There is no single root tree element, the #AssetCatalogTree instance itself represents the root. */ class AssetCatalogTree { - friend class AssetCatalogService; + using ChildMap = AssetCatalogTreeItem::ChildMap; + using ItemIterFn = AssetCatalogTreeItem::ItemIterFn; public: - void foreach_item(const AssetCatalogTreeItem::ItemIterFn callback) const; + /** Ensure an item representing \a path is in the tree, adding it if necessary. */ + void insert_item(const AssetCatalog &catalog); + + void foreach_item(const AssetCatalogTreeItem::ItemIterFn callback); + /** Iterate over root items calling \a callback for each of them, but do not recurse into their + * children. */ + void foreach_root_item(const ItemIterFn callback); protected: /** Child tree items, ordered by their names. */ - AssetCatalogTreeItem::ChildMap children_; + ChildMap root_items_; }; /** Keeps track of which catalogs are defined in a certain file on disk. diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc index 4f1de09e148..b65ae12e5a7 100644 --- a/source/blender/blenkernel/intern/asset_catalog.cc +++ b/source/blender/blenkernel/intern/asset_catalog.cc @@ -64,6 +64,17 @@ AssetCatalog *AssetCatalogService::find_catalog(CatalogID catalog_id) return catalog_uptr_ptr->get(); } +AssetCatalog *AssetCatalogService::find_catalog_by_path(const CatalogPath &path) const +{ + for (auto &catalog : catalogs_.values()) { + if (catalog->path == path) { + return catalog.get(); + } + } + + return nullptr; +} + void AssetCatalogService::delete_catalog(CatalogID catalog_id) { std::unique_ptr<AssetCatalog> *catalog_uptr_ptr = this->catalogs_.lookup_ptr(catalog_id); @@ -104,6 +115,12 @@ AssetCatalog *AssetCatalogService::create_catalog(const CatalogPath &catalog_pat catalog_definition_file_->add_new(catalog_ptr); } + /* The tree may not exist; this happens when no catalog definition file has been loaded yet. When + * the tree is created any in-memory catalogs will be added, so it doesn't need to happen now. */ + if (catalog_tree_) { + catalog_tree_->insert_item(*catalog_ptr); + } + return catalog_ptr; } @@ -268,34 +285,7 @@ std::unique_ptr<AssetCatalogTree> AssetCatalogService::read_into_tree() /* Go through the catalogs, insert each path component into the tree where needed. */ for (auto &catalog : catalogs_.values()) { - const AssetCatalogTreeItem *parent = nullptr; - AssetCatalogTreeItem::ChildMap *insert_to_map = &tree->children_; - - BLI_assert_msg(!ELEM(catalog->path[0], '/', '\\'), - "Malformed catalog path; should not start with a separator"); - - const char *next_slash_ptr; - /* Looks more complicated than it is, this just iterates over path components. E.g. - * "just/some/path" iterates over "just", then "some" then "path". */ - for (const char *name_begin = catalog->path.data(); name_begin && name_begin[0]; - /* Jump to one after the next slash if there is any. */ - name_begin = next_slash_ptr ? next_slash_ptr + 1 : nullptr) { - next_slash_ptr = BLI_path_slash_find(name_begin); - - /* Note that this won't be null terminated. */ - StringRef component_name = next_slash_ptr ? - StringRef(name_begin, next_slash_ptr - name_begin) : - /* Last component in the path. */ - name_begin; - - /* Insert new tree element - if no matching one is there yet! */ - auto [item, was_inserted] = insert_to_map->emplace( - component_name, AssetCatalogTreeItem(component_name, parent)); - - /* Walk further into the path (no matter if a new item was created or not). */ - parent = &item->second; - insert_to_map = &item->second.children_; - } + tree->insert_item(*catalog); } return tree; @@ -306,9 +296,18 @@ void AssetCatalogService::rebuild_tree() this->catalog_tree_ = read_into_tree(); } -AssetCatalogTreeItem::AssetCatalogTreeItem(StringRef name, const AssetCatalogTreeItem *parent) - : name_(name), parent_(parent) +/* ---------------------------------------------------------------------- */ + +AssetCatalogTreeItem::AssetCatalogTreeItem(StringRef name, + CatalogID catalog_id, + const AssetCatalogTreeItem *parent) + : name_(name), catalog_id_(catalog_id), parent_(parent) +{ +} + +CatalogID AssetCatalogTreeItem::get_catalog_id() const { + return catalog_id_; } StringRef AssetCatalogTreeItem::get_name() const @@ -334,20 +333,100 @@ int AssetCatalogTreeItem::count_parents() const return i; } -void AssetCatalogTree::foreach_item(const AssetCatalogTreeItem::ItemIterFn callback) const +bool AssetCatalogTreeItem::has_children() const +{ + return !children_.empty(); +} + +/* ---------------------------------------------------------------------- */ + +/** + * Iterate over path components, calling \a callback for each component. E.g. "just/some/path" + * iterates over "just", then "some" then "path". + */ +static void iterate_over_catalog_path_components( + const CatalogPath &path, + FunctionRef<void(StringRef component_name, bool is_last_component)> callback) +{ + const char *next_slash_ptr; + + for (const char *path_component = path.data(); path_component && path_component[0]; + /* Jump to one after the next slash if there is any. */ + path_component = next_slash_ptr ? next_slash_ptr + 1 : nullptr) { + next_slash_ptr = BLI_path_slash_find(path_component); + + const bool is_last_component = next_slash_ptr == nullptr; + /* Note that this won't be null terminated. */ + const StringRef component_name = is_last_component ? + path_component : + StringRef(path_component, + next_slash_ptr - path_component); + + callback(component_name, is_last_component); + } +} + +void AssetCatalogTree::insert_item(const AssetCatalog &catalog) +{ + const AssetCatalogTreeItem *parent = nullptr; + /* The children for the currently iterated component, where the following component should be + * added to (if not there yet). */ + AssetCatalogTreeItem::ChildMap *current_item_children = &root_items_; + + BLI_assert_msg(!ELEM(catalog.path[0], '/', '\\'), + "Malformed catalog path; should not start with a separator"); + + const CatalogID nil_id{}; + + iterate_over_catalog_path_components( + catalog.path, [&](StringRef component_name, const bool is_last_component) { + /* Insert new tree element - if no matching one is there yet! */ + auto [key_and_item, was_inserted] = current_item_children->emplace( + component_name, + AssetCatalogTreeItem( + component_name, is_last_component ? catalog.catalog_id : nil_id, parent)); + AssetCatalogTreeItem &item = key_and_item->second; + + /* If full path of this catalog already exists as parent path of a previously read catalog, + * we can ensure this tree item's UUID is set here. */ + if (is_last_component && BLI_uuid_is_nil(item.catalog_id_)) { + item.catalog_id_ = catalog.catalog_id; + } + + /* Walk further into the path (no matter if a new item was created or not). */ + parent = &item; + current_item_children = &item.children_; + }); +} + +void AssetCatalogTree::foreach_item(AssetCatalogTreeItem::ItemIterFn callback) { - AssetCatalogTreeItem::foreach_item_recursive(children_, callback); + AssetCatalogTreeItem::foreach_item_recursive(root_items_, callback); } -void AssetCatalogTreeItem::foreach_item_recursive(const AssetCatalogTreeItem::ChildMap &children, +void AssetCatalogTreeItem::foreach_item_recursive(AssetCatalogTreeItem::ChildMap &children, const ItemIterFn callback) { - for (const auto &[key, item] : children) { + for (auto &[key, item] : children) { callback(item); foreach_item_recursive(item.children_, callback); } } +void AssetCatalogTree::foreach_root_item(const ItemIterFn callback) +{ + for (auto &[key, item] : root_items_) { + callback(item); + } +} + +void AssetCatalogTreeItem::foreach_child(const ItemIterFn callback) +{ + for (auto &[key, item] : children_) { + callback(item); + } +} + AssetCatalogTree *AssetCatalogService::get_catalog_tree() { return catalog_tree_.get(); diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc index b40aae5d64a..87ee61b015f 100644 --- a/source/blender/blenkernel/intern/asset_catalog_test.cc +++ b/source/blender/blenkernel/intern/asset_catalog_test.cc @@ -92,6 +92,22 @@ class AssetCatalogTest : public testing::Test { int parent_count; }; + void assert_expected_item(const CatalogPathInfo &expected_path, + const AssetCatalogTreeItem &actual_item) + { + char expected_filename[FILE_MAXFILE]; + /* Is the catalog name as expected? "character", "Ellie", ... */ + BLI_split_file_part(expected_path.name.data(), expected_filename, sizeof(expected_filename)); + EXPECT_EQ(expected_filename, actual_item.get_name()); + /* Does the computed number of parents match? */ + EXPECT_EQ(expected_path.parent_count, actual_item.count_parents()); + EXPECT_EQ(expected_path.name, actual_item.catalog_path()); + } + + /** + * Recursively iterate over all tree items using #AssetCatalogTree::foreach_item() and check if + * the items map exactly to \a expected_paths. + */ void assert_expected_tree_items(AssetCatalogTree *tree, const std::vector<CatalogPathInfo> &expected_paths) { @@ -99,16 +115,43 @@ class AssetCatalogTest : public testing::Test { tree->foreach_item([&](const AssetCatalogTreeItem &actual_item) { ASSERT_LT(i, expected_paths.size()) << "More catalogs in tree than expected; did not expect " << actual_item.catalog_path(); + assert_expected_item(expected_paths[i], actual_item); + i++; + }); + } - char expected_filename[FILE_MAXFILE]; - /* Is the catalog name as expected? "character", "Ellie", ... */ - BLI_split_file_part( - expected_paths[i].name.data(), expected_filename, sizeof(expected_filename)); - EXPECT_EQ(expected_filename, actual_item.get_name()); - /* Does the computed number of parents match? */ - EXPECT_EQ(expected_paths[i].parent_count, actual_item.count_parents()); - EXPECT_EQ(expected_paths[i].name, actual_item.catalog_path()); + /** + * Iterate over the root items of \a tree and check if the items map exactly to \a + * expected_paths. Similar to #assert_expected_tree_items() but calls + * #AssetCatalogTree::foreach_root_item() instead of #AssetCatalogTree::foreach_item(). + */ + void assert_expected_tree_root_items(AssetCatalogTree *tree, + const std::vector<CatalogPathInfo> &expected_paths) + { + int i = 0; + tree->foreach_root_item([&](const AssetCatalogTreeItem &actual_item) { + ASSERT_LT(i, expected_paths.size()) + << "More catalogs in tree root than expected; did not expect " + << actual_item.catalog_path(); + assert_expected_item(expected_paths[i], actual_item); + i++; + }); + } + /** + * Iterate over the child items of \a parent_item and check if the items map exactly to \a + * expected_paths. Similar to #assert_expected_tree_items() but calls + * #AssetCatalogTreeItem::foreach_child() instead of #AssetCatalogTree::foreach_item(). + */ + void assert_expected_tree_item_child_items(AssetCatalogTreeItem *parent_item, + const std::vector<CatalogPathInfo> &expected_paths) + { + int i = 0; + parent_item->foreach_child([&](const AssetCatalogTreeItem &actual_item) { + ASSERT_LT(i, expected_paths.size()) + << "More catalogs in tree item than expected; did not expect " + << actual_item.catalog_path(); + assert_expected_item(expected_paths[i], actual_item); i++; }); } @@ -156,6 +199,87 @@ TEST_F(AssetCatalogTest, load_single_file) EXPECT_EQ("POSES_RUŽENA", poses_ruzena->simple_name); } +TEST_F(AssetCatalogTest, insert_item_into_tree) +{ + { + AssetCatalogTree tree; + std::unique_ptr<AssetCatalog> catalog_empty_path = AssetCatalog::from_path(""); + tree.insert_item(*catalog_empty_path); + + assert_expected_tree_items(&tree, {}); + } + + { + AssetCatalogTree tree; + + std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path("item"); + tree.insert_item(*catalog); + assert_expected_tree_items(&tree, {{"item", 0}}); + + /* Insert child after parent already exists. */ + std::unique_ptr<AssetCatalog> child_catalog = AssetCatalog::from_path("item/child"); + tree.insert_item(*catalog); + assert_expected_tree_items(&tree, {{"item", 0}, {"item/child", 1}}); + + std::vector<CatalogPathInfo> expected_paths; + + /* Test inserting multi-component sub-path. */ + std::unique_ptr<AssetCatalog> grandgrandchild_catalog = AssetCatalog::from_path( + "item/child/grandchild/grandgrandchild"); + tree.insert_item(*catalog); + expected_paths = {{"item", 0}, + {"item/child", 1}, + {"item/child/grandchild", 2}, + {"item/child/grandchild/grandgrandchild", 3}}; + assert_expected_tree_items(&tree, expected_paths); + + std::unique_ptr<AssetCatalog> root_level_catalog = AssetCatalog::from_path("root level"); + tree.insert_item(*catalog); + expected_paths = {{"item", 0}, + {"item/child", 1}, + {"item/child/grandchild", 2}, + {"item/child/grandchild/grandgrandchild", 3}, + {"root level", 0}}; + assert_expected_tree_items(&tree, expected_paths); + } + + { + AssetCatalogTree tree; + + std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path("item/child"); + tree.insert_item(*catalog); + assert_expected_tree_items(&tree, {{"item", 0}, {"item/child", 1}}); + } + + { + AssetCatalogTree tree; + + std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path("white space"); + tree.insert_item(*catalog); + assert_expected_tree_items(&tree, {{"white space", 0}}); + } + + { + AssetCatalogTree tree; + + std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path("/item/white space"); + tree.insert_item(*catalog); + assert_expected_tree_items(&tree, {{"item", 0}, {"item/white space", 1}}); + } + + { + AssetCatalogTree tree; + + std::unique_ptr<AssetCatalog> catalog_unicode_path = AssetCatalog::from_path("Ružena"); + tree.insert_item(*catalog_unicode_path); + assert_expected_tree_items(&tree, {{"Ružena", 0}}); + + catalog_unicode_path = AssetCatalog::from_path("Ružena/Ružena"); + tree.insert_item(*catalog_unicode_path); + assert_expected_tree_items(&tree, {{"Ružena", 0}, {"Ružena/Ružena", 1}}); + } +} + TEST_F(AssetCatalogTest, load_single_file_into_tree) { AssetCatalogService service(asset_library_root_); @@ -182,6 +306,66 @@ TEST_F(AssetCatalogTest, load_single_file_into_tree) assert_expected_tree_items(tree, expected_paths); } +TEST_F(AssetCatalogTest, foreach_in_tree) +{ + { + AssetCatalogTree tree{}; + const std::vector<CatalogPathInfo> no_catalogs{}; + + assert_expected_tree_items(&tree, no_catalogs); + assert_expected_tree_root_items(&tree, no_catalogs); + /* Need a root item to check child items. */ + std::unique_ptr<AssetCatalog> catalog = AssetCatalog::from_path("something"); + tree.insert_item(*catalog); + tree.foreach_root_item([&no_catalogs, this](AssetCatalogTreeItem &item) { + assert_expected_tree_item_child_items(&item, no_catalogs); + }); + } + + AssetCatalogService service(asset_library_root_); + service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt"); + + std::vector<CatalogPathInfo> expected_root_items{{"character", 0}, {"path", 0}}; + AssetCatalogTree *tree = service.get_catalog_tree(); + assert_expected_tree_root_items(tree, expected_root_items); + + /* Test if the direct children of the root item are what's expected. */ + std::vector<std::vector<CatalogPathInfo>> expected_root_child_items = { + /* Children of the "character" root item. */ + {{"character/Ellie", 1}, {"character/Ružena", 1}}, + /* Children of the "path" root item. */ + {{"path/without", 1}}, + }; + int i = 0; + tree->foreach_root_item([&expected_root_child_items, &i, this](AssetCatalogTreeItem &item) { + assert_expected_tree_item_child_items(&item, expected_root_child_items[i]); + i++; + }); +} + +TEST_F(AssetCatalogTest, find_catalog_by_path) +{ + TestableAssetCatalogService service(asset_library_root_); + service.load_from_disk(asset_library_root_ + "/" + + AssetCatalogService::DEFAULT_CATALOG_FILENAME); + + AssetCatalog *catalog; + + EXPECT_EQ(nullptr, service.find_catalog_by_path("")); + catalog = service.find_catalog_by_path("character/Ellie/poselib/white space"); + EXPECT_NE(nullptr, catalog); + EXPECT_EQ(UUID_POSES_ELLIE_WHITESPACE, catalog->catalog_id); + catalog = service.find_catalog_by_path("character/Ružena/poselib"); + EXPECT_NE(nullptr, catalog); + EXPECT_EQ(UUID_POSES_RUZENA, catalog->catalog_id); + + /* "character/Ellie/poselib" is used by two catalogs. Check if it's using the first one. */ + catalog = service.find_catalog_by_path("character/Ellie/poselib"); + EXPECT_NE(nullptr, catalog); + EXPECT_EQ(UUID_POSES_ELLIE, catalog->catalog_id); + EXPECT_NE(UUID_POSES_ELLIE_TRAILING_SLASH, catalog->catalog_id); +} + TEST_F(AssetCatalogTest, write_single_file) { TestableAssetCatalogService service(asset_library_root_); |