From 41d94e42626a8568ed1d257bc0c2400d10928e0f Mon Sep 17 00:00:00 2001
From: CasualPokePlayer <50538166+CasualPokePlayer@users.noreply.github.com>
Date: Tue, 2 May 2023 16:52:48 -0700
Subject: [PATCH] Change up multithreaded rendering in melonDS

Instead of creating a new Task and destroying it every frame, just create a new thread and call the delegate when needed.
Also allow for rendering a frame twice, this is actually possible anyways (albeit rarely done).
This should also help against a potential deadlock due to the Task.Wait call on the UI thread (which has special semantics in WinForms)
Also minor nitpicks in RCheevos code
---
 .../RetroAchievements/RCheevos.GameInfo.cs    |  2 +-
 .../RetroAchievements/RCheevos.Http.cs        |  2 +-
 .../Consoles/Nintendo/NDS/MelonDS.cs          | 52 +++++++++++++++----
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.GameInfo.cs b/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.GameInfo.cs
index 49550ee02a..517a1417d0 100644
--- a/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.GameInfo.cs
+++ b/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.GameInfo.cs
@@ -242,7 +242,7 @@ namespace BizHawk.Client.EmuHawk
 			private LibRCheevos.rc_api_resolve_hash_request_t _apiParams;
 			public int GameID { get; private set; }
 
-			// eh? not sure I want this retried, giving the blocking behavior
+			// eh? not sure I want this retried, given the blocking behavior
 			public override bool ShouldRetry => false;
 
 			protected override void ResponseCallback(byte[] serv_resp)
diff --git a/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.Http.cs b/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.Http.cs
index 9cedbf9c45..d41b73d9fd 100644
--- a/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.Http.cs
+++ b/src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.Http.cs
@@ -156,7 +156,7 @@ namespace BizHawk.Client.EmuHawk
 		{
 			while (_isActive)
 			{
-				if (_inactiveHttpRequests.TryPop(out var request))
+				while (_inactiveHttpRequests.TryPop(out var request))
 				{
 					Task.Run(request.DoRequest);
 					_activeHttpRequests.Add(request);
diff --git a/src/BizHawk.Emulation.Cores/Consoles/Nintendo/NDS/MelonDS.cs b/src/BizHawk.Emulation.Cores/Consoles/Nintendo/NDS/MelonDS.cs
index b6843bac20..6e6d77f251 100644
--- a/src/BizHawk.Emulation.Cores/Consoles/Nintendo/NDS/MelonDS.cs
+++ b/src/BizHawk.Emulation.Cores/Consoles/Nintendo/NDS/MelonDS.cs
@@ -4,7 +4,7 @@ using System.IO;
 using System.IO.Compression;
 using System.Linq;
 using System.Text;
-using System.Threading.Tasks;
+using System.Threading;
 
 using BizHawk.BizInvoke;
 using BizHawk.Common;
@@ -211,7 +211,9 @@ namespace BizHawk.Emulation.Cores.Consoles.Nintendo.NDS
 			if (_frameThreadPtr != IntPtr.Zero)
 			{
 				Console.WriteLine($"Setting up waterbox thread for 0x{(ulong)_frameThreadPtr:X16}");
-				_frameThreadStart = CallingConventionAdapters.GetWaterboxUnsafeUnwrapped().GetDelegateForFunctionPointer<Action>(_frameThreadPtr);
+				_frameThread = new(FrameThreadProc) { IsBackground = true };
+				_frameThread.Start();
+				_frameThreadAction = CallingConventionAdapters.GetWaterboxUnsafeUnwrapped().GetDelegateForFunctionPointer<Action>(_frameThreadPtr);
 				_core.SetThreadStartCallback(_threadstartcb);
 			}
 
@@ -354,24 +356,56 @@ namespace BizHawk.Emulation.Cores.Consoles.Nintendo.NDS
 		}
 
 		private readonly IntPtr _frameThreadPtr;
-		private readonly Action _frameThreadStart;
+		private readonly Action _frameThreadAction;
 		private readonly LibMelonDS.ThreadStartCallback _threadstartcb;
 
-		private Task _frameThreadProcActive;
+		private readonly Thread _frameThread;
+		private readonly SemaphoreSlim _frameThreadStartEvent = new(0, 1);
+		private readonly SemaphoreSlim _frameThreadEndEvent = new(0, 1);
+		private bool _isDisposing;
+		private bool _renderThreadRanThisFrame;
+
+		public override void Dispose()
+		{
+			_isDisposing = true;
+			_frameThreadStartEvent.Release();
+			_frameThread?.Join();
+			_frameThreadStartEvent.Dispose();
+			_frameThreadEndEvent.Dispose();
+			base.Dispose();
+		}
+
+		private void FrameThreadProc()
+		{
+			while (true)
+			{
+				_frameThreadStartEvent.Wait();
+				if (_isDisposing) break;
+				_frameThreadAction();
+				_frameThreadEndEvent.Release();
+			}
+		}
 
 		private void ThreadStartCallback()
 		{
-			if (_frameThreadProcActive != null)
+			if (_renderThreadRanThisFrame)
 			{
-				throw new InvalidOperationException("Attempted to start render thread twice");
+				// This is technically possible due to the game able to force another frame to be rendered by touching vcount
+				// (ALSO MEANS VSYNC NUMBERS ARE KIND OF A LIE)
+				_frameThreadEndEvent.Wait();
 			}
-			_frameThreadProcActive = Task.Run(_frameThreadStart);
+
+			_renderThreadRanThisFrame = true;
+			_frameThreadStartEvent.Release();
 		}
 
 		protected override void FrameAdvancePost()
 		{
-			_frameThreadProcActive?.Wait();
-			_frameThreadProcActive = null;
+			if (_renderThreadRanThisFrame)
+			{
+				_frameThreadEndEvent.Wait();
+				_renderThreadRanThisFrame = false;
+			}
 		}
 
 		protected override void LoadStateBinaryInternal(BinaryReader reader)