[Bf-blender-cvs] [f413bc25f07] temp-asset-browser-catalogs-ui: Cleanup: Replace iterator class, general cleanup
Julian Eisel
noreply at git.blender.org
Thu Sep 23 17:47:39 CEST 2021
Commit: f413bc25f0790439d2408e9250cffac8b0b4ac3e
Author: Julian Eisel
Date: Thu Sep 23 17:45:47 2021 +0200
Branches: temp-asset-browser-catalogs-ui
https://developer.blender.org/rBf413bc25f0790439d2408e9250cffac8b0b4ac3e
Cleanup: Replace iterator class, general cleanup
Instead of the iterator class which requires a bunch of boilerplate
code, use a callback based iterator. Also removed a overly pedantic
assert and wrong friend declarations.
===================================================================
M source/blender/blenkernel/BKE_asset_catalog.hh
M source/blender/blenkernel/intern/asset_catalog.cc
M source/blender/editors/space_file/asset_catalog_tree_view.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh
index 41666e6bae3..6236924cccc 100644
--- a/source/blender/blenkernel/BKE_asset_catalog.hh
+++ b/source/blender/blenkernel/BKE_asset_catalog.hh
@@ -46,7 +46,6 @@ using CatalogFilePath = std::string;
class AssetCatalog;
class AssetCatalogDefinitionFile;
class AssetCatalogTree;
-class AssetCatalogTreeItemIterator;
/* Manages the asset catalogs of a single asset library (i.e. of catalogs defined in a single
* directory hierarchy). */
@@ -129,18 +128,16 @@ class AssetCatalogService {
};
class AssetCatalogTreeItem {
- friend class AssetCatalogService;
friend class AssetCatalogTree;
public:
using ChildMap = std::map<std::string, AssetCatalogTreeItem>;
- using ItemIterFn = FunctionRef<void(const AssetCatalogTreeItem &)>;
+ using ItemIterFn = FunctionRef<void(AssetCatalogTreeItem &)>;
AssetCatalogTreeItem(StringRef name,
const CatalogID &catalog_id,
const AssetCatalogTreeItem *parent = nullptr);
- AssetCatalogTreeItemIterator children();
const 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
@@ -149,7 +146,11 @@ class AssetCatalogTreeItem {
int count_parents() const;
bool has_children() const;
- static void foreach_item_recursive(const ChildMap &children_, const ItemIterFn callback);
+ static void foreach_item_recursive(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. */
@@ -169,49 +170,23 @@ class AssetCatalogTreeItem {
* 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:
/** Ensure an item representing \a path is in the tree, adding it if necessary. */
void insert_item(AssetCatalog &catalog);
- AssetCatalogTreeItemIterator children();
- void foreach_item(const AssetCatalogTreeItem::ItemIterFn callback) const;
+ void foreach_item(const AssetCatalogTreeItem::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_;
};
-/* TODO mostly boilerplate code. Is that worth it? Could alternatively expose the ChildMap
- * directly, and let users iterate over the map and its (key, value) pairs directly. */
-class AssetCatalogTreeItemIterator
- : public std::iterator<std::forward_iterator_tag, AssetCatalogTreeItem> {
- /** #AssetCatalogTreeItemIterator is just a wrapper around the child-maps iterator. That is so we
- * can iterate over the values only of the map's (key, value) pairs. */
- using WrappedIterator = AssetCatalogTreeItem::ChildMap::iterator;
-
- WrappedIterator wrapped_iterator_;
- WrappedIterator wrapped_end_iterator_;
-
- public:
- AssetCatalogTreeItemIterator(WrappedIterator wrapped_iterator,
- WrappedIterator wrapped_end_iterator);
-
- AssetCatalogTreeItemIterator begin() const;
- AssetCatalogTreeItemIterator end() const;
-
- AssetCatalogTreeItem &operator*() const;
- AssetCatalogTreeItem *operator->() const;
-
- AssetCatalogTreeItemIterator &operator++();
- AssetCatalogTreeItemIterator operator++(int);
-
- friend bool operator==(AssetCatalogTreeItemIterator a, AssetCatalogTreeItemIterator b);
- friend bool operator!=(AssetCatalogTreeItemIterator a, AssetCatalogTreeItemIterator b);
-};
-
/** Keeps track of which catalogs are defined in a certain file on disk.
* Only contains non-owning pointers to the #AssetCatalog instances, so ensure the lifetime of this
* class is shorter than that of the #`AssetCatalog`s themselves. */
diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc
index 24081bb5256..589142eb0af 100644
--- a/source/blender/blenkernel/intern/asset_catalog.cc
+++ b/source/blender/blenkernel/intern/asset_catalog.cc
@@ -94,8 +94,6 @@ AssetCatalog *AssetCatalogService::create_catalog(const CatalogPath &catalog_pat
/* So we can std::move(catalog) and still use the non-owning pointer: */
AssetCatalog *const catalog_ptr = catalog.get();
- BLI_assert_msg(find_catalog_from_path(catalog_path) == nullptr,
- "duplicate catalog path not supported");
/* TODO(@sybren): move the `AssetCatalog::from_path()` function to another place, that can reuse
* catalogs when a catalog with the given path is already known, and avoid duplicate catalog IDs.
*/
@@ -298,11 +296,6 @@ AssetCatalogTreeItem::AssetCatalogTreeItem(StringRef name,
{
}
-AssetCatalogTreeItemIterator AssetCatalogTreeItem::children()
-{
- return AssetCatalogTreeItemIterator(children_.begin(), children_.end());
-}
-
const CatalogID &AssetCatalogTreeItem::get_catalog_id() const
{
return catalog_id_;
@@ -338,54 +331,6 @@ bool AssetCatalogTreeItem::has_children() const
/* ---------------------------------------------------------------------- */
-AssetCatalogTreeItemIterator::AssetCatalogTreeItemIterator(WrappedIterator wrapped_iterator,
- WrappedIterator wrapped_end_iterator)
- : wrapped_iterator_(wrapped_iterator), wrapped_end_iterator_(wrapped_end_iterator)
-{
-}
-
-AssetCatalogTreeItemIterator AssetCatalogTreeItemIterator::begin() const
-{
- return *this;
-}
-
-AssetCatalogTreeItemIterator AssetCatalogTreeItemIterator::end() const
-{
- return AssetCatalogTreeItemIterator(wrapped_end_iterator_, wrapped_end_iterator_);
-}
-
-AssetCatalogTreeItem &AssetCatalogTreeItemIterator::operator*() const
-{
- return wrapped_iterator_->second;
-}
-AssetCatalogTreeItem *AssetCatalogTreeItemIterator::operator->() const
-{
- return &wrapped_iterator_->second;
-}
-
-AssetCatalogTreeItemIterator &AssetCatalogTreeItemIterator::operator++()
-{
- ++wrapped_iterator_;
- return *this;
-}
-AssetCatalogTreeItemIterator AssetCatalogTreeItemIterator::operator++(int)
-{
- AssetCatalogTreeItemIterator copy(*this);
- ++wrapped_iterator_;
- return copy;
-}
-
-bool operator==(AssetCatalogTreeItemIterator a, AssetCatalogTreeItemIterator b)
-{
- return a.wrapped_iterator_ == b.wrapped_iterator_;
-}
-bool operator!=(AssetCatalogTreeItemIterator a, AssetCatalogTreeItemIterator b)
-{
- return a.wrapped_iterator_ != b.wrapped_iterator_;
-}
-
-/* ---------------------------------------------------------------------- */
-
void AssetCatalogTree::insert_item(AssetCatalog &catalog)
{
const AssetCatalogTreeItem *parent = nullptr;
@@ -428,25 +373,34 @@ void AssetCatalogTree::insert_item(AssetCatalog &catalog)
}
}
-AssetCatalogTreeItemIterator AssetCatalogTree::children()
-{
- return AssetCatalogTreeItemIterator(children_.begin(), children_.end());
-}
-
-void AssetCatalogTree::foreach_item(const AssetCatalogTreeItem::ItemIterFn callback) const
+void AssetCatalogTree::foreach_item(AssetCatalogTreeItem::ItemIterFn callback)
{
AssetCatalogTreeItem::foreach_item_recursive(children_, 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_child(const ItemIterFn callback)
+{
+ for (auto &[key, item] : children_) {
+ 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/editors/space_file/asset_catalog_tree_view.cc b/source/blender/editors/space_file/asset_catalog_tree_view.cc
index a79b55bd270..7c4dacb934f 100644
--- a/source/blender/editors/space_file/asset_catalog_tree_view.cc
+++ b/source/blender/editors/space_file/asset_catalog_tree_view.cc
@@ -60,8 +60,8 @@ class AssetCatalogTreeView : public uiAbstractTreeView {
void build_tree() override;
private:
- uiBasicTreeViewItem &build_recursive(uiTreeViewItemContainer &view_parent_item,
- AssetCatalogTreeItem &catalog);
+ static uiBasicTreeViewItem &build_recursive(uiTreeViewItemContainer &view_parent_item,
+ AssetCatalogTreeItem &catalog);
};
/* ---------------------------------------------------------------------- */
@@ -127,12 +127,12 @@ void AssetCatalogTreeView::build_tree()
if (library_) {
AssetCatalogTree *catalog_tree = library_->catalog_service->get_catalog_tree();
- for (AssetCatalogTreeItem &item : catalog_tree->children()) {
+ catalog_tree->foreach_child([this](AssetCatalogTreeItem &item) {
uiBasicTreeViewItem &child_view_item = build_recursive(*this, item);
/* Open root-level items by default. */
child_view_item.set_collapsed(false);
- }
+ });
}
add_tree_item<uiBasicTreeViewItem>(
@@ -148,10 +148,8 @@ uiBasicTreeViewItem &AssetCatalogTreeView::build_recursive(
uiBasicTreeViewItem &view_item = view_parent_item.add_tree_item<AssetCatalogTreeViewItem>(
catalog);
- for (AssetCatalogTreeItem &child : catalog.children()) {
- build_recursive(view_item, child);
- }
-
+ catalog.foreach_child(
+ [&view_item](AssetCatalogTreeItem &child) { build_recursive(view_item, child); });
return view_item;
}
More information about the Bf-blender-cvs
mailing list