Remove PostPromotion event

`PostPromotion` is removed, as `IFontHandle.ImFontChanged` now does the job. It also removes the possibility that resources may get disposed while post promotion callback is in progress.

* `IFontHandle.ImFontChanged` is now called with a locked instance of the font.
* `IFontHandle.ImFontLocked`: Added `NewRef` to increase reference count.
This commit is contained in:
Soreepeong 2024-01-23 23:00:47 +09:00
parent fb8beb9370
commit 871deca6e9
17 changed files with 138 additions and 269 deletions

View file

@ -80,16 +80,6 @@ internal sealed class DelegateFontHandle : FontHandle
this.handles.Remove(cgfh);
}
/// <inheritdoc/>
public void InvokeFontHandleImFontChanged()
{
if (this.Substance is not HandleSubstance hs)
return;
foreach (var handle in hs.RelevantHandles)
handle.InvokeImFontChanged();
}
/// <inheritdoc/>
public IFontHandleSubstance NewSubstance(IRefCountable dataRoot)
{
@ -133,6 +123,9 @@ internal sealed class DelegateFontHandle : FontHandle
// Not owned by this class. Do not dispose.
public DelegateFontHandle[] RelevantHandles { get; }
/// <inheritdoc/>
ICollection<FontHandle> IFontHandleSubstance.RelevantHandles => this.RelevantHandles;
/// <inheritdoc/>
public IRefCountable DataRoot { get; }
@ -306,32 +299,5 @@ internal sealed class DelegateFontHandle : FontHandle
}
}
}
/// <inheritdoc/>
public void OnPostPromotion(IFontAtlasBuildToolkitPostPromotion toolkitPostPromotion)
{
foreach (var k in this.RelevantHandles)
{
if (!this.fonts[k].IsNotNullAndLoaded())
continue;
try
{
toolkitPostPromotion.Font = this.fonts[k];
k.CallOnBuildStepChange.Invoke(toolkitPostPromotion);
}
catch (Exception e)
{
this.fonts[k] = default;
this.buildExceptions[k] = e;
Log.Error(
e,
"[{name}:Substance] An error has occurred while during {delegate} PostPromotion call.",
this.Manager.Name,
nameof(FontAtlasBuildStepDelegate));
}
}
}
}
}

View file

@ -677,60 +677,4 @@ internal sealed partial class FontAtlasFactory
}
}
}
/// <summary>
/// Implementations for <see cref="IFontAtlasBuildToolkitPostPromotion"/>.
/// </summary>
private class BuildToolkitPostPromotion : IFontAtlasBuildToolkitPostPromotion
{
private readonly FontAtlasBuiltData builtData;
/// <summary>
/// Initializes a new instance of the <see cref="BuildToolkitPostPromotion"/> class.
/// </summary>
/// <param name="builtData">The built data.</param>
public BuildToolkitPostPromotion(FontAtlasBuiltData builtData) => this.builtData = builtData;
/// <inheritdoc/>
public ImFontPtr Font { get; set; }
/// <inheritdoc/>
public float Scale => this.builtData.Scale;
/// <inheritdoc/>
public bool IsAsyncBuildOperation => true;
/// <inheritdoc/>
public FontAtlasBuildStep BuildStep => FontAtlasBuildStep.PostPromotion;
/// <inheritdoc/>
public ImFontAtlasPtr NewImAtlas => this.builtData.Atlas;
/// <inheritdoc/>
public unsafe ImVectorWrapper<ImFontPtr> Fonts => new(
&this.NewImAtlas.NativePtr->Fonts,
x => ImGuiNative.ImFont_destroy(x->NativePtr));
/// <inheritdoc/>
public T DisposeWithAtlas<T>(T disposable) where T : IDisposable => this.builtData.Garbage.Add(disposable);
/// <inheritdoc/>
public GCHandle DisposeWithAtlas(GCHandle gcHandle) => this.builtData.Garbage.Add(gcHandle);
/// <inheritdoc/>
public void DisposeWithAtlas(Action action) => this.builtData.Garbage.Add(action);
/// <inheritdoc/>
public ImFontPtr GetFont(IFontHandle fontHandle)
{
foreach (var s in this.builtData.Substances)
{
var f = s.GetFontPtr(fontHandle);
if (!f.IsNull())
return f;
}
return default;
}
}
}

View file

@ -486,7 +486,7 @@ internal sealed partial class FontAtlasFactory
}
/// <inheritdoc/>
public Task BuildFontsAsync(bool callPostPromotionOnMainThread = true)
public Task BuildFontsAsync()
{
#if VeryVerboseLog
Log.Verbose("[{name}] Called: {source}.", this.Name, nameof(this.BuildFontsAsync));
@ -519,15 +519,7 @@ internal sealed partial class FontAtlasFactory
if (res.Atlas.IsNull())
return res;
if (callPostPromotionOnMainThread)
{
await this.factory.Framework.RunOnFrameworkThread(
() => this.InvokePostPromotion(rebuildIndex, res, nameof(this.BuildFontsAsync)));
}
else
{
this.InvokePostPromotion(rebuildIndex, res, nameof(this.BuildFontsAsync));
}
this.InvokePostPromotion(rebuildIndex, res, nameof(this.BuildFontsAsync));
return res;
}
@ -536,6 +528,10 @@ internal sealed partial class FontAtlasFactory
private void InvokePostPromotion(int rebuildIndex, FontAtlasBuiltData data, [UsedImplicitly] string source)
{
// Capture the locks inside the lock block, so that the fonts are guaranteed to be the ones just built.
var fontsAndLocks = new List<(FontHandle FontHandle, IFontHandle.ImFontLocked Lock)>();
using var garbage = new DisposeSafety.ScopedFinalizer();
lock (this.syncRoot)
{
if (this.buildIndex != rebuildIndex)
@ -549,56 +545,25 @@ internal sealed partial class FontAtlasFactory
prevBuiltData.ExplicitDisposeIgnoreExceptions();
this.buildTask = EmptyTask;
fontsAndLocks.EnsureCapacity(data.Substances.Sum(x => x.RelevantHandles.Count));
foreach (var substance in data.Substances)
{
substance.Manager.Substance = substance;
foreach (var fontHandle in substance.RelevantHandles)
{
substance.DataRoot.AddRef();
var locked = new IFontHandle.ImFontLocked(substance.GetFontPtr(fontHandle), substance.DataRoot);
fontsAndLocks.Add((fontHandle, garbage.Add(locked)));
}
}
}
lock (this.syncRootPostPromotion)
{
if (this.buildIndex != rebuildIndex)
{
data.ExplicitDisposeIgnoreExceptions();
return;
}
foreach (var substance in data.Substances)
substance.Manager.InvokeFontHandleImFontChanged();
var toolkit = new BuildToolkitPostPromotion(data);
foreach (var substance in data.Substances)
{
try
{
substance.OnPostPromotion(toolkit);
}
catch (Exception e)
{
Log.Error(
e,
"[{name}] {substance} PostPromotion error",
this.Name,
substance.GetType().FullName ?? substance.GetType().Name);
}
}
try
{
this.BuildStepChange?.Invoke(toolkit);
}
catch (Exception e)
{
Log.Error(
e,
"[{name}] {delegateName} PostPromotion error",
this.Name,
nameof(FontAtlasBuildStepDelegate));
}
foreach (var (fontHandle, lockedFont) in fontsAndLocks)
fontHandle.InvokeImFontChanged(lockedFont);
#if VeryVerboseLog
Log.Verbose("[{name}] Built from {source}.", this.Name, source);
Log.Verbose("[{name}] Built from {source}.", this.Name, source);
#endif
}
}
private void ImGuiSceneOnNewRenderFrame()

View file

@ -41,12 +41,12 @@ internal abstract class FontHandle : IFontHandle
}
/// <inheritdoc/>
public event Action<IFontHandle>? ImFontChanged;
public event IFontHandle.ImFontChangedDelegate? ImFontChanged;
/// <summary>
/// Event to be called on the first <see cref="IDisposable.Dispose"/> call.
/// </summary>
protected event Action<IFontHandle>? Disposed;
protected event Action? Disposed;
/// <inheritdoc/>
public Exception? LoadException => this.Manager.Substance?.GetBuildException(this);
@ -70,6 +70,22 @@ internal abstract class FontHandle : IFontHandle
GC.SuppressFinalize(this);
}
/// <summary>
/// Invokes <see cref="IFontHandle.ImFontChanged"/>.
/// </summary>
/// <param name="font">The font, locked during the call of <see cref="ImFontChanged"/>.</param>
public void InvokeImFontChanged(IFontHandle.ImFontLocked font)
{
try
{
this.ImFontChanged?.Invoke(this, font);
}
catch (Exception e)
{
Log.Error(e, $"{nameof(this.InvokeImFontChanged)}: error");
}
}
/// <summary>
/// Obtains an instance of <see cref="ImFontPtr"/> corresponding to this font handle,
/// to be released after rendering the current frame.
@ -220,35 +236,51 @@ internal abstract class FontHandle : IFontHandle
var tcs = new TaskCompletionSource<IFontHandle>();
this.ImFontChanged += OnImFontChanged;
this.Disposed += OnImFontChanged;
this.Disposed += OnDisposed;
if (this.Available)
OnImFontChanged(this);
OnImFontChanged(this, default);
return tcs.Task;
void OnImFontChanged(IFontHandle unused)
void OnImFontChanged(IFontHandle unused, IFontHandle.ImFontLocked unused2)
{
if (tcs.Task.IsCompletedSuccessfully)
return;
this.ImFontChanged -= OnImFontChanged;
this.Disposed -= OnImFontChanged;
if (this.manager is null)
tcs.SetException(new ObjectDisposedException(nameof(GamePrebakedFontHandle)));
else
this.Disposed -= OnDisposed;
try
{
tcs.SetResult(this);
}
catch
{
// ignore
}
}
void OnDisposed()
{
if (tcs.Task.IsCompletedSuccessfully)
return;
this.ImFontChanged -= OnImFontChanged;
this.Disposed -= OnDisposed;
try
{
tcs.SetException(new ObjectDisposedException(nameof(GamePrebakedFontHandle)));
}
catch
{
// ignore
}
}
}
/// <summary>
/// Invokes <see cref="IFontHandle.ImFontChanged"/>.
/// </summary>
protected void InvokeImFontChanged() => this.ImFontChanged.InvokeSafely(this);
/// <summary>
/// Overrideable implementation for <see cref="IDisposable.Dispose"/>.
/// Implementation for <see cref="IDisposable.Dispose"/>.
/// </summary>
/// <param name="disposing">If <c>true</c>, then the function is being called from <see cref="IDisposable.Dispose"/>.</param>
protected virtual void Dispose(bool disposing)
protected void Dispose(bool disposing)
{
if (disposing)
{
@ -256,7 +288,7 @@ internal abstract class FontHandle : IFontHandle
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.Disposed?.InvokeSafely();
this.ImFontChanged = null;
}
}

View file

@ -172,16 +172,6 @@ internal class GamePrebakedFontHandle : FontHandle
}
}
/// <inheritdoc/>
public void InvokeFontHandleImFontChanged()
{
if (this.Substance is not HandleSubstance hs)
return;
foreach (var handle in hs.RelevantHandles)
handle.InvokeImFontChanged();
}
/// <inheritdoc/>
public IFontHandleSubstance NewSubstance(IRefCountable dataRoot)
{
@ -232,6 +222,9 @@ internal class GamePrebakedFontHandle : FontHandle
// Not owned by this class. Do not dispose.
public GamePrebakedFontHandle[] RelevantHandles { get; }
/// <inheritdoc/>
ICollection<FontHandle> IFontHandleSubstance.RelevantHandles => this.RelevantHandles;
/// <inheritdoc/>
public IRefCountable DataRoot { get; }
@ -413,12 +406,6 @@ internal class GamePrebakedFontHandle : FontHandle
}
}
/// <inheritdoc/>
public void OnPostPromotion(IFontAtlasBuildToolkitPostPromotion toolkitPostPromotion)
{
// Irrelevant
}
/// <summary>
/// Creates a new template font.
/// </summary>

View file

@ -32,9 +32,4 @@ internal interface IFontHandleManager : IDisposable
/// <param name="dataRoot">The data root.</param>
/// <returns>The new substance.</returns>
IFontHandleSubstance NewSubstance(IRefCountable dataRoot);
/// <summary>
/// Invokes <see cref="IFontHandle.ImFontChanged"/>.
/// </summary>
void InvokeFontHandleImFontChanged();
}

View file

@ -1,4 +1,6 @@
using Dalamud.Utility;
using System.Collections.Generic;
using Dalamud.Utility;
using ImGuiNET;
@ -32,6 +34,11 @@ internal interface IFontHandleSubstance : IDisposable
[Api10ToDo(Api10ToDoAttribute.DeleteCompatBehavior)]
bool CreateFontOnAccess { get; set; }
/// <summary>
/// Gets the relevant handles.
/// </summary>
public ICollection<FontHandle> RelevantHandles { get; }
/// <summary>
/// Gets the font.
/// </summary>
@ -64,11 +71,4 @@ internal interface IFontHandleSubstance : IDisposable
/// </summary>
/// <param name="toolkitPostBuild">The toolkit.</param>
void OnPostBuild(IFontAtlasBuildToolkitPostBuild toolkitPostBuild);
/// <summary>
/// Called on the specific thread depending on <see cref="IFontAtlasBuildToolkit.IsAsyncBuildOperation"/> after
/// promoting the staging atlas to direct use with <see cref="IFontAtlas"/>.
/// </summary>
/// <param name="toolkitPostPromotion">The toolkit.</param>
void OnPostPromotion(IFontAtlasBuildToolkitPostPromotion toolkitPostPromotion);
}