From 4b60371e64601dba46387f8b7260b3deb770e097 Mon Sep 17 00:00:00 2001 From: riperiperi Date: Mon, 19 Jul 2021 23:10:54 +0100 Subject: [PATCH] Return mapped buffer pointer directly for flush, WriteableRegion for textures (#2494) * Return mapped buffer pointer directly for flush, WriteableRegion for textures A few changes here to generally improve performance, even for platforms not using the persistent buffer flush. - Texture and buffer flush now return a ReadOnlySpan. It's guaranteed that this span is pinned in memory, but it will be overwritten on the next flush from that thread, so it is expected that the data is used before calling again. - As a result, persistent mappings no longer copy to a new array - rather the persistent map is returned directly as a Span<>. A similar host array is used for the glGet flushes instead of allocating new arrays each time. - Texture flushes now do their layout conversion into a WriteableRegion when the texture is not MultiRange, which allows the flush to happen directly into guest memory rather than into a temporary span, then copied over. This avoids another copy when doing layout conversion. Overall, this saves 1 data copy for buffer flush, 1 copy for linear textures with matching source/target stride, and 2 copies for block textures or linear textures with mismatching strides. * Fix tests * Fix array pointer for Mesa/Intel path * Address some feedback * Update method for getting array pointer. --- Ryujinx.Cpu/MemoryManager.cs | 9 ++- Ryujinx.Cpu/MemoryManagerHostMapped.cs | 11 +++- Ryujinx.Graphics.GAL/IRenderer.cs | 2 +- Ryujinx.Graphics.GAL/ITexture.cs | 2 +- Ryujinx.Graphics.Gpu/Image/Texture.cs | 55 +++++++++++++++---- Ryujinx.Graphics.Gpu/Memory/Buffer.cs | 2 +- Ryujinx.Graphics.Gpu/Memory/PhysicalMemory.cs | 5 +- Ryujinx.Graphics.OpenGL/Buffer.cs | 17 ++++-- .../Image/TextureBuffer.cs | 4 +- Ryujinx.Graphics.OpenGL/Image/TextureView.cs | 14 ++--- Ryujinx.Graphics.OpenGL/PersistentBuffers.cs | 32 +++++++---- Ryujinx.Graphics.OpenGL/Renderer.cs | 11 +--- Ryujinx.Graphics.Texture/LayoutConverter.cs | 22 +++++++- .../MockVirtualMemoryManager.cs | 2 +- Ryujinx.Memory/AddressSpaceManager.cs | 3 +- Ryujinx.Memory/IVirtualMemoryManager.cs | 3 +- Ryujinx.Memory/IWritableBlock.cs | 2 + Ryujinx.Memory/WritableRegion.cs | 13 ++++- 18 files changed, 143 insertions(+), 66 deletions(-) diff --git a/Ryujinx.Cpu/MemoryManager.cs b/Ryujinx.Cpu/MemoryManager.cs index 9eb27c4f..153bff62 100644 --- a/Ryujinx.Cpu/MemoryManager.cs +++ b/Ryujinx.Cpu/MemoryManager.cs @@ -237,7 +237,7 @@ namespace Ryujinx.Cpu } /// - public unsafe WritableRegion GetWritableRegion(ulong va, int size) + public unsafe WritableRegion GetWritableRegion(ulong va, int size, bool tracked = false) { if (size == 0) { @@ -246,6 +246,11 @@ namespace Ryujinx.Cpu if (IsContiguousAndMapped(va, size)) { + if (tracked) + { + SignalMemoryTracking(va, (ulong)size, true); + } + return new WritableRegion(null, va, new NativeMemoryManager((byte*)GetHostAddress(va), size).Memory); } else @@ -254,7 +259,7 @@ namespace Ryujinx.Cpu GetSpan(va, size).CopyTo(memory.Span); - return new WritableRegion(this, va, memory); + return new WritableRegion(this, va, memory, tracked); } } diff --git a/Ryujinx.Cpu/MemoryManagerHostMapped.cs b/Ryujinx.Cpu/MemoryManagerHostMapped.cs index 0b4e0e4c..705cedeb 100644 --- a/Ryujinx.Cpu/MemoryManagerHostMapped.cs +++ b/Ryujinx.Cpu/MemoryManagerHostMapped.cs @@ -285,9 +285,16 @@ namespace Ryujinx.Cpu } /// - public WritableRegion GetWritableRegion(ulong va, int size) + public WritableRegion GetWritableRegion(ulong va, int size, bool tracked = false) { - AssertMapped(va, (ulong)size); + if (tracked) + { + SignalMemoryTracking(va, (ulong)size, true); + } + else + { + AssertMapped(va, (ulong)size); + } return _addressSpaceMirror.GetWritableRegion(va, size); } diff --git a/Ryujinx.Graphics.GAL/IRenderer.cs b/Ryujinx.Graphics.GAL/IRenderer.cs index 18f10915..56a40172 100644 --- a/Ryujinx.Graphics.GAL/IRenderer.cs +++ b/Ryujinx.Graphics.GAL/IRenderer.cs @@ -27,7 +27,7 @@ namespace Ryujinx.Graphics.GAL void DeleteBuffer(BufferHandle buffer); - byte[] GetBufferData(BufferHandle buffer, int offset, int size); + ReadOnlySpan GetBufferData(BufferHandle buffer, int offset, int size); Capabilities GetCapabilities(); diff --git a/Ryujinx.Graphics.GAL/ITexture.cs b/Ryujinx.Graphics.GAL/ITexture.cs index ad8fd297..c011b05c 100644 --- a/Ryujinx.Graphics.GAL/ITexture.cs +++ b/Ryujinx.Graphics.GAL/ITexture.cs @@ -14,7 +14,7 @@ namespace Ryujinx.Graphics.GAL ITexture CreateView(TextureCreateInfo info, int firstLayer, int firstLevel); - byte[] GetData(); + ReadOnlySpan GetData(); void SetData(ReadOnlySpan data); void SetData(ReadOnlySpan data, int layer, int level); diff --git a/Ryujinx.Graphics.Gpu/Image/Texture.cs b/Ryujinx.Graphics.Gpu/Image/Texture.cs index c9bff561..a3105cf2 100644 --- a/Ryujinx.Graphics.Gpu/Image/Texture.cs +++ b/Ryujinx.Graphics.Gpu/Image/Texture.cs @@ -4,6 +4,7 @@ using Ryujinx.Graphics.GAL; using Ryujinx.Graphics.Gpu.Memory; using Ryujinx.Graphics.Texture; using Ryujinx.Graphics.Texture.Astc; +using Ryujinx.Memory; using Ryujinx.Memory.Range; using System; using System.Collections.Generic; @@ -821,14 +822,7 @@ namespace Ryujinx.Graphics.Gpu.Image return; // Flushing this format is not supported, as it may have been converted to another host format. } - if (tracked) - { - _physicalMemory.Write(Range, GetTextureDataFromGpu(tracked)); - } - else - { - _physicalMemory.WriteUntracked(Range, GetTextureDataFromGpu(tracked)); - } + FlushTextureDataToGuest(tracked); } /// @@ -864,10 +858,44 @@ namespace Ryujinx.Graphics.Gpu.Image texture = _flushHostTexture = GetScaledHostTexture(1f, _flushHostTexture); } - _physicalMemory.WriteUntracked(Range, GetTextureDataFromGpu(false, texture)); + FlushTextureDataToGuest(false, texture); }); } + /// + /// Gets data from the host GPU, and flushes it to guest memory. + /// + /// + /// This method should be used to retrieve data that was modified by the host GPU. + /// This is not cheap, avoid doing that unless strictly needed. + /// When possible, the data is written directly into guest memory, rather than copied. + /// + /// True if writing the texture data is tracked, false otherwise + /// The specific host texture to flush. Defaults to this texture + private void FlushTextureDataToGuest(bool tracked, ITexture texture = null) + { + if (Range.Count == 1) + { + MemoryRange subrange = Range.GetSubRange(0); + + using (WritableRegion region = _physicalMemory.GetWritableRegion(subrange.Address, (int)subrange.Size, tracked)) + { + GetTextureDataFromGpu(region.Memory.Span, tracked, texture); + } + } + else + { + if (tracked) + { + _physicalMemory.Write(Range, GetTextureDataFromGpu(Span.Empty, true, texture)); + } + else + { + _physicalMemory.WriteUntracked(Range, GetTextureDataFromGpu(Span.Empty, false, texture)); + } + } + } + /// /// Gets data from the host GPU. /// @@ -875,8 +903,11 @@ namespace Ryujinx.Graphics.Gpu.Image /// This method should be used to retrieve data that was modified by the host GPU. /// This is not cheap, avoid doing that unless strictly needed. /// - /// Host texture data - private ReadOnlySpan GetTextureDataFromGpu(bool blacklist, ITexture texture = null) + /// An output span to place the texture data into. If empty, one is generated + /// True if the texture should be blacklisted, false otherwise + /// The specific host texture to flush. Defaults to this texture + /// The span containing the texture data + private ReadOnlySpan GetTextureDataFromGpu(Span output, bool blacklist, ITexture texture = null) { ReadOnlySpan data; @@ -909,6 +940,7 @@ namespace Ryujinx.Graphics.Gpu.Image if (Info.IsLinear) { data = LayoutConverter.ConvertLinearToLinearStrided( + output, Info.Width, Info.Height, Info.FormatInfo.BlockWidth, @@ -920,6 +952,7 @@ namespace Ryujinx.Graphics.Gpu.Image else { data = LayoutConverter.ConvertLinearToBlockLinear( + output, Info.Width, Info.Height, _depth, diff --git a/Ryujinx.Graphics.Gpu/Memory/Buffer.cs b/Ryujinx.Graphics.Gpu/Memory/Buffer.cs index 96e10e77..3f869a50 100644 --- a/Ryujinx.Graphics.Gpu/Memory/Buffer.cs +++ b/Ryujinx.Graphics.Gpu/Memory/Buffer.cs @@ -412,7 +412,7 @@ namespace Ryujinx.Graphics.Gpu.Memory { int offset = (int)(address - Address); - byte[] data = _context.Renderer.GetBufferData(Handle, offset, (int)size); + ReadOnlySpan data = _context.Renderer.GetBufferData(Handle, offset, (int)size); // TODO: When write tracking shaders, they will need to be aware of changes in overlapping buffers. _physicalMemory.WriteUntracked(address, data); diff --git a/Ryujinx.Graphics.Gpu/Memory/PhysicalMemory.cs b/Ryujinx.Graphics.Gpu/Memory/PhysicalMemory.cs index 289e7c1b..8df3c8fb 100644 --- a/Ryujinx.Graphics.Gpu/Memory/PhysicalMemory.cs +++ b/Ryujinx.Graphics.Gpu/Memory/PhysicalMemory.cs @@ -128,10 +128,11 @@ namespace Ryujinx.Graphics.Gpu.Memory /// /// Start address of the range /// Size in bytes to be range + /// True if write tracking is triggered on the span /// A writable region with the data at the specified memory location - public WritableRegion GetWritableRegion(ulong address, int size) + public WritableRegion GetWritableRegion(ulong address, int size, bool tracked = false) { - return _cpuMemory.GetWritableRegion(address, size); + return _cpuMemory.GetWritableRegion(address, size, tracked); } /// diff --git a/Ryujinx.Graphics.OpenGL/Buffer.cs b/Ryujinx.Graphics.OpenGL/Buffer.cs index ecb7dc90..0f6a90e3 100644 --- a/Ryujinx.Graphics.OpenGL/Buffer.cs +++ b/Ryujinx.Graphics.OpenGL/Buffer.cs @@ -55,15 +55,22 @@ namespace Ryujinx.Graphics.OpenGL (IntPtr)size); } - public static byte[] GetData(BufferHandle buffer, int offset, int size) + public static unsafe ReadOnlySpan GetData(Renderer renderer, BufferHandle buffer, int offset, int size) { - GL.BindBuffer(BufferTarget.CopyReadBuffer, buffer.ToInt32()); + if (HwCapabilities.UsePersistentBufferForFlush) + { + return renderer.PersistentBuffers.Default.GetBufferData(buffer, offset, size); + } + else + { + IntPtr target = renderer.PersistentBuffers.Default.GetHostArray(size); - byte[] data = new byte[size]; + GL.BindBuffer(BufferTarget.CopyReadBuffer, buffer.ToInt32()); - GL.GetBufferSubData(BufferTarget.CopyReadBuffer, (IntPtr)offset, size, data); + GL.GetBufferSubData(BufferTarget.CopyReadBuffer, (IntPtr)offset, size, target); - return data; + return new ReadOnlySpan(target.ToPointer(), size); + } } public static void Resize(BufferHandle handle, int size) diff --git a/Ryujinx.Graphics.OpenGL/Image/TextureBuffer.cs b/Ryujinx.Graphics.OpenGL/Image/TextureBuffer.cs index f49a0647..8d407ccb 100644 --- a/Ryujinx.Graphics.OpenGL/Image/TextureBuffer.cs +++ b/Ryujinx.Graphics.OpenGL/Image/TextureBuffer.cs @@ -38,9 +38,9 @@ namespace Ryujinx.Graphics.OpenGL.Image throw new NotSupportedException(); } - public byte[] GetData() + public ReadOnlySpan GetData() { - return Buffer.GetData(_buffer, _bufferOffset, _bufferSize); + return Buffer.GetData(_renderer, _buffer, _bufferOffset, _bufferSize); } public void SetData(ReadOnlySpan data) diff --git a/Ryujinx.Graphics.OpenGL/Image/TextureView.cs b/Ryujinx.Graphics.OpenGL/Image/TextureView.cs index 5e7b5f22..460d1da4 100644 --- a/Ryujinx.Graphics.OpenGL/Image/TextureView.cs +++ b/Ryujinx.Graphics.OpenGL/Image/TextureView.cs @@ -119,7 +119,7 @@ namespace Ryujinx.Graphics.OpenGL.Image _renderer.TextureCopy.Copy(this, (TextureView)destination, srcRegion, dstRegion, linearFilter); } - public byte[] GetData() + public unsafe ReadOnlySpan GetData() { int size = 0; @@ -134,17 +134,11 @@ namespace Ryujinx.Graphics.OpenGL.Image } else { - byte[] data = new byte[size]; + IntPtr target = _renderer.PersistentBuffers.Default.GetHostArray(size); - unsafe - { - fixed (byte* ptr = data) - { - WriteTo((IntPtr)ptr); - } - } + WriteTo(target); - return data; + return new ReadOnlySpan(target.ToPointer(), size); } } diff --git a/Ryujinx.Graphics.OpenGL/PersistentBuffers.cs b/Ryujinx.Graphics.OpenGL/PersistentBuffers.cs index 740f8b98..fe224433 100644 --- a/Ryujinx.Graphics.OpenGL/PersistentBuffers.cs +++ b/Ryujinx.Graphics.OpenGL/PersistentBuffers.cs @@ -1,4 +1,5 @@ using System; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using OpenTK.Graphics.OpenGL; using Ryujinx.Common.Logging; @@ -27,6 +28,9 @@ namespace Ryujinx.Graphics.OpenGL private int _copyBufferHandle; private int _copyBufferSize; + private byte[] _data; + private IntPtr _dataMap; + private void EnsureBuffer(int requiredSize) { if (_copyBufferSize < requiredSize && _copyBufferHandle != 0) @@ -48,6 +52,18 @@ namespace Ryujinx.Graphics.OpenGL } } + public unsafe IntPtr GetHostArray(int requiredSize) + { + if (_data == null || _data.Length < requiredSize) + { + _data = GC.AllocateUninitializedArray(requiredSize, true); + + _dataMap = (IntPtr)Unsafe.AsPointer(ref MemoryMarshal.GetArrayDataReference(_data)); + } + + return _dataMap; + } + private void Sync() { GL.MemoryBarrier(MemoryBarrierFlags.ClientMappedBufferBarrierBit); @@ -63,7 +79,7 @@ namespace Ryujinx.Graphics.OpenGL GL.DeleteSync(sync); } - public byte[] GetTextureData(TextureView view, int size) + public unsafe ReadOnlySpan GetTextureData(TextureView view, int size) { EnsureBuffer(size); @@ -73,16 +89,12 @@ namespace Ryujinx.Graphics.OpenGL GL.BindBuffer(BufferTarget.PixelPackBuffer, 0); - byte[] data = new byte[size]; - Sync(); - Marshal.Copy(_bufferMap, data, 0, size); - - return data; + return new ReadOnlySpan(_bufferMap.ToPointer(), size); } - public byte[] GetBufferData(BufferHandle buffer, int offset, int size) + public unsafe ReadOnlySpan GetBufferData(BufferHandle buffer, int offset, int size) { EnsureBuffer(size); @@ -93,13 +105,9 @@ namespace Ryujinx.Graphics.OpenGL GL.BindBuffer(BufferTarget.CopyWriteBuffer, 0); - byte[] data = new byte[size]; - Sync(); - Marshal.Copy(_bufferMap, data, 0, size); - - return data; + return new ReadOnlySpan(_bufferMap.ToPointer(), size); } public void Dispose() diff --git a/Ryujinx.Graphics.OpenGL/Renderer.cs b/Ryujinx.Graphics.OpenGL/Renderer.cs index 561f0684..c7749228 100644 --- a/Ryujinx.Graphics.OpenGL/Renderer.cs +++ b/Ryujinx.Graphics.OpenGL/Renderer.cs @@ -91,16 +91,9 @@ namespace Ryujinx.Graphics.OpenGL Buffer.Delete(buffer); } - public byte[] GetBufferData(BufferHandle buffer, int offset, int size) + public ReadOnlySpan GetBufferData(BufferHandle buffer, int offset, int size) { - if (HwCapabilities.UsePersistentBufferForFlush) - { - return PersistentBuffers.Default.GetBufferData(buffer, offset, size); - } - else - { - return Buffer.GetData(buffer, offset, size); - } + return Buffer.GetData(this, buffer, offset, size); } public Capabilities GetCapabilities() diff --git a/Ryujinx.Graphics.Texture/LayoutConverter.cs b/Ryujinx.Graphics.Texture/LayoutConverter.cs index 1b7dad2a..970d08cb 100644 --- a/Ryujinx.Graphics.Texture/LayoutConverter.cs +++ b/Ryujinx.Graphics.Texture/LayoutConverter.cs @@ -359,6 +359,7 @@ namespace Ryujinx.Graphics.Texture } public static ReadOnlySpan ConvertLinearToBlockLinear( + Span output, int width, int height, int depth, @@ -373,7 +374,10 @@ namespace Ryujinx.Graphics.Texture SizeInfo sizeInfo, ReadOnlySpan data) { - Span output = new byte[sizeInfo.TotalSize]; + if (output.Length == 0) + { + output = new byte[sizeInfo.TotalSize]; + } int inOffs = 0; @@ -500,6 +504,7 @@ namespace Ryujinx.Graphics.Texture } public static ReadOnlySpan ConvertLinearToLinearStrided( + Span output, int width, int height, int blockWidth, @@ -516,10 +521,21 @@ namespace Ryujinx.Graphics.Texture if (inStride == stride) { - return data; + if (output.Length != 0) + { + data.CopyTo(output); + return output; + } + else + { + return data; + } } - Span output = new byte[h * stride]; + if (output.Length == 0) + { + output = new byte[h * stride]; + } int inOffs = 0; int outOffs = 0; diff --git a/Ryujinx.Memory.Tests/MockVirtualMemoryManager.cs b/Ryujinx.Memory.Tests/MockVirtualMemoryManager.cs index 5051b206..938c9d1d 100644 --- a/Ryujinx.Memory.Tests/MockVirtualMemoryManager.cs +++ b/Ryujinx.Memory.Tests/MockVirtualMemoryManager.cs @@ -49,7 +49,7 @@ namespace Ryujinx.Memory.Tests throw new NotImplementedException(); } - public WritableRegion GetWritableRegion(ulong va, int size) + public WritableRegion GetWritableRegion(ulong va, int size, bool tracked = false) { throw new NotImplementedException(); } diff --git a/Ryujinx.Memory/AddressSpaceManager.cs b/Ryujinx.Memory/AddressSpaceManager.cs index d8ee4746..050b5c05 100644 --- a/Ryujinx.Memory/AddressSpaceManager.cs +++ b/Ryujinx.Memory/AddressSpaceManager.cs @@ -207,9 +207,10 @@ namespace Ryujinx.Memory /// /// Virtual address of the data /// Size of the data + /// True if write tracking is triggered on the span /// A writable region of memory containing the data /// Throw for unhandled invalid or unmapped memory accesses - public unsafe WritableRegion GetWritableRegion(ulong va, int size) + public unsafe WritableRegion GetWritableRegion(ulong va, int size, bool tracked = false) { if (size == 0) { diff --git a/Ryujinx.Memory/IVirtualMemoryManager.cs b/Ryujinx.Memory/IVirtualMemoryManager.cs index 056940cc..2448ba03 100644 --- a/Ryujinx.Memory/IVirtualMemoryManager.cs +++ b/Ryujinx.Memory/IVirtualMemoryManager.cs @@ -87,9 +87,10 @@ namespace Ryujinx.Memory /// /// Virtual address of the data /// Size of the data + /// True if write tracking is triggered on the span /// A writable region of memory containing the data /// Throw for unhandled invalid or unmapped memory accesses - WritableRegion GetWritableRegion(ulong va, int size); + WritableRegion GetWritableRegion(ulong va, int size, bool tracked = false); /// /// Gets a reference for the given type at the specified virtual memory address. diff --git a/Ryujinx.Memory/IWritableBlock.cs b/Ryujinx.Memory/IWritableBlock.cs index c95b754d..36b9f5a6 100644 --- a/Ryujinx.Memory/IWritableBlock.cs +++ b/Ryujinx.Memory/IWritableBlock.cs @@ -5,5 +5,7 @@ namespace Ryujinx.Memory public interface IWritableBlock { void Write(ulong va, ReadOnlySpan data); + + void WriteUntracked(ulong va, ReadOnlySpan data) => Write(va, data); } } diff --git a/Ryujinx.Memory/WritableRegion.cs b/Ryujinx.Memory/WritableRegion.cs index 467ee7f7..21565ea5 100644 --- a/Ryujinx.Memory/WritableRegion.cs +++ b/Ryujinx.Memory/WritableRegion.cs @@ -6,15 +6,17 @@ namespace Ryujinx.Memory { private readonly IWritableBlock _block; private readonly ulong _va; + private readonly bool _tracked; private bool NeedsWriteback => _block != null; public Memory Memory { get; } - public WritableRegion(IWritableBlock block, ulong va, Memory memory) + public WritableRegion(IWritableBlock block, ulong va, Memory memory, bool tracked = false) { _block = block; _va = va; + _tracked = tracked; Memory = memory; } @@ -22,7 +24,14 @@ namespace Ryujinx.Memory { if (NeedsWriteback) { - _block.Write(_va, Memory.Span); + if (_tracked) + { + _block.Write(_va, Memory.Span); + } + else + { + _block.WriteUntracked(_va, Memory.Span); + } } } }