Apply additional fixes to servers' threading

RandomShaper · May 6, 2024 · 52a4e12 · 52a4e12
1 parent 232eee7
commit 52a4e12
Show file tree

Hide file tree

Showing 10 changed files with 142 additions and 141 deletions.
diff --git a/core/templates/command_queue_mt.h b/core/templates/command_queue_mt.h
@@ -388,6 +388,8 @@ class CommandQueueMT {
 		} while (sync_head != sync_head_goal); // Can't use lower-than because of wraparound.
 	}
 
+	void _no_op() {}
+
 public:
 	void lock();
 	void unlock();
@@ -409,18 +411,25 @@ class CommandQueueMT {
 			_flush();
 		}
 	}
+
 	void flush_all() {
 		_flush();
 	}
 
+	void sync() {
+		push_and_sync(this, &CommandQueueMT::_no_op);
+	}
+
 	void wait_and_flush() {
 		ERR_FAIL_COND(pump_task_id == WorkerThreadPool::INVALID_TASK_ID);
 		WorkerThreadPool::get_singleton()->wait_for_task_completion(pump_task_id);
 		_flush();
 	}
 
 	void set_pump_task_id(WorkerThreadPool::TaskID p_task_id) {
+		lock();
 		pump_task_id = p_task_id;
+		unlock();
 	}
 
 	CommandQueueMT();

diff --git a/drivers/gles3/rasterizer_gles3.cpp b/drivers/gles3/rasterizer_gles3.cpp
@@ -395,10 +395,13 @@ void RasterizerGLES3::_blit_render_target_to_screen(RID p_render_target, Display
 	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, GLES3::TextureStorage::system_fbo);
 
 	if (p_first) {
-		Size2i win_size = DisplayServer::get_singleton()->window_get_size();
 		if (p_screen_rect.position != Vector2() || p_screen_rect.size != rt->size) {
 			// Viewport doesn't cover entire window so clear window to black before blitting.
-			glViewport(0, 0, win_size.width, win_size.height);
+			// Querying the actual window size from the DisplayServer would deadlock in separate render thread mode,
+			// so let's set the biggest viewport the implementation supports, to be sure the window is fully covered.
+			GLsizei max_vp[2] = {};
+			glGetIntegerv(GL_MAX_VIEWPORT_DIMS, max_vp);
+			glViewport(0, 0, max_vp[0], max_vp[1]);
 			glClearColor(0.0, 0.0, 0.0, 1.0);
 			glClear(GL_COLOR_BUFFER_BIT);
 		}

diff --git a/servers/physics_server_2d_wrap_mt.cpp b/servers/physics_server_2d_wrap_mt.cpp
@@ -32,45 +32,37 @@
 
 #include "core/os/os.h"
 
-void PhysicsServer2DWrapMT::thread_exit() {
-	exit = true;
+void PhysicsServer2DWrapMT::_assign_mt_ids(WorkerThreadPool::TaskID p_pump_task_id) {
+	server_thread = Thread::get_caller_id();
+	server_task_id = p_pump_task_id;
 }
 
-void PhysicsServer2DWrapMT::thread_step(real_t p_delta) {
-	physics_server_2d->step(p_delta);
-	step_sem.post();
+void PhysicsServer2DWrapMT::_thread_exit() {
+	exit = true;
 }
 
-void PhysicsServer2DWrapMT::thread_loop() {
-	server_thread = Thread::get_caller_id();
-
-	physics_server_2d->init();
-
-	command_queue.set_pump_task_id(server_task_id);
+void PhysicsServer2DWrapMT::_thread_loop() {
 	while (!exit) {
 		WorkerThreadPool::get_singleton()->yield();
 		command_queue.flush_all();
 	}
-
-	command_queue.flush_all();
-
-	physics_server_2d->finish();
 }
 
 /* EVENT QUEUING */
 
 void PhysicsServer2DWrapMT::step(real_t p_step) {
 	if (create_thread) {
-		command_queue.push(this, &PhysicsServer2DWrapMT::thread_step, p_step);
+		command_queue.push(physics_server_2d, &PhysicsServer2D::step, p_step);
 	} else {
-		command_queue.flush_all(); // Flush all pending from other threads.
 		physics_server_2d->step(p_step);
 	}
 }
 
 void PhysicsServer2DWrapMT::sync() {
 	if (create_thread) {
-		step_sem.wait();
+		command_queue.sync();
+	} else {
+		command_queue.flush_all(); // Flush all pending from other threads.
 	}
 	physics_server_2d->sync();
 }
@@ -85,21 +77,26 @@ void PhysicsServer2DWrapMT::end_sync() {
 
 void PhysicsServer2DWrapMT::init() {
 	if (create_thread) {
-		exit = false;
-		server_task_id = WorkerThreadPool::get_singleton()->add_task(callable_mp(this, &PhysicsServer2DWrapMT::thread_loop), true);
-		step_sem.post();
+		WorkerThreadPool::TaskID tid = WorkerThreadPool::get_singleton()->add_task(callable_mp(this, &PhysicsServer2DWrapMT::_thread_loop), true);
+		command_queue.set_pump_task_id(tid);
+		command_queue.push(this, &PhysicsServer2DWrapMT::_assign_mt_ids, tid);
+		command_queue.push_and_sync(physics_server_2d, &PhysicsServer2D::init);
+		DEV_ASSERT(server_task_id == tid);
 	} else {
+		server_thread = Thread::MAIN_ID;
 		physics_server_2d->init();
 	}
 }
 
 void PhysicsServer2DWrapMT::finish() {
 	if (create_thread) {
-		command_queue.push(this, &PhysicsServer2DWrapMT::thread_exit);
+		command_queue.push(physics_server_2d, &PhysicsServer2D::finish);
+		command_queue.push(this, &PhysicsServer2DWrapMT::_thread_exit);
 		if (server_task_id != WorkerThreadPool::INVALID_TASK_ID) {
 			WorkerThreadPool::get_singleton()->wait_for_task_completion(server_task_id);
 			server_task_id = WorkerThreadPool::INVALID_TASK_ID;
 		}
+		server_thread = Thread::MAIN_ID;
 	} else {
 		physics_server_2d->finish();
 	}
@@ -108,9 +105,6 @@ void PhysicsServer2DWrapMT::finish() {
 PhysicsServer2DWrapMT::PhysicsServer2DWrapMT(PhysicsServer2D *p_contained, bool p_create_thread) {
 	physics_server_2d = p_contained;
 	create_thread = p_create_thread;
-	if (!create_thread) {
-		server_thread = Thread::MAIN_ID;
-	}
 }
 
 PhysicsServer2DWrapMT::~PhysicsServer2DWrapMT() {

diff --git a/servers/physics_server_2d_wrap_mt.h b/servers/physics_server_2d_wrap_mt.h
@@ -53,17 +53,14 @@ class PhysicsServer2DWrapMT : public PhysicsServer2D {
 
 	mutable CommandQueueMT command_queue;
 
-	void thread_loop();
-
 	Thread::ID server_thread = Thread::UNASSIGNED_ID;
 	WorkerThreadPool::TaskID server_task_id = WorkerThreadPool::INVALID_TASK_ID;
 	bool exit = false;
-	Semaphore step_sem;
 	bool create_thread = false;
 
-	void thread_step(real_t p_delta);
-
-	void thread_exit();
+	void _assign_mt_ids(WorkerThreadPool::TaskID p_pump_task_id);
+	void _thread_exit();
+	void _thread_loop();
 
 public:
 #define ServerName PhysicsServer2D

diff --git a/servers/physics_server_3d_wrap_mt.cpp b/servers/physics_server_3d_wrap_mt.cpp
@@ -32,45 +32,37 @@
 
 #include "core/os/os.h"
 
-void PhysicsServer3DWrapMT::thread_exit() {
-	exit = true;
+void PhysicsServer3DWrapMT::_assign_mt_ids(WorkerThreadPool::TaskID p_pump_task_id) {
+	server_thread = Thread::get_caller_id();
+	server_task_id = p_pump_task_id;
 }
 
-void PhysicsServer3DWrapMT::thread_step(real_t p_delta) {
-	physics_server_3d->step(p_delta);
-	step_sem.post();
+void PhysicsServer3DWrapMT::_thread_exit() {
+	exit = true;
 }
 
-void PhysicsServer3DWrapMT::thread_loop() {
-	server_thread = Thread::get_caller_id();
-
-	physics_server_3d->init();
-
-	command_queue.set_pump_task_id(server_task_id);
+void PhysicsServer3DWrapMT::_thread_loop() {
 	while (!exit) {
 		WorkerThreadPool::get_singleton()->yield();
 		command_queue.flush_all();
 	}
-
-	command_queue.flush_all(); // flush all
-
-	physics_server_3d->finish();
 }
 
 /* EVENT QUEUING */
 
 void PhysicsServer3DWrapMT::step(real_t p_step) {
 	if (create_thread) {
-		command_queue.push(this, &PhysicsServer3DWrapMT::thread_step, p_step);
+		command_queue.push(physics_server_3d, &PhysicsServer3D::step, p_step);
 	} else {
-		command_queue.flush_all(); // Flush all pending from other threads.
 		physics_server_3d->step(p_step);
 	}
 }
 
 void PhysicsServer3DWrapMT::sync() {
 	if (create_thread) {
-		step_sem.wait();
+		command_queue.sync();
+	} else {
+		command_queue.flush_all(); // Flush all pending from other threads.
 	}
 	physics_server_3d->sync();
 }
@@ -85,21 +77,26 @@ void PhysicsServer3DWrapMT::end_sync() {
 
 void PhysicsServer3DWrapMT::init() {
 	if (create_thread) {
-		exit = false;
-		server_task_id = WorkerThreadPool::get_singleton()->add_task(callable_mp(this, &PhysicsServer3DWrapMT::thread_loop), true);
-		step_sem.post();
+		WorkerThreadPool::TaskID tid = WorkerThreadPool::get_singleton()->add_task(callable_mp(this, &PhysicsServer3DWrapMT::_thread_loop), true);
+		command_queue.set_pump_task_id(tid);
+		command_queue.push(this, &PhysicsServer3DWrapMT::_assign_mt_ids, tid);
+		command_queue.push_and_sync(physics_server_3d, &PhysicsServer3D::init);
+		DEV_ASSERT(server_task_id == tid);
 	} else {
+		server_thread = Thread::MAIN_ID;
 		physics_server_3d->init();
 	}
 }
 
 void PhysicsServer3DWrapMT::finish() {
 	if (create_thread) {
-		command_queue.push(this, &PhysicsServer3DWrapMT::thread_exit);
+		command_queue.push(physics_server_3d, &PhysicsServer3D::finish);
+		command_queue.push(this, &PhysicsServer3DWrapMT::_thread_exit);
 		if (server_task_id != WorkerThreadPool::INVALID_TASK_ID) {
 			WorkerThreadPool::get_singleton()->wait_for_task_completion(server_task_id);
 			server_task_id = WorkerThreadPool::INVALID_TASK_ID;
 		}
+		server_thread = Thread::MAIN_ID;
 	} else {
 		physics_server_3d->finish();
 	}
@@ -108,9 +105,6 @@ void PhysicsServer3DWrapMT::finish() {
 PhysicsServer3DWrapMT::PhysicsServer3DWrapMT(PhysicsServer3D *p_contained, bool p_create_thread) {
 	physics_server_3d = p_contained;
 	create_thread = p_create_thread;
-	if (!create_thread) {
-		server_thread = Thread::MAIN_ID;
-	}
 }
 
 PhysicsServer3DWrapMT::~PhysicsServer3DWrapMT() {

diff --git a/servers/physics_server_3d_wrap_mt.h b/servers/physics_server_3d_wrap_mt.h
@@ -52,17 +52,15 @@ class PhysicsServer3DWrapMT : public PhysicsServer3D {
 
 	mutable CommandQueueMT command_queue;
 
-	void thread_loop();
-
 	Thread::ID server_thread = Thread::UNASSIGNED_ID;
 	WorkerThreadPool::TaskID server_task_id = WorkerThreadPool::INVALID_TASK_ID;
 	bool exit = false;
-	Semaphore step_sem;
 	bool create_thread = false;
 
-	void thread_step(real_t p_delta);
-
-	void thread_exit();
+	void _assign_mt_ids(WorkerThreadPool::TaskID p_pump_task_id);
+	void _thread_exit();
+	void _thread_step(real_t p_delta);
+	void _thread_loop();
 
 public:
 #define ServerName PhysicsServer3D

diff --git a/servers/rendering/renderer_viewport.cpp b/servers/rendering/renderer_viewport.cpp
@@ -678,7 +678,7 @@ void RendererViewport::draw_viewports(bool p_swap_buffers) {
 #endif // _3D_DISABLED
 
 	if (Engine::get_singleton()->is_editor_hint()) {
-		set_default_clear_color(GLOBAL_GET("rendering/environment/defaults/default_clear_color"));
+		RSG::texture_storage->set_default_clear_color(GLOBAL_GET("rendering/environment/defaults/default_clear_color"));
 	}
 
 	if (sorted_active_viewports_dirty) {
@@ -1521,10 +1521,6 @@ void RendererViewport::handle_timestamp(String p_timestamp, uint64_t p_cpu_time,
 	}
 }
 
-void RendererViewport::set_default_clear_color(const Color &p_color) {
-	RSG::texture_storage->set_default_clear_color(p_color);
-}
-
 void RendererViewport::viewport_set_canvas_cull_mask(RID p_viewport, uint32_t p_canvas_cull_mask) {
 	Viewport *viewport = viewport_owner.get_or_null(p_viewport);
 	ERR_FAIL_NULL(viewport);

diff --git a/servers/rendering/renderer_viewport.h b/servers/rendering/renderer_viewport.h
@@ -303,7 +303,6 @@ class RendererViewport {
 
 	void handle_timestamp(String p_timestamp, uint64_t p_cpu_time, uint64_t p_gpu_time);
 
-	void set_default_clear_color(const Color &p_color);
 	void draw_viewports(bool p_swap_buffers);
 
 	bool free(RID p_rid);