IO: Add initial support for File Handlers registration #112466

Merged
Jesse Yurkovich merged 15 commits from guishe/blender:file-handler into main 2023-12-09 05:06:24 +01:00
5 changed files with 58 additions and 63 deletions
Showing only changes of commit f162945b3b - Show all commits

View File

@ -6,10 +6,6 @@
#include "RNA_types.hh"
struct bFileExtension {
char extension[64];
};
struct FileHandlerType {
char idname[OP_MAX_TYPENAME]; /* Unique name. */
guishe marked this conversation as resolved Outdated

Use /** */ doxygen syntax, put all comments on top.

See also https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments

Use `/** */` doxygen syntax, put all comments on top. See also https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments
@ -21,22 +17,20 @@ struct FileHandlerType {
/* String list of file extensions supported by the file handler. */
guishe marked this conversation as resolved Outdated

Should also document the expected format of this string (separator, with or without dot, etc.)

Should also document the expected format of this string (separator, with or without dot, etc.)
char file_extensions_str[OP_MAX_TYPENAME];
guishe marked this conversation as resolved Outdated

I don't think 64 chars is enough here? That's max 12 3-chars extensions... E.g. a handler for image files may support tens of them...

Think it needs its own define, probably at 256 or 512 chars?

I don't think 64 chars is enough here? That's max 12 3-chars extensions... E.g. a handler for image files may support tens of them... Think it needs its own define, probably at 256 or 512 chars?
guishe marked this conversation as resolved Outdated

picky and be separated by

*picky* `and be separated by`
/* Check if file handler can be used. */
/* Check if file handler can be used on file drop. */
bool (*poll_drop)(const struct bContext *C, FileHandlerType *file_handle_type);
/* List of file extensions supported by the file handler. */
blender::Vector<bFileExtension> file_extensions;
blender::Vector<std::string> file_extensions;
/* RNA integration */
ExtensionRNA rna_ext;
};
guishe marked this conversation as resolved Outdated

bFileExtension can just be a std::string?

`bFileExtension` can just be a `std::string`?
void BKE_file_handlers_init();
void BKE_file_handler_add(std::unique_ptr<FileHandlerType> file_handler);
guishe marked this conversation as resolved Outdated

All of these functions need to be properly documented.

All of these functions need to be properly documented.
void BKE_file_handlers_free();
blender::Span<FileHandlerType *> BKE_file_handlers();
void BKE_file_handler_add(FileHandlerType *file_handler);
FileHandlerType *BKE_file_handler_find(const char *idname);
void BKE_file_handler_remove(FileHandlerType *file_handler);
const blender::Vector<FileHandlerType *> BKE_file_handlers();

View File

@ -29,7 +29,6 @@
#include "BKE_brush.hh"
#include "BKE_cachefile.h"
#include "BKE_callbacks.h"
#include "BKE_file_handler.hh"
#include "BKE_global.h"
#include "BKE_idprop.h"
#include "BKE_image.h"
@ -84,8 +83,6 @@ void BKE_blender_free()
IMB_moviecache_destruct();
BKE_node_system_exit();
BKE_file_handlers_free();
}
/** \} */

View File

@ -2,36 +2,60 @@
#include "BKE_file_handler.hh"
static blender::Vector<FileHandlerType *> *file_handlers = nullptr;
#include "BLI_string.h"
brecht marked this conversation as resolved Outdated

I think this can be simplified to static blender::Vector<FileHandlerType> file_handlers. For new C++ we should try to avoid this kind of manual memory allocation.

The only thing is that it should be defined inside a function, so that it is lazily initialized, after the memory allocator is initialized.

I think this can be simplified to `static blender::Vector<FileHandlerType> file_handlers`. For new C++ we should try to avoid this kind of manual memory allocation. The only thing is that it should be defined inside a function, so that it is lazily initialized, after the memory allocator is initialized.

I had to use static blender::RawVector<std::unique_ptr<FileHandlerType>> file_handlers;
Vector will not be released on closing
And had to use a unique_ptr, the RNAStruc pointer can be invalid if the array is rearranged (by removing or adding new elements) with just RawVector<FileHandlerType>

I had to use `static blender::RawVector<std::unique_ptr<FileHandlerType>> file_handlers;` `Vector` will not be released on closing And had to use a `unique_ptr`, the RNAStruc pointer can be invalid if the array is rearranged (by removing or adding new elements) with just `RawVector<FileHandlerType>`

Ok, that makes sense. Calling clear_and_shrink on Vector would have worked as well.

Ok, that makes sense. Calling `clear_and_shrink` on `Vector` would have worked as well.
void BKE_file_handlers_init()
static blender::RawVector<std::unique_ptr<FileHandlerType>> &file_handlers()
{
file_handlers = MEM_new<blender::Vector<FileHandlerType *>>(__func__);
static blender::RawVector<std::unique_ptr<FileHandlerType>> file_handlers;
return file_handlers;
}
void BKE_file_handlers_free()
const blender::Vector<FileHandlerType *> BKE_file_handlers()
brecht marked this conversation as resolved Outdated

I don't think it's necessary to allocate another vector, can't this just return a const RawVector directly? It's less convenient, but I don't think we should be making copies just to iterate over this.

I don't think it's necessary to allocate another vector, can't this just return a `const RawVector` directly? It's less convenient, but I don't think we should be making copies just to iterate over this.

My concern was more about using unique_ptr, but returning the RawVector as a const disables the data ownership transfer, so it really shouldn't be a problem.

My concern was more about using `unique_ptr`, but returning the RawVector as a const disables the data ownership transfer, so it really shouldn't be a problem.
{
for (auto *file_handler : *file_handlers) {
if (file_handler->rna_ext.free) {
file_handler->rna_ext.free(file_handler->rna_ext.data);
}
MEM_delete(file_handler);
blender::Vector<FileHandlerType *> result;
result.reserve(file_handlers().size());
for (const std::unique_ptr<FileHandlerType> &file_handle : file_handlers()) {
result.append(file_handle.get());
}
MEM_delete(file_handlers);
return result;
}
blender::Span<FileHandlerType *> BKE_file_handlers()
FileHandlerType *BKE_file_handler_find(const char *name)
{
return file_handlers->as_span();
auto itr = std::find_if(file_handlers().begin(),
file_handlers().end(),
[name](const std::unique_ptr<FileHandlerType> &file_handler) {
return STREQ(name, file_handler->idname);
});
if (itr != file_handlers().end()) {
return itr->get();
}
return nullptr;
guishe marked this conversation as resolved Outdated

Useless empty line

Useless empty line
}
void BKE_file_handler_add(FileHandlerType *file_handler)
void BKE_file_handler_add(std::unique_ptr<FileHandlerType> file_handler)
{
file_handlers->append(file_handler);
/* Load all extensions from the string list into the list. */
const char char_separator = ';';
const char *char_begin = file_handler->file_extensions_str;
const char *char_end = BLI_strchr_or_end(char_begin, char_separator);
while (char_begin[0]) {
if (char_end - char_begin > 1) {
std::string file_extension(char_begin, char_end - char_begin);
file_handler->file_extensions.append(file_extension);
}
char_begin = char_end[0] ? char_end + 1 : char_end;
char_end = BLI_strchr_or_end(char_begin, char_separator);
}
file_handlers().append(std::move(file_handler));
}
void BKE_file_handler_remove(FileHandlerType *file_handler)
{
file_handlers->remove(file_handlers->first_index_of(file_handler));
MEM_delete(file_handler);
file_handlers().remove_if(
[file_handler](const std::unique_ptr<FileHandlerType> &test_file_handler) {
return test_file_handler.get() == file_handler;
});
}

View File

@ -18,7 +18,6 @@
#include "BKE_screen.hh"
#include "BLI_listbase.h"
#include "BLI_string.h"
#include "RNA_define.hh"
@ -1531,13 +1530,9 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
}
/* Check if we have registered this file handler type before, and remove it. */
for (auto *iter_file_handler_type : BKE_file_handlers()) {
if (STREQ(iter_file_handler_type->idname, dummy_file_handler_type.idname)) {
if (iter_file_handler_type->rna_ext.srna) {
rna_FileHandler_unregister(bmain, iter_file_handler_type->rna_ext.srna);
}
break;
}
auto registered_file_handler = BKE_file_handler_find(dummy_file_handler_type.idname);
if (registered_file_handler) {
rna_FileHandler_unregister(bmain, registered_file_handler->rna_ext.srna);
}
if (!RNA_struct_available_or_report(reports, dummy_file_handler_type.idname)) {
@ -1548,42 +1543,30 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
}
/* Create the new file handler type. */
FileHandlerType *file_handler_type = MEM_new<FileHandlerType>(__func__);
std::unique_ptr<FileHandlerType> file_handler_type = std::make_unique<FileHandlerType>();
*file_handler_type = dummy_file_handler_type;
/* Gather all extensions from a string into a list. */
const char char_separator = ';';
const char *char_begin = file_handler_type->file_extensions_str;
const char *char_end = BLI_strchr_or_end(char_begin, char_separator);
while (char_begin[0]) {
if (char_end - char_begin > 1) {
bFileExtension extension;
BLI_strncpy(extension.extension, char_begin, char_end - char_begin + 1);
file_handler_type->file_extensions.append(extension);
}
char_begin = char_end[0] ? char_end + 1 : char_end;
char_end = BLI_strchr_or_end(char_begin, char_separator);
}
file_handler_type->rna_ext.srna = RNA_def_struct_ptr(
&BLENDER_RNA, file_handler_type->idname, &RNA_FileHandler);
file_handler_type->rna_ext.data = data;
file_handler_type->rna_ext.call = call;
file_handler_type->rna_ext.free = free;
RNA_struct_blender_type_set(file_handler_type->rna_ext.srna, file_handler_type);
RNA_struct_blender_type_set(file_handler_type->rna_ext.srna, file_handler_type.get());
file_handler_type->poll_drop = have_function[0] ? file_handler_poll_drop : nullptr;
BKE_file_handler_add(file_handler_type);
auto srna = file_handler_type->rna_ext.srna;
BKE_file_handler_add(std::move(file_handler_type));
return file_handler_type->rna_ext.srna;
return srna;
}
static StructRNA *rna_FileHandler_refine(PointerRNA *file_handler_ptr)
{
FileHandler *file_handler = (FileHandler *)file_handler_ptr->data;
guishe marked this conversation as resolved

Can this parsing be moved into file_handler.cc?

Can this parsing be moved into `file_handler.cc`?
return (file_handler && file_handler->type->rna_ext.srna) ? file_handler->type->rna_ext.srna :
&RNA_FileHandler;
return (file_handler->type && file_handler->type->rna_ext.srna) ?
file_handler->type->rna_ext.srna :
&RNA_FileHandler;
}
#else /* RNA_RUNTIME */
@ -2339,13 +2322,13 @@ static void rna_def_file_handler(BlenderRNA *brna)
srna = RNA_def_struct(brna, "FileHandler", nullptr);
RNA_def_struct_ui_text(srna, "File Handler Type", "I/O File handler");
guishe marked this conversation as resolved Outdated

That description is not really useful, just rewording the struct label.

Better to get a short explanation of what is a file handler here.

That description is not really useful, just rewording the struct label. Better to get a short explanation of what is a file handler here.

As it is currently it will only add drag and drop support, I think this would fit:

Extends functionality to operators that manages files, such as adding drag and drop support.

As it is currently it will only add drag and drop support, I think this would fit: `Extends functionality to operators that manages files, such as adding drag and drop support.`
RNA_def_struct_sdna(srna, "FileHandler");
RNA_def_struct_refine_func(srna, "rna_FileHandler_refine");
RNA_def_struct_register_funcs(
srna, "rna_FileHandler_register", "rna_FileHandler_unregister", nullptr);
RNA_def_struct_translation_context(srna, BLT_I18NCONTEXT_DEFAULT_BPYRNA);
RNA_def_struct_flag(srna, STRUCT_PUBLIC_NAMESPACE_INHERIT);
/* registration */
prop = RNA_def_property(srna, "bl_idname", PROP_STRING, PROP_NONE);

View File

@ -59,7 +59,6 @@
#include "BKE_addon.h"
#include "BKE_appdir.h"
#include "BKE_file_handler.hh"
#include "BKE_mask.h" /* free mask clipboard */
#include "BKE_material.h" /* BKE_material_copybuf_clear */
#include "BKE_studiolight.h"
@ -217,8 +216,6 @@ void WM_init(bContext *C, int argc, const char **argv)
BKE_addon_pref_type_init();
BKE_file_handlers_init();
BKE_keyconfig_pref_type_init();
wm_operatortype_init();