[Bf-blender-cvs] [2165136740f] master: File/Asset Browser: Refactor how recursive paths are set

Julian Eisel noreply at git.blender.org
Wed Nov 30 19:44:52 CET 2022


Commit: 2165136740f88fe92ebe4e548ef587adb6a3dfdd
Author: Julian Eisel
Date:   Wed Nov 30 18:54:42 2022 +0100
Branches: master
https://developer.blender.org/rB2165136740f88fe92ebe4e548ef587adb6a3dfdd

File/Asset Browser: Refactor how recursive paths are set

When reading directories recursively, the code would first only set the
file name as the relative path and then later iterate over the read files
and prepend the recursed into path, to get the complete path relative to
the recursed into directory. This isn't clear and confused me quite a
bit. And it is not compatible with what we need for creating asset
identifiers, which are introduced in the 2nd following commit.

Instead properly determine the complete relative path when initially
adding the file, and don't change it after. The asset identifier can the
be constructed properly at the time needed.

===================================================================

M	source/blender/editors/space_file/filelist.cc

===================================================================

diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc
index dc59bb1286d..4a27893e985 100644
--- a/source/blender/editors/space_file/filelist.cc
+++ b/source/blender/editors/space_file/filelist.cc
@@ -2888,7 +2888,65 @@ struct TodoDir {
   char *dir;
 };
 
-static int filelist_readjob_list_dir(const char *root,
+struct FileListReadJob {
+  ThreadMutex lock;
+  char main_name[FILE_MAX];
+  Main *current_main;
+  FileList *filelist;
+
+  /** The path currently being read, relative to the filelist root directory. Needed for recursive
+   * reading. The full file path is then composed like: `<filelist root>/<cur_relbase>/<file name>.
+   * (whereby the file name may also be a library path within a .blend, e.g.
+   * `Materials/Material.001`). */
+  char cur_relbase[FILE_MAX_LIBEXTRA];
+
+  /** Set to request a partial read that only adds files representing #Main data (IDs). Used when
+   * #Main may have received changes of interest (e.g. asset removed or renamed). */
+  bool only_main_data;
+
+  /** Shallow copy of #filelist for thread-safe access.
+   *
+   * The job system calls #filelist_readjob_update which moves any read file from #tmp_filelist
+   * into #filelist in a thread-safe way.
+   *
+   * #tmp_filelist also keeps an `AssetLibrary *` so that it can be loaded in the same thread,
+   * and moved to #filelist once all categories are loaded.
+   *
+   * NOTE: #tmp_filelist is freed in #filelist_readjob_free, so any copied pointers need to be
+   * set to nullptr to avoid double-freeing them. */
+  FileList *tmp_filelist;
+};
+
+/**
+ * Append \a filename (or even a path inside of a .blend, like `Material/Material.001`), to the
+ * current relative path being read within the filelist root. The returned string needs freeing
+ * with #MEM_freeN().
+ */
+static char *current_relpath_append(const FileListReadJob *job_params, const char *filename)
+{
+  const char *relbase = job_params->cur_relbase;
+
+  /* Early exit, nothing to join. */
+  if (!relbase[0]) {
+    return BLI_strdup(filename);
+  }
+
+  BLI_assert(relbase[strlen(relbase) - 1] == SEP);
+  BLI_assert(BLI_path_is_rel(relbase));
+
+  char relpath[FILE_MAX_LIBEXTRA];
+  /* Using #BLI_path_join works but isn't needed as `rel_subdir` has a trailing slash. */
+  BLI_string_join(relpath,
+                  sizeof(relpath),
+                  /* + 2 to remove "//" relative path prefix. */
+                  relbase + 2,
+                  filename);
+
+  return BLI_strdup(relpath);
+}
+
+static int filelist_readjob_list_dir(FileListReadJob *job_params,
+                                     const char *root,
                                      ListBase *entries,
                                      const char *filter_glob,
                                      const bool do_lib,
@@ -2911,7 +2969,7 @@ static int filelist_readjob_list_dir(const char *root,
       }
 
       entry = MEM_cnew<FileListInternEntry>(__func__);
-      entry->relpath = static_cast<char *>(MEM_dupallocN(files[i].relname));
+      entry->relpath = current_relpath_append(job_params, files[i].relname);
       entry->st = files[i].s;
 
       BLI_path_join(full_path, FILE_MAX, root, entry->relpath);
@@ -2998,11 +3056,11 @@ enum ListLibOptions {
 };
 ENUM_OPERATORS(ListLibOptions, LIST_LIB_ADD_PARENT);
 
-static FileListInternEntry *filelist_readjob_list_lib_group_create(const int idcode,
-                                                                   const char *group_name)
+static FileListInternEntry *filelist_readjob_list_lib_group_create(
+    const FileListReadJob *job_params, const int idcode, const char *group_name)
 {
   FileListInternEntry *entry = MEM_cnew<FileListInternEntry>(__func__);
-  entry->relpath = BLI_strdup(group_name);
+  entry->relpath = current_relpath_append(job_params, group_name);
   entry->typeflag |= FILE_TYPE_BLENDERLIB | FILE_TYPE_DIR;
   entry->blentype = idcode;
   return entry;
@@ -3012,19 +3070,21 @@ static FileListInternEntry *filelist_readjob_list_lib_group_create(const int idc
  * \warning: This "steals" the asset metadata from \a datablock_info. Not great design but fixing
  *           this requires redesigning things on the caller side for proper ownership management.
  */
-static void filelist_readjob_list_lib_add_datablock(FileList *filelist,
+static void filelist_readjob_list_lib_add_datablock(FileListReadJob *job_params,
                                                     ListBase *entries,
                                                     BLODataBlockInfo *datablock_info,
                                                     const bool prefix_relpath_with_group_name,
                                                     const int idcode,
                                                     const char *group_name)
 {
+  FileList *filelist = job_params->tmp_filelist; /* Use the thread-safe filelist queue. */
   FileListInternEntry *entry = MEM_cnew<FileListInternEntry>(__func__);
   if (prefix_relpath_with_group_name) {
-    entry->relpath = BLI_sprintfN("%s/%s", group_name, datablock_info->name);
+    std::string datablock_path = StringRef(group_name) + "/" + datablock_info->name;
+    entry->relpath = current_relpath_append(job_params, datablock_path.c_str());
   }
   else {
-    entry->relpath = BLI_strdup(datablock_info->name);
+    entry->relpath = current_relpath_append(job_params, datablock_info->name);
   }
   entry->typeflag |= FILE_TYPE_BLENDERLIB;
   if (datablock_info) {
@@ -3048,7 +3108,7 @@ static void filelist_readjob_list_lib_add_datablock(FileList *filelist,
   BLI_addtail(entries, entry);
 }
 
-static void filelist_readjob_list_lib_add_datablocks(FileList *filelist,
+static void filelist_readjob_list_lib_add_datablocks(FileListReadJob *job_params,
                                                      ListBase *entries,
                                                      LinkNode *datablock_infos,
                                                      const bool prefix_relpath_with_group_name,
@@ -3058,12 +3118,12 @@ static void filelist_readjob_list_lib_add_datablocks(FileList *filelist,
   for (LinkNode *ln = datablock_infos; ln; ln = ln->next) {
     struct BLODataBlockInfo *datablock_info = static_cast<BLODataBlockInfo *>(ln->link);
     filelist_readjob_list_lib_add_datablock(
-        filelist, entries, datablock_info, prefix_relpath_with_group_name, idcode, group_name);
+        job_params, entries, datablock_info, prefix_relpath_with_group_name, idcode, group_name);
   }
 }
 
 static void filelist_readjob_list_lib_add_from_indexer_entries(
-    FileList *filelist,
+    FileListReadJob *job_params,
     ListBase *entries,
     const FileIndexerEntries *indexer_entries,
     const bool prefix_relpath_with_group_name)
@@ -3071,7 +3131,7 @@ static void filelist_readjob_list_lib_add_from_indexer_entries(
   for (const LinkNode *ln = indexer_entries->entries; ln; ln = ln->next) {
     FileIndexerEntry *indexer_entry = static_cast<FileIndexerEntry *>(ln->link);
     const char *group_name = BKE_idtype_idcode_to_name(indexer_entry->idcode);
-    filelist_readjob_list_lib_add_datablock(filelist,
+    filelist_readjob_list_lib_add_datablock(job_params,
                                             entries,
                                             &indexer_entry->datablock_info,
                                             prefix_relpath_with_group_name,
@@ -3080,10 +3140,11 @@ static void filelist_readjob_list_lib_add_from_indexer_entries(
   }
 }
 
-static FileListInternEntry *filelist_readjob_list_lib_navigate_to_parent_entry_create(void)
+static FileListInternEntry *filelist_readjob_list_lib_navigate_to_parent_entry_create(
+    const FileListReadJob *job_params)
 {
   FileListInternEntry *entry = MEM_cnew<FileListInternEntry>(__func__);
-  entry->relpath = BLI_strdup(FILENAME_PARENT);
+  entry->relpath = current_relpath_append(job_params, FILENAME_PARENT);
   entry->typeflag |= (FILE_TYPE_BLENDERLIB | FILE_TYPE_DIR);
   return entry;
 }
@@ -3100,7 +3161,7 @@ typedef struct FileIndexer {
   void *user_data;
 } FileIndexer;
 
-static int filelist_readjob_list_lib_populate_from_index(FileList *filelist,
+static int filelist_readjob_list_lib_populate_from_index(FileListReadJob *job_params,
                                                          ListBase *entries,
                                                          const ListLibOptions options,
                                                          const int read_from_index,
@@ -3108,12 +3169,13 @@ static int filelist_readjob_list_lib_populate_from_index(FileList *filelist,
 {
   int navigate_to_parent_len = 0;
   if (options & LIST_LIB_ADD_PARENT) {
-    FileListInternEntry *entry = filelist_readjob_list_lib_navigate_to_parent_entry_create();
+    FileListInternEntry *entry = filelist_readjob_list_lib_navigate_to_parent_entry_create(
+        job_params);
     BLI_addtail(entries, entry);
     navigate_to_parent_len = 1;
   }
 
-  filelist_readjob_list_lib_add_from_indexer_entries(filelist, entries, indexer_entries, true);
+  filelist_readjob_list_lib_add_from_indexer_entries(job_params, entries, indexer_entries, true);
   return read_from_index + navigate_to_parent_len;
 }
 
@@ -3121,7 +3183,7 @@ static int filelist_readjob_list_lib_populate_from_index(FileList *filelist,
  * \return The number of entries found if the \a root path points to a valid library file.
  *         Otherwise returns no value (#std::nullopt).
  */
-static std::optional<int> filelist_readjob_list_lib(FileList *filelist,
+static std::optional<int> filelist_readjob_list_lib(FileListReadJob *job_params,
                                                     const char *root,
                                                     ListBase *entries,
                                                     const ListLibOptions options,
@@ -3161,7 +3223,7 @@ static std::optional<int> filelist_readjob_list_lib(FileList *filelist,
         dir, &indexer_entries, &read_from_index, indexer_runtime->user_data);
     if (indexer_result == FILE_INDEXER_ENTRIES_LOADED) {
       int entries_read = filelist_readjob_list_lib_populate_from_index(
-          filelist, entries, options, rea

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list