From dbea0eb38f5e36a4ac847a9770900deadd5f0e79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Mon, 4 Nov 2024 09:53:46 +0100 Subject: [PATCH] Deprecate the pointless unsafe threading model for rendering --- core/config/project_settings.cpp | 2 +- core/io/resource_loader.cpp | 2 +- core/os/os.h | 10 ++----- doc/classes/ProjectSettings.xml | 4 +-- main/main.cpp | 50 +++++++++++++++++++------------- platform/macos/os_macos.mm | 2 +- 6 files changed, 37 insertions(+), 33 deletions(-) diff --git a/core/config/project_settings.cpp b/core/config/project_settings.cpp index 092177bc154a..45d8d3042eef 100644 --- a/core/config/project_settings.cpp +++ b/core/config/project_settings.cpp @@ -1507,7 +1507,7 @@ ProjectSettings::ProjectSettings() { GLOBAL_DEF("display/window/frame_pacing/android/enable_frame_pacing", true); GLOBAL_DEF(PropertyInfo(Variant::INT, "display/window/frame_pacing/android/swappy_mode", PROPERTY_HINT_ENUM, "pipeline_forced_on,auto_fps_pipeline_forced_on,auto_fps_auto_pipeline"), 2); - custom_prop_info["rendering/driver/threads/thread_model"] = PropertyInfo(Variant::INT, "rendering/driver/threads/thread_model", PROPERTY_HINT_ENUM, "Single-Unsafe,Single-Safe,Multi-Threaded"); + GLOBAL_DEF("rendering/driver/run_on_separate_thread", false); GLOBAL_DEF("physics/2d/run_on_separate_thread", false); GLOBAL_DEF("physics/3d/run_on_separate_thread", false); diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index c8c7d430ccd5..728a20c7dba0 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -872,7 +872,7 @@ bool ResourceLoader::_ensure_load_progress() { // Some servers may need a new engine iteration to allow the load to progress. // Since the only known one is the rendering server (in single thread mode), let's keep it simple and just sync it. // This may be refactored in the future to support other servers and have less coupling. - if (OS::get_singleton()->get_render_thread_mode() == OS::RENDER_SEPARATE_THREAD) { + if (OS::get_singleton()->is_separate_thread_rendering_enabled()) { return false; // Not needed. } RenderingServer::get_singleton()->sync(); diff --git a/core/os/os.h b/core/os/os.h index 4bb177eb77a4..a56f9c8eecd4 100644 --- a/core/os/os.h +++ b/core/os/os.h @@ -91,19 +91,13 @@ class OS { typedef void (*ImeCallback)(void *p_inp, const String &p_text, Point2 p_selection); typedef bool (*HasServerFeatureCallback)(const String &p_feature); - enum RenderThreadMode { - RENDER_THREAD_UNSAFE, - RENDER_THREAD_SAFE, - RENDER_SEPARATE_THREAD - }; - protected: friend class Main; // Needed by tests to setup command-line args. friend int test_main(int argc, char *argv[]); HasServerFeatureCallback has_server_feature_callback = nullptr; - RenderThreadMode _render_thread_mode = RENDER_THREAD_SAFE; + bool _separate_thread_render = false; // Functions used by Main to initialize/deinitialize the OS. void add_logger(Logger *p_logger); @@ -261,7 +255,7 @@ class OS { virtual uint64_t get_static_memory_peak_usage() const; virtual Dictionary get_memory_info() const; - RenderThreadMode get_render_thread_mode() const { return _render_thread_mode; } + bool is_separate_thread_rendering_enabled() const { return _separate_thread_render; } virtual String get_locale() const; String get_locale_language() const; diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index 3358e6838c5f..56d284572b5a 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -2449,8 +2449,8 @@ If [code]true[/code], performs a previous depth pass before rendering 3D materials. This increases performance significantly in scenes with high overdraw, when complex materials and lighting are used. However, in scenes with few occluded surfaces, the depth prepass may reduce performance. If your game is viewed from a fixed angle that makes it easy to avoid overdraw (such as top-down or side-scrolling perspective), consider disabling the depth prepass to improve performance. This setting can be changed at run-time to optimize performance depending on the scene currently being viewed. [b]Note:[/b] Depth prepass is only supported when using the Forward+ or Compatibility rendering method. When using the Mobile rendering method, there is no depth prepass performed. - - The thread model to use for rendering. Rendering on a thread may improve performance, but synchronizing to the main thread can cause a bit more jitter. + + If [code]true[/code], the rendering server runs on its own thread. This may improve performance, but synchronizing to the main thread can cause a bit more jitter. Default background clear color. Overridable per [Viewport] using its [Environment]. See [member Environment.background_mode] and [member Environment.background_color] in particular. To change this default color programmatically, use [method RenderingServer.set_default_clear_color]. diff --git a/main/main.cpp b/main/main.cpp index e8086db9d3c7..43614ad80ca1 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -560,7 +560,11 @@ void Main::print_help(const char *p_binary) { print_help_option("--path ", "Path to a project ( must contain a \"project.godot\" file).\n"); print_help_option("-u, --upwards", "Scan folders upwards for project.godot file.\n"); print_help_option("--main-pack ", "Path to a pack (.pck) file to load.\n"); - print_help_option("--render-thread ", "Render thread mode (\"unsafe\", \"safe\", \"separate\").\n"); +#ifdef DISABLE_DEPRECATED + print_help_option("--render-thread ", "Render thread mode (\"safe\", \"separate\").\n"); +#else + print_help_option("--render-thread ", "Render thread mode (\"unsafe\" [deprecated], \"safe\", \"separate\").\n"); +#endif print_help_option("--remote-fs
", "Remote filesystem ([:] address).\n"); print_help_option("--remote-fs-password ", "Password for remote filesystem.\n"); @@ -1002,7 +1006,7 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph bool skip_breakpoints = false; String main_pack; bool quiet_stdout = false; - int rtm = -1; + int separate_thread_render = -1; // Tri-state: -1 = not set, 0 = false, 1 = true. String remotefs; String remotefs_pass; @@ -1397,13 +1401,21 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph if (N) { if (N->get() == "safe") { - rtm = OS::RENDER_THREAD_SAFE; + separate_thread_render = 0; +#ifndef DISABLE_DEPRECATED } else if (N->get() == "unsafe") { - rtm = OS::RENDER_THREAD_UNSAFE; + OS::get_singleton()->print("The --render-thread unsafe option is unsupported in Godot 4 and will be removed.\n"); + separate_thread_render = 0; +#endif } else if (N->get() == "separate") { - rtm = OS::RENDER_SEPARATE_THREAD; + separate_thread_render = 1; } else { - OS::get_singleton()->print("Unknown render thread mode, aborting.\nValid options are 'unsafe', 'safe' and 'separate'.\n"); + OS::get_singleton()->print("Unknown render thread mode, aborting.\n"); +#ifdef DISABLE_DEPRECATED + OS::get_singleton()->print("Valid options are 'safe' and 'separate'.\n"); +#else + OS::get_singleton()->print("Valid options are 'unsafe', 'safe' and 'separate'.\n"); +#endif goto error; } @@ -2474,20 +2486,18 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph } #endif - if (rtm == -1) { - rtm = GLOBAL_DEF("rendering/driver/threads/thread_model", OS::RENDER_THREAD_SAFE); + if (separate_thread_render == -1) { + separate_thread_render = (int)GLOBAL_DEF("rendering/driver/run_on_separate_thread", false); } - if (rtm >= 0 && rtm < 3) { - if (editor || project_manager) { - // Editor and project manager cannot run with rendering in a separate thread (they will crash on startup). - rtm = OS::RENDER_THREAD_SAFE; - } + if (editor || project_manager) { + // Editor and project manager cannot run with rendering in a separate thread (they will crash on startup). + separate_thread_render = 0; + } #if !defined(THREADS_ENABLED) - rtm = OS::RENDER_THREAD_SAFE; + separate_thread_render = 0; #endif - OS::get_singleton()->_render_thread_mode = OS::RenderThreadMode(rtm); - } + OS::get_singleton()->_separate_thread_render = separate_thread_render; /* Determine audio and video drivers */ @@ -3059,10 +3069,10 @@ Error Main::setup2(bool p_show_boot_logo) { } } - if (OS::get_singleton()->_render_thread_mode == OS::RENDER_SEPARATE_THREAD) { - WARN_PRINT("The Multi-Threaded rendering thread model is experimental. Feel free to try it since it will eventually become a stable feature.\n" + if (OS::get_singleton()->_separate_thread_render) { + WARN_PRINT("The separate rendering thread feature is experimental. Feel free to try it since it will eventually become a stable feature.\n" "However, bear in mind that at the moment it can lead to project crashes or instability.\n" - "So, unless you want to test the engine, use the Single-Safe option in the project settings instead."); + "So, unless you want to test the engine, disable the \"rendering/driver/run_on_separate_thread\" project setting to false."); } /* Initialize Pen Tablet Driver */ @@ -3101,7 +3111,7 @@ Error Main::setup2(bool p_show_boot_logo) { { OS::get_singleton()->benchmark_begin_measure("Servers", "Rendering"); - rendering_server = memnew(RenderingServerDefault(OS::get_singleton()->get_render_thread_mode() == OS::RENDER_SEPARATE_THREAD)); + rendering_server = memnew(RenderingServerDefault(OS::get_singleton()->is_separate_thread_rendering_enabled())); rendering_server->init(); //rendering_server->call_set_use_vsync(OS::get_singleton()->_use_vsync); diff --git a/platform/macos/os_macos.mm b/platform/macos/os_macos.mm index d9086b8c3823..8181f1fc85ed 100644 --- a/platform/macos/os_macos.mm +++ b/platform/macos/os_macos.mm @@ -51,7 +51,7 @@ // Do not redraw when rendering is done from the separate thread, it will conflict with the OpenGL context updates. DisplayServerMacOS *ds = (DisplayServerMacOS *)DisplayServer::get_singleton(); - if (get_singleton()->get_main_loop() && ds && (get_singleton()->get_render_thread_mode() != RENDER_SEPARATE_THREAD) && !ds->get_is_resizing()) { + if (get_singleton()->get_main_loop() && ds && !get_singleton()->is_separate_thread_rendering_enabled() && !ds->get_is_resizing()) { Main::force_redraw(); if (!Main::is_iterating()) { // Avoid cyclic loop. Main::iteration();