Improve handling for unmapped GPU resources (#2083)

* Improve handling for unmapped GPU resources

- Fixed a memory tracking bug that would set protection on empty PTEs
- When a texture's memory is (partially) unmapped, all pool references are forcibly removed and the texture must be rediscovered to draw with it. This will also force the texture discovery to always compare the texture's range for a match.
- RegionHandles now know if they are unmapped, and automatically unset their dirty flag when unmapped.
- Partial texture sync now loads only the region of texture that has been modified. Unmapped memory tracking handles cause dirty flags for a texture group handle to be ignored.

This greatly improves the emulator's stability for newer UE4 games.

* Address feedback, fix MultiRange slice

Fixed an issue where the size of the multi-range slice would be miscalculated.

* Update Ryujinx.Memory/Range/MultiRange.cs (feedback)

Co-authored-by: Mary <thog@protonmail.com>

Co-authored-by: Mary <thog@protonmail.com>

5 files changed, 136 insertions, 11 deletions

diff --git a/Ryujinx.Graphics.Gpu/Image/Pool.cs b/Ryujinx.Graphics.Gpu/Image/Pool.cs
index c5aef77f..855f6344 100644
--- a/Ryujinx.Graphics.Gpu/Image/Pool.cs
+++ b/Ryujinx.Graphics.Gpu/Image/Pool.cs
@@ -117,7 +117,7 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// Performs the disposal of all resources stored on the pool.
         /// It's an error to try using the pool after disposal.
         /// </summary>
-        public void Dispose()
+        public virtual void Dispose()
         {
             if (Items != null)
             {
diff --git a/Ryujinx.Graphics.Gpu/Image/Texture.cs b/Ryujinx.Graphics.Gpu/Image/Texture.cs
index 4d4091cb..26e102a9 100644
--- a/Ryujinx.Graphics.Gpu/Image/Texture.cs
+++ b/Ryujinx.Graphics.Gpu/Image/Texture.cs
@@ -21,6 +21,12 @@ namespace Ryujinx.Graphics.Gpu.Image
         // This method uses much more memory so we want to avoid it if possible.
         private const int ByteComparisonSwitchThreshold = 4;
 
+        private struct TexturePoolOwner
+        {
+            public TexturePool Pool;
+            public int ID;
+        }
+
         private GpuContext _context;
 
         private SizeInfo _sizeInfo;
@@ -64,7 +70,13 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// Set when a texture has been changed size. This indicates that it may need to be
         /// changed again when obtained as a sampler.
         /// </summary>
-        public bool ChangedSize { get; internal set; }
+        public bool ChangedSize { get; private set; }
+
+        /// <summary>
+        /// Set when a texture's GPU VA has ever been partially or fully unmapped. 
+        /// This indicates that the range must be fully checked when matching the texture.
+        /// </summary>
+        public bool ChangedMapping { get; private set; }
 
         private int _depth;
         private int _layers;
@@ -121,6 +133,7 @@ namespace Ryujinx.Graphics.Gpu.Image
         public bool IsView => _viewStorage != this;
 
         private int _referenceCount;
+        private List<TexturePoolOwner> _poolOwners;
 
         /// <summary>
         /// Constructs a new instance of the cached GPU texture.
@@ -190,6 +203,7 @@ namespace Ryujinx.Graphics.Gpu.Image
             _viewStorage = this;
 
             _views = new List<Texture>();
+            _poolOwners = new List<TexturePoolOwner>();
         }
 
         /// <summary>
@@ -1219,6 +1233,20 @@ namespace Ryujinx.Graphics.Gpu.Image
         }
 
         /// <summary>
+        /// Increments the reference count and records the given texture pool and ID as a pool owner.
+        /// </summary>
+        /// <param name="pool">The texture pool this texture has been added to</param>
+        /// <param name="id">The ID of the reference to this texture in the pool</param>
+        public void IncrementReferenceCount(TexturePool pool, int id)
+        {
+            lock (_poolOwners)
+            {
+                _poolOwners.Add(new TexturePoolOwner { Pool = pool, ID = id });
+            }
+            _referenceCount++;
+        }
+
+        /// <summary>
         /// Decrements the texture reference count.
         /// When the reference count hits zero, the texture may be deleted and can't be used anymore.
         /// </summary>
@@ -1245,6 +1273,48 @@ namespace Ryujinx.Graphics.Gpu.Image
         }
 
         /// <summary>
+        /// Decrements the texture reference count, also removing an associated pool owner reference.
+        /// When the reference count hits zero, the texture may be deleted and can't be used anymore.
+        /// </summary>
+        /// <param name="pool">The texture pool this texture is being removed from</param>
+        /// <param name="id">The ID of the reference to this texture in the pool</param>
+        /// <returns>True if the texture is now referenceless, false otherwise</returns>
+        public bool DecrementReferenceCount(TexturePool pool, int id = -1)
+        {
+            lock (_poolOwners)
+            {
+                int references = _poolOwners.RemoveAll(entry => entry.Pool == pool && entry.ID == id || id == -1);
+
+                if (references == 0)
+                {
+                    // This reference has already been removed.
+                    return _referenceCount <= 0;
+                }
+
+                Debug.Assert(references == 1);
+            }
+
+            return DecrementReferenceCount();
+        }
+
+        /// <summary>
+        /// Forcibly remove this texture from all pools that reference it.
+        /// </summary>
+        /// <param name="deferred">Indicates if the removal is being done from another thread.</param>
+        public void RemoveFromPools(bool deferred)
+        {
+            lock (_poolOwners)
+            {
+                foreach (var owner in _poolOwners)
+                {
+                    owner.Pool.ForceRemove(this, owner.ID, deferred);
+                }
+
+                _poolOwners.Clear();
+            }
+        }
+
+        /// <summary>
         /// Delete the texture if it is not used anymore.
         /// The texture is considered unused when the reference count is zero,
         /// and it has no child views.
@@ -1282,7 +1352,11 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// </summary>
         public void Unmapped()
         {
+            ChangedMapping = true;
+
             IsModified = false; // We shouldn't flush this texture, as its memory is no longer mapped.
+
+            RemoveFromPools(true);
         }
 
         /// <summary>
diff --git a/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs b/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
index 5d150559..39567a82 100644
--- a/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
+++ b/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
@@ -122,6 +122,7 @@ namespace Ryujinx.Graphics.Gpu.Image
             {
                 bool dirty = false;
                 bool anyModified = false;
+                bool anyUnmapped = false;
 
                 for (int i = 0; i < regionCount; i++)
                 {
@@ -130,6 +131,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                     bool modified = group.Modified;
                     bool handleDirty = false;
                     bool handleModified = false;
+                    bool handleUnmapped = false;
 
                     foreach (CpuRegionHandle handle in group.Handles)
                     {
@@ -140,6 +142,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                         }
                         else
                         {
+                            handleUnmapped |= handle.Unmapped;
                             handleModified |= modified;
                         }
                     }
@@ -159,18 +162,20 @@ namespace Ryujinx.Graphics.Gpu.Image
                         dirty |= handleDirty;
                     }
 
+                    anyUnmapped |= handleUnmapped;
+
                     if (group.NeedsCopy)
                     {
                         // The texture we copied from is still being written to. Copy from it again the next time this texture is used.
                         texture.SignalGroupDirty();
                     }
 
-                    _loadNeeded[baseHandle + i] = handleDirty;
+                    _loadNeeded[baseHandle + i] = handleDirty && !handleUnmapped;
                 }
 
                 if (dirty)
                 {
-                    if (_handles.Length > 1 && (anyModified || split))
+                    if (anyUnmapped || (_handles.Length > 1 && (anyModified || split)))
                     {
                         // Partial texture invalidation. Only update the layers/levels with dirty flags of the storage.
 
@@ -194,8 +199,6 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// <param name="regionCount">The number of handles to synchronize</param>
         private void SynchronizePartial(int baseHandle, int regionCount)
         {
-            ReadOnlySpan<byte> fullData = _context.PhysicalMemory.GetSpan(Storage.Range);
-
             for (int i = 0; i < regionCount; i++)
             {
                 if (_loadNeeded[baseHandle + i])
@@ -212,7 +215,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                             int endOffset = (offsetIndex + 1 == _allOffsets.Length) ? (int)Storage.Size : _allOffsets[offsetIndex + 1];
                             int size = endOffset - offset;
 
-                            ReadOnlySpan<byte> data = fullData.Slice(offset, size);
+                            ReadOnlySpan<byte> data = _context.PhysicalMemory.GetSpan(Storage.Range.GetSlice((ulong)offset, (ulong)size));
 
                             data = Storage.ConvertToHostCompatibleFormat(data, info.BaseLevel, true);
 
diff --git a/Ryujinx.Graphics.Gpu/Image/TextureManager.cs b/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
index f13c3443..07bc7c7e 100644
--- a/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
+++ b/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
@@ -715,7 +715,9 @@ namespace Ryujinx.Graphics.Gpu.Image
                     // we know the textures are located at the same memory region.
                     // If they don't, it may still be mapped to the same physical region, so we
                     // do a more expensive check to tell if they are mapped into the same physical regions.
-                    if (overlap.Info.GpuAddress != info.GpuAddress && !_context.MemoryManager.CompareRange(overlap.Range, info.GpuAddress))
+                    // If the GPU VA for the texture has ever been unmapped, then the range must be checked regardless.
+                    if ((overlap.Info.GpuAddress != info.GpuAddress || overlap.ChangedMapping) && 
+                        !_context.MemoryManager.CompareRange(overlap.Range, info.GpuAddress))
                     {
                         continue;
                     }
diff --git a/Ryujinx.Graphics.Gpu/Image/TexturePool.cs b/Ryujinx.Graphics.Gpu/Image/TexturePool.cs
index 58e881ca..eece2a79 100644
--- a/Ryujinx.Graphics.Gpu/Image/TexturePool.cs
+++ b/Ryujinx.Graphics.Gpu/Image/TexturePool.cs
@@ -2,6 +2,7 @@ using Ryujinx.Common.Logging;
 using Ryujinx.Graphics.GAL;
 using Ryujinx.Graphics.Texture;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 
 namespace Ryujinx.Graphics.Gpu.Image
@@ -12,6 +13,7 @@ namespace Ryujinx.Graphics.Gpu.Image
     class TexturePool : Pool<Texture, TextureDescriptor>
     {
         private int _sequenceNumber;
+        private readonly ConcurrentQueue<Texture> _dereferenceQueue = new ConcurrentQueue<Texture>();
 
         /// <summary>
         /// Intrusive linked list node used on the texture pool cache.
@@ -53,6 +55,8 @@ namespace Ryujinx.Graphics.Gpu.Image
 
                 TextureInfo info = GetInfo(descriptor, out int layerSize);
 
+                ProcessDereferenceQueue();
+
                 texture = Context.Methods.TextureManager.FindOrCreateTexture(TextureSearchFlags.ForSampler, info, layerSize);
 
                 // If this happens, then the texture address is invalid, we can't add it to the cache.
@@ -61,7 +65,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                     return null;
                 }
 
-                texture.IncrementReferenceCount();
+                texture.IncrementReferenceCount(this, id);
 
                 Items[id] = texture;
 
@@ -93,12 +97,47 @@ namespace Ryujinx.Graphics.Gpu.Image
         }
 
         /// <summary>
+        /// Forcibly remove a texture from this pool's items.
+        /// If deferred, the dereference will be queued to occur on the render thread.
+        /// </summary>
+        /// <param name="texture">The texture being removed</param>
+        /// <param name="id">The ID of the texture in this pool</param>
+        /// <param name="deferred">If true, queue the dereference to happen on the render thread, otherwise dereference immediately</param>
+        public void ForceRemove(Texture texture, int id, bool deferred)
+        {
+            Items[id] = null;
+
+            if (deferred)
+            {
+                _dereferenceQueue.Enqueue(texture);
+            }
+            else
+            {
+                texture.DecrementReferenceCount();
+            }
+        }
+
+        /// <summary>
+        /// Process the dereference queue, decrementing the reference count for each texture in it.
+        /// This is used to ensure that texture disposal happens on the render thread.
+        /// </summary>
+        private void ProcessDereferenceQueue()
+        {
+            while (_dereferenceQueue.TryDequeue(out Texture toRemove))
+            {
+                toRemove.DecrementReferenceCount();
+            }
+        }
+
+        /// <summary>
         /// Implementation of the texture pool range invalidation.
         /// </summary>
         /// <param name="address">Start address of the range of the texture pool</param>
         /// <param name="size">Size of the range being invalidated</param>
         protected override void InvalidateRangeImpl(ulong address, ulong size)
         {
+            ProcessDereferenceQueue();
+
             ulong endAddress = address + size;
 
             for (; address < endAddress; address += DescriptorSize)
@@ -118,7 +157,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                         continue;
                     }
 
-                    texture.DecrementReferenceCount();
+                    texture.DecrementReferenceCount(this, id);
 
                     Items[id] = null;
                 }
@@ -342,7 +381,14 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// <param name="item">The texture to be deleted</param>
         protected override void Delete(Texture item)
         {
-            item?.DecrementReferenceCount();
+            item?.DecrementReferenceCount(this);
+        }
+
+        public override void Dispose()
+        {
+            ProcessDereferenceQueue();
+
+            base.Dispose();
         }
     }
 }
\ No newline at end of file