[Bf-blender-cvs] [1bf0f1a1192] temp-asset-browser-catalogs: Use existing C-APIs for path and filesystem handling
Julian Eisel
noreply at git.blender.org
Tue Sep 21 16:05:01 CEST 2021
Commit: 1bf0f1a1192c5a15aae5718603f2d2266715edb4
Author: Julian Eisel
Date: Tue Sep 21 16:01:31 2021 +0200
Branches: temp-asset-browser-catalogs
https://developer.blender.org/rB1bf0f1a1192c5a15aae5718603f2d2266715edb4
Use existing C-APIs for path and filesystem handling
The `ghc::filesystem` we wanted to use for a platform compatible
replacement of `std::filesystem` (see D12197) doesn't behave as we
expect or want to. We could probably find a solution, but it's not worth
the extra time. To move this forward, we can switch the asset catalogs
to use Blender's C functions for path and filesystem handling.
Differential Revision: https://developer.blender.org/D12538
===================================================================
M source/blender/blenkernel/BKE_asset_catalog.hh
M source/blender/blenkernel/BKE_asset_library.hh
M source/blender/blenkernel/intern/asset_catalog.cc
M source/blender/blenkernel/intern/asset_catalog_test.cc
M source/blender/blenkernel/intern/asset_library.cc
M source/blender/blenkernel/intern/asset_library_test.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh
index 7483c82754e..7ed2dc187e8 100644
--- a/source/blender/blenkernel/BKE_asset_catalog.hh
+++ b/source/blender/blenkernel/BKE_asset_catalog.hh
@@ -24,7 +24,6 @@
# error This is a C++ header. The C interface is yet to be implemented/designed.
#endif
-#include "BLI_filesystem.hh"
#include "BLI_function_ref.hh"
#include "BLI_map.hh"
#include "BLI_string_ref.hh"
@@ -40,7 +39,9 @@ namespace blender::bke {
using CatalogID = UUID;
using CatalogPath = std::string;
using CatalogPathComponent = std::string;
-using CatalogFilePath = filesystem::path;
+/* Would be nice to be able to use `std::filesystem::path` for this, but it's currently not
+ * available on the minimum macOS target version. */
+using CatalogFilePath = std::string;
class AssetCatalog;
class AssetCatalogDefinitionFile;
diff --git a/source/blender/blenkernel/BKE_asset_library.hh b/source/blender/blenkernel/BKE_asset_library.hh
index d12130ca368..68f7481574e 100644
--- a/source/blender/blenkernel/BKE_asset_library.hh
+++ b/source/blender/blenkernel/BKE_asset_library.hh
@@ -28,18 +28,14 @@
#include "BKE_asset_catalog.hh"
-#include <filesystem>
#include <memory>
namespace blender::bke {
-/* TODO(@sybren): revisit after D12117 has a conclusion. */
-namespace fs = blender::filesystem;
-
struct AssetLibrary {
std::unique_ptr<AssetCatalogService> catalog_service;
- void load(const fs::path &library_root_directory);
+ void load(StringRefNull library_root_directory);
};
} // namespace blender::bke
diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc
index 4519f796c40..618888d44c5 100644
--- a/source/blender/blenkernel/intern/asset_catalog.cc
+++ b/source/blender/blenkernel/intern/asset_catalog.cc
@@ -20,14 +20,12 @@
#include "BKE_asset_catalog.hh"
-#include "BLI_filesystem.hh"
+#include "BLI_fileops.h"
+#include "BLI_path_util.h"
#include "BLI_string_ref.hh"
-#include <filesystem>
#include <fstream>
-namespace fs = blender::filesystem;
-
namespace blender::bke {
const char AssetCatalogService::PATH_SEPARATOR = '/';
@@ -95,6 +93,16 @@ AssetCatalog *AssetCatalogService::create_catalog(const CatalogPath &catalog_pat
return catalog_ptr;
}
+static std::string asset_definition_default_file_path_from_dir(StringRef asset_library_root)
+{
+ char file_path[PATH_MAX];
+ BLI_join_dirfile(file_path,
+ sizeof(file_path),
+ asset_library_root.data(),
+ AssetCatalogService::DEFAULT_CATALOG_FILENAME.data());
+ return file_path;
+}
+
void AssetCatalogService::ensure_catalog_definition_file()
{
if (catalog_definition_file_) {
@@ -102,7 +110,7 @@ void AssetCatalogService::ensure_catalog_definition_file()
}
auto cdf = std::make_unique<AssetCatalogDefinitionFile>();
- cdf->file_path = asset_library_root_ / DEFAULT_CATALOG_FILENAME;
+ cdf->file_path = asset_definition_default_file_path_from_dir(asset_library_root_);
catalog_definition_file_ = std::move(cdf);
}
@@ -117,8 +125,8 @@ bool AssetCatalogService::ensure_asset_library_root()
return false;
}
- if (fs::exists(asset_library_root_)) {
- if (!fs::is_directory(asset_library_root_)) {
+ if (BLI_exists(asset_library_root_.data())) {
+ if (!BLI_is_dir(asset_library_root_.data())) {
std::cerr << "AssetCatalogService: " << asset_library_root_
<< " exists but is not a directory, this is not a supported situation."
<< std::endl;
@@ -131,7 +139,7 @@ bool AssetCatalogService::ensure_asset_library_root()
/* Ensure the root directory exists. */
std::error_code err_code;
- if (!fs::create_directories(asset_library_root_, err_code)) {
+ if (!BLI_dir_create_recursive(asset_library_root_.data())) {
std::cerr << "AssetCatalogService: error creating directory " << asset_library_root_ << ": "
<< err_code << std::endl;
return false;
@@ -148,17 +156,20 @@ void AssetCatalogService::load_from_disk()
void AssetCatalogService::load_from_disk(const CatalogFilePath &file_or_directory_path)
{
- fs::file_status status = fs::status(file_or_directory_path);
- switch (status.type()) {
- case fs::file_type::regular:
- load_single_file(file_or_directory_path);
- break;
- case fs::file_type::directory:
- load_directory_recursive(file_or_directory_path);
- break;
- default:
- // TODO(@sybren): throw an appropriate exception.
- return;
+ BLI_stat_t status;
+ if (BLI_stat(file_or_directory_path.data(), &status) == -1) {
+ // TODO(@sybren): throw an appropriate exception.
+ return;
+ }
+
+ if (S_ISREG(status.st_mode)) {
+ load_single_file(file_or_directory_path);
+ }
+ else if (S_ISDIR(status.st_mode)) {
+ load_directory_recursive(file_or_directory_path);
+ }
+ else {
+ // TODO(@sybren): throw an appropriate exception.
}
/* TODO: Should there be a sanitize step? E.g. to remove catalogs with identical paths? */
@@ -170,13 +181,13 @@ void AssetCatalogService::load_directory_recursive(const CatalogFilePath &direct
{
// TODO(@sybren): implement proper multi-file support. For now, just load
// the default file if it is there.
- CatalogFilePath file_path = directory_path / DEFAULT_CATALOG_FILENAME;
- fs::file_status fs_status = fs::status(file_path);
+ CatalogFilePath file_path = asset_definition_default_file_path_from_dir(directory_path);
- if (!fs::exists(fs_status)) {
+ if (!BLI_exists(file_path.data())) {
/* No file to be loaded is perfectly fine. */
return;
}
+
this->load_single_file(file_path);
}
@@ -289,18 +300,25 @@ 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()) {
- /* #fs::path adds useful behavior to the path. Remember that on Windows it uses "\" as
- * separator! For catalogs it should always be "/". Use #fs::path::generic_string if needed. */
- fs::path catalog_path = catalog->path;
-
const AssetCatalogTreeItem *parent = nullptr;
AssetCatalogTreeItem::ChildMap *insert_to_map = &tree->children_;
- BLI_assert_msg(catalog_path.is_relative() && !catalog_path.has_root_path(),
- "Malformed catalog path: Path should be a relative path, with no root-name or "
- "root-directory as defined by std::filesystem::path.");
- for (const fs::path &component : catalog_path) {
- std::string component_name = component.string();
+ BLI_assert_msg(!ELEM(catalog->path[0], '/', '\\'),
+ "Malformed catalog path: Path should be formatted like a relative path");
+
+ 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(
diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc
index 65cff8c58df..48179576a33 100644
--- a/source/blender/blenkernel/intern/asset_catalog_test.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_test.cc
@@ -21,13 +21,10 @@
#include "BKE_asset_catalog.hh"
#include "BLI_fileops.h"
+#include "BLI_path_util.h"
#include "testing/testing.h"
-#include <filesystem>
-
-namespace fs = blender::filesystem;
-
namespace blender::bke::tests {
/* UUIDs from lib/tests/asset_library/blender_assets.cats.txt */
@@ -61,12 +58,12 @@ class AssetCatalogTest : public testing::Test {
void SetUp() override
{
- const fs::path test_files_dir = blender::tests::flags_test_asset_dir();
+ const std::string test_files_dir = blender::tests::flags_test_asset_dir();
if (test_files_dir.empty()) {
FAIL();
}
- asset_library_root_ = test_files_dir / "asset_library";
+ asset_library_root_ = test_files_dir + "/" + "asset_library";
temp_library_path_ = "";
}
@@ -74,33 +71,31 @@ class AssetCatalogTest : public testing::Test {
CatalogFilePath use_temp_path()
{
const CatalogFilePath tempdir = BKE_tempdir_session();
- temp_library_path_ = tempdir / "test-temporary-path";
+ temp_library_path_ = tempdir + "test-temporary-path";
return temp_library_path_;
}
- int count_path_parents(const fs::path &path)
- {
- int counter = 0;
- for (const fs::path &segment : path.parent_path()) {
- counter++;
- UNUSED_VARS(segment);
- }
- return counter;
- }
+ struct CatalogPathInfo {
+ StringRef name;
+ int parent_count;
+ };
void assert_expected_tree_items(AssetCatalogTree *tree,
- const std::vector<fs::path> &expected_paths)
+ const std::vector<CatalogPathInfo> &expected_paths)
{
int i = 0;
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();
+ char expected_filename[FILE_MAXFILE];
/* Is the catalog name as expected? "character", "Ellie", ... */
- EXPECT_EQ(expected_paths[i].filename().string(), actual_item.get_name());
- /* Does the number of parents match? */
- EXPECT_EQ(count_path_parents(expected_paths[
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list