From 233a865c7837376ee5d558a2b3043bf1a2381f66 Mon Sep 17 00:00:00 2001
From: Exter-N <exter-n@exter-n.fr>
Date: Thu, 31 Aug 2023 20:05:39 +0200
Subject: [PATCH] Material editor: use a SafeHandle for texture swapping

---
 .../MaterialPreview/LiveColorSetPreviewer.cs  | 37 +++++---------
 .../Interop/SafeHandles/SafeTextureHandle.cs  | 48 +++++++++++++++++++
 2 files changed, 59 insertions(+), 26 deletions(-)
 create mode 100644 Penumbra/Interop/SafeHandles/SafeTextureHandle.cs

diff --git a/Penumbra/Interop/MaterialPreview/LiveColorSetPreviewer.cs b/Penumbra/Interop/MaterialPreview/LiveColorSetPreviewer.cs
index 18afa949..f927aa43 100644
--- a/Penumbra/Interop/MaterialPreview/LiveColorSetPreviewer.cs
+++ b/Penumbra/Interop/MaterialPreview/LiveColorSetPreviewer.cs
@@ -5,6 +5,7 @@ using Dalamud.Plugin.Services;
 using FFXIVClientStructs.FFXIV.Client.Graphics.Kernel;
 using FFXIVClientStructs.FFXIV.Client.Graphics.Render;
 using Penumbra.GameData.Files;
+using Penumbra.Interop.SafeHandles;
 
 namespace Penumbra.Interop.MaterialPreview;
 
@@ -16,8 +17,8 @@ public sealed unsafe class LiveColorSetPreviewer : LiveMaterialPreviewerBase
 
     private readonly Framework _framework;
 
-    private readonly Texture** _colorSetTexture;
-    private readonly Texture*  _originalColorSetTexture;
+    private readonly Texture**         _colorSetTexture;
+    private readonly SafeTextureHandle _originalColorSetTexture;
 
     private Half[] _colorSet;
     private bool   _updatePending;
@@ -40,12 +41,10 @@ public sealed unsafe class LiveColorSetPreviewer : LiveMaterialPreviewerBase
 
         _colorSetTexture = colorSetTextures + (MaterialInfo.ModelSlot * 4 + MaterialInfo.MaterialSlot);
 
-        _originalColorSetTexture = *_colorSetTexture;
+        _originalColorSetTexture = new SafeTextureHandle(*_colorSetTexture, true);
         if (_originalColorSetTexture == null)
             throw new InvalidOperationException("Material doesn't have a color set");
 
-        Structs.TextureUtility.IncRef(_originalColorSetTexture);
-
         _colorSet      = new Half[TextureLength];
         _updatePending = true;
 
@@ -59,15 +58,9 @@ public sealed unsafe class LiveColorSetPreviewer : LiveMaterialPreviewerBase
         base.Clear(disposing, reset);
 
         if (reset)
-        {
-            var oldTexture = (Texture*)Interlocked.Exchange(ref *(nint*)_colorSetTexture, (nint)_originalColorSetTexture);
-            if (oldTexture != null)
-                Structs.TextureUtility.DecRef(oldTexture);
-        }
-        else
-        {
-            Structs.TextureUtility.DecRef(_originalColorSetTexture);
-        }
+            _originalColorSetTexture.Exchange(ref *(nint*)_colorSetTexture);
+
+        _originalColorSetTexture.Dispose();
     }
 
     public void ScheduleUpdate()
@@ -89,8 +82,8 @@ public sealed unsafe class LiveColorSetPreviewer : LiveMaterialPreviewerBase
         textureSize[0] = TextureWidth;
         textureSize[1] = TextureHeight;
 
-        var newTexture = Structs.TextureUtility.Create2D(Device.Instance(), textureSize, 1, 0x2460, 0x80000804, 7);
-        if (newTexture == null)
+        using var texture = new SafeTextureHandle(Structs.TextureUtility.Create2D(Device.Instance(), textureSize, 1, 0x2460, 0x80000804, 7), false);
+        if (texture.IsInvalid)
             return;
 
         bool success;
@@ -98,20 +91,12 @@ public sealed unsafe class LiveColorSetPreviewer : LiveMaterialPreviewerBase
         {
             fixed (Half* colorSet = _colorSet)
             {
-                success = Structs.TextureUtility.InitializeContents(newTexture, colorSet);
+                success = Structs.TextureUtility.InitializeContents(texture.Texture, colorSet);
             }
         }
 
         if (success)
-        {
-            var oldTexture = (Texture*)Interlocked.Exchange(ref *(nint*)_colorSetTexture, (nint)newTexture);
-            if (oldTexture != null)
-                Structs.TextureUtility.DecRef(oldTexture);
-        }
-        else
-        {
-            Structs.TextureUtility.DecRef(newTexture);
-        }
+            texture.Exchange(ref *(nint*)_colorSetTexture);
     }
 
     protected override bool IsStillValid()
diff --git a/Penumbra/Interop/SafeHandles/SafeTextureHandle.cs b/Penumbra/Interop/SafeHandles/SafeTextureHandle.cs
new file mode 100644
index 00000000..36cd4612
--- /dev/null
+++ b/Penumbra/Interop/SafeHandles/SafeTextureHandle.cs
@@ -0,0 +1,48 @@
+using System;
+using System.Runtime.InteropServices;
+using System.Threading;
+using FFXIVClientStructs.FFXIV.Client.Graphics.Render;
+using Penumbra.Interop.Structs;
+
+namespace Penumbra.Interop.SafeHandles;
+
+public unsafe class SafeTextureHandle : SafeHandle
+{
+    public Texture* Texture => (Texture*)handle;
+
+    public override bool IsInvalid => handle == 0;
+
+    public SafeTextureHandle(Texture* handle, bool incRef, bool ownsHandle = true) : base(0, ownsHandle)
+    {
+        if (incRef && !ownsHandle)
+            throw new ArgumentException("Non-owning SafeTextureHandle with IncRef is unsupported");
+        if (incRef && handle != null)
+            TextureUtility.IncRef(handle);
+        SetHandle((nint)handle);
+    }
+
+    public void Exchange(ref nint ppTexture)
+    {
+        lock (this)
+        {
+            handle = Interlocked.Exchange(ref ppTexture, handle);
+        }
+    }
+
+    public static SafeTextureHandle CreateInvalid()
+        => new(null, incRef: false);
+
+    protected override bool ReleaseHandle()
+    {
+        nint handle;
+        lock (this)
+        {
+            handle = this.handle;
+            this.handle = 0;
+        }
+        if (handle != 0)
+            TextureUtility.DecRef((Texture*)handle);
+
+        return true;
+    }
+}