From 42b6f8fd4b5e8a7031922ccd3df56daf996e1783 Mon Sep 17 00:00:00 2001 From: Soreepeong Date: Mon, 26 Feb 2024 04:16:36 +0900 Subject: [PATCH] fix disposes and add TextureWrapIconSource --- .../ImGuiNotification/IActiveNotification.cs | 25 ++----- .../ImGuiNotification/INotification.cs | 9 ++- .../IconSource/TextureWrapIconSource.cs | 63 ++++++++++++++++ .../Internal/ActiveNotification.cs | 72 ++++++++----------- .../Internal/NotificationManager.cs | 26 +++++-- .../Internal/NotificationUtilities.cs | 23 +++++- .../ImGuiNotification/Notification.cs | 7 ++ .../Windows/Data/Widgets/ImGuiWidget.cs | 34 ++++++--- Dalamud/Interface/UiBuilder.cs | 1 + .../Plugin/Services/INotificationManager.cs | 8 ++- 10 files changed, 187 insertions(+), 81 deletions(-) create mode 100644 Dalamud/Interface/ImGuiNotification/IconSource/TextureWrapIconSource.cs diff --git a/Dalamud/Interface/ImGuiNotification/IActiveNotification.cs b/Dalamud/Interface/ImGuiNotification/IActiveNotification.cs index 0a8f656b9..3ae1a76ce 100644 --- a/Dalamud/Interface/ImGuiNotification/IActiveNotification.cs +++ b/Dalamud/Interface/ImGuiNotification/IActiveNotification.cs @@ -56,11 +56,6 @@ public interface IActiveNotification : INotification /// new NotificationType Type { get; set; } - /// Gets or sets the icon source. - /// Setting a new value to this property does not change the icon. Use to do so. - /// - new INotificationIconSource? IconSource { get; set; } - /// new DateTime Expiry { get; set; } @@ -86,25 +81,19 @@ public interface IActiveNotification : INotification /// This includes when the hide animation is being played. bool IsDismissed { get; } - /// Clones this notification as a . - /// A new instance of . - Notification CloneNotification(); - /// Dismisses this notification. void DismissNow(); - /// Updates the notification data. - /// - /// Call to update the icon using the new . - /// If is true, then this function is a no-op. - /// - /// The new notification entry. - void Update(INotification newNotification); - - /// Loads the icon again using . + /// Loads the icon again using the same . /// If is true, then this function is a no-op. void UpdateIcon(); + /// Disposes the previous icon source, take ownership of the new icon source, + /// and calls . + /// Thew new icon source. + /// If is true, then this function is a no-op. + void UpdateIconSource(INotificationIconSource? newIconSource); + /// Generates a new value to use for . /// The new value. internal static long CreateNewId() => Interlocked.Increment(ref idCounter); diff --git a/Dalamud/Interface/ImGuiNotification/INotification.cs b/Dalamud/Interface/ImGuiNotification/INotification.cs index 6b47b69f4..e80ff96c0 100644 --- a/Dalamud/Interface/ImGuiNotification/INotification.cs +++ b/Dalamud/Interface/ImGuiNotification/INotification.cs @@ -4,7 +4,7 @@ using Dalamud.Interface.Internal.Notifications; namespace Dalamud.Interface.ImGuiNotification; /// Represents a notification. -public interface INotification +public interface INotification : IDisposable { /// Gets the content body of the notification. string Content { get; } @@ -16,10 +16,15 @@ public interface INotification NotificationType Type { get; } /// Gets the icon source. - /// The following icon sources are currently available.
+ /// + /// The assigned value will be disposed upon the call on this instance of + /// .
+ ///
+ /// The following icon sources are currently available.
///
    ///
    • ///
    • + ///
    • ///
    • ///
    • ///
    • diff --git a/Dalamud/Interface/ImGuiNotification/IconSource/TextureWrapIconSource.cs b/Dalamud/Interface/ImGuiNotification/IconSource/TextureWrapIconSource.cs new file mode 100644 index 000000000..b3d4392cf --- /dev/null +++ b/Dalamud/Interface/ImGuiNotification/IconSource/TextureWrapIconSource.cs @@ -0,0 +1,63 @@ +using System.Numerics; +using System.Threading; + +using Dalamud.Interface.ImGuiNotification.Internal; +using Dalamud.Interface.Internal; +using Dalamud.Plugin.Internal.Types; + +namespace Dalamud.Interface.ImGuiNotification.IconSource; + +/// Represents the use of future as the icon of a notification. +/// If there was no texture loaded for any reason, the plugin icon will be displayed instead. +public sealed class TextureWrapIconSource : INotificationIconSource.IInternal +{ + private IDalamudTextureWrap? wrap; + + /// Initializes a new instance of the class. + /// The texture wrap to handle over the ownership. + /// + /// If true, this class will own the passed , and you must not call + /// on the passed wrap. + /// If false, this class will create a new reference of the passed wrap, and you should call + /// on the passed wrap. + /// In both cases, this class must be disposed after use. + public TextureWrapIconSource(IDalamudTextureWrap? wrap, bool takeOwnership) => + this.wrap = takeOwnership ? wrap : wrap?.CreateWrapSharingLowLevelResource(); + + /// Gets the underlying texture wrap. + public IDalamudTextureWrap? Wrap => this.wrap; + + /// + public INotificationIconSource Clone() => new TextureWrapIconSource(this.wrap, false); + + /// + public void Dispose() + { + if (Interlocked.Exchange(ref this.wrap, null) is { } w) + w.Dispose(); + } + + /// + INotificationMaterializedIcon INotificationIconSource.IInternal.Materialize() => + new MaterializedIcon(this.wrap?.CreateWrapSharingLowLevelResource()); + + private sealed class MaterializedIcon : INotificationMaterializedIcon + { + private IDalamudTextureWrap? wrap; + + public MaterializedIcon(IDalamudTextureWrap? wrap) => this.wrap = wrap; + + public void Dispose() + { + if (Interlocked.Exchange(ref this.wrap, null) is { } w) + w.Dispose(); + } + + public void DrawIcon(Vector2 minCoord, Vector2 maxCoord, Vector4 color, LocalPlugin? initiatorPlugin) => + NotificationUtilities.DrawTexture( + this.wrap, + minCoord, + maxCoord, + initiatorPlugin); + } +} diff --git a/Dalamud/Interface/ImGuiNotification/Internal/ActiveNotification.cs b/Dalamud/Interface/ImGuiNotification/Internal/ActiveNotification.cs index 2e82af6a3..3f14ec50b 100644 --- a/Dalamud/Interface/ImGuiNotification/Internal/ActiveNotification.cs +++ b/Dalamud/Interface/ImGuiNotification/Internal/ActiveNotification.cs @@ -43,7 +43,10 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable /// The initiator plugin. Use null if originated by Dalamud. public ActiveNotification(Notification underlyingNotification, LocalPlugin? initiatorPlugin) { - this.underlyingNotification = underlyingNotification with { }; + this.underlyingNotification = underlyingNotification with + { + IconSource = underlyingNotification.IconSource?.Clone(), + }; this.InitiatorPlugin = initiatorPlugin; this.showEasing = new InCubic(NotificationConstants.ShowAnimationDuration); this.hideEasing = new OutCubic(NotificationConstants.HideAnimationDuration); @@ -51,7 +54,16 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable this.showEasing.Start(); this.progressEasing.Start(); - this.UpdateIcon(); + try + { + this.UpdateIcon(); + } + catch (Exception e) + { + // Ignore the one caused from ctor only; other UpdateIcon calls are from plugins, and they should handle the + // error accordingly. + Log.Error(e, $"{nameof(ActiveNotification)}#{this.Id} ctor: {nameof(this.UpdateIcon)} failed and ignored."); + } } /// @@ -114,17 +126,8 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable } } - /// - public INotificationIconSource? IconSource - { - get => this.underlyingNotification.IconSource; - set - { - if (this.IsDismissed) - return; - this.underlyingNotification.IconSource = value; - } - } + /// + public INotificationIconSource? IconSource => this.underlyingNotification.IconSource; /// public DateTime Expiry @@ -264,23 +267,14 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable /// public void Dispose() { - this.ClearIconTask(); - this.underlyingNotification.IconSource = null; + this.ClearMaterializedIcon(); + this.underlyingNotification.Dispose(); this.Dismiss = null; this.Click = null; this.DrawActions = null; this.InitiatorPlugin = null; } - /// - public Notification CloneNotification() - { - var newValue = this.underlyingNotification with { }; - if (this.newProgress is { } p) - newValue.Progress = p; - return newValue; - } - /// public void DismissNow() => this.DismissNow(NotificationDismissReason.Programmatical); @@ -504,30 +498,26 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable return windowSize.Y; } - /// - public void Update(INotification newNotification) - { - if (this.IsDismissed) - return; - this.Content = newNotification.Content; - this.Title = newNotification.Title; - this.Type = newNotification.Type; - this.IconSource = newNotification.IconSource; - this.Expiry = newNotification.Expiry; - this.Interactable = newNotification.Interactable; - this.HoverExtendDuration = newNotification.HoverExtendDuration; - this.newProgress = newNotification.Progress; - } - /// public void UpdateIcon() { if (this.IsDismissed) return; - this.ClearIconTask(); + this.ClearMaterializedIcon(); this.MaterializedIcon = (this.IconSource as INotificationIconSource.IInternal)?.Materialize(); } + /// + public void UpdateIconSource(INotificationIconSource? newIconSource) + { + if (this.IsDismissed || this.underlyingNotification.IconSource == newIconSource) + return; + + this.underlyingNotification.IconSource?.Dispose(); + this.underlyingNotification.IconSource = newIconSource; + this.UpdateIcon(); + } + /// Removes non-Dalamud invocation targets from events. public void RemoveNonDalamudInvocations() { @@ -567,7 +557,7 @@ internal sealed class ActiveNotification : IActiveNotification, IDisposable } } - private void ClearIconTask() + private void ClearMaterializedIcon() { this.MaterializedIcon?.Dispose(); this.MaterializedIcon = null; diff --git a/Dalamud/Interface/ImGuiNotification/Internal/NotificationManager.cs b/Dalamud/Interface/ImGuiNotification/Internal/NotificationManager.cs index e5a27550c..fdea6146a 100644 --- a/Dalamud/Interface/ImGuiNotification/Internal/NotificationManager.cs +++ b/Dalamud/Interface/ImGuiNotification/Internal/NotificationManager.cs @@ -56,8 +56,9 @@ internal class NotificationManager : INotificationManager, IServiceType, IDispos } /// - public IActiveNotification AddNotification(Notification notification) + public IActiveNotification AddNotification(Notification notification, bool disposeNotification = true) { + using var disposer = disposeNotification ? notification : null; var an = new ActiveNotification(notification, null); this.pendingNotifications.Add(an); return an; @@ -65,10 +66,13 @@ internal class NotificationManager : INotificationManager, IServiceType, IDispos /// Adds a notification originating from a plugin. /// The notification. + /// Dispose when this function returns. /// The source plugin. - /// The new notification. - public IActiveNotification AddNotification(Notification notification, LocalPlugin plugin) + /// The added notification. + /// will be honored even on exceptions. + public IActiveNotification AddNotification(Notification notification, bool disposeNotification, LocalPlugin plugin) { + using var disposer = disposeNotification ? notification : null; var an = new ActiveNotification(notification, plugin); this.pendingNotifications.Add(an); return an; @@ -88,7 +92,8 @@ internal class NotificationManager : INotificationManager, IServiceType, IDispos Content = content, Title = title, Type = type, - }); + }, + true); /// Draw all currently queued notifications. public void Draw() @@ -101,7 +106,14 @@ internal class NotificationManager : INotificationManager, IServiceType, IDispos var maxWidth = Math.Max(320 * ImGuiHelpers.GlobalScale, viewportSize.X / 3); - this.notifications.RemoveAll(x => x.UpdateAnimations()); + this.notifications.RemoveAll(static x => + { + if (!x.UpdateAnimations()) + return false; + + x.Dispose(); + return true; + }); foreach (var tn in this.notifications) height += tn.Draw(maxWidth, height) + NotificationConstants.ScaledWindowGap; } @@ -127,9 +139,9 @@ internal class NotificationManagerPluginScoped : INotificationManager, IServiceT this.localPlugin = localPlugin; /// - public IActiveNotification AddNotification(Notification notification) + public IActiveNotification AddNotification(Notification notification, bool disposeNotification = true) { - var an = this.notificationManagerService.AddNotification(notification, this.localPlugin); + var an = this.notificationManagerService.AddNotification(notification, disposeNotification, this.localPlugin); _ = this.notifications.TryAdd(an, 0); an.Dismiss += (a, unused) => this.notifications.TryRemove(an, out _); return an; diff --git a/Dalamud/Interface/ImGuiNotification/Internal/NotificationUtilities.cs b/Dalamud/Interface/ImGuiNotification/Internal/NotificationUtilities.cs index 3bf8add07..3e24f628c 100644 --- a/Dalamud/Interface/ImGuiNotification/Internal/NotificationUtilities.cs +++ b/Dalamud/Interface/ImGuiNotification/Internal/NotificationUtilities.cs @@ -23,7 +23,22 @@ internal static class NotificationUtilities Vector2 maxCoord, LocalPlugin? initiatorPlugin) { - if (texture is null) + var handle = nint.Zero; + var size = Vector2.Zero; + if (texture is not null) + { + try + { + handle = texture.ImGuiHandle; + size = texture.Size; + } + catch + { + // must have been disposed or something; ignore the texture + } + } + + if (handle == nint.Zero) { var dam = Service.Get(); if (initiatorPlugin is null) @@ -46,14 +61,16 @@ internal static class NotificationUtilities }; } } + + handle = texture.ImGuiHandle; + size = texture.Size; } - var size = texture.Size; if (size.X > maxCoord.X - minCoord.X) size *= (maxCoord.X - minCoord.X) / size.X; if (size.Y > maxCoord.Y - minCoord.Y) size *= (maxCoord.Y - minCoord.Y) / size.Y; ImGui.SetCursorPos(((minCoord + maxCoord) - size) / 2); - ImGui.Image(texture.ImGuiHandle, size); + ImGui.Image(handle, size); } } diff --git a/Dalamud/Interface/ImGuiNotification/Notification.cs b/Dalamud/Interface/ImGuiNotification/Notification.cs index 97279d6c1..3b452bd2d 100644 --- a/Dalamud/Interface/ImGuiNotification/Notification.cs +++ b/Dalamud/Interface/ImGuiNotification/Notification.cs @@ -31,4 +31,11 @@ public sealed record Notification : INotification /// public float Progress { get; set; } = 1f; + + /// + public void Dispose() + { + this.IconSource?.Dispose(); + this.IconSource = null; + } } diff --git a/Dalamud/Interface/Internal/Windows/Data/Widgets/ImGuiWidget.cs b/Dalamud/Interface/Internal/Windows/Data/Widgets/ImGuiWidget.cs index 74dc8939c..1093ca4b8 100644 --- a/Dalamud/Interface/Internal/Windows/Data/Widgets/ImGuiWidget.cs +++ b/Dalamud/Interface/Internal/Windows/Data/Widgets/ImGuiWidget.cs @@ -79,27 +79,26 @@ internal class ImGuiWidget : IDataWindowWidget NotificationTemplate.IconSourceTitles.Length); switch (this.notificationTemplate.IconSourceInt) { - case 1: - case 2: + case 1 or 2: ImGui.InputText( "Icon Text##iconSourceText", ref this.notificationTemplate.IconSourceText, 255); break; - case 3: + case 3 or 4: ImGui.Combo( "Icon Source##iconSourceAssetCombo", ref this.notificationTemplate.IconSourceAssetInt, NotificationTemplate.AssetSources, NotificationTemplate.AssetSources.Length); break; - case 4: + case 5 or 7: ImGui.InputText( "Game Path##iconSourceText", ref this.notificationTemplate.IconSourceText, 255); break; - case 5: + case 6 or 8: ImGui.InputText( "File Path##iconSourceText", ref this.notificationTemplate.IconSourceText, @@ -170,17 +169,31 @@ internal class ImGuiWidget : IDataWindowWidget (FontAwesomeIcon)(this.notificationTemplate.IconSourceText.Length == 0 ? 0 : this.notificationTemplate.IconSourceText[0])), - 3 => new TextureWrapTaskIconSource( + 3 => new TextureWrapIconSource( + Service.Get().GetDalamudTextureWrap( + Enum.Parse( + NotificationTemplate.AssetSources[ + this.notificationTemplate.IconSourceAssetInt])), + false), + 4 => new TextureWrapTaskIconSource( () => Service.Get().GetDalamudTextureWrapAsync( Enum.Parse( NotificationTemplate.AssetSources[ this.notificationTemplate.IconSourceAssetInt]))), - 4 => new GamePathIconSource(this.notificationTemplate.IconSourceText), - 5 => new FilePathIconSource(this.notificationTemplate.IconSourceText), + 5 => new GamePathIconSource(this.notificationTemplate.IconSourceText), + 6 => new FilePathIconSource(this.notificationTemplate.IconSourceText), + 7 => new TextureWrapIconSource( + Service.Get().GetTextureFromGame(this.notificationTemplate.IconSourceText), + false), + 8 => new TextureWrapIconSource( + Service.Get().GetTextureFromFile( + new(this.notificationTemplate.IconSourceText)), + false), _ => null, }, - }); + }, + true); switch (this.notificationTemplate.ProgressMode) { case 2: @@ -276,9 +289,12 @@ internal class ImGuiWidget : IDataWindowWidget "None (use Type)", "SeIconChar", "FontAwesomeIcon", + "TextureWrap from DalamudAssets", "TextureWrapTask from DalamudAssets", "GamePath", "FilePath", + "TextureWrap from GamePath", + "TextureWrap from FilePath", }; public static readonly string[] AssetSources = diff --git a/Dalamud/Interface/UiBuilder.cs b/Dalamud/Interface/UiBuilder.cs index 64ff0cc45..1237c9c1f 100644 --- a/Dalamud/Interface/UiBuilder.cs +++ b/Dalamud/Interface/UiBuilder.cs @@ -581,6 +581,7 @@ public sealed class UiBuilder : IDisposable Type = type, Expiry = DateTime.Now + TimeSpan.FromMilliseconds(msDelay), }, + true, this.localPlugin); _ = this.notifications.TryAdd(an, 0); an.Dismiss += (a, unused) => this.notifications.TryRemove(an, out _); diff --git a/Dalamud/Plugin/Services/INotificationManager.cs b/Dalamud/Plugin/Services/INotificationManager.cs index 1d31ddd35..441cc31f7 100644 --- a/Dalamud/Plugin/Services/INotificationManager.cs +++ b/Dalamud/Plugin/Services/INotificationManager.cs @@ -11,6 +11,12 @@ public interface INotificationManager /// Adds a notification. /// /// The new notification. + /// + /// Dispose when this function returns, even if the function throws an exception. + /// Set to false to reuse for multiple calls to this function, in which case, + /// you should call on the value supplied to at a + /// later time. + /// /// The added notification. - IActiveNotification AddNotification(Notification notification); + IActiveNotification AddNotification(Notification notification, bool disposeNotification = true); }