Merge pull request #10591 from hrydgard/gl-render-manager-exit-fixes

GL render manager exit fixes

Core/Core.cpp

@@ -189,10 +189,7 @@ void UpdateRunLoop() {
 		return;
 	}
 	NativeUpdate();
-
-	if (GetUIState() != UISTATE_EXIT) {
-		NativeRender(graphicsContext);
-	}
+	NativeRender(graphicsContext);
 }
 
 void KeepScreenAwake() {
@@ -268,6 +265,7 @@ reswitch:
 		if (GetUIState() != UISTATE_INGAME) {
 			CoreStateProcessed();
 			if (GetUIState() == UISTATE_EXIT) {
+				UpdateRunLoop();
 				return;
 			}
 			Core_RunLoop(ctx);

Qt/QtMain.cpp

@@ -191,7 +191,6 @@ static int mainInternal(QApplication &a) {
 }
 
 void MainUI::EmuThreadFunc() {
-	ILOG("In emu thread");
 	setCurrentThreadName("Emu");
 
 	// There's no real requirement that NativeInit happen on this thread, though it can't hurt...
@@ -204,12 +203,11 @@ void MainUI::EmuThreadFunc() {
 		SDL_PumpEvents();
 	#endif
 		updateAccelerometer();
-		time_update();
 		UpdateRunLoop();
 	}
-	NativeShutdownGraphics();
-
 	emuThreadState = (int)EmuThreadState::STOPPED;
+
+	NativeShutdownGraphics();
 }
 
 void MainUI::EmuThreadStart() {
@@ -219,6 +217,9 @@ void MainUI::EmuThreadStart() {
 
 void MainUI::EmuThreadStop() {
 	emuThreadState = (int)EmuThreadState::QUIT_REQUESTED;
+}
+
+void MainUI::EmuThreadJoin() {
 	emuThread.join();
 	emuThread = std::thread();
 }
@@ -246,6 +247,11 @@ MainUI::~MainUI()
 	if (emuThreadState != (int)EmuThreadState::DISABLED) {
 		ILOG("EmuThreadStop");
 		EmuThreadStop();
+		while (emuThreadState != (int)EmuThreadState::STOPPED) {
+			// Need to keep eating frames to allow the EmuThread to exit correctly.
+			graphicsContext->ThreadFrame();
+		}
+		EmuThreadJoin();
 	}
 #if defined(MOBILE_DEVICE)
 	delete acc;

Qt/QtMain.h

@@ -120,6 +120,7 @@ protected:
 	void EmuThreadFunc();
 	void EmuThreadStart();
 	void EmuThreadStop();
+	void EmuThreadJoin();
 
 private:
 	QtDummyGraphicsContext *graphicsContext;

SDL/SDLMain.cpp

@@ -432,6 +432,9 @@ static void EmuThreadStart(GraphicsContext *context) {
 
 static void EmuThreadStop() {
 	emuThreadState = (int)EmuThreadState::QUIT_REQUESTED;
+}
+
+static void EmuThreadJoin() {
 	emuThread.join();
 	emuThread = std::thread();
 }
@@ -638,13 +641,13 @@ int main(int argc, char *argv[]) {
 		useEmuThread = false;
 	}
 
+	SDL_SetWindowTitle(window, (app_name_nice + " " + PPSSPP_GIT_VERSION).c_str());
+
 	// Since we render from the main thread, there's nothing done here, but we call it to avoid confusion.
 	if (!graphicsContext->InitFromRenderThread(&error_message)) {
 		printf("Init from thread error: '%s'\n", error_message.c_str());
 	}
 
-	SDL_SetWindowTitle(window, (app_name_nice + " " + PPSSPP_GIT_VERSION).c_str());
-
 #ifdef MOBILE_DEVICE
 	SDL_ShowCursor(SDL_DISABLE);
 #endif
@@ -892,7 +895,14 @@ int main(int argc, char *argv[]) {
 		framecount++;
 	}
 
-	EmuThreadStop();
+	if (useEmuThread) {
+		EmuThreadStop();
+		while (emuThreadState != (int)EmuThreadState::STOPPED) {
+			// Need to keep eating frames to allow the EmuThread to exit correctly.
+			graphicsContext->ThreadFrame();
+		}
+		EmuThreadJoin();
+	}
 
 	delete joystick;
 
@@ -902,6 +912,7 @@ int main(int argc, char *argv[]) {
 	graphicsContext->Shutdown();
 	graphicsContext->ThreadEnd();
 	graphicsContext->ShutdownFromRenderThread();
+
 	NativeShutdown();
 	delete graphicsContext;
 

Windows/EmuThread.cpp

@@ -24,99 +24,94 @@
 #include "Core/Config.h"
 #include "thread/threadutil.h"
 
-static std::mutex emuThreadLock;
+enum class EmuThreadState {
+	DISABLED,
+	START_REQUESTED,
+	RUNNING,
+	QUIT_REQUESTED,
+	STOPPED,
+};
+
 static std::thread emuThread;
-static std::atomic<int> emuThreadState;
+static std::atomic<int> emuThreadState((int)EmuThreadState::DISABLED);
 
-static std::mutex renderThreadLock;
-static std::thread renderThread;
-static std::atomic<int> renderThreadReady;
-
-static bool useRenderThread;
-static bool renderThreadFailed;
-static bool renderThreadSucceeded;
+static std::thread mainThread;
+static bool useEmuThread;
 static std::string g_error_message;
+static bool g_inLoop;
 
 extern std::vector<std::wstring> GetWideCmdLine();
 
 class GraphicsContext;
-
 static GraphicsContext *g_graphicsContext;
 
-enum EmuThreadStatus : int {
-	THREAD_NONE = 0,
-	THREAD_INIT,
-	THREAD_CORE_LOOP,
-	THREAD_SHUTDOWN,
-	THREAD_END,
-};
+void EmuThreadFunc(GraphicsContext *graphicsContext);
+void MainThreadFunc();
 
-void EmuThreadFunc();
-void RenderThreadFunc();
-
-void EmuThread_Start(bool separateRenderThread) {
-	std::lock_guard<std::mutex> guard(emuThreadLock);
-	emuThread = std::thread(&EmuThreadFunc);
-	useRenderThread = separateRenderThread;
-	if (useRenderThread) {
-		renderThread = std::thread(&RenderThreadFunc);
-	}
+// On most other platforms, we let the "main" thread become the render thread and
+// start a separate emu thread from that, if needed. Should probably switch to that
+// to make it the same on all platforms.
+void MainThread_Start(bool separateEmuThread) {
+	useEmuThread = separateEmuThread;
+	mainThread = std::thread(&MainThreadFunc);
 }
 
-void EmuThread_Stop() {
+void MainThread_Stop() {
 	// Already stopped?
-	{
-		std::lock_guard<std::mutex> guard(emuThreadLock);
-		if (emuThreadState == THREAD_END)
-			return;
-	}
-
 	UpdateUIState(UISTATE_EXIT);
-	Core_Stop();
-	Core_WaitInactive(800);
-	emuThread.join();
-	if (useRenderThread) {
-		renderThread.join();
-	}
-
-	PostMessage(MainWindow::GetHWND(), MainWindow::WM_USER_UPDATE_UI, 0, 0);
+	mainThread.join();
 }
 
-bool EmuThread_Ready() {
-	return emuThreadState == THREAD_CORE_LOOP;
+bool MainThread_Ready() {
+	return g_inLoop;
 }
 
-void RenderThreadFunc() {
-	setCurrentThreadName("Render");
-	renderThreadFailed = false;
-	renderThreadSucceeded = false;
-	while (!g_graphicsContext) {
-		sleep_ms(10);
-		continue;
-	}
-
-	std::string error_message;
-	if (!g_graphicsContext->InitFromRenderThread(&error_message)) {
-		g_error_message = error_message;
-		renderThreadFailed = true;
-		return;
-	} else {
-		renderThreadSucceeded = true;
-	}
-
-	g_graphicsContext->ThreadStart();
-	while (g_graphicsContext->ThreadFrame()) {
-		continue;
-	}
-	g_graphicsContext->ThreadEnd();
-	g_graphicsContext->ShutdownFromRenderThread();
-}
-
-void EmuThreadFunc() {
-	emuThreadState = THREAD_INIT;
-
+static void EmuThreadFunc(GraphicsContext *graphicsContext) {
 	setCurrentThreadName("Emu");
 
+	// There's no real requirement that NativeInit happen on this thread.
+	// We just call the update/render loop here.
+	emuThreadState = (int)EmuThreadState::RUNNING;
+
+	NativeInitGraphics(graphicsContext);
+
+	while (emuThreadState != (int)EmuThreadState::QUIT_REQUESTED) {
+		// We're here again, so the game quit.  Restart Core_Run() which controls the UI.
+		// This way they can load a new game.
+		if (!Core_IsActive())
+			UpdateUIState(UISTATE_MENU);
+		Core_Run(g_graphicsContext);
+	}
+
+	emuThreadState = (int)EmuThreadState::STOPPED;
+
+	NativeShutdownGraphics();
+}
+
+static void EmuThreadStart(GraphicsContext *graphicsContext) {
+	emuThreadState = (int)EmuThreadState::START_REQUESTED;
+	emuThread = std::thread(&EmuThreadFunc, graphicsContext);
+}
+
+static void EmuThreadStop() {
+	emuThreadState = (int)EmuThreadState::QUIT_REQUESTED;
+}
+
+static void EmuThreadJoin() {
+	emuThread.join();
+	emuThread = std::thread();
+	ILOG("EmuThreadJoin - joined");
+}
+
+void MainThreadFunc() {
+	if (useEmuThread) {
+		// We'll start up a separate thread we'll call Emu
+		setCurrentThreadName("Render");
+	} else {
+		// This is both Emu and Render.
+		setCurrentThreadName("Emu");
+	}
+
 	host = new WindowsHost(MainWindow::GetHInstance(), MainWindow::GetHWND(), MainWindow::GetDisplayHWND());
 	host->SetWindowTitle(nullptr);
 
@@ -139,18 +134,8 @@ void EmuThreadFunc() {
 	bool success = host->InitGraphics(&error_string, &g_graphicsContext);
 
 	if (success) {
-		if (!useRenderThread) {
-			// This is also the render thread.
-			success = g_graphicsContext->InitFromRenderThread(&error_string);
-		} else {
-			while (!renderThreadFailed && !renderThreadSucceeded) {
-				sleep_ms(10);
-			}
-			success = renderThreadSucceeded;
-			if (!success) {
-				error_string = g_error_message;
-			}
-		}
+		// Main thread is the render thread.
+		success = g_graphicsContext->InitFromRenderThread(&error_string);
 	}
 
 	if (!success) {
@@ -202,8 +187,12 @@ void EmuThreadFunc() {
 		exit(1);
 	}
 
-	NativeInitGraphics(g_graphicsContext);
-	NativeResized();
+	GraphicsContext *graphicsContext = g_graphicsContext;
+
+	if (!useEmuThread) {
+		NativeInitGraphics(graphicsContext);
+		NativeResized();
+	}
 
 	INFO_LOG(BOOT, "Done.");
 	_dbg_update_();
@@ -213,30 +202,59 @@ void EmuThreadFunc() {
 		goto shutdown;
 	}
 
-	emuThreadState = THREAD_CORE_LOOP;
+	g_inLoop = true;
+
+	if (useEmuThread) {
+		EmuThreadStart(graphicsContext);
+	}
+	graphicsContext->ThreadStart();
 
 	if (g_Config.bBrowse)
 		PostMessage(MainWindow::GetHWND(), WM_COMMAND, ID_FILE_LOAD, 0);
 
-	Core_EnableStepping(FALSE);
+	Core_EnableStepping(false);
 
-	while (GetUIState() != UISTATE_EXIT) {
-		// We're here again, so the game quit.  Restart Core_Run() which controls the UI.
-		// This way they can load a new game.
-		if (!Core_IsActive())
-			UpdateUIState(UISTATE_MENU);
-		Core_Run(g_graphicsContext);
+	if (useEmuThread) {
+		while (emuThreadState != (int)EmuThreadState::DISABLED) {
+			graphicsContext->ThreadFrame();
+			if (GetUIState() == UISTATE_EXIT) {
+				break;
+			}
+		}
+	} else {
+		while (GetUIState() != UISTATE_EXIT) {
+			// We're here again, so the game quit.  Restart Core_Run() which controls the UI.
+			// This way they can load a new game.
+			if (!Core_IsActive())
+				UpdateUIState(UISTATE_MENU);
+			Core_Run(g_graphicsContext);
+		}
+	}
+	Core_Stop();
+	Core_WaitInactive(800);
+
+	g_inLoop = false;
+
+	if (useEmuThread) {
+		EmuThreadStop();
+		while (emuThreadState != (int)EmuThreadState::STOPPED) {
+			// Need to keep eating frames to allow the EmuThread to exit correctly.
+			graphicsContext->ThreadFrame();
+		}
+		EmuThreadJoin();
 	}
 
 shutdown:
-	emuThreadState = THREAD_SHUTDOWN;
 
-	NativeShutdownGraphics();
-	if (!useRenderThread)
-		g_graphicsContext->ShutdownFromRenderThread();
+	if (!useEmuThread) {
+		NativeShutdownGraphics();
+	}
+
+	g_graphicsContext->ThreadEnd();
+	g_graphicsContext->ShutdownFromRenderThread();
 
 	// NativeShutdown deletes the graphics context through host->ShutdownGraphics().
 	NativeShutdown();
-	emuThreadState = THREAD_END;
-}
 
+	PostMessage(MainWindow::GetHWND(), MainWindow::WM_USER_UPDATE_UI, 0, 0);
+}

Windows/EmuThread.h

@@ -17,6 +17,6 @@
 
 #pragma once
 
-void EmuThread_Start(bool separateRenderThread);
-void EmuThread_Stop();
-bool EmuThread_Ready();
+void MainThread_Start(bool separateRenderThread);
+void MainThread_Stop();
+bool MainThread_Ready();

Windows/GPU/WindowsGLContext.cpp

@@ -386,8 +386,6 @@ void WindowsGLContext::SwapInterval(int interval) {
 }
 
 void WindowsGLContext::Shutdown() {
-	if (renderManager_)
-		renderManager_->StopThread();
 	glslang::FinalizeProcess();
 }
 

Windows/MainWindow.cpp

@@ -795,7 +795,7 @@ namespace MainWindow
 
 		case WM_COMMAND:
 			{
-				if (!EmuThread_Ready())
+				if (!MainThread_Ready())
 					return DefWindowProc(hWnd, message, wParam, lParam);
 
 				MainWindowMenu_Process(hWnd, wParam);
@@ -844,7 +844,7 @@ namespace MainWindow
 
 		case WM_DROPFILES:
 			{
-				if (!EmuThread_Ready())
+				if (!MainThread_Ready())
 					return DefWindowProc(hWnd, message, wParam, lParam);
 
 				HDROP hdrop = (HDROP)wParam;
@@ -866,9 +866,8 @@ namespace MainWindow
 
 		case WM_CLOSE:
 			InputDevice::StopPolling();
-			EmuThread_Stop();
+			MainThread_Stop();
 			WindowsRawInput::Shutdown();
-
 			return DefWindowProc(hWnd,message,wParam,lParam);
 
 		case WM_DESTROY:
@@ -914,10 +913,10 @@ namespace MainWindow
 		case WM_USER_RESTART_EMUTHREAD:
 			NativeSetRestarting();
 			InputDevice::StopPolling();
-			EmuThread_Stop();
+			MainThread_Stop();
 			coreState = CORE_POWERUP;
 			ResetUIState();
-			EmuThread_Start(false);
+			MainThread_Start(false);
 			InputDevice::BeginPolling();
 			break;
 

Windows/main.cpp

@@ -532,7 +532,7 @@ int WINAPI WinMain(HINSTANCE _hInstance, HINSTANCE hPrevInstance, LPSTR szCmdLin
 	// Emu thread (and render thread, if any) is always running!
 	// Only OpenGL uses an externally managed render thread (due to GL's single-threaded context design). Vulkan
 	// manages its own render thread.
-	EmuThread_Start(g_Config.iGPUBackend == (int)GPUBackend::OPENGL);
+	MainThread_Start(g_Config.iGPUBackend == (int)GPUBackend::OPENGL);
 	InputDevice::BeginPolling();
 
 	HACCEL hAccelTable = LoadAccelerators(_hInstance, (LPCTSTR)IDR_ACCELS);
@@ -586,9 +586,6 @@ int WINAPI WinMain(HINSTANCE _hInstance, HINSTANCE hPrevInstance, LPSTR szCmdLin
 
 	InputDevice::StopPolling();
 
-	// The emuthread calls NativeShutdown when shutting down.
-	EmuThread_Stop();
-
 	MainWindow::DestroyDebugWindows();
 	DialogManager::DestroyAll();
 	timeEndPeriod(1);

android/jni/AndroidEGLContext.h

@@ -14,6 +14,9 @@ public:
 	Draw::DrawContext *GetDrawContext() override {
 		return draw_;
 	}
+	bool Initialized() override {
+		return draw_ != nullptr;
+	}
 
 private:
 	Draw::DrawContext *draw_;

android/jni/AndroidGraphicsContext.h

@@ -22,4 +22,5 @@ public:
 	// This is different than the base class function since on
 	// Android (EGL, Vulkan) we do have all this info on the render thread.
 	virtual bool InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) = 0;
+	virtual bool Initialized() = 0;
 };

android/jni/AndroidJavaGLContext.cpp

@@ -20,6 +20,7 @@ bool AndroidJavaEGLGraphicsContext::InitFromRenderThread(ANativeWindow *wnd, int
 
 void AndroidJavaEGLGraphicsContext::ShutdownFromRenderThread() {
 	ILOG("AndroidJavaEGLGraphicsContext::Shutdown");
+	renderManager_->WaitUntilQueueIdle();
 	renderManager_ = nullptr;  // owned by draw_.
 	delete draw_;
 	draw_ = nullptr;

android/jni/AndroidJavaGLContext.h

@@ -15,6 +15,10 @@ public:
 		delete draw_;
 	}
 
+	bool Initialized() override {
+		return draw_ != nullptr;
+	}
+
 	// This performs the actual initialization,
 	bool InitFromRenderThread(ANativeWindow *wnd, int desiredBackbufferSizeX, int desiredBackbufferSizeY, int backbufferFormat, int androidVersion) override;
 

android/jni/AndroidVulkanContext.h

@@ -22,6 +22,10 @@ public:
 	Draw::DrawContext *GetDrawContext() override {
 		return draw_;
 	}
+	bool Initialized() override {
+		return draw_ != nullptr;
+	}
+
 private:
 	VulkanContext *g_Vulkan = nullptr;
 	Draw::DrawContext *draw_ = nullptr;

android/jni/app-android.cpp

@@ -171,6 +171,20 @@ static void EmuThreadFunc() {
 	gJvm->AttachCurrentThread(&env, nullptr);
 
 	setCurrentThreadName("Emu");
+	ILOG("Entering emu thread");
+
+	// Wait for render loop to get started.
+	if (!graphicsContext || !graphicsContext->Initialized()) {
+		ILOG("Runloop: Waiting for displayInit...");
+		while (!graphicsContext || !graphicsContext->Initialized()) {
+			sleep_ms(20);
+		}
+	} else {
+		ILOG("Runloop: Graphics context available! %p", graphicsContext);
+	}
+	NativeInitGraphics(graphicsContext);
+
+	ILOG("Graphics initialized. Entering loop.");
 
 	// There's no real requirement that NativeInit happen on this thread.
 	// We just call the update/render loop here.
@@ -179,18 +193,31 @@ static void EmuThreadFunc() {
 		UpdateRunLoopAndroid(env);
 	}
 	emuThreadState = (int)EmuThreadState::STOPPED;
+
+	NativeShutdownGraphics();
+
 	gJvm->DetachCurrentThread();
+	ILOG("Leaving emu thread");
 }
 
-static void EmuThreadStart(JNIEnv *env) {
+static void EmuThreadStart() {
+	ILOG("EmuThreadStart");
 	emuThreadState = (int)EmuThreadState::START_REQUESTED;
 	emuThread = std::thread(&EmuThreadFunc);
 }
 
+// Call EmuThreadStop first, then keep running the GPU (or eat commands)
+// as long as emuThreadState isn't STOPPED and/or there are still things queued up.
+// Only after that, call EmuThreadJoin.
 static void EmuThreadStop() {
+	ILOG("EmuThreadStop - stopping...");
 	emuThreadState = (int)EmuThreadState::QUIT_REQUESTED;
+}
+
+static void EmuThreadJoin() {
 	emuThread.join();
 	emuThread = std::thread();
+	ILOG("EmuThreadJoin - joined");
 }
 
 static void ProcessFrameCommands(JNIEnv *env);
@@ -429,9 +456,8 @@ retry:
 
 	if (useCPUThread) {
 		ILOG("NativeApp.init() - launching emu thread");
-		EmuThreadStart(env);
+		EmuThreadStart();
 	}
-	ILOG("NativeApp.init() -- end");
 }
 
 extern "C" void Java_org_ppsspp_ppsspp_NativeApp_audioInit(JNIEnv *, jclass) {
@@ -480,8 +506,13 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_pause(JNIEnv *, jclass) {
 }
 
 extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) {
-	if (useCPUThread)
+	if (useCPUThread && graphicsContext) {
 		EmuThreadStop();
+		while (emuThreadState != (int)EmuThreadState::STOPPED) {
+			graphicsContext->ThreadFrame();
+		}
+		EmuThreadJoin();
+	}
 
 	ILOG("NativeApp.shutdown() -- begin");
 	if (renderer_inited) {
@@ -507,11 +538,28 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env,
 	// We should be running on the render thread here.
 	std::string errorMessage;
 	if (renderer_inited) {
+		// Would be really nice if we could get something on the GL thread immediately when shutting down...
 		ILOG("NativeApp.displayInit() restoring");
 		graphicsContext->ThreadEnd();
+		if (useCPUThread) {
+			EmuThreadStop();
+			while (emuThreadState != (int)EmuThreadState::STOPPED) {
+				graphicsContext->ThreadFrame();
+			}
+			EmuThreadJoin();
+		} else {
+			NativeShutdownGraphics();
+		}
 		graphicsContext->ShutdownFromRenderThread();
 
+		ILOG("Shut down both threads. Now let's bring it up again!");
+
 		graphicsContext->InitFromRenderThread(nullptr, 0, 0, 0, 0);
+		if (useCPUThread) {
+			EmuThreadStart();
+		} else {
+			NativeInitGraphics(graphicsContext);
+		}
 		graphicsContext->ThreadStart();
 		ILOG("Restored.");
 	} else {
@@ -564,15 +612,6 @@ extern "C" void JNICALL Java_org_ppsspp_ppsspp_NativeApp_backbufferResize(JNIEnv
 }
 
 void UpdateRunLoopAndroid(JNIEnv *env) {
-	// Wait for render loop to get started.
-	if (!renderer_inited) {
-		ILOG("Runloop: Waiting for displayInit");
-		while (!renderer_inited) {
-			sleep_ms(20);
-		}
-		NativeInitGraphics(graphicsContext);
-	}
-
 	NativeUpdate();
 
 	NativeRender(graphicsContext);

ext/native/thread/prioritizedworkqueue.cpp

@@ -112,7 +112,7 @@ PrioritizedWorkQueueItem *PrioritizedWorkQueue::Pop() {
 static std::thread *workThread;
 
 static void threadfunc(PrioritizedWorkQueue *wq) {
-	setCurrentThreadName("PrioritizedWorkQueue");
+	setCurrentThreadName("PrioQueue");
 	while (true) {
 		PrioritizedWorkQueueItem *item = wq->Pop();
 		if (!item) {
@@ -126,7 +126,7 @@ static void threadfunc(PrioritizedWorkQueue *wq) {
 }
 
 void ProcessWorkQueueOnThreadWhile(PrioritizedWorkQueue *wq) {
-	workThread = new std::thread(std::bind(&threadfunc, wq));
+	workThread = new std::thread([=](){threadfunc(wq);});
 }
 
 void StopProcessingWorkQueue(PrioritizedWorkQueue *wq) {
@@ -135,5 +135,5 @@ void StopProcessingWorkQueue(PrioritizedWorkQueue *wq) {
 		workThread->join();
 		delete workThread;
 	}
-	workThread = 0;
+	workThread = nullptr;
 }