diff --git a/src/graphics/host_gpu/renderer/cache/textureCache.cpp b/src/graphics/host_gpu/renderer/cache/textureCache.cpp index 4180a3d9f..f3fdc38ca 100644 --- a/src/graphics/host_gpu/renderer/cache/textureCache.cpp +++ b/src/graphics/host_gpu/renderer/cache/textureCache.cpp @@ -297,9 +297,6 @@ void TextureCache::TouchImage(Image& image) { void TextureCache::MarkAsMaybeDirty(ImageId id, Image& image) { image.MarkMaybeCpuDirty(); - if (image.NeedsMaybeCpuHash()) { - image.SetMaybeCpuHash(image.HashGuestEdges()); - } UntrackImage(id); } @@ -1144,12 +1141,7 @@ void TextureCache::RefreshImage(ImageId id) { } TrackImage(id); if (image.IsMaybeCpuDirty()) { - const auto hash = image.HashGuestEdges(); - if (image.NeedsMaybeCpuHash()) { - image.SetMaybeCpuHash(hash); - return; - } - (void)image.ResolveMaybeCpuHash(hash); + (void)image.ResolveMaybeCpuHash(image.HashGuestEdges()); } bool cpu_dirty = image.IsBufferModified() || image.IsDefinitelyCpuDirty(); if (image.info.metadata.compression != VideoOutCompression::Uncompressed) { @@ -1444,7 +1436,6 @@ vk::ImageView TextureCache::FindDepthTarget(ImageId id, const ImageDesc& desc) { } TouchImage(image); image.MarkGpuModified(); - image.usage.depth_target = true; if (desc.info.HasStencil()) { const auto layers = image.info.resources.layers; EXIT_IF(layers == 0 || image.info.data.size % layers != 0 || @@ -1860,7 +1851,7 @@ void TextureCache::InvalidateCpuAliases(uint64_t address, uint64_t size) { continue; } if (owner->Overlaps(address, size)) { - owner->InvalidateCpuWrite(address, size); + owner->MarkCpuDirty(); UntrackImage(id); continue; } diff --git a/src/graphics/host_gpu/renderer/image/image.h b/src/graphics/host_gpu/renderer/image/image.h index f45009495..e46966203 100644 --- a/src/graphics/host_gpu/renderer/image/image.h +++ b/src/graphics/host_gpu/renderer/image/image.h @@ -29,10 +29,8 @@ struct CachedImageView { }; struct ImageUsage { - bool texture = false; bool storage = false; bool render_target = false; - bool depth_target = false; bool video_out = false; }; @@ -70,40 +68,25 @@ public: void CopyImageWithBuffer(Image& source, Buffer& buffer, TileManager& tiler); void CopyMip(Image& source, uint32_t mip, uint32_t layer); - void InvalidateCpuWrite(uint64_t vaddr, uint64_t size) { - if (ImageRangeOverlaps(info.data.address, info.data.size, vaddr, size)) { - m_cpu_dirty = true; - m_maybe_cpu_dirty = false; - m_maybe_hash_valid = false; - } else if (ImagePageRangesOverlap(info.data.address, info.data.size, vaddr, size)) { - m_maybe_cpu_dirty = true; - } + void MarkCpuDirty() { + m_cpu_dirty = true; + m_maybe_cpu_dirty = false; } [[nodiscard]] bool IsCpuDirty() const { return m_cpu_dirty || m_maybe_cpu_dirty; } [[nodiscard]] bool IsDefinitelyCpuDirty() const { return m_cpu_dirty; } [[nodiscard]] bool IsMaybeCpuDirty() const { return m_maybe_cpu_dirty; } void MarkMaybeCpuDirty() { - if (!m_cpu_dirty) { + if (!IsCpuDirty()) { + m_maybe_cpu_hash = HashGuestEdges(); m_maybe_cpu_dirty = true; } } - [[nodiscard]] bool NeedsMaybeCpuHash() const { - return m_maybe_cpu_dirty && !m_maybe_hash_valid; - } - void SetMaybeCpuHash(uint64_t hash) { - if (!NeedsMaybeCpuHash()) { - EXIT("image cannot initialize maybe-dirty hash\n"); - } - m_maybe_cpu_hash = hash; - m_maybe_hash_valid = true; - } [[nodiscard]] bool ResolveMaybeCpuHash(uint64_t hash) { - if (!m_maybe_cpu_dirty || !m_maybe_hash_valid || m_cpu_dirty) { + if (!m_maybe_cpu_dirty || m_cpu_dirty) { EXIT("image cannot resolve maybe-dirty hash\n"); } - m_maybe_cpu_dirty = false; - m_maybe_hash_valid = false; + m_maybe_cpu_dirty = false; m_cpu_dirty |= hash != m_maybe_cpu_hash; return m_cpu_dirty; } @@ -112,9 +95,8 @@ public: if (!IsCpuDirty()) { EXIT("clean image cannot complete a refresh\n"); } - m_cpu_dirty = false; - m_maybe_cpu_dirty = false; - m_maybe_hash_valid = false; + m_cpu_dirty = false; + m_maybe_cpu_dirty = false; } [[nodiscard]] bool IsGpuModified() const noexcept { return m_gpu_modified; } @@ -167,12 +149,11 @@ private: GraphicContext& m_graphics; CommandScheduler& m_scheduler; - uint64_t m_maybe_cpu_hash = 0; - bool m_cpu_dirty = false; - bool m_maybe_cpu_dirty = false; - bool m_maybe_hash_valid = false; - bool m_gpu_modified = false; - bool m_buffer_modified = false; + uint64_t m_maybe_cpu_hash = 0; + bool m_cpu_dirty = false; + bool m_maybe_cpu_dirty = false; + bool m_gpu_modified = false; + bool m_buffer_modified = false; }; namespace ImageOps { diff --git a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp index 7683bfb35..b8063ccd5 100644 --- a/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp +++ b/src/graphics/host_gpu/renderer/pipeline/descriptors.cpp @@ -873,7 +873,6 @@ void RenderExecutor::RebindImages(PreparedBindings& prepared) { auto& image = texture_cache.GetImage(binding.image_id); const bool storage = binding.desc.type == TextureCache::BindingType::Storage; image.usage.storage |= storage; - image.usage.texture |= !storage; } } diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index bae331f1c..a42d7abc8 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -5455,7 +5455,7 @@ public: name, "dynamic views", first_view != nullptr && repeated_view == first_view && mapped_view != nullptr && mapped_view != first_view && - image.views.size() == 2 && !image.usage.texture && + image.views.size() == 2 && !image.IsGpuModified() && !image.IsCpuDirty() && !TextureCacheTestAccess::PendingDownload(texture_cache, first), "TextureCache::FindTexture did not preserve sampled view " @@ -7245,6 +7245,48 @@ public: texture_cache.GetImage(fault_b_image).IsGpuModified(), "the surviving image could not reconcile its CPU write"); + // Further writes on the unprotected page must retain the first baseline. + Require(name, "same-page repeated-write fault", + resources.HandleFault(PageFaultAccess::Write, + base + padding_fault_offset), + "the padding write fault was not handled"); + constexpr uint32_t fault_a_cpu = 0x81828384u; + std::memcpy(memory + 0x8000, &fault_a_cpu, sizeof(fault_a_cpu)); + texture_cache.InvalidateMemory(base + padding_fault_offset, 1); + Require(name, "repeated same-page invalidation", + texture_cache.GetImage(fault_a_image).IsMaybeCpuDirty() && + !texture_cache.GetImage(fault_a_image).IsDefinitelyCpuDirty(), + "a repeated padding notification lost maybe-dirty ownership"); + texture_cache.InvalidateMemory(base + 0x8010, sizeof(fault_b)); + Require(name, "definite write supersedes maybe-dirty state", + texture_cache.GetImage(fault_b_image).IsDefinitelyCpuDirty() && + !texture_cache.GetImage(fault_b_image).IsMaybeCpuDirty(), + "an overlapping CPU write retained the padding-only dirty state"); + texture_cache.UpdateImage(fault_a_image); + texture_cache.UpdateImage(fault_b_image); + + auto fault_readback = CreateHostBuffer( + name, 2 * sizeof(uint32_t), vk::BufferUsageFlagBits::eTransferDst, {0, 0}); + vk::BufferImageCopy fault_copy{}; + fault_copy.imageSubresource = {vk::ImageAspectFlagBits::eColor, 0, 0, 1}; + fault_copy.imageExtent = {1, 1, 1}; + for (const auto id : {fault_a_image, fault_b_image}) { + texture_cache.GetImage(id).Download( + {&fault_copy, 1}, fault_readback.buffer, fault_copy.bufferOffset, + sizeof(uint32_t)); + fault_copy.bufferOffset += sizeof(uint32_t); + } + HostReadBarrier(fault_readback.buffer, fault_readback.size, + vk::PipelineStageFlagBits::eTransfer, + vk::AccessFlagBits::eTransferWrite); + scheduler.Finish(); + Require(name, "same-page CPU refresh contents", + ReadBuffer(name, fault_readback, 2) == + std::vector{fault_a_cpu, fault_b_cpu}, + "repeated padding invalidation lost a CPU write or definite " + "invalidation failed to refresh the native image"); + DestroyBuffer(&fault_readback); + constexpr uint64_t publish_image_offset = 0x26000; constexpr uint64_t publish_buffer_offset = 0x26010; constexpr uint32_t publish_image_value = 0x31415926u; @@ -12669,7 +12711,7 @@ public: null_image.info.data.Empty() && !null_image.binding.is_bound && !null_image.binding.is_target && !null_image.binding.needs_rebind && - null_image.usage.texture && null_image.usage.storage && + null_image.usage.storage && null_image.IsGpuModified() && !TextureCacheTestAccess::PendingDownload(texture_cache, null_image_id), @@ -13344,8 +13386,7 @@ public: texture_cache.GetImage(storage_id).binding.force_general && texture_cache.GetImage(storage_id).binding.shader_write && texture_cache.GetImage(storage_id).IsGpuModified() && - texture_cache.GetImage(storage_id).usage.storage && - texture_cache.GetImage(storage_id).usage.texture, + texture_cache.GetImage(storage_id).usage.storage, "the production graphics binding path did not complete vertex " "storage acquisition before pixel sampling"); descriptor_pipelines.push_back(RenderExecutorTestAccess::CommitBindings( @@ -13671,8 +13712,7 @@ public: phased_depth.desc.info.metadata.stencil_compressed && !phased_depth.depth_clear_enable && !texture_cache.IsMeta(phased_depth.desc.info.metadata.range.address) && - !texture_cache.GetImage(phased_depth.image_id).IsGpuModified() && - !texture_cache.GetImage(phased_depth.image_id).usage.depth_target, + !texture_cache.GetImage(phased_depth.image_id).IsGpuModified(), "valid PS5 texture-compatibility policy changed logical depth image " "identity or discovery side effects"); @@ -13754,8 +13794,7 @@ public: !texture_cache.IsMetaCleared( phased_depth.desc.info.metadata.range.address, phased_depth.desc.view_info.base_layer) && - texture_cache.GetImage(phased_depth.image_id).IsGpuModified() && - texture_cache.GetImage(phased_depth.image_id).usage.depth_target, + texture_cache.GetImage(phased_depth.image_id).IsGpuModified(), "final depth acquisition did not publish and consume HTile " "state at the final acquisition boundary"); RenderExecutorTestAccess::ResetBindings(executor); @@ -14457,7 +14496,7 @@ public: array_binding.images[0].desc.view_info.layer_count == 2 && expanded_array.backing.layers == 2 && - expanded_array.IsGpuModified() && expanded_array.usage.texture, + expanded_array.IsGpuModified(), "raw array view did not expand and reuse the Color2D backing"); RenderColorInfo rebound_array_target{}; @@ -14755,13 +14794,11 @@ public: !texture_cache.FindImageFromRange(stencil_address, stencil_layout.size, false) && !texture_cache.GetImage(depth_id).binding.is_bound && - !texture_cache.GetImage(depth_id).binding.is_target && - !texture_cache.GetImage(depth_id).usage.depth_target, + !texture_cache.GetImage(depth_id).binding.is_target, "FindImage created depth acquisition state or a stencil proxy"); RenderExecutorTestAccess::BindRenderTarget(executor, depth_id); Require(name, "depth prefetch purity", texture_cache.GetImage(depth_id).binding.is_target && - !texture_cache.GetImage(depth_id).usage.depth_target && !texture_cache.FindImageFromRange(stencil_address, stencil_layout.size, false), "depth prefetch performed final target acquisition"); @@ -14892,7 +14929,6 @@ public: texture_cache.GetImage(target_parent_id).binding.is_target && texture_cache.GetImage(target_parent_id).usage.render_target && texture_cache.GetImage(target_parent_id).IsGpuModified() && - texture_cache.GetImage(depth_id).usage.depth_target && texture_cache.GetImage(depth_id).IsGpuModified() && TextureCacheTestAccess::PendingDownload(texture_cache, target_parent_id) && @@ -14989,8 +15025,7 @@ public: context.GetRenderExecutor().RebindImages(prepared); Require(name, "first stencil acquisition", prepared.images[0].image_id == sampled_stencil_id && - prepared.images[0].image_view != nullptr && - texture_cache.GetImage(sampled_stencil_id).usage.texture, + prepared.images[0].image_view != nullptr, "the first sampled-stencil image was not finally acquired"); RenderExecutorTestAccess::ResetBindings(executor); @@ -15019,8 +15054,7 @@ public: context.GetRenderExecutor().RebindImages(redirected); Require(name, "second-pass stencil acquisition", redirected.images[0].image_id == depth_id && - redirected.images[0].image_view != nullptr && - texture_cache.GetImage(depth_id).usage.texture, + redirected.images[0].image_view != nullptr, "RebindImages did not acquire the associated depth owner"); RenderExecutorTestAccess::ResetBindings(executor);