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>

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++;
             }

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;

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)
             {

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>
@@ -1218,6 +1232,20 @@ namespace Ryujinx.Graphics.Gpu.Image
             _referenceCount++;
         }
 
+        /// <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.
@@ -1244,6 +1272,48 @@ namespace Ryujinx.Graphics.Gpu.Image
             return newRefCount <= 0;
         }
 
+        /// <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,
@@ -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>

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);
 

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;
                     }

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;
 
@@ -92,6 +96,39 @@ namespace Ryujinx.Graphics.Gpu.Image
             return texture;
         }
 
+        /// <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>
@@ -99,6 +136,8 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// <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();
         }
     }
 }

Ryujinx.Memory/Range/MultiRange.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 
 namespace Ryujinx.Memory.Range
 {
@@ -75,6 +76,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>

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();
                 }
             }

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>
+        /// <param name="mapped">True if the region handle starts mapped</param>
-        internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, bool dirty = true)
+        internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, bool mapped = true)
         {
-            Dirty = dirty;
+            Dirty = mapped;
+            Unmapped = !mapped;
             Address = address;
             Size = size;
             EndAddress = address + size;
@@ -128,6 +130,23 @@ namespace Ryujinx.Memory.Tracking
             _regions.Add(region);
         }
 
+        /// <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>

Ryujinx.Memory/Tracking/VirtualRegion.cs

@@ -66,6 +66,18 @@ namespace Ryujinx.Memory.Tracking
             UpdatePhysicalChildren();
         }
 
+        /// <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>