Fix external tool drawing with new GuiApi methods.

This removes the old obsolete methods (that didn't do anything; any scripts using them already didn't work).
This is somewhat a partial revert of commit 7749d02 which forced external tools to put all drawing operations for a frame inside WithSurface. And addresses the regression from commit 0c8ce93 that made that method of drawing from external tools no longer work. (by exposing an alternative that plays nicely with other tools)

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

@@ -55,8 +55,6 @@ public sealed class GuiApi : IGuiApi
 
 		public bool HasGUISurface => _GUISurface != null;
 
-		private bool _frameStarted = false;
-
 		public GuiApi(Action<string> logCallback, DisplayManagerBase displayManager)
 		{
 			LogCallback = logCallback;
@@ -114,50 +112,42 @@ public void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc)
 			}
 		}
 
-		/// <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()
 		{
 			// 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
-			_GUISurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.EmuCore, true);
-			_clientSurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.Client, true);
-			_frameStarted = true;
-		}
-		/// <summary>
-		/// Displays the current drawing surfaces.
-		/// More drawing can still happen on these surfaces until BeginFrame is called.
-		/// </summary>
-		public void EndFrame()
-		{
-			if (_frameStarted)
+
+			// There might be multiple API users at once and therefore multiple GuiApi instances.
+			// In that case, we don't want to get a new surface. We want the current one.
+			var locked = _displayManager.PeekApiHawkLockedSurface(DisplaySurfaceID.EmuCore);
+			if (locked != null)
 			{
-				_displayManager.UnlockApiHawkSurface(_GUISurface);
-				_displayManager.UnlockApiHawkSurface(_clientSurface);
-				_frameStarted = false;
+				// Someone else has locked surfaces. Use those.
+				_GUISurface = locked;
+				_clientSurface = _displayManager.PeekApiHawkLockedSurface(DisplaySurfaceID.Client);
+				return;
 			}
-		}
+
+			if (_GUISurface != _displayManager.GetCurrentSurface(DisplaySurfaceID.EmuCore))
+				_GUISurface = _displayManager.GetCurrentSurface(DisplaySurfaceID.EmuCore);
+			else
+				_GUISurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.EmuCore, true);
+
+			if (_clientSurface != _displayManager.GetCurrentSurface(DisplaySurfaceID.Client))
+				_clientSurface = _displayManager.GetCurrentSurface(DisplaySurfaceID.Client);
+			else
+				_clientSurface = _displayManager.LockApiHawkSurface(DisplaySurfaceID.Client, true);
 
 
-		public void DrawNew(string name, bool clear)
+		}
+		public void EndFrame()
 		{
-			switch (name)
-			{
-				case null:
-				case "emu":
-					LogCallback("the `DrawNew(\"emu\")` function has been deprecated");
-					return;
-				case "native":
-					throw new InvalidOperationException("the ability to draw in the margins with `DrawNew(\"native\")` has been removed");
-				default:
-					throw new InvalidOperationException("invalid surface name");
-			}
+			if (_displayManager.PeekApiHawkLockedSurface(DisplaySurfaceID.EmuCore) == _GUISurface)
+				_displayManager.UnlockApiHawkSurface(_GUISurface);
+			if (_displayManager.PeekApiHawkLockedSurface(DisplaySurfaceID.Client) == _clientSurface)
+				_displayManager.UnlockApiHawkSurface(_clientSurface);
 		}
 
-		public void DrawFinish() => LogCallback("the `DrawFinish()` function has been deprecated");
-
 		public void SetPadding(int all) => _padding = (all, all, all, all);
 
 		public void SetPadding(int x, int y) => _padding = (x / 2, y / 2, x / 2 + x & 1, y / 2 + y & 1);

src/BizHawk.Client.Common/Api/Interfaces/IGuiApi.cs

@@ -12,11 +12,16 @@ public interface IGuiApi : IDisposable, IExternalApi
 
 		void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc);
 
-		[Obsolete]
-		void DrawNew(string name, bool clear = true);
-
-		[Obsolete]
-		void DrawFinish();
+		/// <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>
+		void BeginFrame();
+		/// <summary>
+		/// Displays the current drawing surfaces.
+		/// More drawing can still happen on these surfaces until BeginFrame is called.
+		/// </summary>
+		void EndFrame();
 
 		[Obsolete]
 		bool HasGUISurface { get; }

src/BizHawk.Client.Common/DisplayManager/DisplayManagerBase.cs

@@ -979,6 +979,9 @@ private void LoadCustomFont(Stream fontStream)
 		public IDisplaySurface PeekApiHawkLockedSurface(DisplaySurfaceID surfaceID)
 			=> _apiHawkIDToSurface.TryGetValue(surfaceID, out var surface) ? surface : null;
 
+		public IDisplaySurface GetCurrentSurface(DisplaySurfaceID surfaceID)
+			=> _apiHawkSurfaceSets[surfaceID].GetCurrent();
+
 		public IDisplaySurface LockApiHawkSurface(DisplaySurfaceID surfaceID, bool clear)
 		{
 			if (_apiHawkIDToSurface.ContainsKey(surfaceID))

src/BizHawk.Client.Common/lua/CommonLibs/GuiLuaLibrary.cs

@@ -20,18 +20,6 @@ public GuiLuaLibrary(ILuaLibraries luaLibsImpl, ApiContainer apiContainer, Actio
 		private DisplaySurfaceID UseOrFallback(string surfaceName)
 			=> DisplaySurfaceIDParser.Parse(surfaceName) ?? _rememberedSurfaceID;
 
-#pragma warning disable CS0612
-		[LuaDeprecatedMethod]
-		[LuaMethod("DrawNew", "Changes drawing target to the specified lua surface name. This may clobber any previous drawing to this surface (pass false if you don't want it to)")]
-		public void DrawNew(string name, bool? clear = true)
-			=> APIs.Gui.DrawNew(name, clear ?? true);
-
-		[LuaDeprecatedMethod]
-		[LuaMethod("DrawFinish", "Finishes drawing to the current lua surface and causes it to get displayed.")]
-		public void DrawFinish()
-			=> APIs.Gui.DrawFinish();
-#pragma warning restore CS0612
-
 		[LuaMethodExample("gui.addmessage( \"Some message\" );")]
 		[LuaMethod("addmessage", "Adds a message to the OSD's message area")]
 		public void AddMessage(string message)

src/BizHawk.Tests/Client.Common/Api/ExternalToolTests.cs

@@ -1,10 +1,15 @@
-using System.IO;
+using System.Drawing;
+using System.IO;
 using System.Linq;
 using System.Reflection;
+
+using BizHawk.Bizware.BizwareGL;
 using BizHawk.Client.Common;
 using BizHawk.Emulation.Common;
+using BizHawk.Tests.Client.Common.Lua;
 using BizHawk.Tests.Implementations;
 using BizHawk.Tests.Mocks;
+
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace BizHawk.Tests.Client.Common.Api
@@ -71,5 +76,45 @@ public void TestExternalToolCanUseApi()
 			TestExternalAPI externalApi = LoadExternalTool(toolManager);
 			Assert.AreEqual(mainFormApi.Emulator.AsVideoProviderOrDefault().BufferHeight, externalApi.APIs.EmuClient.BufferHeight());
 		}
+
+		[TestMethod]
+		public void TestExternalToolCanDraw()
+		{
+			IMainFormForApi mainFormApi = new MockMainFormForApi(new NullEmulator());
+			DisplayManagerBase displayManager = new TestDisplayManager(mainFormApi.Emulator);
+			TestToolManager toolManager = new TestToolManager(mainFormApi, config, displayManager);
+
+			TestExternalAPI externalApi = LoadExternalTool(toolManager);
+			IGuiApi guiApi = externalApi.APIs.Gui;
+
+			guiApi.DrawPixel(2, 2, Color.Blue, DisplaySurfaceID.Client);
+			BitmapBufferVideoProvider vp = new BitmapBufferVideoProvider(new BitmapBuffer(8, 8));
+			var buffer = displayManager.RenderOffscreenLua(vp);
+			Assert.AreEqual(buffer.GetPixel(2, 2), Color.Blue.ToArgb());
+		}
+
+		[TestMethod]
+		public void TestExternalToolCanDrawWhileLuaIsRunning()
+		{
+			IMainFormForApi mainFormApi = new MockMainFormForApi(new NullEmulator());
+			DisplayManagerBase displayManager = new TestDisplayManager(mainFormApi.Emulator);
+			TestToolManager toolManager = new TestToolManager(mainFormApi, config, displayManager);
+
+			TestExternalAPI externalApi = LoadExternalTool(toolManager);
+			IGuiApi guiApi = externalApi.APIs.Gui;
+			TestLuaDrawing luaTester = new TestLuaDrawing(mainFormApi, config, displayManager); // not the default constructor; we need to share a DisplayManager at least
+
+			// trigger some frames with both to ensure we can cycle through buffers
+			guiApi.BeginFrame();
+			guiApi.EndFrame();
+			luaTester.luaLibraries.CallFrameBeforeEvent();
+			luaTester.luaLibraries.CallFrameAfterEvent();
+
+			// draw
+			guiApi.DrawPixel(2, 2, Color.Blue, DisplaySurfaceID.Client);
+			BitmapBufferVideoProvider vp = new BitmapBufferVideoProvider(new BitmapBuffer(8, 8));
+			var buffer = displayManager.RenderOffscreenLua(vp);
+			Assert.AreEqual(Color.Blue.ToArgb(), buffer.GetPixel(2, 2));
+		}
 	}
 }

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

@@ -14,10 +14,26 @@ public class TestLuaDrawing
 	{
 #pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type.
 		// null values are initialized in the setup method
-		private ILuaLibraries luaLibraries = null;
+		internal ILuaLibraries luaLibraries = null;
 		private DisplayManagerBase displayManager = null;
 #pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type.
 
+		public TestLuaDrawing() { }
+
+		internal TestLuaDrawing(IMainFormForApi mainForm, Config config, DisplayManagerBase displayManager)
+		{
+			this.displayManager = displayManager;
+
+			IGameInfo gameInfo = new GameInfo();
+			luaLibraries = new TestLuaLibraries(
+				mainForm,
+				displayManager,
+				config,
+				gameInfo
+			);
+			luaLibraries.Restart(config, gameInfo);
+		}
+
 		private const string pathToTestLuaScripts = "Client.Common/lua/LuaScripts";
 
 		[TestInitialize]