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>

11 files changed, 229 insertions, 15 deletions

diff --git a/Ryujinx.Cpu/MemoryManager.cs b/Ryujinx.Cpu/MemoryManager.cs
index 83c288ae..59654e8b 100644
--- a/Ryujinx.Cpu/MemoryManager.cs
+++ b/Ryujinx.Cpu/MemoryManager.cs
@@ -627,7 +627,7 @@ namespace Ryujinx.Cpu
                 {
                     pte = Volatile.Read(ref pageRef);
                 }
-                while (Interlocked.CompareExchange(ref pageRef, (pte & invTagMask) | tag, pte) != pte);
+                while (pte != 0 && Interlocked.CompareExchange(ref pageRef, (pte & invTagMask) | tag, pte) != pte);
 
                 pageStart++;
             }
diff --git a/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs b/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs
index f4391aad..6a530b0e 100644
--- a/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs
+++ b/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs
@@ -8,6 +8,7 @@ namespace Ryujinx.Cpu.Tracking
         private readonly RegionHandle _impl;
 
         public bool Dirty => _impl.Dirty;
+        public bool Unmapped => _impl.Unmapped;
         public ulong Address => _impl.Address;
         public ulong Size => _impl.Size;
         public ulong EndAddress => _impl.EndAddress;
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
diff --git a/Ryujinx.Memory/Range/MultiRange.cs b/Ryujinx.Memory/Range/MultiRange.cs
index b40daa8a..3d505c7c 100644
--- a/Ryujinx.Memory/Range/MultiRange.cs
+++ b/Ryujinx.Memory/Range/MultiRange.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 
 namespace Ryujinx.Memory.Range
 {
@@ -76,6 +77,53 @@ namespace Ryujinx.Memory.Range
         }
 
         /// <summary>
+        /// Gets a slice of the multi-range.
+        /// </summary>
+        /// <param name="offset">Offset of the slice into the multi-range in bytes</param>
+        /// <param name="size">Size of the slice in bytes</param>
+        /// <returns>A new multi-range representing the given slice of this one</returns>
+        public MultiRange GetSlice(ulong offset, ulong size)
+        {
+            if (HasSingleRange)
+            {
+                if (_singleRange.Size - offset < size)
+                {
+                    throw new ArgumentOutOfRangeException(nameof(size));
+                }
+
+                return new MultiRange(_singleRange.Address + offset, size);
+            }
+            else
+            {
+                var ranges = new List<MemoryRange>();
+
+                foreach (MemoryRange range in _ranges)
+                {
+                    if ((long)offset <= 0)
+                    {
+                        ranges.Add(new MemoryRange(range.Address, Math.Min(size, range.Size)));
+                        size -= range.Size;
+                    }
+                    else if (offset < range.Size)
+                    {
+                        ulong sliceSize = Math.Min(size, range.Size - offset);
+                        ranges.Add(new MemoryRange(range.Address + offset, sliceSize));
+                        size -= sliceSize;
+                    }
+
+                    if ((long)size <= 0)
+                    {
+                        break;
+                    }
+
+                    offset -= range.Size;
+                }
+
+                return new MultiRange(ranges.ToArray());
+            }
+        }
+
+        /// <summary>
         /// Gets the physical region at the specified index.
         /// </summary>
         /// <param name="index">Index of the physical region</param>
diff --git a/Ryujinx.Memory/Tracking/MemoryTracking.cs b/Ryujinx.Memory/Tracking/MemoryTracking.cs
index aff223e8..6485e566 100644
--- a/Ryujinx.Memory/Tracking/MemoryTracking.cs
+++ b/Ryujinx.Memory/Tracking/MemoryTracking.cs
@@ -74,6 +74,14 @@ namespace Ryujinx.Memory.Tracking
                 for (int i = 0; i < count; i++)
                 {
                     VirtualRegion region = results[i];
+
+                    // If the region has been fully remapped, signal that it has been mapped again.
+                    bool remapped = _memoryManager.IsRangeMapped(region.Address, region.Size);
+                    if (remapped)
+                    {
+                        region.SignalMappingChanged(true);
+                    }
+
                     region.RecalculatePhysicalChildren();
                     region.UpdateProtection();
                 }
@@ -99,6 +107,7 @@ namespace Ryujinx.Memory.Tracking
                 for (int i = 0; i < count; i++)
                 {
                     VirtualRegion region = results[i];
+                    region.SignalMappingChanged(false);
                     region.RecalculatePhysicalChildren();
                 }
             }
diff --git a/Ryujinx.Memory/Tracking/RegionHandle.cs b/Ryujinx.Memory/Tracking/RegionHandle.cs
index 4da184dd..da3ee99a 100644
--- a/Ryujinx.Memory/Tracking/RegionHandle.cs
+++ b/Ryujinx.Memory/Tracking/RegionHandle.cs
@@ -12,6 +12,7 @@ namespace Ryujinx.Memory.Tracking
     public class RegionHandle : IRegionHandle, IRange
     {
         public bool Dirty { get; private set; }
+        public bool Unmapped { get; private set; }
 
         public ulong Address { get; }
         public ulong Size { get; }
@@ -37,10 +38,11 @@ namespace Ryujinx.Memory.Tracking
         /// <param name="tracking">Tracking object for the target memory block</param>
         /// <param name="address">Virtual address of the region to track</param>
         /// <param name="size">Size of the region to track</param>
-        /// <param name="dirty">Initial value of the dirty flag</param>
-        internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, bool dirty = true)
+        /// <param name="mapped">True if the region handle starts mapped</param>
+        internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, bool mapped = true)
         {
-            Dirty = dirty;
+            Dirty = mapped;
+            Unmapped = !mapped;
             Address = address;
             Size = size;
             EndAddress = address + size;
@@ -129,6 +131,23 @@ namespace Ryujinx.Memory.Tracking
         }
 
         /// <summary>
+        /// Signal that this handle has been mapped or unmapped.
+        /// </summary>
+        /// <param name="mapped">True if the handle has been mapped, false if unmapped</param>
+        internal void SignalMappingChanged(bool mapped)
+        {
+            if (Unmapped == mapped)
+            {
+                Unmapped = !mapped;
+
+                if (Unmapped)
+                {
+                    Dirty = false;
+                }
+            }
+        }
+
+        /// <summary>
         /// Check if this region overlaps with another.
         /// </summary>
         /// <param name="address">Base address</param>
diff --git a/Ryujinx.Memory/Tracking/VirtualRegion.cs b/Ryujinx.Memory/Tracking/VirtualRegion.cs
index 15a11568..fcf2fbe0 100644
--- a/Ryujinx.Memory/Tracking/VirtualRegion.cs
+++ b/Ryujinx.Memory/Tracking/VirtualRegion.cs
@@ -67,6 +67,18 @@ namespace Ryujinx.Memory.Tracking
         }
 
         /// <summary>
+        /// Signal that this region has been mapped or unmapped.
+        /// </summary>
+        /// <param name="mapped">True if the region has been mapped, false if unmapped</param>
+        public void SignalMappingChanged(bool mapped)
+        {
+            foreach (RegionHandle handle in Handles)
+            {
+                handle.SignalMappingChanged(mapped);
+            }
+        }
+
+        /// <summary>
         /// Gets the strictest permission that the child handles demand. Assumes that the tracking lock has been obtained.
         /// </summary>
         /// <returns>Protection level that this region demands</returns>