Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate the pointless unsafe threading model for rendering #98383

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Deprecate the pointless unsafe threading model for rendering #98383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion core/config/project_settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,11 @@ 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");
#ifdef DISABLE_DEPRECATED
custom_prop_info["rendering/driver/threads/thread_model"] = PropertyInfo(Variant::INT, "rendering/driver/threads/thread_model", PROPERTY_HINT_ENUM, "Safe:1,Separate");
#else
custom_prop_info["rendering/driver/threads/thread_model"] = PropertyInfo(Variant::INT, "rendering/driver/threads/thread_model", PROPERTY_HINT_ENUM, "Unsafe (deprecated),Safe,Separate");
#endif
GLOBAL_DEF("physics/2d/run_on_separate_thread", false);
GLOBAL_DEF("physics/3d/run_on_separate_thread", false);

Expand Down
2 changes: 1 addition & 1 deletion core/io/resource_loader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions core/os/os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class OS {
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);
Expand Down Expand Up @@ -261,7 +261,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;
Expand Down
50 changes: 30 additions & 20 deletions main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,11 @@ void Main::print_help(const char *p_binary) {
print_help_option("--path <directory>", "Path to a project (<directory> 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 <file>", "Path to a pack (.pck) file to load.\n");
print_help_option("--render-thread <mode>", "Render thread mode (\"unsafe\", \"safe\", \"separate\").\n");
#ifdef DISABLE_DEPRECATED
print_help_option("--render-thread <mode>", "Render thread mode (\"safe\", \"separate\").\n");
#else
print_help_option("--render-thread <mode>", "Render thread mode (\"unsafe\" [deprecated], \"safe\", \"separate\").\n");
#endif
print_help_option("--remote-fs <address>", "Remote filesystem (<host/IP>[:<port>] address).\n");
print_help_option("--remote-fs-password <password>", "Password for remote filesystem.\n");

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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/threads/thread_model", OS::RENDER_THREAD_SAFE) == OS::RENDER_SEPARATE_THREAD;
}

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 */

Expand Down Expand Up @@ -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, set the \"rendering/driver/threads/thread_model\" project setting to 'Safe'.");
}

/* Initialize Pen Tablet Driver */
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion platform/macos/os_macos.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down