From 78eaa8c235d7737b404e74228271b566ec8b60a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= <hrydgard@gmail.com>
Date: Tue, 16 May 2023 23:14:54 +0200
Subject: [PATCH] Make sure we never copy GLRRenderThreadTask objects

---
 Common/GPU/OpenGL/GLRenderManager.cpp | 32 +++++++++++++--------------
 Common/GPU/OpenGL/GLRenderManager.h   | 13 +++++++----
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/Common/GPU/OpenGL/GLRenderManager.cpp b/Common/GPU/OpenGL/GLRenderManager.cpp
index a1b4a201c4..8e259a722c 100644
--- a/Common/GPU/OpenGL/GLRenderManager.cpp
+++ b/Common/GPU/OpenGL/GLRenderManager.cpp
@@ -129,7 +129,7 @@ bool GLRenderManager::ThreadFrame() {
 		return false;
 	}
 
-	GLRRenderThreadTask task;
+	GLRRenderThreadTask *task = nullptr;
 
 	// In case of syncs or other partial completion, we keep going until we complete a frame.
 	while (true) {
@@ -147,7 +147,7 @@ bool GLRenderManager::ThreadFrame() {
 
 		// We got a task! We can now have pushMutex_ unlocked, allowing the host to
 		// push more work when it feels like it, and just start working.
-		if (task.runType == GLRRunType::EXIT) {
+		if (task->runType == GLRRunType::EXIT) {
 			// Oh, host wanted out. Let's leave, and also let's notify the host.
 			// This is unlike Vulkan too which can just block on the thread existing.
 			std::unique_lock<std::mutex> lock(syncMutex_);
@@ -157,11 +157,13 @@ bool GLRenderManager::ThreadFrame() {
 		}
 
 		// Render the scene.
-		VLOG("  PULL: Frame %d RUN (%0.3f)", task.frame, time_now_d());
-		if (Run(task)) {
+		VLOG("  PULL: Frame %d RUN (%0.3f)", task->frame, time_now_d());
+		if (Run(*task)) {
 			// Swap requested, so we just bail the loop.
+			delete task;
 			break;
 		}
+		delete task;
 	};
 
 	return true;
@@ -174,9 +176,7 @@ void GLRenderManager::StopThread() {
 		run_ = false;
 
 		std::unique_lock<std::mutex> lock(pushMutex_);
-		GLRRenderThreadTask exitTask{};
-		exitTask.runType = GLRRunType::EXIT;
-		renderThreadQueue_.push(exitTask);
+		renderThreadQueue_.push(new GLRRenderThreadTask(GLRRunType::EXIT));
 		pushCondVar_.notify_one();
 	} else {
 		WARN_LOG(G3D, "GL submission thread was already paused.");
@@ -377,15 +377,14 @@ void GLRenderManager::Finish() {
 	frameData_[curFrame].deleter.Take(deleter_);
 
 	VLOG("PUSH: Finish, pushing task. curFrame = %d", curFrame);
-	GLRRenderThreadTask task;
-	task.frame = curFrame;
-	task.runType = GLRRunType::PRESENT;
+	GLRRenderThreadTask *task = new GLRRenderThreadTask(GLRRunType::PRESENT);
+	task->frame = curFrame;
 
 	{
 		std::unique_lock<std::mutex> lock(pushMutex_);
 		renderThreadQueue_.push(task);
-		renderThreadQueue_.back().initSteps = std::move(initSteps_);
-		renderThreadQueue_.back().steps = std::move(steps_);
+		renderThreadQueue_.back()->initSteps = std::move(initSteps_);
+		renderThreadQueue_.back()->steps = std::move(steps_);
 		initSteps_.clear();
 		steps_.clear();
 		pushCondVar_.notify_one();
@@ -507,14 +506,13 @@ void GLRenderManager::FlushSync() {
 	{
 		VLOG("PUSH: Frame[%d].readyForRun = true (sync)", curFrame_);
 
-		GLRRenderThreadTask task;
-		task.frame = curFrame_;
-		task.runType = GLRRunType::SYNC;
+		GLRRenderThreadTask *task = new GLRRenderThreadTask(GLRRunType::SYNC);
+		task->frame = curFrame_;
 
 		std::unique_lock<std::mutex> lock(pushMutex_);
 		renderThreadQueue_.push(task);
-		renderThreadQueue_.back().initSteps = std::move(initSteps_);
-		renderThreadQueue_.back().steps = std::move(steps_);
+		renderThreadQueue_.back()->initSteps = std::move(initSteps_);
+		renderThreadQueue_.back()->steps = std::move(steps_);
 		pushCondVar_.notify_one();
 		steps_.clear();
 	}
diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h
index c324287031..71304eddfe 100644
--- a/Common/GPU/OpenGL/GLRenderManager.h
+++ b/Common/GPU/OpenGL/GLRenderManager.h
@@ -203,14 +203,19 @@ enum class GLRRunType {
 class GLRenderManager;
 class GLPushBuffer;
 
-// These are enqueued from the main thread,
-// and the render thread pops them off
+// These are enqueued from the main thread, and the render thread pops them off
 struct GLRRenderThreadTask {
+	GLRRenderThreadTask(GLRRunType _runType) : runType(_runType) {}
+
 	std::vector<GLRStep *> steps;
 	std::vector<GLRInitStep> initSteps;
 
-	int frame;
+	int frame = -1;
 	GLRRunType runType;
+
+	// Avoid copying these by accident.
+	GLRRenderThreadTask(GLRRenderThreadTask &) = delete;
+	GLRRenderThreadTask& operator =(GLRRenderThreadTask &) = delete;
 };
 
 // Note: The GLRenderManager is created and destroyed on the render thread, and the latter
@@ -866,7 +871,7 @@ private:
 	std::mutex pushMutex_;
 	std::condition_variable pushCondVar_;
 
-	std::queue<GLRRenderThreadTask> renderThreadQueue_;
+	std::queue<GLRRenderThreadTask *> renderThreadQueue_;
 
 	// For readbacks and other reasons we need to sync with the render thread.
 	std::mutex syncMutex_;