From 5479149e79a9a91aff74ebbb1c4adf250ca93137 Mon Sep 17 00:00:00 2001 From: Soreepeong Date: Tue, 23 Jan 2024 20:51:29 +0900 Subject: [PATCH] Lock font resources on Push and miscellaneous direct accesses These changes ensure that using a font under some other thread's ownership from the UI thread for rendering into ImGui purposes always work. * `FontHandle`: * Moved common code from `DelegateFontHandle` and `GamePrebakedFontHandle`. * Added `LockUntilPostFrame` so that the obtained `ImFontPtr` and its accompanying resources are kept valid until everything is rendered. * Added more code comments to `Try/Lock`. * Moved font access thread checking logic from `InterfaceManager` to `LockUntilPostFrame`. * `Push`ing a font will now also perform `LockUntilPostFrame`. * `GameFontHandle`: Make the property `ImFont` a forwarder to `FontHandle.LockUntilPostFrame`. * `InterfaceManager`: * Added companion logic to `FontHandle.LockUntilPostFrame`. * Accessing default/icon/mono fonts will forward to `FontHandle.LockUntilPostFrame`. * Changed `List` to `ConcurrentBag` as texture disposal can be done outside the main thread, and a race condition is possible. --- Dalamud/Interface/GameFonts/GameFontHandle.cs | 26 +- .../Interface/Internal/InterfaceManager.cs | 92 +++--- .../Interface/ManagedFontAtlas/IFontHandle.cs | 21 +- .../Internals/DelegateFontHandle.cs | 135 +-------- .../ManagedFontAtlas/Internals/FontHandle.cs | 263 ++++++++++++++++++ .../Internals/GamePrebakedFontHandle.cs | 132 +-------- .../Internals/SimplePushedFont.cs | 7 +- Dalamud/Interface/UiBuilder.cs | 2 +- 8 files changed, 331 insertions(+), 347 deletions(-) create mode 100644 Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs diff --git a/Dalamud/Interface/GameFonts/GameFontHandle.cs b/Dalamud/Interface/GameFonts/GameFontHandle.cs index 7bda27eae..4c472c032 100644 --- a/Dalamud/Interface/GameFonts/GameFontHandle.cs +++ b/Dalamud/Interface/GameFonts/GameFontHandle.cs @@ -15,15 +15,16 @@ namespace Dalamud.Interface.GameFonts; [Api10ToDo(Api10ToDoAttribute.DeleteCompatBehavior)] public sealed class GameFontHandle : IFontHandle { - private readonly IFontHandle.IInternal fontHandle; + private readonly GamePrebakedFontHandle fontHandle; private readonly FontAtlasFactory fontAtlasFactory; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class.
+ /// Ownership of is transferred. /// - /// The wrapped . + /// The wrapped . /// An instance of . - internal GameFontHandle(IFontHandle.IInternal fontHandle, FontAtlasFactory fontAtlasFactory) + internal GameFontHandle(GamePrebakedFontHandle fontHandle, FontAtlasFactory fontAtlasFactory) { this.fontHandle = fontHandle; this.fontAtlasFactory = fontAtlasFactory; @@ -42,9 +43,15 @@ public sealed class GameFontHandle : IFontHandle /// public bool Available => this.fontHandle.Available; - /// - [Obsolete($"Use {nameof(Push)}, and then use {nameof(ImGui.GetFont)} instead.", false)] - public ImFontPtr ImFont => this.fontHandle.ImFont; + /// + /// Gets the font.
+ /// Use of this properly is safe only from the UI thread.
+ /// Use if the intended purpose of this property is .
+ /// Futures changes may make simple not enough.
+ /// If you need to access a font outside the UI thread, use . + /// + [Obsolete($"Use {nameof(Push)}-{nameof(ImGui.GetFont)} or {nameof(Lock)} instead.", false)] + public ImFontPtr ImFont => this.fontHandle.LockUntilPostFrame(); /// /// Gets the font style. Only applicable for . @@ -66,10 +73,7 @@ public sealed class GameFontHandle : IFontHandle /// public IFontHandle.ImFontLocked Lock() => this.fontHandle.Lock(); - /// - /// Pushes the font. - /// - /// An that can be used to pop the font on dispose. + /// public IDisposable Push() => this.fontHandle.Push(); /// diff --git a/Dalamud/Interface/Internal/InterfaceManager.cs b/Dalamud/Interface/Internal/InterfaceManager.cs index e1b714ee8..25baa5e29 100644 --- a/Dalamud/Interface/Internal/InterfaceManager.cs +++ b/Dalamud/Interface/Internal/InterfaceManager.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -20,8 +21,6 @@ using Dalamud.Interface.ManagedFontAtlas.Internals; using Dalamud.Interface.Style; using Dalamud.Interface.Utility; using Dalamud.Interface.Windowing; -using Dalamud.Plugin.Internal; -using Dalamud.Plugin.Internal.Types; using Dalamud.Utility; using Dalamud.Utility.Timing; using ImGuiNET; @@ -63,15 +62,9 @@ internal class InterfaceManager : IDisposable, IServiceType /// public const float DefaultFontSizePx = (DefaultFontSizePt * 4.0f) / 3.0f; - private const int NonMainThreadFontAccessWarningCheckInterval = 10000; - private static readonly ConditionalWeakTable NonMainThreadFontAccessWarning = new(); - private static long nextNonMainThreadFontAccessWarningCheck; + private readonly ConcurrentBag deferredDisposeTextures = new(); + private readonly ConcurrentBag deferredDisposeImFontLockeds = new(); - private readonly List deferredDisposeTextures = new(); - - [ServiceManager.ServiceDependency] - private readonly Framework framework = Service.Get(); - [ServiceManager.ServiceDependency] private readonly WndProcHookManager wndProcHookManager = Service.Get(); @@ -127,34 +120,37 @@ internal class InterfaceManager : IDisposable, IServiceType /// Gets the default ImGui font.
/// Accessing this static property outside of the main thread is dangerous and not supported. /// - public static ImFontPtr DefaultFont => WhenFontsReady().DefaultFontHandle!.ImFont.OrElse(ImGui.GetIO().FontDefault); + public static ImFontPtr DefaultFont => + WhenFontsReady().DefaultFontHandle!.LockUntilPostFrame().OrElse(ImGui.GetIO().FontDefault); /// /// Gets an included FontAwesome icon font.
/// Accessing this static property outside of the main thread is dangerous and not supported. /// - public static ImFontPtr IconFont => WhenFontsReady().IconFontHandle!.ImFont.OrElse(ImGui.GetIO().FontDefault); + public static ImFontPtr IconFont => + WhenFontsReady().IconFontHandle!.LockUntilPostFrame().OrElse(ImGui.GetIO().FontDefault); /// /// Gets an included monospaced font.
/// Accessing this static property outside of the main thread is dangerous and not supported. /// - public static ImFontPtr MonoFont => WhenFontsReady().MonoFontHandle!.ImFont.OrElse(ImGui.GetIO().FontDefault); + public static ImFontPtr MonoFont => + WhenFontsReady().MonoFontHandle!.LockUntilPostFrame().OrElse(ImGui.GetIO().FontDefault); /// /// Gets the default font handle. /// - public IFontHandle.IInternal? DefaultFontHandle { get; private set; } + public FontHandle? DefaultFontHandle { get; private set; } /// /// Gets the icon font handle. /// - public IFontHandle.IInternal? IconFontHandle { get; private set; } + public FontHandle? IconFontHandle { get; private set; } /// /// Gets the mono font handle. /// - public IFontHandle.IInternal? MonoFontHandle { get; private set; } + public FontHandle? MonoFontHandle { get; private set; } /// /// Gets or sets the pointer to ImGui.IO(), when it was last used. @@ -408,6 +404,15 @@ internal class InterfaceManager : IDisposable, IServiceType this.deferredDisposeTextures.Add(wrap); } + /// + /// Enqueue an to be disposed at the end of the frame. + /// + /// The disposable. + public void EnqueueDeferredDispose(in IFontHandle.ImFontLocked locked) + { + this.deferredDisposeImFontLockeds.Add(locked); + } + /// /// Get video memory information. /// @@ -466,29 +471,6 @@ internal class InterfaceManager : IDisposable, IServiceType if (im?.dalamudAtlas is not { } atlas) throw new InvalidOperationException($"Tried to access fonts before {nameof(ContinueConstruction)} call."); - if (!ThreadSafety.IsMainThread && nextNonMainThreadFontAccessWarningCheck < Environment.TickCount64) - { - nextNonMainThreadFontAccessWarningCheck = - Environment.TickCount64 + NonMainThreadFontAccessWarningCheckInterval; - var stack = new StackTrace(); - if (Service.GetNullable()?.FindCallingPlugin(stack) is { } plugin) - { - if (!NonMainThreadFontAccessWarning.TryGetValue(plugin, out _)) - { - NonMainThreadFontAccessWarning.Add(plugin, new()); - Log.Warning( - "[IM] {pluginName}: Accessing fonts outside the main thread is deprecated.\n{stack}", - plugin.Name, - stack); - } - } - else - { - // Dalamud internal should be made safe right now - throw new InvalidOperationException("Attempted to access fonts outside the main thread."); - } - } - if (!atlas.HasBuiltAtlas) atlas.BuildTask.GetAwaiter().GetResult(); return im; @@ -673,28 +655,38 @@ internal class InterfaceManager : IDisposable, IServiceType var pRes = this.presentHook!.Original(swapChain, syncInterval, presentFlags); RenderImGui(this.scene!); - this.DisposeTextures(); + this.CleanupPostImGuiRender(); return pRes; } RenderImGui(this.scene!); - this.DisposeTextures(); + this.CleanupPostImGuiRender(); return this.presentHook!.Original(swapChain, syncInterval, presentFlags); } - private void DisposeTextures() + private void CleanupPostImGuiRender() { - if (this.deferredDisposeTextures.Count > 0) + if (!this.deferredDisposeTextures.IsEmpty) { - Log.Verbose("[IM] Disposing {Count} textures", this.deferredDisposeTextures.Count); - foreach (var texture in this.deferredDisposeTextures) + var count = 0; + while (this.deferredDisposeTextures.TryTake(out var d)) { - texture.RealDispose(); + count++; + d.RealDispose(); } - this.deferredDisposeTextures.Clear(); + Log.Verbose("[IM] Disposing {Count} textures", count); + } + + if (!this.deferredDisposeImFontLockeds.IsEmpty) + { + // Not logging; the main purpose of this is to keep resources used for rendering the frame to be kept + // referenced until the resources are actually done being used, and it is expected that this will be + // frequent. + while (this.deferredDisposeImFontLockeds.TryTake(out var d)) + d.Dispose(); } } @@ -709,9 +701,9 @@ internal class InterfaceManager : IDisposable, IServiceType .CreateFontAtlas(nameof(InterfaceManager), FontAtlasAutoRebuildMode.Disable); using (this.dalamudAtlas.SuppressAutoRebuild()) { - this.DefaultFontHandle = (IFontHandle.IInternal)this.dalamudAtlas.NewDelegateFontHandle( + this.DefaultFontHandle = (FontHandle)this.dalamudAtlas.NewDelegateFontHandle( e => e.OnPreBuild(tk => tk.AddDalamudDefaultFont(DefaultFontSizePx))); - this.IconFontHandle = (IFontHandle.IInternal)this.dalamudAtlas.NewDelegateFontHandle( + this.IconFontHandle = (FontHandle)this.dalamudAtlas.NewDelegateFontHandle( e => e.OnPreBuild( tk => tk.AddFontAwesomeIconFont( new() @@ -720,7 +712,7 @@ internal class InterfaceManager : IDisposable, IServiceType GlyphMinAdvanceX = DefaultFontSizePx, GlyphMaxAdvanceX = DefaultFontSizePx, }))); - this.MonoFontHandle = (IFontHandle.IInternal)this.dalamudAtlas.NewDelegateFontHandle( + this.MonoFontHandle = (FontHandle)this.dalamudAtlas.NewDelegateFontHandle( e => e.OnPreBuild( tk => tk.AddDalamudAssetFont( DalamudAsset.InconsolataRegular, diff --git a/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs index 94edc9777..97d345925 100644 --- a/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs +++ b/Dalamud/Interface/ManagedFontAtlas/IFontHandle.cs @@ -21,21 +21,6 @@ public interface IFontHandle : IDisposable /// event Action ImFontChanged; - /// - /// Represents a reference counting handle for fonts. Dalamud internal use only. - /// - internal interface IInternal : IFontHandle - { - /// - /// Gets the font.
- /// Use of this properly is safe only from the UI thread.
- /// Use if the intended purpose of this property is .
- /// Futures changes may make simple not enough.
- /// If you need to access a font outside the UI thread, consider using . - /// - ImFontPtr ImFont { get; } - } - /// /// Gets the load exception, if it failed to load. Otherwise, it is null. /// @@ -45,7 +30,6 @@ public interface IFontHandle : IDisposable /// Gets a value indicating whether this font is ready for use. /// /// - /// Once set to true, it will remain true.
/// Use directly if you want to keep the current ImGui font if the font is not ready.
/// Alternatively, use to wait for this property to become true. /// @@ -103,14 +87,13 @@ public interface IFontHandle : IDisposable private IRefCountable? owner; /// - /// Initializes a new instance of the struct, - /// and incrase the reference count of . + /// Initializes a new instance of the struct. + /// Ownership of reference of is transferred. /// /// The contained font. /// The owner. internal ImFontLocked(ImFontPtr imFont, IRefCountable owner) { - owner.AddRef(); this.ImFont = imFont; this.owner = owner; } diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/DelegateFontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/DelegateFontHandle.cs index e1c18e923..53a836511 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/DelegateFontHandle.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/DelegateFontHandle.cs @@ -1,164 +1,35 @@ using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; -using Dalamud.Interface.Internal; using Dalamud.Interface.Utility; using Dalamud.Logging.Internal; using Dalamud.Utility; using ImGuiNET; -using Serilog; - namespace Dalamud.Interface.ManagedFontAtlas.Internals; /// /// A font handle representing a user-callback generated font. /// -internal class DelegateFontHandle : IFontHandle.IInternal +internal sealed class DelegateFontHandle : FontHandle { - private readonly List pushedFonts = new(8); - - private IFontHandleManager? manager; - private long lastCumulativePresentCalls; - /// /// Initializes a new instance of the class. /// /// An instance of . /// Callback for . public DelegateFontHandle(IFontHandleManager manager, FontAtlasBuildStepDelegate callOnBuildStepChange) + : base(manager) { - this.manager = manager; this.CallOnBuildStepChange = callOnBuildStepChange; } - /// - public event Action? ImFontChanged; - - private event Action? Disposed; - /// /// Gets the function to be called on build step changes. /// public FontAtlasBuildStepDelegate CallOnBuildStepChange { get; } - /// - public Exception? LoadException => this.ManagerNotDisposed.Substance?.GetBuildException(this); - - /// - public bool Available => this.ImFont.IsNotNullAndLoaded(); - - /// - public ImFontPtr ImFont => this.ManagerNotDisposed.Substance?.GetFontPtr(this) ?? default; - - private IFontHandleManager ManagerNotDisposed => - this.manager ?? throw new ObjectDisposedException(nameof(GamePrebakedFontHandle)); - - /// - public void Dispose() - { - if (this.pushedFonts.Count > 0) - Log.Warning($"{nameof(IFontHandle)}.{nameof(IDisposable.Dispose)}: fonts were still in a stack."); - this.manager?.FreeFontHandle(this); - this.manager = null; - this.Disposed?.InvokeSafely(this); - this.ImFontChanged = null; - } - - /// - public IFontHandle.ImFontLocked Lock() - { - IFontHandleSubstance? prevSubstance = default; - while (true) - { - var substance = this.ManagerNotDisposed.Substance; - if (substance is null) - throw new InvalidOperationException(); - if (substance == prevSubstance) - throw new ObjectDisposedException(nameof(DelegateFontHandle)); - - prevSubstance = substance; - try - { - substance.DataRoot.AddRef(); - } - catch (ObjectDisposedException) - { - continue; - } - - try - { - var fontPtr = substance.GetFontPtr(this); - if (fontPtr.IsNull()) - continue; - return new(fontPtr, substance.DataRoot); - } - finally - { - substance.DataRoot.Release(); - } - } - } - - /// - public IDisposable Push() - { - ThreadSafety.AssertMainThread(); - var cumulativePresentCalls = Service.GetNullable()?.CumulativePresentCalls ?? 0L; - if (this.lastCumulativePresentCalls != cumulativePresentCalls) - { - this.lastCumulativePresentCalls = cumulativePresentCalls; - if (this.pushedFonts.Count > 0) - { - Log.Warning( - $"{nameof(this.Push)} has been called, but the handle-private stack was not empty. " + - $"You might be missing a call to {nameof(this.Pop)}."); - this.pushedFonts.Clear(); - } - } - - var rented = SimplePushedFont.Rent(this.pushedFonts, this.ImFont, this.Available); - this.pushedFonts.Add(rented); - return rented; - } - - /// - public void Pop() - { - ThreadSafety.AssertMainThread(); - this.pushedFonts[^1].Dispose(); - } - - /// - public Task WaitAsync() - { - if (this.Available) - return Task.FromResult(this); - - var tcs = new TaskCompletionSource(); - this.ImFontChanged += OnImFontChanged; - this.Disposed += OnImFontChanged; - if (this.Available) - OnImFontChanged(this); - return tcs.Task; - - void OnImFontChanged(IFontHandle unused) - { - if (tcs.Task.IsCompletedSuccessfully) - return; - - this.ImFontChanged -= OnImFontChanged; - this.Disposed -= OnImFontChanged; - if (this.manager is null) - tcs.SetException(new ObjectDisposedException(nameof(GamePrebakedFontHandle))); - else - tcs.SetResult(this); - } - } - /// /// Manager for s. /// @@ -216,7 +87,7 @@ internal class DelegateFontHandle : IFontHandle.IInternal return; foreach (var handle in hs.RelevantHandles) - handle.ImFontChanged?.InvokeSafely(handle); + handle.InvokeImFontChanged(); } /// diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs new file mode 100644 index 000000000..93b17f86e --- /dev/null +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/FontHandle.cs @@ -0,0 +1,263 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +using Dalamud.Interface.Internal; +using Dalamud.Interface.Utility; +using Dalamud.Plugin.Internal; +using Dalamud.Plugin.Internal.Types; +using Dalamud.Utility; + +using ImGuiNET; + +using Serilog; + +namespace Dalamud.Interface.ManagedFontAtlas.Internals; + +/// +/// Default implementation for . +/// +internal abstract class FontHandle : IFontHandle +{ + private const int NonMainThreadFontAccessWarningCheckInterval = 10000; + private static readonly ConditionalWeakTable NonMainThreadFontAccessWarning = new(); + private static long nextNonMainThreadFontAccessWarningCheck; + + private readonly InterfaceManager interfaceManager; + private readonly List pushedFonts = new(8); + + private IFontHandleManager? manager; + private long lastCumulativePresentCalls; + + /// + /// Initializes a new instance of the class. + /// + /// An instance of . + protected FontHandle(IFontHandleManager manager) + { + this.interfaceManager = Service.Get(); + this.manager = manager; + } + + /// + public event Action? ImFontChanged; + + /// + /// Event to be called on the first call. + /// + protected event Action? Disposed; + + /// + public Exception? LoadException => this.Manager.Substance?.GetBuildException(this); + + /// + public bool Available => (this.Manager.Substance?.GetFontPtr(this) ?? default).IsNotNullAndLoaded(); + + /// + /// Gets the associated . + /// + /// When the object has already been disposed. + protected IFontHandleManager Manager => this.manager ?? throw new ObjectDisposedException(this.GetType().Name); + + /// + public void Dispose() + { + if (this.manager is null) + return; + + this.Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Obtains an instance of corresponding to this font handle, + /// to be released after rendering the current frame. + /// + /// The font pointer, or default if unavailble. + /// + /// Behavior is undefined on access outside the main thread. + /// + public ImFontPtr LockUntilPostFrame() + { + if (this.TryLock(out _) is not { } locked) + return default; + + if (!ThreadSafety.IsMainThread && nextNonMainThreadFontAccessWarningCheck < Environment.TickCount64) + { + nextNonMainThreadFontAccessWarningCheck = + Environment.TickCount64 + NonMainThreadFontAccessWarningCheckInterval; + var stack = new StackTrace(); + if (Service.GetNullable()?.FindCallingPlugin(stack) is { } plugin) + { + if (!NonMainThreadFontAccessWarning.TryGetValue(plugin, out _)) + { + NonMainThreadFontAccessWarning.Add(plugin, new()); + Log.Warning( + "[IM] {pluginName}: Accessing fonts outside the main thread is deprecated.\n{stack}", + plugin.Name, + stack); + } + } + else + { + // Dalamud internal should be made safe right now + throw new InvalidOperationException("Attempted to access fonts outside the main thread."); + } + } + + this.interfaceManager.EnqueueDeferredDispose(locked); + return locked.ImFont; + } + + /// + /// Attempts to lock the fully constructed instance of corresponding to the this + /// , for use in any thread.
+ /// Modification of the font will exhibit undefined behavior if some other thread also uses the font. + /// + /// The error message, if any. + /// + /// An instance of that must be disposed after use on success; + /// null with populated on failure. + /// + /// Still may be thrown. + public IFontHandle.ImFontLocked? TryLock(out string? errorMessage) + { + IFontHandleSubstance? prevSubstance = default; + while (true) + { + var substance = this.Manager.Substance; + + // Does the associated IFontAtlas have a built substance? + if (substance is null) + { + errorMessage = "The font atlas has not been built yet."; + return null; + } + + // Did we loop (because it did not have the requested font), + // and are the fetched substance same between loops? + if (substance == prevSubstance) + { + errorMessage = "The font atlas did not built the requested handle yet."; + return null; + } + + prevSubstance = substance; + + // Try to lock the substance. + try + { + substance.DataRoot.AddRef(); + } + catch (ObjectDisposedException) + { + // If it got invalidated, it's probably because a new substance is incoming. Try again. + continue; + } + + var fontPtr = substance.GetFontPtr(this); + if (fontPtr.IsNull()) + { + // The font for the requested handle is unavailable. Release the reference and try again. + substance.DataRoot.Release(); + continue; + } + + // Transfer the ownership of reference. + errorMessage = null; + return new(fontPtr, substance.DataRoot); + } + } + + /// + public IFontHandle.ImFontLocked Lock() => + this.TryLock(out var errorMessage) ?? throw new InvalidOperationException(errorMessage); + + /// + public IDisposable Push() + { + ThreadSafety.AssertMainThread(); + + // Warn if the client is not properly managing the pushed font stack. + var cumulativePresentCalls = this.interfaceManager.CumulativePresentCalls; + if (this.lastCumulativePresentCalls != cumulativePresentCalls) + { + this.lastCumulativePresentCalls = cumulativePresentCalls; + if (this.pushedFonts.Count > 0) + { + Log.Warning( + $"{nameof(this.Push)} has been called, but the handle-private stack was not empty. " + + $"You might be missing a call to {nameof(this.Pop)}."); + this.pushedFonts.Clear(); + } + } + + var font = default(ImFontPtr); + if (this.TryLock(out _) is { } locked) + { + font = locked.ImFont; + this.interfaceManager.EnqueueDeferredDispose(locked); + } + + var rented = SimplePushedFont.Rent(this.pushedFonts, font); + this.pushedFonts.Add(rented); + return rented; + } + + /// + public void Pop() + { + ThreadSafety.AssertMainThread(); + this.pushedFonts[^1].Dispose(); + } + + /// + public Task WaitAsync() + { + if (this.Available) + return Task.FromResult(this); + + var tcs = new TaskCompletionSource(); + this.ImFontChanged += OnImFontChanged; + this.Disposed += OnImFontChanged; + if (this.Available) + OnImFontChanged(this); + return tcs.Task; + + void OnImFontChanged(IFontHandle unused) + { + if (tcs.Task.IsCompletedSuccessfully) + return; + + this.ImFontChanged -= OnImFontChanged; + this.Disposed -= OnImFontChanged; + if (this.manager is null) + tcs.SetException(new ObjectDisposedException(nameof(GamePrebakedFontHandle))); + else + tcs.SetResult(this); + } + } + + /// + /// Invokes . + /// + protected void InvokeImFontChanged() => this.ImFontChanged.InvokeSafely(this); + + /// + /// Overrideable implementation for . + /// + /// If true, then the function is being called from . + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + if (this.pushedFonts.Count > 0) + Log.Warning($"{nameof(IFontHandle)}.{nameof(IDisposable.Dispose)}: fonts were still in a stack."); + this.Manager.FreeFontHandle(this); + this.manager = null; + this.Disposed?.InvokeSafely(this); + this.ImFontChanged = null; + } + } +} diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/GamePrebakedFontHandle.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/GamePrebakedFontHandle.cs index 0e8301785..e062405b8 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/GamePrebakedFontHandle.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/GamePrebakedFontHandle.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reactive.Disposables; -using System.Threading.Tasks; using Dalamud.Game.Text; using Dalamud.Interface.GameFonts; @@ -16,8 +15,6 @@ using ImGuiNET; using Lumina.Data.Files; -using Serilog; - using Vector4 = System.Numerics.Vector4; namespace Dalamud.Interface.ManagedFontAtlas.Internals; @@ -25,7 +22,7 @@ namespace Dalamud.Interface.ManagedFontAtlas.Internals; /// /// A font handle that uses the game's built-in fonts, optionally with some styling. /// -internal class GamePrebakedFontHandle : IFontHandle.IInternal +internal class GamePrebakedFontHandle : FontHandle { /// /// The smallest value of . @@ -37,17 +34,13 @@ internal class GamePrebakedFontHandle : IFontHandle.IInternal /// public static readonly char SeIconCharMax = (char)Enum.GetValues().Max(); - private readonly List pushedFonts = new(8); - - private IFontHandleManager? manager; - private long lastCumulativePresentCalls; - /// /// Initializes a new instance of the class. /// /// An instance of . /// Font to use. public GamePrebakedFontHandle(IFontHandleManager manager, GameFontStyle style) + : base(manager) { if (!Enum.IsDefined(style.FamilyAndSize) || style.FamilyAndSize == GameFontFamilyAndSize.Undefined) throw new ArgumentOutOfRangeException(nameof(style), style, null); @@ -55,15 +48,9 @@ internal class GamePrebakedFontHandle : IFontHandle.IInternal if (style.SizePt <= 0) throw new ArgumentException($"{nameof(style.SizePt)} must be a positive number.", nameof(style)); - this.manager = manager; this.FontStyle = style; } - /// - public event Action? ImFontChanged; - - private event Action? Disposed; - /// /// Provider for for `common/font/fontNN.tex`. /// @@ -107,119 +94,6 @@ internal class GamePrebakedFontHandle : IFontHandle.IInternal /// public GameFontStyle FontStyle { get; } - /// - public Exception? LoadException => this.ManagerNotDisposed.Substance?.GetBuildException(this); - - /// - public bool Available => this.ImFont.IsNotNullAndLoaded(); - - /// - public ImFontPtr ImFont => this.ManagerNotDisposed.Substance?.GetFontPtr(this) ?? default; - - private IFontHandleManager ManagerNotDisposed => - this.manager ?? throw new ObjectDisposedException(nameof(GamePrebakedFontHandle)); - - /// - public void Dispose() - { - this.manager?.FreeFontHandle(this); - this.manager = null; - this.Disposed?.InvokeSafely(this); - this.ImFontChanged = null; - } - - /// - public IFontHandle.ImFontLocked Lock() - { - IFontHandleSubstance? prevSubstance = default; - while (true) - { - var substance = this.ManagerNotDisposed.Substance; - if (substance is null) - throw new InvalidOperationException(); - if (substance == prevSubstance) - throw new ObjectDisposedException(nameof(DelegateFontHandle)); - - prevSubstance = substance; - try - { - substance.DataRoot.AddRef(); - } - catch (ObjectDisposedException) - { - continue; - } - - try - { - var fontPtr = substance.GetFontPtr(this); - if (fontPtr.IsNull()) - continue; - return new(fontPtr, substance.DataRoot); - } - finally - { - substance.DataRoot.Release(); - } - } - } - - /// - public IDisposable Push() - { - ThreadSafety.AssertMainThread(); - var cumulativePresentCalls = Service.GetNullable()?.CumulativePresentCalls ?? 0L; - if (this.lastCumulativePresentCalls != cumulativePresentCalls) - { - this.lastCumulativePresentCalls = cumulativePresentCalls; - if (this.pushedFonts.Count > 0) - { - Log.Warning( - $"{nameof(this.Push)} has been called, but the handle-private stack was not empty. " + - $"You might be missing a call to {nameof(this.Pop)}."); - this.pushedFonts.Clear(); - } - } - - var rented = SimplePushedFont.Rent(this.pushedFonts, this.ImFont, this.Available); - this.pushedFonts.Add(rented); - return rented; - } - - /// - public void Pop() - { - ThreadSafety.AssertMainThread(); - this.pushedFonts[^1].Dispose(); - } - - /// - public Task WaitAsync() - { - if (this.Available) - return Task.FromResult(this); - - var tcs = new TaskCompletionSource(); - this.ImFontChanged += OnImFontChanged; - this.Disposed += OnImFontChanged; - if (this.Available) - OnImFontChanged(this); - return tcs.Task; - - void OnImFontChanged(IFontHandle unused) - { - if (tcs.Task.IsCompletedSuccessfully) - return; - - this.ImFontChanged -= OnImFontChanged; - this.Disposed -= OnImFontChanged; - if (this.manager is null) - tcs.SetException(new ObjectDisposedException(nameof(GamePrebakedFontHandle))); - else - tcs.SetResult(this); - } - } - /// public override string ToString() => $"{nameof(GamePrebakedFontHandle)}({this.FontStyle})"; @@ -305,7 +179,7 @@ internal class GamePrebakedFontHandle : IFontHandle.IInternal return; foreach (var handle in hs.RelevantHandles) - handle.ImFontChanged?.InvokeSafely(handle); + handle.InvokeImFontChanged(); } /// diff --git a/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs b/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs index 3f7255386..0642e7be1 100644 --- a/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs +++ b/Dalamud/Interface/ManagedFontAtlas/Internals/SimplePushedFont.cs @@ -28,17 +28,14 @@ internal sealed class SimplePushedFont : IDisposable /// /// The -private stack. /// The font pointer being pushed. - /// Whether to push. /// this. - public static SimplePushedFont Rent(List stack, ImFontPtr fontPtr, bool push) + public static SimplePushedFont Rent(List stack, ImFontPtr fontPtr) { - push &= !fontPtr.IsNull(); - var rented = Pool.Get(); Debug.Assert(rented.font.IsNull(), "Rented object must not have its font set"); rented.stack = stack; - if (push) + if (fontPtr.IsNotNullAndLoaded()) { rented.font = fontPtr; ImGui.PushFont(fontPtr); diff --git a/Dalamud/Interface/UiBuilder.cs b/Dalamud/Interface/UiBuilder.cs index 1134704ee..ea3803f35 100644 --- a/Dalamud/Interface/UiBuilder.cs +++ b/Dalamud/Interface/UiBuilder.cs @@ -498,7 +498,7 @@ public sealed class UiBuilder : IDisposable [Obsolete($"Use {nameof(this.FontAtlas)}.{nameof(IFontAtlas.NewGameFontHandle)} instead.", false)] [Api10ToDo(Api10ToDoAttribute.DeleteCompatBehavior)] public GameFontHandle GetGameFontHandle(GameFontStyle style) => new( - (IFontHandle.IInternal)this.FontAtlas.NewGameFontHandle(style), + (GamePrebakedFontHandle)this.FontAtlas.NewGameFontHandle(style), Service.Get()); ///