From 68dc16803c0a23993aa89193dd99cbdc73e14940 Mon Sep 17 00:00:00 2001 From: Soreepeong Date: Tue, 23 Jan 2024 23:39:25 +0900 Subject: [PATCH] Turn ImFontLocked into a class As `ImFontLocked` utilizes a reference counter, changed it to a class so that at worst case we still got the destructor to decrease the reference count. --- .../Interface/Internal/InterfaceManager.cs | 34 ++++++---- .../Interface/ManagedFontAtlas/IFontHandle.cs | 67 ++++++++++++++----- .../FontAtlasFactory.Implementation.cs | 4 +- .../ManagedFontAtlas/Internals/FontHandle.cs | 2 +- .../Internals/SimplePushedFont.cs | 2 +- 5 files changed, 75 insertions(+), 34 deletions(-) diff --git a/Dalamud/Interface/Internal/InterfaceManager.cs b/Dalamud/Interface/Internal/InterfaceManager.cs index 18bb95799..82299a136 100644 --- a/Dalamud/Interface/Internal/InterfaceManager.cs +++ b/Dalamud/Interface/Internal/InterfaceManager.cs @@ -79,7 +79,7 @@ internal class InterfaceManager : IDisposable, IServiceType private Hook? resizeBuffersHook; private IFontAtlas? dalamudAtlas; - private IFontHandle.ImFontLocked defaultFontResourceLock; + private IFontHandle.ImFontLocked? defaultFontResourceLock; // can't access imgui IO before first present call private bool lastWantCapture = false; @@ -243,6 +243,8 @@ internal class InterfaceManager : IDisposable, IServiceType Disposer(); this.wndProcHookManager.PreWndProc -= this.WndProcHookManagerOnPreWndProc; + this.defaultFontResourceLock?.Dispose(); // lock outlives handle and atlas + this.defaultFontResourceLock = null; this.dalamudAtlas?.Dispose(); this.scene?.Dispose(); return; @@ -727,22 +729,26 @@ internal class InterfaceManager : IDisposable, IServiceType tk.GetFont(this.MonoFontHandle), missingOnly: true); }); - this.DefaultFontHandle.ImFontChanged += (_, font) => Service.Get().RunOnFrameworkThread( - () => - { - // Update the ImGui default font. - unsafe + this.DefaultFontHandle.ImFontChanged += (_, font) => + { + var fontLocked = font.NewRef(); + Service.Get().RunOnFrameworkThread( + () => { - ImGui.GetIO().NativePtr->FontDefault = font; - } + // Update the ImGui default font. + unsafe + { + ImGui.GetIO().NativePtr->FontDefault = fontLocked; + } - // Update the reference to the resources of the default font. - this.defaultFontResourceLock.Dispose(); - this.defaultFontResourceLock = font.NewRef(); + // Update the reference to the resources of the default font. + this.defaultFontResourceLock?.Dispose(); + this.defaultFontResourceLock = fontLocked; - // Broadcast to auto-rebuilding instances. - this.AfterBuildFonts?.Invoke(); - }); + // Broadcast to auto-rebuilding instances. + this.AfterBuildFonts?.Invoke(); + }); + }; } // This will wait for scene on its own. We just wait for this.dalamudAtlas.BuildTask in this.InitScene. diff --git a/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs index d58a89e56..dd3775236 100644 --- a/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs +++ b/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs @@ -1,9 +1,14 @@ -using System.Threading.Tasks; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Dalamud.Interface.Utility; using Dalamud.Utility; using ImGuiNET; +using Microsoft.Extensions.ObjectPool; + namespace Dalamud.Interface.ManagedFontAtlas; /// @@ -80,26 +85,23 @@ public interface IFontHandle : IDisposable /// The wrapper for , guaranteeing that the associated data will be available as long as /// this struct is not disposed. /// - public struct ImFontLocked : IDisposable + public class ImFontLocked : IDisposable { - /// - /// The associated . - /// - public ImFontPtr ImFont; + // Using constructor instead of DefaultObjectPoolProvider, since we do not want the pool to call Dispose. + private static readonly ObjectPool Pool = + new DefaultObjectPool(new DefaultPooledObjectPolicy()); private IRefCountable? owner; /// - /// Initializes a new instance of the struct. - /// Ownership of reference of is transferred. + /// Finalizes an instance of the class. /// - /// The contained font. - /// The owner. - internal ImFontLocked(ImFontPtr imFont, IRefCountable owner) - { - this.ImFont = imFont; - this.owner = owner; - } + ~ImFontLocked() => this.FreeOwner(); + + /// + /// Gets the associated . + /// + public ImFontPtr ImFont { get; private set; } public static implicit operator ImFontPtr(ImFontLocked l) => l.ImFont; @@ -109,16 +111,47 @@ public interface IFontHandle : IDisposable /// Creates a new instance of with an additional reference to the owner. /// /// The new locked instance. - public readonly ImFontLocked NewRef() + public ImFontLocked NewRef() { if (this.owner is null) throw new ObjectDisposedException(nameof(ImFontLocked)); + + var rented = Pool.Get(); + rented.owner = this.owner; + rented.ImFont = this.ImFont; + this.owner.AddRef(); - return new(this.ImFont, this.owner); + return rented; } /// + [SuppressMessage( + "Usage", + "CA1816:Dispose methods should call SuppressFinalize", + Justification = "Dispose returns this object to the pool.")] public void Dispose() + { + this.FreeOwner(); + Pool.Return(this); + } + + /// + /// Initializes a new instance of the class. + /// Ownership of reference of is transferred. + /// + /// The contained font. + /// The owner. + /// The rented instance of . + internal static ImFontLocked Rent(ImFontPtr font, IRefCountable owner) + { + var rented = Pool.Get(); + Debug.Assert(rented.ImFont.IsNull(), "Rented object must not have its font set"); + rented.ImFont = font; + rented.owner = owner; + return rented; + } + + private void FreeOwner() { if (this.owner is null) return; diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/FontAtlasFactory.Implementation.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/FontAtlasFactory.Implementation.cs index 7fadf669d..06bc5b7ab 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/FontAtlasFactory.Implementation.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/FontAtlasFactory.Implementation.cs @@ -557,7 +557,9 @@ internal sealed partial class FontAtlasFactory foreach (var fontHandle in substance.RelevantHandles) { substance.DataRoot.AddRef(); - var locked = new IFontHandle.ImFontLocked(substance.GetFontPtr(fontHandle), substance.DataRoot); + var locked = IFontHandle.ImFontLocked.Rent( + substance.GetFontPtr(fontHandle), + substance.DataRoot); fontsAndLocks.Add((fontHandle, garbage.Add(locked))); } } diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs index d01b0df87..f8291cc51 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs @@ -182,7 +182,7 @@ internal abstract class FontHandle : IFontHandle // Transfer the ownership of reference. errorMessage = null; - return new(fontPtr, substance.DataRoot); + return IFontHandle.ImFontLocked.Rent(fontPtr, substance.DataRoot); } } diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs index 0642e7be1..0c96025ac 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs @@ -28,7 +28,7 @@ internal sealed class SimplePushedFont : IDisposable /// /// The -private stack. /// The font pointer being pushed. - /// this. + /// The rented instance of . public static SimplePushedFont Rent(List stack, ImFontPtr fontPtr) { var rented = Pool.Get();