griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Cleanup: Miscellaneous C++ changes to file handlers

Browse Source 
- Move code to C++ namespace for blenkernel
- Remove unnecessary prefixes based on namespace change
- Remove use of `RawVector` for function-scoped static variable
- Use `StringRef` instead of char pointer
- Use safer `STRNCPY` instead of `strcpy` in tests
- Give span instead of vector to users of API

Pull Request: https://projects.blender.org/blender/blender/pulls/116808
This commit is contained in:
Hans Goudey
2024-01-05 05:35:29 +01:00
parent 709dcc50cf
commit 4a95ead054
5 changed files with 111 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
source/blender/blenkernel/BKE_file_handler.hh
View File

@@ -2,6 +2,8 @@
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "BLI_span.hh"
#include "BLI_string_ref.hh"
#include "BLI_vector.hh"
#include "DNA_windowmanager_types.h"
@@ -10,6 +12,8 @@
struct bContext;
namespace blender::bke {
#define FH_MAX_FILE_EXTENSIONS_STR 512
struct FileHandlerType {
@@ -27,7 +31,7 @@ struct FileHandlerType {
bool (*poll_drop)(const bContext *C, FileHandlerType *file_handle_type);
/** List of file extensions supported by the file handler. */
blender::Vector<std::string> file_extensions;
Vector<std::string> file_extensions;
/** RNA integration. */
ExtensionRNA rna_ext;
@@ -40,14 +44,18 @@ struct FileHandlerType {
*
* The new `file_handler` is expected to have a unique `FileHandlerType.idname`.
*/
void BKE_file_handler_add(std::unique_ptr<FileHandlerType> file_handler);
void file_handler_add(std::unique_ptr<FileHandlerType> file_handler);
/** Returns a `file_handler` that have a specific `idname`, otherwise return `nullptr`. */
FileHandlerType *BKE_file_handler_find(const char *idname);
FileHandlerType *file_handler_find(StringRef idname);
/** Removes and frees a specific `file_handler` from the `file_handlers` list, the `file_handler`
* pointer will be not longer valid for use. */
void BKE_file_handler_remove(FileHandlerType *file_handler);
/**
* Removes and frees a specific `file_handler` from the `file_handlers` list, the `file_handler`
* pointer will be not longer valid for use.
*/
void file_handler_remove(FileHandlerType *file_handler);
/** Return a reference of the #RawVector with all `file_handlers` registered. */
const blender::RawVector<std::unique_ptr<FileHandlerType>> &BKE_file_handlers();
/** Return pointers to all registered file handlers. */
Span<std::unique_ptr<FileHandlerType>> file_handlers();
} // namespace blender::bke

26
source/blender/blenkernel/intern/file_handler.cc
View File

@@ -6,23 +6,25 @@
#include "BLI_string.h"
static blender::RawVector<std::unique_ptr<FileHandlerType>> &file_handlers()
namespace blender::bke {
static Vector<std::unique_ptr<FileHandlerType>> &file_handlers_vector()
{
static blender::RawVector<std::unique_ptr<FileHandlerType>> file_handlers;
static Vector<std::unique_ptr<FileHandlerType>> file_handlers;
return file_handlers;
}
const blender::RawVector<std::unique_ptr<FileHandlerType>> &BKE_file_handlers()
Span<std::unique_ptr<FileHandlerType>> file_handlers()
{
return file_handlers();
return file_handlers_vector();
}
FileHandlerType *BKE_file_handler_find(const char *name)
FileHandlerType *file_handler_find(const StringRef name)
{
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);
return name == file_handler->idname;
});
if (itr != file_handlers().end()) {
return itr->get();
@@ -30,9 +32,9 @@ FileHandlerType *BKE_file_handler_find(const char *name)
return nullptr;
}
void BKE_file_handler_add(std::unique_ptr<FileHandlerType> file_handler)
void file_handler_add(std::unique_ptr<FileHandlerType> file_handler)
{
BLI_assert(BKE_file_handler_find(file_handler->idname) == nullptr);
BLI_assert(file_handler_find(file_handler->idname) == nullptr);
/** Load all extensions from the string list into the list. */
const char char_separator = ';';
@@ -47,13 +49,15 @@ void BKE_file_handler_add(std::unique_ptr<FileHandlerType> file_handler)
char_end = BLI_strchr_or_end(char_begin, char_separator);
}
file_handlers().append(std::move(file_handler));
file_handlers_vector().append(std::move(file_handler));
}
void BKE_file_handler_remove(FileHandlerType *file_handler)
void file_handler_remove(FileHandlerType *file_handler)
{
file_handlers().remove_if(
file_handlers_vector().remove_if(
[file_handler](const std::unique_ptr<FileHandlerType> &test_file_handler) {
return test_file_handler.get() == file_handler;
});
}
} // namespace blender::bke

119
source/blender/blenkernel/intern/file_handler_test.cc
View File

@@ -3,11 +3,14 @@
* SPDX-License-Identifier: Apache-2.0 */
#include "BKE_file_handler.hh"
#include "BLI_string.h"
#include "testing/testing.h"
namespace blender::tests {
namespace blender::bke::tests {
#define MAX_FILE_HANDLERS_TEST_SIZE 8
static FileHandlerType *file_handlers[MAX_FILE_HANDLERS_TEST_SIZE];
static FileHandlerType *test_file_handlers[MAX_FILE_HANDLERS_TEST_SIZE];
static void file_handler_add_test(const int test_number,
const char *idname,
@@ -17,20 +20,20 @@ static void file_handler_add_test(const int test_number,
{
EXPECT_LE(test_number, MAX_FILE_HANDLERS_TEST_SIZE);
EXPECT_GE(test_number, 1);
EXPECT_EQ(BKE_file_handlers().size(), test_number - 1);
EXPECT_EQ(file_handlers().size(), test_number - 1);
std::unique_ptr<FileHandlerType> file_handler = std::make_unique<FileHandlerType>();
file_handlers[test_number - 1] = file_handler.get();
test_file_handlers[test_number - 1] = file_handler.get();
strcpy(file_handler->idname, idname);
strcpy(file_handler->file_extensions_str, file_extensions_str);
strcpy(file_handler->label, label);
STRNCPY(file_handler->idname, idname);
STRNCPY(file_handler->file_extensions_str, file_extensions_str);
STRNCPY(file_handler->label, label);
BKE_file_handler_add(std::move(file_handler));
EXPECT_EQ(BKE_file_handlers().size(), test_number);
EXPECT_EQ(BKE_file_handlers()[test_number - 1].get(), file_handlers[test_number - 1]);
EXPECT_EQ(BKE_file_handlers()[test_number - 1]->file_extensions, expected_file_extensions);
file_handler_add(std::move(file_handler));
EXPECT_EQ(file_handlers().size(), test_number);
EXPECT_EQ(file_handlers()[test_number - 1].get(), test_file_handlers[test_number - 1]);
EXPECT_EQ(file_handlers()[test_number - 1]->file_extensions, expected_file_extensions);
}
TEST(file_handler, add)
@@ -51,63 +54,63 @@ TEST(file_handler, add)
TEST(file_handler, find)
{
EXPECT_EQ(BKE_file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender1"), file_handlers[0]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender2"), file_handlers[1]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender3"), file_handlers[2]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender4"), file_handlers[3]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender5"), file_handlers[4]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender6"), file_handlers[5]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender7"), file_handlers[6]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender8"), file_handlers[7]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blende"), nullptr);
EXPECT_EQ(BKE_file_handler_find("TstFH_blen"), nullptr);
EXPECT_EQ(file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE);
EXPECT_EQ(file_handler_find("Test_FH_blender1"), test_file_handlers[0]);
EXPECT_EQ(file_handler_find("Test_FH_blender2"), test_file_handlers[1]);
EXPECT_EQ(file_handler_find("Test_FH_blender3"), test_file_handlers[2]);
EXPECT_EQ(file_handler_find("Test_FH_blender4"), test_file_handlers[3]);
EXPECT_EQ(file_handler_find("Test_FH_blender5"), test_file_handlers[4]);
EXPECT_EQ(file_handler_find("Test_FH_blender6"), test_file_handlers[5]);
EXPECT_EQ(file_handler_find("Test_FH_blender7"), test_file_handlers[6]);
EXPECT_EQ(file_handler_find("Test_FH_blender8"), test_file_handlers[7]);
EXPECT_EQ(file_handler_find("Test_FH_blende"), nullptr);
EXPECT_EQ(file_handler_find("TstFH_blen"), nullptr);
}
TEST(file_handler, remove)
{
EXPECT_EQ(BKE_file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE);
EXPECT_EQ(file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE);
BKE_file_handler_remove(BKE_file_handler_find("Test_FH_blender2"));
file_handler_remove(file_handler_find("Test_FH_blender2"));
EXPECT_EQ(BKE_file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE - 1);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender2"), nullptr);
EXPECT_EQ(file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE - 1);
EXPECT_EQ(file_handler_find("Test_FH_blender2"), nullptr);
/** `FileHandlerType` pointer in `file_handlers[1]` is not longer valid. */
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender1"), file_handlers[0]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender3"), file_handlers[2]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender4"), file_handlers[3]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender5"), file_handlers[4]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender6"), file_handlers[5]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender7"), file_handlers[6]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender8"), file_handlers[7]);
/** `FileHandlerType` pointer in `test_file_handlers[1]` is not longer valid. */
EXPECT_EQ(file_handler_find("Test_FH_blender1"), test_file_handlers[0]);
EXPECT_EQ(file_handler_find("Test_FH_blender3"), test_file_handlers[2]);
EXPECT_EQ(file_handler_find("Test_FH_blender4"), test_file_handlers[3]);
EXPECT_EQ(file_handler_find("Test_FH_blender5"), test_file_handlers[4]);
EXPECT_EQ(file_handler_find("Test_FH_blender6"), test_file_handlers[5]);
EXPECT_EQ(file_handler_find("Test_FH_blender7"), test_file_handlers[6]);
EXPECT_EQ(file_handler_find("Test_FH_blender8"), test_file_handlers[7]);
EXPECT_EQ(BKE_file_handlers()[0].get(), file_handlers[0]);
EXPECT_EQ(BKE_file_handlers()[1].get(), file_handlers[2]);
EXPECT_EQ(BKE_file_handlers()[2].get(), file_handlers[3]);
EXPECT_EQ(BKE_file_handlers()[3].get(), file_handlers[4]);
EXPECT_EQ(BKE_file_handlers()[4].get(), file_handlers[5]);
EXPECT_EQ(BKE_file_handlers()[5].get(), file_handlers[6]);
EXPECT_EQ(BKE_file_handlers()[6].get(), file_handlers[7]);
EXPECT_EQ(file_handlers()[0].get(), test_file_handlers[0]);
EXPECT_EQ(file_handlers()[1].get(), test_file_handlers[2]);
EXPECT_EQ(file_handlers()[2].get(), test_file_handlers[3]);
EXPECT_EQ(file_handlers()[3].get(), test_file_handlers[4]);
EXPECT_EQ(file_handlers()[4].get(), test_file_handlers[5]);
EXPECT_EQ(file_handlers()[5].get(), test_file_handlers[6]);
EXPECT_EQ(file_handlers()[6].get(), test_file_handlers[7]);
BKE_file_handler_remove(BKE_file_handler_find("Test_FH_blender8"));
file_handler_remove(file_handler_find("Test_FH_blender8"));
EXPECT_EQ(BKE_file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE - 2);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender8"), nullptr);
EXPECT_EQ(file_handlers().size(), MAX_FILE_HANDLERS_TEST_SIZE - 2);
EXPECT_EQ(file_handler_find("Test_FH_blender8"), nullptr);
/** `FileHandlerType` pointer in `file_handlers[7]` is not longer valid. */
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender1"), file_handlers[0]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender3"), file_handlers[2]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender4"), file_handlers[3]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender5"), file_handlers[4]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender6"), file_handlers[5]);
EXPECT_EQ(BKE_file_handler_find("Test_FH_blender7"), file_handlers[6]);
/** `FileHandlerType` pointer in `test_file_handlers[7]` is not longer valid. */
EXPECT_EQ(file_handler_find("Test_FH_blender1"), test_file_handlers[0]);
EXPECT_EQ(file_handler_find("Test_FH_blender3"), test_file_handlers[2]);
EXPECT_EQ(file_handler_find("Test_FH_blender4"), test_file_handlers[3]);
EXPECT_EQ(file_handler_find("Test_FH_blender5"), test_file_handlers[4]);
EXPECT_EQ(file_handler_find("Test_FH_blender6"), test_file_handlers[5]);
EXPECT_EQ(file_handler_find("Test_FH_blender7"), test_file_handlers[6]);
EXPECT_EQ(BKE_file_handlers()[0].get(), file_handlers[0]);
EXPECT_EQ(BKE_file_handlers()[1].get(), file_handlers[2]);
EXPECT_EQ(BKE_file_handlers()[2].get(), file_handlers[3]);
EXPECT_EQ(BKE_file_handlers()[3].get(), file_handlers[4]);
EXPECT_EQ(BKE_file_handlers()[4].get(), file_handlers[5]);
EXPECT_EQ(BKE_file_handlers()[5].get(), file_handlers[6]);
EXPECT_EQ(file_handlers()[0].get(), test_file_handlers[0]);
EXPECT_EQ(file_handlers()[1].get(), test_file_handlers[2]);
EXPECT_EQ(file_handlers()[2].get(), test_file_handlers[3]);
EXPECT_EQ(file_handlers()[3].get(), test_file_handlers[4]);
EXPECT_EQ(file_handlers()[4].get(), test_file_handlers[5]);
EXPECT_EQ(file_handlers()[5].get(), test_file_handlers[6]);
}
} // namespace blender::tests
} // namespace blender::bke::tests

10
source/blender/makesdna/DNA_screen_types.h
View File

@@ -32,6 +32,14 @@ struct wmDrawBuffer;
struct wmTimer;
struct wmTooltipState;
struct Panel_Runtime;
#ifdef __cplusplus
namespace blender::bke {
struct FileHandlerType;
}
using FileHandlerTypeHandle = blender::bke::FileHandlerType;
#else
typedef struct FileHandlerTypeHandle FileHandlerTypeHandle;
#endif
/* TODO: Doing this is quite ugly :)
* Once the top-bar is merged bScreen should be refactored to use ScrAreaMap. */
@@ -826,5 +834,5 @@ ENUM_OPERATORS(AssetShelfSettings_DisplayFlag, ASSETSHELF_SHOW_NAMES);
typedef struct FileHandler {
DNA_DEFINE_CXX_METHODS(FileHandler)
/** Runtime. */
struct FileHandlerType *type;
FileHandlerTypeHandle *type;
} FileHandler;

18
source/blender/makesrna/intern/rna_ui.cc
View File

@@ -1455,7 +1455,8 @@ static void rna_UILayout_property_decorate_set(PointerRNA *ptr, bool value)
/* File Handler */
static bool file_handler_poll_drop(const bContext *C, FileHandlerType *file_handler_type)
static bool file_handler_poll_drop(const bContext *C,
blender::bke::FileHandlerType *file_handler_type)
{
extern FunctionRNA rna_FileHandler_poll_drop_func;
@@ -1480,7 +1481,8 @@ static bool file_handler_poll_drop(const bContext *C, FileHandlerType *file_hand
static bool rna_FileHandler_unregister(Main * /*bmain*/, StructRNA *type)
{
FileHandlerType *file_handler_type = static_cast<FileHandlerType *>(
using namespace blender;
bke::FileHandlerType *file_handler_type = static_cast<bke::FileHandlerType *>(
RNA_struct_blender_type_get(type));
if (!file_handler_type) {
@@ -1490,7 +1492,7 @@ static bool rna_FileHandler_unregister(Main * /*bmain*/, StructRNA *type)
RNA_struct_free_extension(type, &file_handler_type->rna_ext);
RNA_struct_free(&BLENDER_RNA, type);
BKE_file_handler_remove(file_handler_type);
bke::file_handler_remove(file_handler_type);
return true;
}
@@ -1503,8 +1505,8 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
StructCallbackFunc call,
StructFreeFunc free)
{
FileHandlerType dummy_file_handler_type{};
using namespace blender;
bke::FileHandlerType dummy_file_handler_type{};
FileHandler dummy_file_handler{};
dummy_file_handler.type = &dummy_file_handler_type;
@@ -1530,7 +1532,7 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
}
/* Check if there is a file handler registered with the same `idname`, and remove it. */
auto registered_file_handler = BKE_file_handler_find(dummy_file_handler_type.idname);
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);
}
@@ -1543,7 +1545,7 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
}
/* Create the new file handler type. */
std::unique_ptr<FileHandlerType> file_handler_type = std::make_unique<FileHandlerType>();
auto file_handler_type = std::make_unique<bke::FileHandlerType>();
*file_handler_type = dummy_file_handler_type;
file_handler_type->rna_ext.srna = RNA_def_struct_ptr(
@@ -1556,7 +1558,7 @@ static StructRNA *rna_FileHandler_register(Main *bmain,
file_handler_type->poll_drop = have_function[0] ? file_handler_poll_drop : nullptr;
auto srna = file_handler_type->rna_ext.srna;
BKE_file_handler_add(std::move(file_handler_type));
bke::file_handler_add(std::move(file_handler_type));
return srna;
}