From dcf10561b996cdba111c5a3c3fe128781ab44021 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Wed, 15 Nov 2023 21:36:25 -0300 Subject: [PATCH] Fix missing texture flush for draw then DMA copy sequence without render target change (#5933) * Unbind render targets before DMA copy * Move DirtyAction to TextureGroupHandle * Fix lost copy dependency bug * XML doc --- .../Engine/Dma/DmaClass.cs | 1 + src/Ryujinx.Graphics.Gpu/Image/Texture.cs | 7 +- .../Image/TextureGroup.cs | 38 +--------- .../Image/TextureGroupHandle.cs | 45 ++++++++---- .../Image/TextureManager.cs | 72 +++++++++++++++++-- 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 199cc423e..b1921bd51 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 04c2b615f..f1615b388 100644 --- a/src/Ryujinx.Graphics.Gpu/Image/Texture.cs +++ b/src/Ryujinx.Graphics.Gpu/Image/Texture.cs @@ -101,11 +101,6 @@ namespace Ryujinx.Graphics.Gpu.Image /// public bool AlwaysFlushOnOverlap { get; private set; } - /// - /// Indicates that the texture was modified since the last time it was flushed. - /// - public bool ModifiedSinceLastFlush { get; set; } - /// /// Increments when the host texture is swapped, or when the texture is removed from all pools. /// @@ -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 e828cb9f1..b93f3832d 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 /// /// The texture that has been modified /// True if this texture is being bound, false if unbound - /// Indicates if the modified flag should be set - 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); } }); } @@ -993,26 +992,6 @@ namespace Ryujinx.Graphics.Gpu.Image } } - /// - /// The action to perform when a memory tracking handle is flipped to dirty. - /// This notifies overlapping textures that the memory needs to be synchronized. - /// - /// The handle that a dirty flag was set on - 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(); - } - } - } - /// /// Generate a CpuRegionHandle for a given address and size range in CPU VA. /// @@ -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 717792e04..72b743906 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); + } + } + + /// + /// The action to perform when a memory tracking handle is flipped to dirty. + /// This notifies overlapping textures that the memory needs to be synchronized. + /// + 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; } /// @@ -304,17 +330,9 @@ namespace Ryujinx.Graphics.Gpu.Image /// /// True if this handle is being bound, false if unbound /// The GPU context to register a sync action on - /// Indicates if the modified flag should be set - 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 e9f58314f..fa278bbd9 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]; } /// @@ -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) @@ -466,6 +493,31 @@ namespace Ryujinx.Graphics.Gpu.Image _context.Renderer.Pipeline.SetRenderTargets(_rtHostColors, _rtHostDs); } + /// + /// Marks all currently bound render target textures as modified, and also makes them be set as modified again on next use. + /// + 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; + } + } + } + /// /// Forces the texture and sampler pools to be re-loaded from the cache on next use. /// @@ -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; } }