From e616b0fa62988bc168e1dfa94bf433cf0e7b1450 Mon Sep 17 00:00:00 2001 From: cosmonaut Date: Wed, 4 Oct 2023 14:45:17 -0700 Subject: [PATCH] resource management and logging improvements --- src/Game.cs | 8 ++-- src/Graphics/FencePool.cs | 12 ++--- src/Graphics/GraphicsDevice.cs | 45 ++++++++++++++++--- src/Graphics/GraphicsResource.cs | 33 +++++++++++--- .../GraphicsResourceDisposalHandle.cs | 26 +++++++++++ src/Graphics/Resources/Fence.cs | 13 +----- src/Graphics/Resources/Texture.cs | 16 +++---- src/Input/Input.cs | 10 ++--- src/Logger.cs | 43 +++++++++++------- src/Window.cs | 2 +- 10 files changed, 144 insertions(+), 64 deletions(-) create mode 100644 src/Graphics/GraphicsResourceDisposalHandle.cs diff --git a/src/Game.cs b/src/Game.cs index 4376ad0b..1a33a567 100644 --- a/src/Game.cs +++ b/src/Game.cs @@ -70,7 +70,7 @@ namespace MoonWorks if (SDL.SDL_Init(SDL.SDL_INIT_VIDEO | SDL.SDL_INIT_TIMER | SDL.SDL_INIT_GAMECONTROLLER) < 0) { - System.Console.WriteLine("Failed to initialize SDL!"); + Logger.LogError("Failed to initialize SDL!"); return; } @@ -235,6 +235,8 @@ namespace MoonWorks Draw(alpha); accumulatedDrawTime -= FramerateCapTimeSpan; + + GraphicsDevice.FlushEmergencyDisposalQueue(); } } @@ -338,14 +340,14 @@ namespace MoonWorks var index = evt.cdevice.which; if (SDL.SDL_IsGameController(index) == SDL.SDL_bool.SDL_TRUE) { - System.Console.WriteLine($"New controller detected!"); + Logger.LogInfo("New controller detected!"); Inputs.AddGamepad(index); } } private void HandleControllerRemoved(SDL.SDL_Event evt) { - System.Console.WriteLine($"Controller removal detected!"); + Logger.LogInfo("Controller removal detected!"); Inputs.RemoveGamepad(evt.cdevice.which); } diff --git a/src/Graphics/FencePool.cs b/src/Graphics/FencePool.cs index d0e6935d..22a3a240 100644 --- a/src/Graphics/FencePool.cs +++ b/src/Graphics/FencePool.cs @@ -1,11 +1,11 @@ -using System.Collections.Generic; +using System.Collections.Concurrent; namespace MoonWorks.Graphics { internal class FencePool { private GraphicsDevice GraphicsDevice; - private Queue Fences = new Queue(); + private ConcurrentQueue Fences = new ConcurrentQueue(); public FencePool(GraphicsDevice graphicsDevice) { @@ -14,12 +14,14 @@ namespace MoonWorks.Graphics public Fence Obtain() { - if (Fences.Count == 0) + if (Fences.TryDequeue(out var fence)) + { + return fence; + } + else { return new Fence(GraphicsDevice); } - - return Fences.Dequeue(); } public void Return(Fence fence) diff --git a/src/Graphics/GraphicsDevice.cs b/src/Graphics/GraphicsDevice.cs index ed2262f8..3d3a7e13 100644 --- a/src/Graphics/GraphicsDevice.cs +++ b/src/Graphics/GraphicsDevice.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using RefreshCS; @@ -87,6 +88,12 @@ namespace MoonWorks.Graphics /// True if successfully claimed. public bool ClaimWindow(Window window, PresentMode presentMode) { + if (window.Claimed) + { + Logger.LogError("Window already claimed!"); + return false; + } + var success = Conversions.ByteToBool( Refresh.Refresh_ClaimWindow( Handle, @@ -101,7 +108,7 @@ namespace MoonWorks.Graphics window.SwapchainFormat = GetSwapchainFormat(window); if (window.SwapchainTexture == null) { - window.SwapchainTexture = new Texture(this, IntPtr.Zero, window.SwapchainFormat, 0, 0); + window.SwapchainTexture = new Texture(this); } } @@ -113,11 +120,19 @@ namespace MoonWorks.Graphics /// public void UnclaimWindow(Window window) { - Refresh.Refresh_UnclaimWindow( - Handle, - window.Handle - ); - window.Claimed = false; + if (window.Claimed) + { + Refresh.Refresh_UnclaimWindow( + Handle, + window.Handle + ); + window.Claimed = false; + + // The swapchain texture doesn't actually have a permanent texture reference, so we zero the handle before disposing. + window.SwapchainTexture.Handle = IntPtr.Zero; + window.SwapchainTexture.Dispose(); + window.SwapchainTexture = null; + } } /// @@ -129,6 +144,7 @@ namespace MoonWorks.Graphics { if (!window.Claimed) { + Logger.LogError("Cannot set present mode on unclaimed window!"); return; } @@ -338,6 +354,21 @@ namespace MoonWorks.Graphics } } + ConcurrentQueue emergencyDisposalQueue = new ConcurrentQueue(); + + internal void RegisterForEmergencyDisposal(GraphicsResourceDisposalHandle handle) + { + emergencyDisposalQueue.Enqueue(handle); + } + + internal void FlushEmergencyDisposalQueue() + { + while (emergencyDisposalQueue.TryDequeue(out var handle)) + { + handle.Dispose(this); + } + } + protected virtual void Dispose(bool disposing) { if (!IsDisposed) @@ -359,6 +390,8 @@ namespace MoonWorks.Graphics Refresh.Refresh_DestroyDevice(Handle); } + FlushEmergencyDisposalQueue(); + IsDisposed = true; } } diff --git a/src/Graphics/GraphicsResource.cs b/src/Graphics/GraphicsResource.cs index 8b53a269..c610df26 100644 --- a/src/Graphics/GraphicsResource.cs +++ b/src/Graphics/GraphicsResource.cs @@ -16,23 +16,31 @@ namespace MoonWorks.Graphics { Device = device; - if (trackResource) + weakReference = new WeakReference(this); + Device.AddResourceReference(weakReference); + } + + internal GraphicsResourceDisposalHandle CreateDisposalHandle() { - weakReference = new WeakReference(this); - Device.AddResourceReference(weakReference); - } + return new GraphicsResourceDisposalHandle + { + QueueDestroyAction = QueueDestroyFunction, + ResourceHandle = Handle + }; } protected virtual void Dispose(bool disposing) { if (!IsDisposed) { - if (weakReference != null) + if (Handle != IntPtr.Zero) { QueueDestroyFunction(Device.Handle, Handle); Device.RemoveResourceReference(weakReference); weakReference.SetTarget(null); + weakReference = null; + Handle = IntPtr.Zero; } IsDisposed = true; @@ -41,8 +49,19 @@ namespace MoonWorks.Graphics ~GraphicsResource() { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method - Dispose(disposing: false); + #if DEBUG + if (!IsDisposed && Device != null && !Device.IsDisposed) + { + // If you see this log message, you leaked a graphics resource without disposing it! + // This means your game may eventually run out of native memory for mysterious reasons. + Logger.LogWarn($"A resource of type {GetType().Name} was not Disposed."); + } + #endif + + // While we only log in debug builds, in both debug and release builds we want to free + // any native resources associated with this object at the earliest opportunity. + // This will at least prevent you from running out of memory rapidly. + Device.RegisterForEmergencyDisposal(CreateDisposalHandle()); } public void Dispose() diff --git a/src/Graphics/GraphicsResourceDisposalHandle.cs b/src/Graphics/GraphicsResourceDisposalHandle.cs new file mode 100644 index 00000000..c6c28378 --- /dev/null +++ b/src/Graphics/GraphicsResourceDisposalHandle.cs @@ -0,0 +1,26 @@ +using System; + +namespace MoonWorks.Graphics +{ + // This allows us to defer native disposal calls from the finalizer thread. + internal struct GraphicsResourceDisposalHandle + { + internal Action QueueDestroyAction; + internal IntPtr ResourceHandle; + + public void Dispose(GraphicsDevice device) + { + if (device == null) + { + throw new ArgumentNullException(nameof(device)); + } + + if (QueueDestroyAction == null) + { + return; + } + + QueueDestroyAction(device.Handle, ResourceHandle); + } + } +} diff --git a/src/Graphics/Resources/Fence.cs b/src/Graphics/Resources/Fence.cs index ae59ddfb..fd380ae5 100644 --- a/src/Graphics/Resources/Fence.cs +++ b/src/Graphics/Resources/Fence.cs @@ -12,9 +12,9 @@ namespace MoonWorks.Graphics /// public class Fence : GraphicsResource { - protected override Action QueueDestroyFunction => Release; + protected override Action QueueDestroyFunction => Refresh.Refresh_ReleaseFence; - internal Fence(GraphicsDevice device) : base(device, true) + internal Fence(GraphicsDevice device) : base(device) { } @@ -22,14 +22,5 @@ namespace MoonWorks.Graphics { Handle = handle; } - - private void Release(nint deviceHandle, nint fenceHandle) - { - if (fenceHandle != IntPtr.Zero) - { - // This will only be called if the client forgot to release a handle. Oh no! - Refresh.Refresh_ReleaseFence(deviceHandle, fenceHandle); - } - } } } diff --git a/src/Graphics/Resources/Texture.cs b/src/Graphics/Resources/Texture.cs index 0af8454f..8f1c9d0c 100644 --- a/src/Graphics/Resources/Texture.cs +++ b/src/Graphics/Resources/Texture.cs @@ -303,18 +303,14 @@ namespace MoonWorks.Graphics // Used by AcquireSwapchainTexture. // Should not be tracked, because swapchain textures are managed by Vulkan. internal Texture( - GraphicsDevice device, - IntPtr handle, - TextureFormat format, - uint width, - uint height - ) : base(device, false) + GraphicsDevice device + ) : base(device) { - Handle = handle; + Handle = IntPtr.Zero; - Format = format; - Width = width; - Height = height; + Format = TextureFormat.R8G8B8A8; + Width = 0; + Height = 0; Depth = 1; IsCube = false; LevelCount = 1; diff --git a/src/Input/Input.cs b/src/Input/Input.cs index 4df0b270..d2ea27ed 100644 --- a/src/Input/Input.cs +++ b/src/Input/Input.cs @@ -119,19 +119,19 @@ namespace MoonWorks.Input var openResult = SDL.SDL_GameControllerOpen(index); if (openResult == 0) { - System.Console.WriteLine($"Error opening gamepad!"); - System.Console.WriteLine(SDL.SDL_GetError()); + Logger.LogError("Error opening gamepad!"); + Logger.LogError(SDL.SDL_GetError()); } else { Gamepads[slot].Register(openResult); - System.Console.WriteLine($"Gamepad added to slot {slot}!"); + Logger.LogInfo($"Gamepad added to slot {slot}!"); } return; } } - System.Console.WriteLine("Too many gamepads already!"); + Logger.LogInfo("Too many gamepads already!"); } internal void RemoveGamepad(int joystickInstanceID) @@ -142,7 +142,7 @@ namespace MoonWorks.Input { SDL.SDL_GameControllerClose(Gamepads[slot].Handle); Gamepads[slot].Unregister(); - System.Console.WriteLine($"Removing gamepad from slot {slot}!"); + Logger.LogInfo($"Removing gamepad from slot {slot}!"); return; } } diff --git a/src/Logger.cs b/src/Logger.cs index 1d0ce7ce..76821e73 100644 --- a/src/Logger.cs +++ b/src/Logger.cs @@ -5,9 +5,9 @@ namespace MoonWorks { public static class Logger { - public static Action LogInfo; - public static Action LogWarn; - public static Action LogError; + public static Action LogInfo = LogInfoDefault; + public static Action LogWarn = LogWarnDefault; + public static Action LogError = LogErrorDefault; private static RefreshCS.Refresh.Refresh_LogFunc LogInfoFunc = RefreshLogInfo; private static RefreshCS.Refresh.Refresh_LogFunc LogWarnFunc = RefreshLogWarn; @@ -15,19 +15,6 @@ namespace MoonWorks internal static void Initialize() { - if (Logger.LogInfo == null) - { - Logger.LogInfo = Console.WriteLine; - } - if (Logger.LogWarn == null) - { - Logger.LogWarn = Console.WriteLine; - } - if (Logger.LogError == null) - { - Logger.LogError = Console.WriteLine; - } - Refresh.Refresh_HookLogFunctions( LogInfoFunc, LogWarnFunc, @@ -35,6 +22,30 @@ namespace MoonWorks ); } + private static void LogInfoDefault(string str) + { + Console.ForegroundColor = ConsoleColor.Green; + Console.Write("INFO: "); + Console.ForegroundColor = ConsoleColor.White; + Console.WriteLine(str); + } + + private static void LogWarnDefault(string str) + { + Console.ForegroundColor = ConsoleColor.Yellow; + Console.Write("WARN: "); + Console.ForegroundColor = ConsoleColor.White; + Console.WriteLine(str); + } + + private static void LogErrorDefault(string str) + { + Console.ForegroundColor = ConsoleColor.Red; + Console.Write("ERROR: "); + Console.ForegroundColor = ConsoleColor.White; + Console.WriteLine(str); + } + private static void RefreshLogInfo(IntPtr msg) { LogInfo(UTF8_ToManaged(msg)); diff --git a/src/Window.cs b/src/Window.cs index 6ae5676b..958c9bba 100644 --- a/src/Window.cs +++ b/src/Window.cs @@ -16,7 +16,7 @@ namespace MoonWorks public ScreenMode ScreenMode { get; private set; } public uint Width { get; private set; } public uint Height { get; private set; } - internal Texture SwapchainTexture { get; set; } = null; + internal Texture SwapchainTexture { get; set; } public bool Claimed { get; internal set; } public MoonWorks.Graphics.TextureFormat SwapchainFormat { get; internal set; }