[Bf-blender-cvs] [40ed613609d] ui-asset-view-template: Cleanup: First round of polishing (C++ style, type safety, naming, etc)
Julian Eisel
noreply at git.blender.org
Thu Mar 18 19:48:08 CET 2021
Commit: 40ed613609d1a8dbab9ea9ef85dd34142471b9c1
Author: Julian Eisel
Date: Thu Mar 18 19:42:05 2021 +0100
Branches: ui-asset-view-template
https://developer.blender.org/rB40ed613609d1a8dbab9ea9ef85dd34142471b9c1
Cleanup: First round of polishing (C++ style, type safety, naming, etc)
* Follow "rule of five"
* Explicit conversion constructors
* Separate inteface declarations from definitions (readability)
* Avoid clang-tidy error
* Naming
....
===================================================================
M source/blender/editors/asset/asset_list.cc
M source/blender/editors/include/ED_asset.h
M source/blender/editors/interface/interface_template_asset_view.cc
===================================================================
diff --git a/source/blender/editors/asset/asset_list.cc b/source/blender/editors/asset/asset_list.cc
index 641304d6ff1..accf27f0da3 100644
--- a/source/blender/editors/asset/asset_list.cc
+++ b/source/blender/editors/asset/asset_list.cc
@@ -25,11 +25,13 @@
#include <optional>
+#include "BKE_context.h"
#include "BKE_screen.h"
#include "BLI_function_ref.hh"
#include "BLI_hash.hh"
#include "BLI_map.hh"
+#include "BLI_utility_mixins.hh"
#include "DNA_asset_types.h"
#include "DNA_space_types.h"
@@ -40,6 +42,9 @@
#include "ED_fileselect.h"
#include "ED_screen.h"
+#include "WM_api.h"
+#include "WM_types.h"
+
/* XXX uses private header of file-space. */
#include "../space_file/filelist.h"
@@ -48,35 +53,43 @@ using namespace blender;
/**
* Wrapper to add logic to the AssetLibraryReference DNA struct.
*/
-struct AssetLibraryReferenceWrapper {
+class AssetLibraryReferenceWrapper {
const AssetLibraryReference &reference_;
- AssetLibraryReferenceWrapper(const AssetLibraryReference &reference) : reference_(reference)
- {
- }
+ public:
+ /* Intentionally not `explicit`, allow implicit conversion for convienience. Might have to be
+ * NOLINT */
+ AssetLibraryReferenceWrapper(const AssetLibraryReference &reference);
~AssetLibraryReferenceWrapper() = default;
friend bool operator==(const AssetLibraryReferenceWrapper &a,
- const AssetLibraryReferenceWrapper &b)
- {
- return (a.reference_.type == b.reference_.type) &&
- (a.reference_.type == ASSET_LIBRARY_CUSTOM) ?
- (a.reference_.custom_library_index == b.reference_.custom_library_index) :
- true;
- }
+ const AssetLibraryReferenceWrapper &b);
+ uint64_t hash() const;
+};
- uint64_t hash() const
- {
- uint64_t hash1 = DefaultHash<decltype(reference_.type)>{}(reference_.type);
- if (reference_.type != ASSET_LIBRARY_CUSTOM) {
- return hash1;
- }
+AssetLibraryReferenceWrapper::AssetLibraryReferenceWrapper(const AssetLibraryReference &reference)
+ : reference_(reference)
+{
+}
+
+bool operator==(const AssetLibraryReferenceWrapper &a, const AssetLibraryReferenceWrapper &b)
+{
+ return (a.reference_.type == b.reference_.type) && (a.reference_.type == ASSET_LIBRARY_CUSTOM) ?
+ (a.reference_.custom_library_index == b.reference_.custom_library_index) :
+ true;
+}
- uint64_t hash2 = DefaultHash<decltype(reference_.custom_library_index)>{}(
- reference_.custom_library_index);
- return hash1 ^ (hash2 * 33); /* Copied from DefaultHash for std::pair. */
+uint64_t AssetLibraryReferenceWrapper::hash() const
+{
+ uint64_t hash1 = DefaultHash<decltype(reference_.type)>{}(reference_.type);
+ if (reference_.type != ASSET_LIBRARY_CUSTOM) {
+ return hash1;
}
-};
+
+ uint64_t hash2 = DefaultHash<decltype(reference_.custom_library_index)>{}(
+ reference_.custom_library_index);
+ return hash1 ^ (hash2 * 33); /* Copied from DefaultHash for std::pair. */
+}
/* -------------------------------------------------------------------- */
/** \name Asset list API
@@ -84,196 +97,235 @@ struct AssetLibraryReferenceWrapper {
* Internally re-uses #FileList from the File Browser. It does all the heavy lifting already.
* \{ */
-#include "BKE_context.h"
-#include "WM_api.h"
-#include "WM_types.h"
-
-class AssetList {
- static void filelist_free_wrapper(FileList *list)
+/**
+ * RAII wrapper for `FileList`
+ */
+class FileListWrapper : NonCopyable {
+ static void filelist_free_fn(FileList *list)
{
filelist_free(list);
MEM_freeN(list);
}
- using FileList_Ptr = std::unique_ptr<FileList, decltype(&filelist_free_wrapper)>;
- FileList_Ptr filelist_;
- AssetLibraryReference library_ref_;
-
- struct wmTimer *previews_timer = nullptr;
+ std::unique_ptr<FileList, decltype(&filelist_free_fn)> file_list_;
public:
- AssetList() = delete;
- AssetList(eFileSelectType filesel_type, const AssetLibraryReference &asset_library_ref)
- : filelist_(filelist_new(filesel_type), filelist_free_wrapper),
- library_ref_(asset_library_ref)
+ explicit FileListWrapper(eFileSelectType filesel_type)
+ : file_list_(filelist_new(filesel_type), filelist_free_fn)
{
}
- AssetList(AssetList &&other)
- : filelist_(std::move(other.filelist_)), library_ref_(other.library_ref_)
- {
- }
- AssetList(const AssetList &) = delete;
- ~AssetList()
+ FileListWrapper(FileListWrapper &&other) = default;
+ FileListWrapper &operator=(FileListWrapper &&other) = default;
+ ~FileListWrapper()
{
/* Destructs the owned pointer. */
- filelist_ = nullptr;
+ file_list_ = nullptr;
}
- void setup(const AssetFilterSettings *filter_settings = nullptr)
+ operator FileList *() const
{
- FileList *files = filelist_.get();
-
- /* TODO there should only be one (FileSelectAssetLibraryUID vs. AssetLibraryReference). */
- FileSelectAssetLibraryUID file_asset_lib_ref;
- file_asset_lib_ref.type = library_ref_.type;
- file_asset_lib_ref.custom_library_index = library_ref_.custom_library_index;
-
- bUserAssetLibrary *user_library = NULL;
+ return file_list_.get();
+ }
+};
- /* Ensure valid repository, or fall-back to local one. */
- if (library_ref_.type == ASSET_LIBRARY_CUSTOM) {
- BLI_assert(library_ref_.custom_library_index >= 0);
+class PreviewTimer : NonCopyable {
+ /* Non-owning! The Window-Manager registers and owns this. */
+ wmTimer *timer_ = nullptr;
- user_library = BKE_preferences_asset_library_find_from_index(
- &U, library_ref_.custom_library_index);
+ public:
+ void ensureRunning(const bContext *C)
+ {
+ if (!timer_) {
+ timer_ = WM_event_add_timer_notifier(
+ CTX_wm_manager(C), CTX_wm_window(C), NC_ASSET | ND_ASSET_LIST_PREVIEW, 0.01);
}
+ }
- /* Relevant bits from file_refresh(). */
- /* TODO pass options properly. */
- filelist_setrecursion(files, 1);
- filelist_setsorting(files, FILE_SORT_ALPHA, false);
- filelist_setlibrary(files, &file_asset_lib_ref);
- /* TODO different filtering settings require the list to be reread. That's a no-go for when we
- * want to allow showing the same asset library with different filter settings (as in,
- * different ID types). The filelist needs to be made smarter somehow, maybe goes together with
- * the plan to separate the view (preview caching, filtering, etc. ) from the data. */
- filelist_setfilter_options(
- files,
- filter_settings != nullptr,
- true,
- true, /* Just always hide parent, prefer to not add an extra user option for this. */
- FILE_TYPE_BLENDERLIB,
- filter_settings ? filter_settings->id_types : FILTER_ID_ALL,
- true,
- "",
- "");
-
- char path[FILE_MAXDIR] = "";
- if (user_library) {
- BLI_strncpy(path, user_library->path, sizeof(path));
- filelist_setdir(files, path);
- }
- else {
- filelist_setdir(files, path);
+ void stop(const bContext *C)
+ {
+ if (timer_) {
+ WM_event_remove_timer_notifier(CTX_wm_manager(C), CTX_wm_window(C), timer_);
+ timer_ = nullptr;
}
}
+};
- void fetch(const bContext &C)
- {
- FileList *files = filelist_.get();
+class AssetList : NonCopyable {
+ FileListWrapper filelist_;
+ AssetLibraryReference library_ref_;
+ PreviewTimer previews_timer;
- if (filelist_needs_force_reset(files)) {
- filelist_readjob_stop(CTX_wm_manager(&C), CTX_data_scene(&C));
- filelist_clear(files);
- }
+ public:
+ AssetList() = delete;
+ AssetList(eFileSelectType filesel_type, const AssetLibraryReference &asset_library_ref);
+ AssetList(AssetList &&other) = default;
+ ~AssetList() = default;
+
+ void setup(const AssetFilterSettings *filter_settings = nullptr);
+ void fetch(const bContext &C);
+ void ensurePreviewsJob(bContext *C);
+
+ bool needsRefetch() const;
+ void iterate(AssetListIterFn fn) const;
+ bool listen(const wmNotifier ¬ifier) const;
+ void tagMainDataDirty() const;
+ void remapID(ID *id_old, ID *id_new) const;
+ StringRef filepath() const;
+};
- if (filelist_needs_reading(files)) {
- if (!filelist_pending(files)) {
- filelist_readjob_start(files, NC_ASSET | ND_ASSET_LIST_READING, &C);
- }
- }
- filelist_sort(files);
- filelist_filter(files);
+AssetList::AssetList(eFileSelectType filesel_type, const AssetLibraryReference &asset_library_ref)
+ : filelist_(filesel_type), library_ref_(asset_library_ref)
+{
+}
+
+void AssetList::setup(const AssetFilterSettings *filter_settings)
+{
+ FileList *files = filelist_;
+
+ /* TODO there should only be one (FileSelectAssetLibraryUID vs. AssetLibraryReference). */
+ FileSelectAssetLibraryUID file_asset_lib_ref;
+ file_asset_lib_ref.type = library_ref_.type;
+ file_asset_lib_ref.custom_library_index = library_ref_.custom_library_index;
+
+ bUserAssetLibrary *user_library = nullptr;
+
+ /* Ensure valid repository, or fall-back to local one. */
+ if (library_ref_.type == ASSET_LIBRARY_CUSTOM) {
+ BLI_assert(library_ref_.custom_library_index >= 0);
+
+ user_library = BKE_preferences_asset_library_find_from_index(
+ &U, library_ref_.custom_library_index);
}
- bool needsRefetch() const
- {
- return filelist_needs_force_reset(filelist_.get());
+ /* Relevant bits from file_refresh(). */
+ /* TODO pass options properly. */
+ filelist_setrecursion(files, 1);
+ filelist_setsorting(files, FILE_SORT_ALPHA, false);
+ filelist_setlibrary(files, &file_asset_lib_ref);
+ /* TODO different filtering settings require the list to be reread. That's a no-go for when we
+ * want to allow showing the same asset library with different filter settings (as in,
+ * different ID types). The filelist needs to be made smarter somehow, maybe goes together with
+ * the plan to separate the view (preview caching, filtering, etc. ) from the data. */
+ filelist_setfilter_options(
+ files,
+ filter_settings != nullptr,
+ true,
+ true, /* Just always hide parent, prefer to not add an extra user option for this. */
+ FILE_TYPE_BLENDERLIB,
+ filter_settings ?
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list