From 7537a0521f3b0dea7cd169d90b8800f6cdd9b2da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 17 Jan 2023 14:26:16 +0100 Subject: [PATCH 1/2] Simplify ResourceLoader error callbacks --- core/io/resource_loader.cpp | 3 --- core/io/resource_loader.h | 14 ++++++-------- editor/editor_node.cpp | 17 +++++------------ editor/editor_node.h | 10 ++++------ 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index a27341dd2cc..f852e8d3824 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -1136,10 +1136,7 @@ void ResourceLoader::initialize() {} void ResourceLoader::finalize() {} ResourceLoadErrorNotify ResourceLoader::err_notify = nullptr; -void *ResourceLoader::err_notify_ud = nullptr; - DependencyErrorNotify ResourceLoader::dep_err_notify = nullptr; -void *ResourceLoader::dep_err_notify_ud = nullptr; bool ResourceLoader::create_missing_resources_if_class_unavailable = false; bool ResourceLoader::abort_on_missing_resource = true; diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h index ffe9d5de9a8..21651d3df5b 100644 --- a/core/io/resource_loader.h +++ b/core/io/resource_loader.h @@ -89,8 +89,8 @@ public: VARIANT_ENUM_CAST(ResourceFormatLoader::CacheMode) -typedef void (*ResourceLoadErrorNotify)(void *p_ud, const String &p_text); -typedef void (*DependencyErrorNotify)(void *p_ud, const String &p_loading, const String &p_which, const String &p_type); +typedef void (*ResourceLoadErrorNotify)(const String &p_text); +typedef void (*DependencyErrorNotify)(const String &p_loading, const String &p_which, const String &p_type); typedef Error (*ResourceLoaderImport)(const String &p_path); typedef void (*ResourceLoadedCallback)(Ref p_resource, const String &p_path); @@ -220,22 +220,20 @@ public: static void notify_load_error(const String &p_err) { if (err_notify) { - err_notify(err_notify_ud, p_err); + err_notify(p_err); } } - static void set_error_notify_func(void *p_ud, ResourceLoadErrorNotify p_err_notify) { + static void set_error_notify_func(ResourceLoadErrorNotify p_err_notify) { err_notify = p_err_notify; - err_notify_ud = p_ud; } static void notify_dependency_error(const String &p_path, const String &p_dependency, const String &p_type) { if (dep_err_notify) { - dep_err_notify(dep_err_notify_ud, p_path, p_dependency, p_type); + dep_err_notify(p_path, p_dependency, p_type); } } - static void set_dependency_error_notify_func(void *p_ud, DependencyErrorNotify p_err_notify) { + static void set_dependency_error_notify_func(DependencyErrorNotify p_err_notify) { dep_err_notify = p_err_notify; - dep_err_notify_ud = p_ud; } static void set_abort_on_missing_resources(bool p_abort) { abort_on_missing_resource = p_abort; } diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index 16128603b54..7d2517180e3 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -4115,16 +4115,9 @@ void EditorNode::notify_all_debug_sessions_exited() { } void EditorNode::add_io_error(const String &p_error) { - _load_error_notify(singleton, p_error); -} - -void EditorNode::_load_error_notify(void *p_ud, const String &p_text) { - EditorNode *en = static_cast(p_ud); - if (en && en->load_error_dialog) { - en->load_errors->add_image(en->gui_base->get_theme_icon(SNAME("Error"), SNAME("EditorIcons"))); - en->load_errors->add_text(p_text + "\n"); - en->load_error_dialog->attach_and_popup_centered_ratio(0.5); - } + singleton->load_errors->add_image(singleton->gui_base->get_theme_icon(SNAME("Error"), SNAME("EditorIcons"))); + singleton->load_errors->add_text(p_error + "\n"); + singleton->load_error_dialog->attach_and_popup_centered_ratio(0.5); } bool EditorNode::_find_scene_in_use(Node *p_node, const String &p_path) const { @@ -6731,8 +6724,8 @@ EditorNode::EditorNode() { } ResourceLoader::set_abort_on_missing_resources(false); - ResourceLoader::set_error_notify_func(this, _load_error_notify); - ResourceLoader::set_dependency_error_notify_func(this, _dependency_error_report); + ResourceLoader::set_error_notify_func(&EditorNode::add_io_error); + ResourceLoader::set_dependency_error_notify_func(&EditorNode::_dependency_error_report); { // Register importers at the beginning, so dialogs are created with the right extensions. diff --git a/editor/editor_node.h b/editor/editor_node.h index 9917fa16bc6..28422390e69 100644 --- a/editor/editor_node.h +++ b/editor/editor_node.h @@ -504,12 +504,11 @@ private: static int plugin_init_callback_count; static Vector _init_callbacks; - static void _dependency_error_report(void *ud, const String &p_path, const String &p_dep, const String &p_type) { - EditorNode *en = static_cast(ud); - if (!en->dependency_errors.has(p_path)) { - en->dependency_errors[p_path] = HashSet(); + static void _dependency_error_report(const String &p_path, const String &p_dep, const String &p_type) { + if (!singleton->dependency_errors.has(p_path)) { + singleton->dependency_errors[p_path] = HashSet(); } - en->dependency_errors[p_path].insert(p_dep + "::" + p_type); + singleton->dependency_errors[p_path].insert(p_dep + "::" + p_type); } static Ref _file_dialog_get_icon(const String &p_path); @@ -518,7 +517,6 @@ private: static void _editor_file_dialog_register(EditorFileDialog *p_dialog); static void _editor_file_dialog_unregister(EditorFileDialog *p_dialog); - static void _load_error_notify(void *p_ud, const String &p_text); static void _file_access_close_error_notify(const String &p_str); static void _print_handler(void *p_this, const String &p_string, bool p_error, bool p_rich); From 45d0b38076a88f81b4d061ddae1ebf543e8ffc13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Thu, 19 Jan 2023 13:45:04 +0100 Subject: [PATCH 2/2] Enhance thread safety of loaders and importers --- core/io/resource_loader.h | 6 ++++-- editor/editor_node.cpp | 1 + editor/editor_node.h | 1 + editor/import/editor_import_collada.cpp | 7 ------- editor/import/resource_importer_shader_file.cpp | 2 +- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h index 21651d3df5b..592befb6039 100644 --- a/core/io/resource_loader.h +++ b/core/io/resource_loader.h @@ -218,18 +218,20 @@ public: static void set_timestamp_on_load(bool p_timestamp) { timestamp_on_load = p_timestamp; } static bool get_timestamp_on_load() { return timestamp_on_load; } + // Loaders can safely use this regardless which thread they are running on. static void notify_load_error(const String &p_err) { if (err_notify) { - err_notify(p_err); + callable_mp_static(err_notify).bind(p_err).call_deferred(); } } static void set_error_notify_func(ResourceLoadErrorNotify p_err_notify) { err_notify = p_err_notify; } + // Loaders can safely use this regardless which thread they are running on. static void notify_dependency_error(const String &p_path, const String &p_dependency, const String &p_type) { if (dep_err_notify) { - dep_err_notify(p_path, p_dependency, p_type); + callable_mp_static(dep_err_notify).bind(p_path, p_dependency, p_type).call_deferred(); } } static void set_dependency_error_notify_func(DependencyErrorNotify p_err_notify) { diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index 7d2517180e3..2d9f9645b8b 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -4115,6 +4115,7 @@ void EditorNode::notify_all_debug_sessions_exited() { } void EditorNode::add_io_error(const String &p_error) { + DEV_ASSERT(Thread::get_caller_id() == Thread::get_main_id()); singleton->load_errors->add_image(singleton->gui_base->get_theme_icon(SNAME("Error"), SNAME("EditorIcons"))); singleton->load_errors->add_text(p_error + "\n"); singleton->load_error_dialog->attach_and_popup_centered_ratio(0.5); diff --git a/editor/editor_node.h b/editor/editor_node.h index 28422390e69..221637be1cc 100644 --- a/editor/editor_node.h +++ b/editor/editor_node.h @@ -505,6 +505,7 @@ private: static Vector _init_callbacks; static void _dependency_error_report(const String &p_path, const String &p_dep, const String &p_type) { + DEV_ASSERT(Thread::get_caller_id() == Thread::get_main_id()); if (!singleton->dependency_errors.has(p_path)) { singleton->dependency_errors[p_path] = HashSet(); } diff --git a/editor/import/editor_import_collada.cpp b/editor/import/editor_import_collada.cpp index 1ffede65023..5f714e44885 100644 --- a/editor/import/editor_import_collada.cpp +++ b/editor/import/editor_import_collada.cpp @@ -1782,15 +1782,8 @@ Node *EditorSceneFormatImporterCollada::import_scene(const String &p_path, uint3 ERR_FAIL_COND_V_MSG(err != OK, nullptr, "Cannot load scene from file '" + p_path + "'."); if (state.missing_textures.size()) { - /* - for(int i=0;ipush_back(state.missing_textures[i]); } } diff --git a/editor/import/resource_importer_shader_file.cpp b/editor/import/resource_importer_shader_file.cpp index ba48fc90291..1275e5b85ae 100644 --- a/editor/import/resource_importer_shader_file.cpp +++ b/editor/import/resource_importer_shader_file.cpp @@ -106,7 +106,7 @@ Error ResourceImporterShaderFile::import(const String &p_source_file, const Stri if (err != OK) { if (!ShaderFileEditor::singleton->is_visible_in_tree()) { - EditorNode::get_singleton()->add_io_error(vformat(TTR("Error importing GLSL shader file: '%s'. Open the file in the filesystem dock in order to see the reason."), p_source_file)); + callable_mp_static(&EditorNode::add_io_error).bind(vformat(TTR("Error importing GLSL shader file: '%s'. Open the file in the filesystem dock in order to see the reason."), p_source_file)).call_deferred(); } }