This addresses the regression from commit 0c8ce93 that made external …

…tools unable to draw.

This removes the old obsolete methods IGuiApi.DrawNew and DrawFinish (they didn't do anything; any scripts using them already didn't work).

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;
@@ -103,61 +101,71 @@ private IDisplaySurface GetRelevantSurface(DisplaySurfaceID? surfaceID)
 
 		public void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc)
 		{
+			BeginFrame();
 			_usingSurfaceID = surfaceID;
+
 			try
 			{
 				drawingCallsFunc();
 			}
 			finally
 			{
 				_usingSurfaceID = null;
+				EndFrame();
+			}
+		}
+
+		private IDisplaySurface LockSurface(DisplaySurfaceID surface)
+		{
+			// 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
+
+			// 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(surface);
+			if (locked != null)
+			{
+				// Someone else has locked surfaces. Use those.
+				return locked;
 			}
+
+			IDisplaySurface current = surface == DisplaySurfaceID.EmuCore ? _GUISurface : _clientSurface;
+			if (current != _displayManager.GetCurrentSurface(surface))
+				return _displayManager.GetCurrentSurface(surface);
+			else
+				return _displayManager.LockApiHawkSurface(surface, true);
+		}
+		private void UnlockSurface(DisplaySurfaceID surface)
+		{
+			IDisplaySurface current = surface == DisplaySurfaceID.EmuCore ? _GUISurface : _clientSurface;
+			if (current != null && _displayManager.PeekApiHawkLockedSurface(surface) == current)
+				_displayManager.UnlockApiHawkSurface(current);
 		}
 
 		/// <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)
+		/// This method is safe to call multiple times.
+		/// 
+		/// This is how the LuaConsole can manage drawing in a way that supports having multiple scripts. (WithSurface does not support that)
+		/// This method and WithSurface will probably be replaced with an event-based method at some point.
 		/// </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;
+			_GUISurface = LockSurface(DisplaySurfaceID.EmuCore);
+			_clientSurface = LockSurface(DisplaySurfaceID.Client);
 		}
 		/// <summary>
 		/// Displays the current drawing surfaces.
 		/// More drawing can still happen on these surfaces until BeginFrame is called.
+		/// This method is safe to call multiple times.
 		/// </summary>
 		public void EndFrame()
 		{
-			if (_frameStarted)
-			{
-				_displayManager.UnlockApiHawkSurface(_GUISurface);
-				_displayManager.UnlockApiHawkSurface(_clientSurface);
-				_frameStarted = false;
-			}
-		}
-
-
-		public void DrawNew(string name, bool clear)
-		{
-			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");
-			}
+			UnlockSurface(DisplaySurfaceID.EmuCore);
+			UnlockSurface(DisplaySurfaceID.Client);
 		}
 
-		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

@@ -10,14 +10,13 @@ public interface IGuiApi : IDisposable, IExternalApi
 		ImageAttributes GetAttributes();
 		void SetAttributes(ImageAttributes a);
 
+		/// <summary>
+		/// This is the intended way for external tools to draw. All drawing calls (per surfaceID) should be done in a single call to WithSurface.
+		/// Each call will clear the given surface before calling the drawingCallsFunc.
+		/// This method will probably be replaced with an event-based method at some point.
+		/// </summary>
 		void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc);
 
-		[Obsolete]
-		void DrawNew(string name, bool clear = true);
-
-		[Obsolete]
-		void DrawFinish();
-
 		[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,44 @@ 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.WithSurface(DisplaySurfaceID.Client, () => { });
+			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/Api/GuiApiTests.cs

@@ -3,7 +3,6 @@
 using BizHawk.Bizware.BizwareGL;
 using BizHawk.Client.Common;
 using BizHawk.Emulation.Common;
-
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace BizHawk.Tests.Client.Common.Api
@@ -79,5 +78,20 @@ public void TestDrawAfterBeginFrameIsVisibleOnlyAfterEndFrame()
 			buffer = displayManager.RenderOffscreenLua(vp);
 			Assert.AreEqual(buffer.GetPixel(2, 2), Color.Red.ToArgb());
 		}
+
+		[TestMethod]
+		public void TestWithSurfaceClearsSurface()
+		{
+			BitmapBufferVideoProvider vp = new BitmapBufferVideoProvider(new BitmapBuffer(8, 8));
+
+			guiApi.WithSurface(DisplaySurfaceID.Client, () => guiApi.DrawPixel(2, 2, Color.Blue));
+			var buffer = displayManager.RenderOffscreenLua(vp);
+			Assert.AreEqual(Color.Blue.ToArgb(), buffer.GetPixel(2, 2));
+
+			guiApi.WithSurface(DisplaySurfaceID.Client, () => { });
+			buffer = displayManager.RenderOffscreenLua(vp);
+			Assert.AreEqual(Color.Black.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]