fix(PluginInstaller): make sure that icons load on the render thread in changelogs, to prevent access violations in DX

Dalamud/Interface/Internal/Windows/PluginImageCache.cs

@@ -239,9 +239,20 @@ namespace Dalamud.Interface.Internal.Windows
             var interfaceManager = Service<InterfaceManager>.Get();
             var pluginManager = Service<PluginManager>.Get();
 
-            static bool TryLoadIcon(byte[] bytes, string loc, PluginManifest manifest, InterfaceManager interfaceManager, out TextureWrap icon)
+            static bool TryLoadIcon(byte[] bytes, string? loc, PluginManifest manifest, InterfaceManager interfaceManager, out TextureWrap? icon)
             {
-                icon = interfaceManager.LoadImage(bytes);
+                // FIXME(goat): This is a hack around this call failing randomly in certain situations. Might be related to not being called on the main thread.
+                try
+                {
+                    icon = interfaceManager.LoadImage(bytes);
+                }
+                catch (AccessViolationException ex)
+                {
+                    Log.Error(ex, "Access violation during load plugin icon from {Loc}", loc);
+
+                    icon = null;
+                    return false;
+                }
 
                 if (icon == null)
                 {
@@ -332,9 +343,20 @@ namespace Dalamud.Interface.Internal.Windows
             var interfaceManager = Service<InterfaceManager>.Get();
             var pluginManager = Service<PluginManager>.Get();
 
-            static bool TryLoadImage(int i, byte[] bytes, string loc, PluginManifest manifest, InterfaceManager interfaceManager, out TextureWrap image)
+            static bool TryLoadImage(int i, byte[] bytes, string loc, PluginManifest manifest, InterfaceManager interfaceManager, out TextureWrap? image)
             {
-                image = interfaceManager.LoadImage(bytes);
+                // FIXME(goat): This is a hack around this call failing randomly in certain situations. Might be related to not being called on the main thread.
+                try
+                {
+                    image = interfaceManager.LoadImage(bytes);
+                }
+                catch (AccessViolationException ex)
+                {
+                    Log.Error(ex, "Access violation during load plugin image from {Loc}", loc);
+
+                    image = null;
+                    return false;
+                }
 
                 if (image == null)
                 {
@@ -351,43 +373,41 @@ namespace Dalamud.Interface.Internal.Windows
                 return true;
             }
 
-            if (plugin != null && plugin.IsDev)
+            if (plugin is { IsDev: true })
             {
                 var files = this.GetPluginImageFileInfos(plugin);
-                if (files != null)
+
+                var didAny = false;
+                var pluginImages = new TextureWrap[files.Count];
+                for (var i = 0; i < files.Count; i++)
                 {
-                    var didAny = false;
+                    var file = files[i];
-                    var pluginImages = new TextureWrap[files.Count];
-                    for (var i = 0; i < files.Count; i++)
-                    {
-                        var file = files[i];
 
-                        if (file == null)
+                    if (file == null)
-                            continue;
+                        continue;
 
-                        Log.Verbose($"Loading image{i + 1} for {manifest.InternalName} from {file.FullName}");
+                    Log.Verbose($"Loading image{i + 1} for {manifest.InternalName} from {file.FullName}");
-                        var bytes = await File.ReadAllBytesAsync(file.FullName);
+                    var bytes = await File.ReadAllBytesAsync(file.FullName);
 
-                        if (!TryLoadImage(i, bytes, file.FullName, manifest, interfaceManager, out var image))
+                    if (!TryLoadImage(i, bytes, file.FullName, manifest, interfaceManager, out var image))
-                            continue;
+                        continue;
 
-                        Log.Verbose($"Plugin image{i + 1} for {manifest.InternalName} loaded from disk");
+                    Log.Verbose($"Plugin image{i + 1} for {manifest.InternalName} loaded from disk");
-                        pluginImages[i] = image;
+                    pluginImages[i] = image;
 
-                        didAny = true;
+                    didAny = true;
-                    }
+                }
 
-                    if (didAny)
+                if (didAny)
-                    {
+                {
-                        Log.Verbose($"Plugin images for {manifest.InternalName} loaded from disk");
+                    Log.Verbose($"Plugin images for {manifest.InternalName} loaded from disk");
 
-                        if (pluginImages.Contains(null))
+                    if (pluginImages.Contains(null))
-                            pluginImages = pluginImages.Where(image => image != null).ToArray();
+                        pluginImages = pluginImages.Where(image => image != null).ToArray();
 
-                        this.pluginImagesMap[manifest.InternalName] = pluginImages;
+                    this.pluginImagesMap[manifest.InternalName] = pluginImages;
 
-                        return;
+                    return;
-                    }
                 }
 
                 // Dev plugins are likely going to look like a main repo plugin, the InstalledFrom field is going to be null.

Dalamud/Interface/Internal/Windows/PluginInstaller/DalamudChangelogEntry.cs

@@ -15,11 +15,9 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
         /// Initializes a new instance of the <see cref="DalamudChangelogEntry"/> class.
         /// </summary>
         /// <param name="changelog">The changelog.</param>
-        /// <param name="icon">The icon.</param>
+        public DalamudChangelogEntry(DalamudChangelog changelog)
-        public DalamudChangelogEntry(DalamudChangelog changelog, TextureWrap icon)
         {
             this.changelog = changelog;
-            this.Icon = icon;
 
             var changelogText = string.Empty;
             for (var i = 0; i < changelog.Changes.Count; i++)
@@ -45,9 +43,6 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
         /// <inheritdoc/>
         public string Text { get; init; }
 
-        /// <inheritdoc/>
-        public TextureWrap Icon { get; init; }
-
         /// <inheritdoc/>
         public DateTime Date => this.changelog.Date;
     }

Dalamud/Interface/Internal/Windows/PluginInstaller/IChangelogEntry.cs

@@ -24,11 +24,6 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
         /// </summary>
         string Text { get; }
 
-        /// <summary>
-        /// Gets the icon of the entry.
-        /// </summary>
-        TextureWrap Icon { get; }
-
         /// <summary>
         /// Gets the date of the entry.
         /// </summary>

Dalamud/Interface/Internal/Windows/PluginInstaller/PluginChangelogEntry.cs

@@ -11,17 +11,13 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
     /// </summary>
     internal class PluginChangelogEntry : IChangelogEntry
     {
-        private readonly LocalPlugin plugin;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="PluginChangelogEntry"/> class.
         /// </summary>
         /// <param name="plugin">The plugin manifest.</param>
-        /// <param name="icon">The icon.</param>
+        public PluginChangelogEntry(LocalPlugin plugin)
-        public PluginChangelogEntry(LocalPlugin plugin, TextureWrap icon)
         {
-            this.plugin = plugin;
+            this.Plugin = plugin;
-            this.Icon = icon;
 
             if (plugin.Manifest.Changelog.IsNullOrEmpty())
                 throw new ArgumentException("Manifest has no changelog.");
@@ -34,19 +30,21 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
             this.Version = version!.ToString();
         }
 
+        /// <summary>
+        /// Gets the respective plugin.
+        /// </summary>
+        public LocalPlugin Plugin { get; private set; }
+
         /// <inheritdoc/>
-        public string Title => this.plugin.Manifest.Name;
+        public string Title => this.Plugin.Manifest.Name;
 
         /// <inheritdoc/>
         public string Version { get; init; }
 
         /// <inheritdoc/>
-        public string Text => this.plugin.Manifest.Changelog!;
+        public string Text => this.Plugin.Manifest.Changelog!;
 
         /// <inheritdoc/>
-        public TextureWrap Icon { get; init; }
+        public DateTime Date => DateTimeOffset.FromUnixTimeSeconds(this.Plugin.Manifest.LastUpdate).DateTime;
-
-        /// <inheritdoc/>
-        public DateTime Date => DateTimeOffset.FromUnixTimeSeconds(this.plugin.Manifest.LastUpdate).DateTime;
     }
 }

Dalamud/Interface/Internal/Windows/PluginInstaller/PluginInstallerWindow.cs

@@ -467,52 +467,6 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
             }
         }
 
-        /*
-        private void DrawPluginTabBar()
-        {
-            ImGui.SetCursorPosY(ImGui.GetCursorPosY() - (5 * ImGuiHelpers.GlobalScale));
-
-            ImGui.PushStyleVar(ImGuiStyleVar.ItemSpacing, ImGuiHelpers.ScaledVector2(1, 3));
-
-            if (ImGui.BeginTabBar("PluginsTabBar", ImGuiTabBarFlags.NoTooltip))
-            {
-                this.DrawPluginTab(Locs.TabTitle_AvailablePlugins, this.DrawAvailablePluginList);
-                this.DrawPluginTab(Locs.TabTitle_InstalledPlugins, this.DrawInstalledPluginList);
-
-                if (this.hasDevPlugins)
-                {
-                    this.DrawPluginTab(Locs.TabTitle_InstalledDevPlugins, this.DrawInstalledDevPluginList);
-                    this.DrawPluginTab("Image/Icon Tester", this.DrawImageTester);
-                }
-            }
-
-            ImGui.PopStyleVar();
-        }
-        */
-
-        /*
-        private void DrawPluginTab(string title, Action drawPluginList)
-        {
-            if (ImGui.BeginTabItem(title))
-            {
-                ImGui.BeginChild($"Scrolling{title}", ImGuiHelpers.ScaledVector2(0, -30), true, ImGuiWindowFlags.HorizontalScrollbar | ImGuiWindowFlags.NoBackground);
-
-                ImGui.SetCursorPosY(ImGui.GetCursorPosY() - 5);
-
-                var ready = this.DrawPluginListLoading();
-
-                if (ready)
-                {
-                    drawPluginList();
-                }
-
-                ImGui.EndChild();
-
-                ImGui.EndTabItem();
-            }
-        }
-        */
-
         private void DrawChangelogList(bool displayDalamud, bool displayPlugins)
         {
             if (this.pluginListInstalled.Count == 0)
@@ -526,14 +480,7 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
                                                     && !plugin.Manifest.Changelog.IsNullOrEmpty())
                                    .Select(x =>
                                    {
-                                       var iconTex = this.imageCache.DefaultIcon;
+                                       var changelog = new PluginChangelogEntry(x);
-                                       var hasIcon = this.imageCache.TryGetIcon(x, x.Manifest, x.Manifest.IsThirdParty, out var cachedIconTex);
-                                       if (hasIcon && cachedIconTex != null)
-                                       {
-                                           iconTex = cachedIconTex;
-                                       }
-
-                                       var changelog = new PluginChangelogEntry(x, iconTex);
                                        return (IChangelogEntry)changelog;
                                    });
 
@@ -542,12 +489,12 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
             {
                 changelogs = pluginChangelogs
                              .Concat(this.dalamudChangelogManager.Changelogs.Select(
-                                         x => new DalamudChangelogEntry(x, this.imageCache.CorePluginIcon)));
+                                         x => new DalamudChangelogEntry(x)));
             }
             else if (displayDalamud && this.dalamudChangelogManager.Changelogs != null)
             {
                 changelogs = this.dalamudChangelogManager.Changelogs.Select(
-                    x => new DalamudChangelogEntry(x, this.imageCache.CorePluginIcon));
+                    x => new DalamudChangelogEntry(x));
             }
             else if (displayPlugins)
             {
@@ -1256,7 +1203,22 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
 
             var iconSize = ImGuiHelpers.ScaledVector2(64, 64);
 
-            ImGui.Image(log.Icon.ImGuiHandle, iconSize);
+            TextureWrap icon;
+            if (log is PluginChangelogEntry pluginLog)
+            {
+                icon = this.imageCache.DefaultIcon;
+                var hasIcon = this.imageCache.TryGetIcon(pluginLog.Plugin, pluginLog.Plugin.Manifest, pluginLog.Plugin.Manifest.IsThirdParty, out var cachedIconTex);
+                if (hasIcon && cachedIconTex != null)
+                {
+                    icon = cachedIconTex;
+                }
+            }
+            else
+            {
+                icon = this.imageCache.CorePluginIcon;
+            }
+
+            ImGui.Image(icon.ImGuiHandle, iconSize);
             ImGui.SameLine();
 
             ImGuiHelpers.ScaledDummy(5);
@@ -2055,8 +2017,10 @@ namespace Dalamud.Interface.Internal.Windows.PluginInstaller
                 (manifest.Tags != null && manifest.Tags.Contains(searchString, StringComparer.InvariantCultureIgnoreCase)));
         }
 
-        private (bool IsInstalled, LocalPlugin Plugin) IsManifestInstalled(RemotePluginManifest manifest)
+        private (bool IsInstalled, LocalPlugin Plugin) IsManifestInstalled(RemotePluginManifest? manifest)
         {
+            if (manifest == null) return (false, default);
+
             var plugin = this.pluginListInstalled.FirstOrDefault(plugin => plugin.Manifest.InternalName == manifest.InternalName);
             var isInstalled = plugin != default;