Fix missing texture flush for draw then DMA copy sequence without render target change (#5933)1.1.1087

* Unbind render targets before DMA copy * Move DirtyAction to TextureGroupHandle * Fix lost copy dependency bug * XML doc
author: gdkchan <gab.dark.100@gmail.com> 2023-11-15 21:36:25 -0300
committer: GitHub <noreply@github.com> 2023-11-15 21:36:25 -0300
commit: dcf10561b996cdba111c5a3c3fe128781ab44021 (patch)
tree: 6649be78f1ea86ff09f151bae622dc5b55fda669
parent: cdc8fed64fc8e51fe626b0a369902932db0ec49c (diff)
5 files changed, 102 insertions, 61 deletions
diff --git a/src/Ryujinx.Graphics.Gpu/Engine/Dma/DmaClass.cs b/src/Ryujinx.Graphics.Gpu/Engine/Dma/DmaClass.cs
index 199cc423..b1921bd5 100644
--- a/src/Ryujinx.Graphics.Gpu/Engine/Dma/DmaClass.cs
+++ b/src/Ryujinx.Graphics.Gpu/Engine/Dma/DmaClass.cs
@@ -211,6 +211,7 @@ namespace Ryujinx.Graphics.Gpu.Engine.Dma
             int xCount = (int)_state.State.LineLengthIn;
             int yCount = (int)_state.State.LineCount;
 
+            _channel.TextureManager.RefreshModifiedTextures();
             _3dEngine.CreatePendingSyncs();
             _3dEngine.FlushUboDirty();
 
diff --git a/src/Ryujinx.Graphics.Gpu/Image/Texture.cs b/src/Ryujinx.Graphics.Gpu/Image/Texture.cs
index 04c2b615..f1615b38 100644
--- a/src/Ryujinx.Graphics.Gpu/Image/Texture.cs
+++ b/src/Ryujinx.Graphics.Gpu/Image/Texture.cs
@@ -102,11 +102,6 @@ namespace Ryujinx.Graphics.Gpu.Image
         public bool AlwaysFlushOnOverlap { get; private set; }
 
         /// <summary>
-        /// Indicates that the texture was modified since the last time it was flushed.
-        /// </summary>
-        public bool ModifiedSinceLastFlush { get; set; }
-
-        /// <summary>
         /// Increments when the host texture is swapped, or when the texture is removed from all pools.
         /// </summary>
         public int InvalidatedSequence { get; private set; }
@@ -1443,7 +1438,7 @@ namespace Ryujinx.Graphics.Gpu.Image
             if (_modifiedStale || Group.HasCopyDependencies || Group.HasFlushBuffer)
             {
                 _modifiedStale = false;
-                Group.SignalModifying(this, bound, bound || ModifiedSinceLastFlush || Group.HasCopyDependencies || Group.HasFlushBuffer);
+                Group.SignalModifying(this, bound);
             }
 
             _physicalMemory.TextureCache.Lift(this);
diff --git a/src/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs b/src/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
index e828cb9f..b93f3832 100644
--- a/src/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
+++ b/src/Ryujinx.Graphics.Gpu/Image/TextureGroup.cs
@@ -709,8 +709,7 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// </summary>
         /// <param name="texture">The texture that has been modified</param>
         /// <param name="bound">True if this texture is being bound, false if unbound</param>
-        /// <param name="setModified">Indicates if the modified flag should be set</param>
-        public void SignalModifying(Texture texture, bool bound, bool setModified)
+        public void SignalModifying(Texture texture, bool bound)
         {
             ModifiedSequence = _context.GetModifiedSequence();
 
@@ -722,7 +721,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                 {
                     TextureGroupHandle group = _handles[baseHandle + i];
 
-                    group.SignalModifying(bound, _context, setModified);
+                    group.SignalModifying(bound, _context);
                 }
             });
         }
@@ -994,26 +993,6 @@ namespace Ryujinx.Graphics.Gpu.Image
         }
 
         /// <summary>
-        /// The action to perform when a memory tracking handle is flipped to dirty.
-        /// This notifies overlapping textures that the memory needs to be synchronized.
-        /// </summary>
-        /// <param name="groupHandle">The handle that a dirty flag was set on</param>
-        private void DirtyAction(TextureGroupHandle groupHandle)
-        {
-            // Notify all textures that belong to this handle.
-
-            Storage.SignalGroupDirty();
-
-            lock (groupHandle.Overlaps)
-            {
-                foreach (Texture overlap in groupHandle.Overlaps)
-                {
-                    overlap.SignalGroupDirty();
-                }
-            }
-        }
-
-        /// <summary>
         /// Generate a CpuRegionHandle for a given address and size range in CPU VA.
         /// </summary>
         /// <param name="address">The start address of the tracked region</param>
@@ -1084,11 +1063,6 @@ namespace Ryujinx.Graphics.Gpu.Image
                 views,
                 result.ToArray());
 
-            foreach (RegionHandle handle in result)
-            {
-                handle.RegisterDirtyEvent(() => DirtyAction(groupHandle));
-            }
-
             return groupHandle;
         }
 
@@ -1360,11 +1334,6 @@ namespace Ryujinx.Graphics.Gpu.Image
 
                 var groupHandle = new TextureGroupHandle(this, 0, Storage.Size, _views, 0, 0, 0, _allOffsets.Length, cpuRegionHandles);
 
-                foreach (RegionHandle handle in cpuRegionHandles)
-                {
-                    handle.RegisterDirtyEvent(() => DirtyAction(groupHandle));
-                }
-
                 handles = new TextureGroupHandle[] { groupHandle };
             }
             else
@@ -1620,6 +1589,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                     if ((ignore == null || !handle.HasDependencyTo(ignore)) && handle.Modified)
                     {
                         handle.Modified = false;
+                        handle.DeferredCopy = null;
                         Storage.SignalModifiedDirty();
 
                         lock (handle.Overlaps)
@@ -1666,8 +1636,6 @@ namespace Ryujinx.Graphics.Gpu.Image
                 return;
             }
 
-            Storage.ModifiedSinceLastFlush = false;
-
             // There is a small gap here where the action is removed but _actionRegistered is still 1.
             // In this case it will skip registering the action, but here we are already handling it,
             // so there shouldn't be any issue as it's the same handler for all actions.
diff --git a/src/Ryujinx.Graphics.Gpu/Image/TextureGroupHandle.cs b/src/Ryujinx.Graphics.Gpu/Image/TextureGroupHandle.cs
index 717792e0..72b74390 100644
--- a/src/Ryujinx.Graphics.Gpu/Image/TextureGroupHandle.cs
+++ b/src/Ryujinx.Graphics.Gpu/Image/TextureGroupHandle.cs
@@ -152,6 +152,32 @@ namespace Ryujinx.Graphics.Gpu.Image
                 // Linear textures are presumed to be used for readback initially.
                 _flushBalance = FlushBalanceThreshold + FlushBalanceIncrement;
             }
+
+            foreach (RegionHandle handle in handles)
+            {
+                handle.RegisterDirtyEvent(DirtyAction);
+            }
+        }
+
+        /// <summary>
+        /// The action to perform when a memory tracking handle is flipped to dirty.
+        /// This notifies overlapping textures that the memory needs to be synchronized.
+        /// </summary>
+        private void DirtyAction()
+        {
+            // Notify all textures that belong to this handle.
+
+            _group.Storage.SignalGroupDirty();
+
+            lock (Overlaps)
+            {
+                foreach (Texture overlap in Overlaps)
+                {
+                    overlap.SignalGroupDirty();
+                }
+            }
+
+            DeferredCopy = null;
         }
 
         /// <summary>
@@ -304,17 +330,9 @@ namespace Ryujinx.Graphics.Gpu.Image
         /// </summary>
         /// <param name="bound">True if this handle is being bound, false if unbound</param>
         /// <param name="context">The GPU context to register a sync action on</param>
-        /// <param name="setModified">Indicates if the modified flag should be set</param>
-        public void SignalModifying(bool bound, GpuContext context, bool setModified)
+        public void SignalModifying(bool bound, GpuContext context)
         {
-            if (setModified)
-            {
-                SignalModified(context);
-            }
-            else
-            {
-                RegisterSync(context);
-            }
+            SignalModified(context);
 
             if (!bound && _syncActionRegistered && NextSyncCopies())
             {
@@ -457,7 +475,6 @@ namespace Ryujinx.Graphics.Gpu.Image
         public void DeferCopy(TextureGroupHandle copyFrom)
         {
             Modified = false;
-
             DeferredCopy = copyFrom;
 
             _group.Storage.SignalGroupDirty();
@@ -514,7 +531,7 @@ namespace Ryujinx.Graphics.Gpu.Image
                 {
                     existing.Other.Handle.CreateCopyDependency(this);
 
-                    if (copyToOther)
+                    if (copyToOther && Modified)
                     {
                         existing.Other.Handle.DeferCopy(this);
                     }
@@ -558,10 +575,10 @@ namespace Ryujinx.Graphics.Gpu.Image
                 if (fromHandle != null)
                 {
                     // Only copy if the copy texture is still modified.
-                    // It will be set as unmodified if new data is written from CPU, as the data previously in the texture will flush.
+                    // DeferredCopy will be set to null if new data is written from CPU (see the DirtyAction method).
                     // It will also set as unmodified if a copy is deferred to it.
 
-                    shouldCopy = fromHandle.Modified;
+                    shouldCopy = true;
 
                     if (fromHandle._bindCount == 0)
                     {
diff --git a/src/Ryujinx.Graphics.Gpu/Image/TextureManager.cs b/src/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
index e9f58314..fa278bbd 100644
--- a/src/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
+++ b/src/Ryujinx.Graphics.Gpu/Image/TextureManager.cs
@@ -20,8 +20,10 @@ namespace Ryujinx.Graphics.Gpu.Image
 
         private readonly Texture[] _rtColors;
         private readonly ITexture[] _rtHostColors;
+        private readonly bool[] _rtColorsBound;
         private Texture _rtDepthStencil;
         private ITexture _rtHostDs;
+        private bool _rtDsBound;
 
         public int ClipRegionWidth { get; private set; }
         public int ClipRegionHeight { get; private set; }
@@ -51,6 +53,7 @@ namespace Ryujinx.Graphics.Gpu.Image
 
             _rtColors = new Texture[Constants.TotalRenderTargets];
             _rtHostColors = new ITexture[Constants.TotalRenderTargets];
+            _rtColorsBound = new bool[Constants.TotalRenderTargets];
         }
 
         /// <summary>
@@ -154,7 +157,14 @@ namespace Ryujinx.Graphics.Gpu.Image
 
             if (_rtColors[index] != color)
             {
-                _rtColors[index]?.SignalModifying(false);
+                if (_rtColorsBound[index])
+                {
+                    _rtColors[index]?.SignalModifying(false);
+                }
+                else
+                {
+                    _rtColorsBound[index] = true;
+                }
 
                 if (color != null)
                 {
@@ -180,7 +190,14 @@ namespace Ryujinx.Graphics.Gpu.Image
 
             if (_rtDepthStencil != depthStencil)
             {
-                _rtDepthStencil?.SignalModifying(false);
+                if (_rtDsBound)
+                {
+                    _rtDepthStencil?.SignalModifying(false);
+                }
+                else
+                {
+                    _rtDsBound = true;
+                }
 
                 if (depthStencil != null)
                 {
@@ -419,7 +436,12 @@ namespace Ryujinx.Graphics.Gpu.Image
             if (dsTexture != null)
             {
                 hostDsTexture = dsTexture.HostTexture;
-                dsTexture.ModifiedSinceLastFlush = true;
+
+                if (!_rtDsBound)
+                {
+                    dsTexture.SignalModifying(true);
+                    _rtDsBound = true;
+                }
             }
 
             if (_rtHostDs != hostDsTexture)
@@ -436,7 +458,12 @@ namespace Ryujinx.Graphics.Gpu.Image
                 if (texture != null)
                 {
                     hostTexture = texture.HostTexture;
-                    texture.ModifiedSinceLastFlush = true;
+
+                    if (!_rtColorsBound[index])
+                    {
+                        texture.SignalModifying(true);
+                        _rtColorsBound[index] = true;
+                    }
                 }
 
                 if (_rtHostColors[index] != hostTexture)
@@ -467,6 +494,31 @@ namespace Ryujinx.Graphics.Gpu.Image
         }
 
         /// <summary>
+        /// Marks all currently bound render target textures as modified, and also makes them be set as modified again on next use.
+        /// </summary>
+        public void RefreshModifiedTextures()
+        {
+            Texture dsTexture = _rtDepthStencil;
+
+            if (dsTexture != null && _rtDsBound)
+            {
+                dsTexture.SignalModifying(false);
+                _rtDsBound = false;
+            }
+
+            for (int index = 0; index < _rtColors.Length; index++)
+            {
+                Texture texture = _rtColors[index];
+
+                if (texture != null && _rtColorsBound[index])
+                {
+                    texture.SignalModifying(false);
+                    _rtColorsBound[index] = false;
+                }
+            }
+        }
+
+        /// <summary>
         /// Forces the texture and sampler pools to be re-loaded from the cache on next use.
         /// </summary>
         public void ReloadPools()
@@ -502,11 +554,19 @@ namespace Ryujinx.Graphics.Gpu.Image
 
             for (int i = 0; i < _rtColors.Length; i++)
             {
-                _rtColors[i]?.DecrementReferenceCount();
+                if (_rtColorsBound[i])
+                {
+                    _rtColors[i]?.DecrementReferenceCount();
+                }
+
                 _rtColors[i] = null;
             }
 
-            _rtDepthStencil?.DecrementReferenceCount();
+            if (_rtDsBound)
+            {
+                _rtDepthStencil?.DecrementReferenceCount();
+            }
+
             _rtDepthStencil = null;
         }
     }
author	gdkchan <gab.dark.100@gmail.com>	2023-11-15 21:36:25 -0300
committer	GitHub <noreply@github.com>	2023-11-15 21:36:25 -0300
commit	dcf10561b996cdba111c5a3c3fe128781ab44021 (patch)
tree	6649be78f1ea86ff09f151bae622dc5b55fda669
parent	cdc8fed64fc8e51fe626b0a369902932db0ec49c (diff)