Update GuiApi so that TestDrawingWithTwoScripts passes. This fixes the bug where if two lua scripts draw stuff on the same frame only one script's drawings are visible.

Replaces a bunch of obscure ThisIsTheLuaHack and locking/unlocking code with a simple pair of methods.

src/BizHawk.Client.Common/Api/ApiManager.cs

@@ -100,7 +100,6 @@ namespace BizHawk.Client.Common
 		{
 			_luaContainer?.Dispose();
 			_luaContainer = Register(serviceProvider, logCallback, mainForm, displayManager, inputManager, movieSession, toolManager, config, emulator, game);
-			((GuiApi) _luaContainer.Gui).EnableLuaAutolockHack = true;
 			return _luaContainer;
 		}
 	}

src/BizHawk.Client.Common/Api/Classes/GuiApi.cs

@@ -53,14 +53,19 @@ namespace BizHawk.Client.Common
 
 		private DisplaySurfaceID? _usingSurfaceID = null;
 
-		public bool EnableLuaAutolockHack = false;
-
 		public bool HasGUISurface => _GUISurface != null;
 
+		private bool _frameStarted = false;
+
 		public GuiApi(Action<string> logCallback, DisplayManagerBase displayManager)
 		{
 			LogCallback = logCallback;
 			_displayManager = displayManager;
+
+			// These will set the drawing surfaces and display them.
+			// This allows drawing and seeing results without any actual frames occurring.
+			BeginFrame();
+			EndFrame();
 		}
 
 		private SolidBrush GetBrush(Color color)
@@ -85,68 +90,19 @@ namespace BizHawk.Client.Common
 		private IDisplaySurface GetRelevantSurface(DisplaySurfaceID? surfaceID)
 		{
 			var nnID = surfaceID ?? _usingSurfaceID ?? throw new Exception();
-			void ThisIsTheLuaAutolockHack()
-			{
-				try
-				{
-					UnlockSurface(nnID);
-					LockSurface(nnID);
-				}
-				catch (InvalidOperationException ex)
-				{
-					LogCallback(ex.ToString());
-				}
-			}
 			switch (nnID)
 			{
 				case DisplaySurfaceID.EmuCore:
-					if (_GUISurface == null && EnableLuaAutolockHack) ThisIsTheLuaAutolockHack();
 					return _GUISurface;
 				case DisplaySurfaceID.Client:
-					if (_clientSurface == null && EnableLuaAutolockHack) ThisIsTheLuaAutolockHack();
 					return _clientSurface;
 				default:
 					throw new Exception();
 			}
 		}
 
-		private void LockSurface(DisplaySurfaceID surfaceID)
-		{
-			switch (surfaceID)
-			{
-				case DisplaySurfaceID.EmuCore:
-					if (_GUISurface != null) throw new InvalidOperationException("attempt to lock surface without unlocking previous");
-					_GUISurface = _displayManager.LockApiHawkSurface(surfaceID, clear: true);
-					break;
-				case DisplaySurfaceID.Client:
-					if (_clientSurface != null) throw new InvalidOperationException("attempt to lock surface without unlocking previous");
-					_clientSurface = _displayManager.LockApiHawkSurface(surfaceID, clear: true);
-					break;
-				default:
-					throw new ArgumentException(message: "not a valid enum member", paramName: nameof(surfaceID));
-			}
-		}
-
-		private void UnlockSurface(DisplaySurfaceID surfaceID)
-		{
-			switch (surfaceID)
-			{
-				case DisplaySurfaceID.EmuCore:
-					if (_GUISurface != null) _displayManager.UnlockApiHawkSurface(_GUISurface);
-					_GUISurface = null;
-					break;
-				case DisplaySurfaceID.Client:
-					if (_clientSurface != null) _displayManager.UnlockApiHawkSurface(_clientSurface);
-					_clientSurface = null;
-					break;
-				default:
-					throw new ArgumentException(message: "not a valid enum member", paramName: nameof(surfaceID));
-			}
-		}
-
 		public void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc)
 		{
-			LockSurface(surfaceID);
 			_usingSurfaceID = surfaceID;
 			try
 			{
@@ -155,26 +111,35 @@ namespace BizHawk.Client.Common
 			finally
 			{
 				_usingSurfaceID = null;
-				UnlockSurface(surfaceID);
 			}
 		}
 
-		public readonly ref struct LuaAutoUnlockHack
+		/// <summary>
+		/// Starts drawing on new surfaces and clears them, but does not display the new surfaces.
+		/// Use this with EndFrame for double-buffered display (avoid flickering during drawing operations)
+		/// </summary>
+		public void BeginFrame()
 		{
-			private readonly GuiApi _guiApi;
+			// I think the "Lock" stuff is mis-named. All it actually does it track what's been "locked" and use that as the current surface. (and disallow re-locking or re-unlocking)
-
+			// Anyway, calling LockSurface will return a cleared surface we can draw on without displaying it. --SuuperW
-			internal LuaAutoUnlockHack(GuiApi guiApi)
+			_GUISurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.EmuCore, true);
-				=> _guiApi = guiApi;
+			_clientSurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.Client, true);
-
+			_frameStarted = true;
-			public void Dispose()
+		}
+		/// <summary>
+		/// Displays the current drawing surfaces.
+		/// More drawing can still happen on these surfaces until BeginFrame is called.
+		/// </summary>
+		public void EndFrame()
 		{
-				_guiApi.UnlockSurface(DisplaySurfaceID.EmuCore);
+			if (_frameStarted)
-				_guiApi.UnlockSurface(DisplaySurfaceID.Client);
+			{
+				_displayManager.UnlockApiHawkSurface(_GUISurface);
+				_displayManager.UnlockApiHawkSurface(_clientSurface);
+				_frameStarted = false;
 			}
 		}
 
-		public LuaAutoUnlockHack ThisIsTheLuaAutoUnlockHack()
-			=> new(this);
 
 		public void DrawNew(string name, bool clear)
 		{
@@ -638,8 +603,7 @@ namespace BizHawk.Client.Common
 
 		public void Dispose()
 		{
-			UnlockSurface(DisplaySurfaceID.EmuCore);
+			EndFrame();
-			UnlockSurface(DisplaySurfaceID.Client);
 			foreach (var brush in _solidBrushes.Values) brush.Dispose();
 			foreach (var brush in _pens.Values) brush.Dispose();
 		}

src/BizHawk.Client.Common/lua/LuaLibrariesBase.cs

@@ -169,8 +169,6 @@ namespace BizHawk.Client.Common
 
 		public void CallSaveStateEvent(string name)
 		{
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
-
 			try
 			{
 				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_SAVESTATE).ToList())
@@ -186,7 +184,7 @@ namespace BizHawk.Client.Common
 
 		public void CallLoadStateEvent(string name)
 		{
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
+			GuiAPI.BeginFrame();
 
 			try
 			{
@@ -199,13 +197,15 @@ namespace BizHawk.Client.Common
 			{
 				LogToLuaConsole($"error running function attached by lua function event.onloadstate\nError message: {e.Message}");
 			}
+
+			GuiAPI.EndFrame();
 		}
 
 		public void CallFrameBeforeEvent()
 		{
 			if (IsUpdateSupressed) return;
 
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
+			GuiAPI.BeginFrame();
 
 			try
 			{
@@ -224,8 +224,6 @@ namespace BizHawk.Client.Common
 		{
 			if (IsUpdateSupressed) return;
 
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
-
 			try
 			{
 				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_POSTFRAME).ToList())
@@ -237,12 +235,12 @@ namespace BizHawk.Client.Common
 			{
 				LogToLuaConsole($"error running function attached by lua function event.onframeend\nError message: {e.Message}");
 			}
+
+			GuiAPI.EndFrame();
 		}
 
 		public void CallExitEvent(LuaFile lf)
 		{
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
-
 			foreach (var exitCallback in RegisteredFunctions
 				.Where(l => l.Event == NamedLuaFunction.EVENT_TYPE_ENGINESTOP
 					&& (l.LuaFile.Path == lf.Path || ReferenceEquals(l.LuaFile.Thread, lf.Thread)))
@@ -312,7 +310,6 @@ namespace BizHawk.Client.Common
 		public (bool WaitForFrame, bool Terminated) ResumeScript(LuaFile lf)
 		{
 			_currThread = lf.Thread;
-			using var luaAutoUnlockHack = GuiAPI.ThisIsTheLuaAutoUnlockHack();
 
 			try
 			{

src/BizHawk.Tests/Client.Common/lua/TestLuaDrawing.cs

@@ -71,7 +71,7 @@ namespace BizHawk.Tests.Client.Common.Lua
 			var buffer = displayManager.RenderOffscreenLua(vp);
 
 			Assert.AreEqual(buffer.GetPixel(2, 2), Color.Red.ToArgb());
-			Assert.AreEqual(buffer.GetPixel(2, 4), 0xff00ff00);
+			Assert.AreEqual((uint)buffer.GetPixel(2, 4), 0xff00ff00);
 		}
 
 	}