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
This commit is contained in:
gdkchan 2023-11-15 21:36:25 -03:00 committed by GitHub
parent cdc8fed64f
commit dcf10561b9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 102 additions and 61 deletions

View file

@ -211,6 +211,7 @@ namespace Ryujinx.Graphics.Gpu.Engine.Dma
int xCount = (int)_state.State.LineLengthIn; int xCount = (int)_state.State.LineLengthIn;
int yCount = (int)_state.State.LineCount; int yCount = (int)_state.State.LineCount;
_channel.TextureManager.RefreshModifiedTextures();
_3dEngine.CreatePendingSyncs(); _3dEngine.CreatePendingSyncs();
_3dEngine.FlushUboDirty(); _3dEngine.FlushUboDirty();

View file

@ -101,11 +101,6 @@ namespace Ryujinx.Graphics.Gpu.Image
/// </summary> /// </summary>
public bool AlwaysFlushOnOverlap { get; private set; } 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> /// <summary>
/// Increments when the host texture is swapped, or when the texture is removed from all pools. /// Increments when the host texture is swapped, or when the texture is removed from all pools.
/// </summary> /// </summary>
@ -1443,7 +1438,7 @@ namespace Ryujinx.Graphics.Gpu.Image
if (_modifiedStale || Group.HasCopyDependencies || Group.HasFlushBuffer) if (_modifiedStale || Group.HasCopyDependencies || Group.HasFlushBuffer)
{ {
_modifiedStale = false; _modifiedStale = false;
Group.SignalModifying(this, bound, bound || ModifiedSinceLastFlush || Group.HasCopyDependencies || Group.HasFlushBuffer); Group.SignalModifying(this, bound);
} }
_physicalMemory.TextureCache.Lift(this); _physicalMemory.TextureCache.Lift(this);

View file

@ -709,8 +709,7 @@ namespace Ryujinx.Graphics.Gpu.Image
/// </summary> /// </summary>
/// <param name="texture">The texture that has been modified</param> /// <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="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)
public void SignalModifying(Texture texture, bool bound, bool setModified)
{ {
ModifiedSequence = _context.GetModifiedSequence(); ModifiedSequence = _context.GetModifiedSequence();
@ -722,7 +721,7 @@ namespace Ryujinx.Graphics.Gpu.Image
{ {
TextureGroupHandle group = _handles[baseHandle + i]; TextureGroupHandle group = _handles[baseHandle + i];
group.SignalModifying(bound, _context, setModified); group.SignalModifying(bound, _context);
} }
}); });
} }
@ -993,26 +992,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> /// <summary>
/// Generate a CpuRegionHandle for a given address and size range in CPU VA. /// Generate a CpuRegionHandle for a given address and size range in CPU VA.
/// </summary> /// </summary>
@ -1084,11 +1063,6 @@ namespace Ryujinx.Graphics.Gpu.Image
views, views,
result.ToArray()); result.ToArray());
foreach (RegionHandle handle in result)
{
handle.RegisterDirtyEvent(() => DirtyAction(groupHandle));
}
return 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); 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 }; handles = new TextureGroupHandle[] { groupHandle };
} }
else else
@ -1620,6 +1589,7 @@ namespace Ryujinx.Graphics.Gpu.Image
if ((ignore == null || !handle.HasDependencyTo(ignore)) && handle.Modified) if ((ignore == null || !handle.HasDependencyTo(ignore)) && handle.Modified)
{ {
handle.Modified = false; handle.Modified = false;
handle.DeferredCopy = null;
Storage.SignalModifiedDirty(); Storage.SignalModifiedDirty();
lock (handle.Overlaps) lock (handle.Overlaps)
@ -1666,8 +1636,6 @@ namespace Ryujinx.Graphics.Gpu.Image
return; return;
} }
Storage.ModifiedSinceLastFlush = false;
// There is a small gap here where the action is removed but _actionRegistered is still 1. // 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, // 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. // so there shouldn't be any issue as it's the same handler for all actions.

View file

@ -152,6 +152,32 @@ namespace Ryujinx.Graphics.Gpu.Image
// Linear textures are presumed to be used for readback initially. // Linear textures are presumed to be used for readback initially.
_flushBalance = FlushBalanceThreshold + FlushBalanceIncrement; _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> /// <summary>
@ -304,17 +330,9 @@ namespace Ryujinx.Graphics.Gpu.Image
/// </summary> /// </summary>
/// <param name="bound">True if this handle is being bound, false if unbound</param> /// <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="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)
public void SignalModifying(bool bound, GpuContext context, bool setModified)
{
if (setModified)
{ {
SignalModified(context); SignalModified(context);
}
else
{
RegisterSync(context);
}
if (!bound && _syncActionRegistered && NextSyncCopies()) if (!bound && _syncActionRegistered && NextSyncCopies())
{ {
@ -457,7 +475,6 @@ namespace Ryujinx.Graphics.Gpu.Image
public void DeferCopy(TextureGroupHandle copyFrom) public void DeferCopy(TextureGroupHandle copyFrom)
{ {
Modified = false; Modified = false;
DeferredCopy = copyFrom; DeferredCopy = copyFrom;
_group.Storage.SignalGroupDirty(); _group.Storage.SignalGroupDirty();
@ -514,7 +531,7 @@ namespace Ryujinx.Graphics.Gpu.Image
{ {
existing.Other.Handle.CreateCopyDependency(this); existing.Other.Handle.CreateCopyDependency(this);
if (copyToOther) if (copyToOther && Modified)
{ {
existing.Other.Handle.DeferCopy(this); existing.Other.Handle.DeferCopy(this);
} }
@ -558,10 +575,10 @@ namespace Ryujinx.Graphics.Gpu.Image
if (fromHandle != null) if (fromHandle != null)
{ {
// Only copy if the copy texture is still modified. // 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. // It will also set as unmodified if a copy is deferred to it.
shouldCopy = fromHandle.Modified; shouldCopy = true;
if (fromHandle._bindCount == 0) if (fromHandle._bindCount == 0)
{ {

View file

@ -20,8 +20,10 @@ namespace Ryujinx.Graphics.Gpu.Image
private readonly Texture[] _rtColors; private readonly Texture[] _rtColors;
private readonly ITexture[] _rtHostColors; private readonly ITexture[] _rtHostColors;
private readonly bool[] _rtColorsBound;
private Texture _rtDepthStencil; private Texture _rtDepthStencil;
private ITexture _rtHostDs; private ITexture _rtHostDs;
private bool _rtDsBound;
public int ClipRegionWidth { get; private set; } public int ClipRegionWidth { get; private set; }
public int ClipRegionHeight { get; private set; } public int ClipRegionHeight { get; private set; }
@ -51,6 +53,7 @@ namespace Ryujinx.Graphics.Gpu.Image
_rtColors = new Texture[Constants.TotalRenderTargets]; _rtColors = new Texture[Constants.TotalRenderTargets];
_rtHostColors = new ITexture[Constants.TotalRenderTargets]; _rtHostColors = new ITexture[Constants.TotalRenderTargets];
_rtColorsBound = new bool[Constants.TotalRenderTargets];
} }
/// <summary> /// <summary>
@ -153,8 +156,15 @@ namespace Ryujinx.Graphics.Gpu.Image
bool changesScale = (hasValue != (_rtColors[index] != null)) || (hasValue && RenderTargetScale != color.ScaleFactor); bool changesScale = (hasValue != (_rtColors[index] != null)) || (hasValue && RenderTargetScale != color.ScaleFactor);
if (_rtColors[index] != color) if (_rtColors[index] != color)
{
if (_rtColorsBound[index])
{ {
_rtColors[index]?.SignalModifying(false); _rtColors[index]?.SignalModifying(false);
}
else
{
_rtColorsBound[index] = true;
}
if (color != null) if (color != null)
{ {
@ -179,8 +189,15 @@ namespace Ryujinx.Graphics.Gpu.Image
bool changesScale = (hasValue != (_rtDepthStencil != null)) || (hasValue && RenderTargetScale != depthStencil.ScaleFactor); bool changesScale = (hasValue != (_rtDepthStencil != null)) || (hasValue && RenderTargetScale != depthStencil.ScaleFactor);
if (_rtDepthStencil != depthStencil) if (_rtDepthStencil != depthStencil)
{
if (_rtDsBound)
{ {
_rtDepthStencil?.SignalModifying(false); _rtDepthStencil?.SignalModifying(false);
}
else
{
_rtDsBound = true;
}
if (depthStencil != null) if (depthStencil != null)
{ {
@ -419,7 +436,12 @@ namespace Ryujinx.Graphics.Gpu.Image
if (dsTexture != null) if (dsTexture != null)
{ {
hostDsTexture = dsTexture.HostTexture; hostDsTexture = dsTexture.HostTexture;
dsTexture.ModifiedSinceLastFlush = true;
if (!_rtDsBound)
{
dsTexture.SignalModifying(true);
_rtDsBound = true;
}
} }
if (_rtHostDs != hostDsTexture) if (_rtHostDs != hostDsTexture)
@ -436,7 +458,12 @@ namespace Ryujinx.Graphics.Gpu.Image
if (texture != null) if (texture != null)
{ {
hostTexture = texture.HostTexture; hostTexture = texture.HostTexture;
texture.ModifiedSinceLastFlush = true;
if (!_rtColorsBound[index])
{
texture.SignalModifying(true);
_rtColorsBound[index] = true;
}
} }
if (_rtHostColors[index] != hostTexture) if (_rtHostColors[index] != hostTexture)
@ -466,6 +493,31 @@ namespace Ryujinx.Graphics.Gpu.Image
_context.Renderer.Pipeline.SetRenderTargets(_rtHostColors, _rtHostDs); _context.Renderer.Pipeline.SetRenderTargets(_rtHostColors, _rtHostDs);
} }
/// <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> /// <summary>
/// Forces the texture and sampler pools to be re-loaded from the cache on next use. /// Forces the texture and sampler pools to be re-loaded from the cache on next use.
/// </summary> /// </summary>
@ -501,12 +553,20 @@ namespace Ryujinx.Graphics.Gpu.Image
_samplerPoolCache.Dispose(); _samplerPoolCache.Dispose();
for (int i = 0; i < _rtColors.Length; i++) for (int i = 0; i < _rtColors.Length; i++)
{
if (_rtColorsBound[i])
{ {
_rtColors[i]?.DecrementReferenceCount(); _rtColors[i]?.DecrementReferenceCount();
}
_rtColors[i] = null; _rtColors[i] = null;
} }
if (_rtDsBound)
{
_rtDepthStencil?.DecrementReferenceCount(); _rtDepthStencil?.DecrementReferenceCount();
}
_rtDepthStencil = null; _rtDepthStencil = null;
} }
} }