diff --git a/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.cs b/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.cs index f8ddcab8..fa2f0912 100644 --- a/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.cs +++ b/src/SharpEmu.Libs/VideoOut/VulkanVideoPresenter.cs @@ -1777,6 +1777,57 @@ internal static unsafe class VulkanVideoPresenter uint depth) => checked(GetGuestImageByteCount(format, width, height) * Math.Max(depth, 1u)); + /// + /// Upper bound on the backing extent that guest CPU-write tracking is + /// armed over. Deliberately equal to the presenter-side re-upload budget + /// used by the AGC flip/acquire sync path: arming a range larger than the + /// sync path is willing to read back would fault and dirty forever without + /// ever producing a re-upload, so it is pure cost. + /// + internal const ulong MaxTrackedGuestImageBytes = 128UL * 1024UL * 1024UL; + + /// + /// Decides whether a guest surface is eligible for CPU-write tracking. + /// The predicate is byte-based on purpose: the cost that actually scales + /// with surface size is the dirty re-upload (one allocation plus a guest + /// memory read of the whole extent per dirty flip), not the arming itself + /// (one mprotect and, per write burst, one fault for the whole range). + /// A resolution cap was the wrong proxy — it ignored bytes-per-texel and + /// volume depth while excluding the 4K UI sheets that most need + /// invalidation. + /// + internal static bool ShouldTrackGuestImageWrites(ulong byteCount) => + byteCount != 0 && byteCount <= MaxTrackedGuestImageBytes; + + /// + /// Decides whether a sampled guest image whose backing memory the parse + /// thread just re-read should be re-uploaded from those bytes. + /// + /// is a latch that flips false the first + /// time an address is used as a render target and never flips back, so it + /// cannot be the sole gate: a font atlas or UI sheet that was also + /// rendered into is permanently frozen at its first upload afterwards. + /// The write tracker answers the real question. A parse-time generation + /// above zero means a guest CPU store was observed on the backing range, + /// and a generation the last upload does not already cover means those + /// bytes are newer than the host image. + /// + /// + /// Requiring a positive generation keeps the pure GPU-feedback case + /// (render into an image, then sample it) safe: such a surface is tracked + /// but never CPU-written, so its generation stays zero and the live image + /// is preserved instead of being overwritten with guest memory. + /// + /// + internal static bool ShouldRefreshGuestImageFromCpu( + bool isCpuBacked, + long textureWriteGeneration, + bool hasUploadedGeneration, + long uploadedGeneration) => + isCpuBacked || + (textureWriteGeneration > 0 && + (!hasUploadedGeneration || uploadedGeneration != textureWriteGeneration)); + // Maps a UNORM swapchain format to the sRGB view of the same bit layout, // or Undefined when no counterpart exists. Used to encode linear-float // guest flips on their way into a UNORM swapchain. @@ -8217,8 +8268,7 @@ internal static unsafe class VulkanVideoPresenter out TextureResource resource) { resource = default!; - if (!guestImage.IsCpuBacked || - guestImage.Width != texture.Width || + if (guestImage.Width != texture.Width || guestImage.Height != texture.Height || guestImage.Depth != GetGuestTextureDepth(texture.Type, texture.Depth) || IsGuestTexture3D(guestImage.Type) != IsGuestTexture3D(texture.Type) || @@ -8228,6 +8278,32 @@ internal static unsafe class VulkanVideoPresenter return false; } + // IsCpuBacked alone used to gate this path, but it is a latch that + // flips false the first time the address is used as a render target + // and never flips back. On PS5 that address is unified memory: a + // surface that was rendered into once and is later rewritten by the + // guest CPU (glyph atlas rasterization, a 4K UI sheet redrawn on the + // brightness screen) must still be re-read. Fall back on the write + // tracker, which reports genuine CPU stores and leaves pure + // render-into-then-sample feedback untouched. + bool hasUploadedGeneration; + long uploadedGeneration; + lock (_gate) + { + hasUploadedGeneration = _cpuBackedUploadGenerations.TryGetValue( + texture.Address, + out uploadedGeneration); + } + + if (!ShouldRefreshGuestImageFromCpu( + guestImage.IsCpuBacked, + texture.WriteGeneration, + hasUploadedGeneration, + uploadedGeneration)) + { + return false; + } + var rowLength = texture.TileMode == 0 ? Math.Max(texture.Pitch, texture.Width) : texture.Width; @@ -13348,20 +13424,33 @@ internal static unsafe class VulkanVideoPresenter retained.IsCpuBacked = false; retained.CpuContentFingerprint = 0; _guestImages.Add(target.Address, retained); + var retainedByteCount = GetTextureByteCount( + target.Format, + target.Width, + target.Height, + depth); lock (_gate) { _cpuBackedUploadGenerations.Remove(target.Address); _guestImageExtents[target.Address] = ( target.Width, target.Height, - GetTextureByteCount( - target.Format, - target.Width, - target.Height, - depth)); + retainedByteCount); } - TrackCpuBackedGuestImage(retained); + // Arm the exact extent the flip/acquire sync path would read + // back, budgeted by bytes rather than by resolution: the old + // 1920x1080 cap left every 4K surface permanently + // un-invalidated, so a guest CPU rewrite of one was never + // reflected and the sample served stale bytes. + if (ShouldTrackGuestImageWrites(retainedByteCount)) + { + SharpEmu.HLE.GuestImageWriteTracker.Track( + target.Address, + retainedByteCount, + CurrentGuestWorkSequenceForDiagnostics, + "vulkan.render-target"); + } if (_traceGuestImageEvents) { @@ -13500,19 +13589,31 @@ internal static unsafe class VulkanVideoPresenter SetDebugName(ObjectType.Framebuffer, framebuffer.Handle, $"{debugName} framebuffer"); } _guestImages.Add(target.Address, resource); + var createdByteCount = GetTextureByteCount( + target.Format, + target.Width, + target.Height, + depth); lock (_gate) { _guestImageExtents[target.Address] = ( target.Width, target.Height, - GetTextureByteCount( - target.Format, - target.Width, - target.Height, - depth)); + createdByteCount); } - TrackCpuBackedGuestImage(resource); + // See the retained-variant path above: track the full backing + // extent under a byte budget instead of a resolution cap so + // oversized render targets the guest later rewrites with the CPU + // are re-uploaded on the next sample. + if (ShouldTrackGuestImageWrites(createdByteCount)) + { + SharpEmu.HLE.GuestImageWriteTracker.Track( + target.Address, + createdByteCount, + CurrentGuestWorkSequenceForDiagnostics, + "vulkan.render-target"); + } if (_traceGuestImageEvents) { @@ -13526,24 +13627,25 @@ internal static unsafe class VulkanVideoPresenter private void TrackCpuBackedGuestImage(GuestImageResource image) { - // Arm ≤1080p guest images so native CPU stores fault. Drain skips - // false overlap dirties with a 4 KiB zero probe unless IsCpuBacked. - if (image.Width == 0 || - image.Height == 0 || - image.Width > 1920 || - image.Height > 1080) + if (image.Width == 0 || image.Height == 0) { return; } var depth = Math.Max(image.Depth, 1u); + var byteCount = GetVulkanImageByteCount( + image.Format, + image.Width, + image.Height, + depth); + if (!ShouldTrackGuestImageWrites(byteCount)) + { + return; + } + SharpEmu.HLE.GuestImageWriteTracker.Track( image.Address, - GetVulkanImageByteCount( - image.Format, - image.Width, - image.Height, - depth), + byteCount, CurrentGuestWorkSequenceForDiagnostics, "vulkan.render-target"); } diff --git a/tests/SharpEmu.Libs.Tests/VideoOut/VulkanGuestImageCpuSyncPolicyTests.cs b/tests/SharpEmu.Libs.Tests/VideoOut/VulkanGuestImageCpuSyncPolicyTests.cs new file mode 100644 index 00000000..cc393af7 --- /dev/null +++ b/tests/SharpEmu.Libs.Tests/VideoOut/VulkanGuestImageCpuSyncPolicyTests.cs @@ -0,0 +1,170 @@ +// Copyright (C) 2026 SharpEmu Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +using SharpEmu.Libs.VideoOut; +using Xunit; + +namespace SharpEmu.Libs.Tests.VideoOut; + +/// +/// Guards the two policy decisions that let a guest CPU rewrite reach a host +/// image that the GPU also renders into. Both used to be answered by proxies +/// that silently excluded real surfaces: a 1920x1080 arming cap (which left +/// every 4K target un-invalidated) and an IsCpuBacked latch (which flips false +/// the first time an address is used as a render target and never flips back). +/// The Vulkan device code around them cannot be unit-tested without a device, +/// so the decisions themselves are exercised here. +/// +public sealed class VulkanGuestImageCpuSyncPolicyTests +{ + private const uint Rgba8Format = 10; + private const uint Rgba16FFormat = 11; + private const uint Rgba32FFormat = 14; + + [Theory] + // 4K UI sheets are the whole point of the change: under the old + // resolution cap none of these were ever armed. + [InlineData(Rgba8Format, 3840u, 2160u)] + [InlineData(Rgba16FFormat, 3840u, 2160u)] + [InlineData(Rgba32FFormat, 3840u, 2160u)] + // Supersampled and unusual-aspect targets above 1080p in one axis only. + [InlineData(Rgba8Format, 2560u, 1440u)] + [InlineData(Rgba8Format, 1920u, 2160u)] + [InlineData(Rgba8Format, 3840u, 1080u)] + public void OversizedTargetsRemainEligibleForWriteTracking( + uint format, + uint width, + uint height) + { + var byteCount = VulkanVideoPresenter.GetGuestImageByteCount( + format, + width, + height, + depth: 1); + + Assert.True(VulkanVideoPresenter.ShouldTrackGuestImageWrites(byteCount)); + } + + [Fact] + public void SubHdTargetsRemainEligibleForWriteTracking() + { + var byteCount = VulkanVideoPresenter.GetGuestImageByteCount( + Rgba8Format, + 1280u, + 720u, + depth: 1); + + Assert.True(VulkanVideoPresenter.ShouldTrackGuestImageWrites(byteCount)); + } + + [Fact] + public void EmptyExtentIsNotTracked() + { + Assert.False(VulkanVideoPresenter.ShouldTrackGuestImageWrites(0)); + } + + [Fact] + public void ExtentBeyondTheReUploadBudgetIsNotTracked() + { + // Arming beyond the budget the flip/acquire sync path is willing to + // read back can only cost faults; it can never produce a re-upload. + Assert.True( + VulkanVideoPresenter.ShouldTrackGuestImageWrites( + VulkanVideoPresenter.MaxTrackedGuestImageBytes)); + Assert.False( + VulkanVideoPresenter.ShouldTrackGuestImageWrites( + VulkanVideoPresenter.MaxTrackedGuestImageBytes + 1)); + } + + [Fact] + public void VolumeDepthCountsAgainstTheTrackingBudget() + { + // A resolution cap could not see volume depth at all. 512^3 RGBA8 is + // 512 MiB of backing memory behind a "512x512" surface. + var byteCount = VulkanVideoPresenter.GetGuestImageByteCount( + Rgba8Format, + 512u, + 512u, + depth: 512u); + + Assert.False(VulkanVideoPresenter.ShouldTrackGuestImageWrites(byteCount)); + } + + [Fact] + public void CpuBackedImageAlwaysRefreshes() + { + Assert.True( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: true, + textureWriteGeneration: -1, + hasUploadedGeneration: false, + uploadedGeneration: 0)); + } + + [Fact] + public void RenderTargetLatchNoLongerBlocksAnObservedCpuWrite() + { + // The surface was rendered into (IsCpuBacked latched false) and the + // guest CPU then rewrote its backing memory, so the parse thread + // shipped fresh texels carrying a newer generation than the upload. + Assert.True( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: false, + textureWriteGeneration: 3, + hasUploadedGeneration: true, + uploadedGeneration: 2)); + } + + [Fact] + public void ObservedCpuWriteRefreshesEvenWithNoRecordedUpload() + { + // The render-target recreate/retain paths drop the recorded upload + // generation; a tracked CPU write must still win. + Assert.True( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: false, + textureWriteGeneration: 1, + hasUploadedGeneration: false, + uploadedGeneration: 0)); + } + + [Fact] + public void GpuFeedbackSurfaceKeepsItsLiveImage() + { + // Tracked but never CPU-written: generation zero. Render-into-then- + // sample must not be overwritten with guest memory. + Assert.False( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: false, + textureWriteGeneration: 0, + hasUploadedGeneration: false, + uploadedGeneration: 0)); + } + + [Fact] + public void UntrackedSurfaceKeepsItsLiveImage() + { + // -1 is the "no tracker generation" sentinel the parse thread ships + // when the range is not tracked (or tracking is disabled entirely, + // as on Windows). + Assert.False( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: false, + textureWriteGeneration: -1, + hasUploadedGeneration: false, + uploadedGeneration: 0)); + } + + [Fact] + public void AlreadyUploadedGenerationDoesNotRefreshAgain() + { + // The upload recorded this exact generation, so the host image is + // current: re-uploading every draw would restage the whole surface. + Assert.False( + VulkanVideoPresenter.ShouldRefreshGuestImageFromCpu( + isCpuBacked: false, + textureWriteGeneration: 4, + hasUploadedGeneration: true, + uploadedGeneration: 4)); + } +}