From 2ff3487dbcd1bb43ccdcbdbe205663983867f1c4 Mon Sep 17 00:00:00 2001 From: Lander Gallastegi Date: Wed, 27 May 2026 13:49:34 +0200 Subject: [PATCH] video_core: check if depth image is valid before using it (#4479) * check if depth image is valid before using it * fix uid usage --- CMakeLists.txt | 1 + src/common/incremental_id.h | 22 +++++++++++++++++++ .../renderer_vulkan/vk_rasterizer.cpp | 4 ++-- src/video_core/texture_cache/image.cpp | 3 +++ src/video_core/texture_cache/image.h | 16 ++++++++++++-- .../texture_cache/texture_cache.cpp | 6 ++--- src/video_core/texture_cache/texture_cache.h | 17 ++++++++++++++ 7 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 src/common/incremental_id.h diff --git a/CMakeLists.txt b/CMakeLists.txt index e9d66d9d0..b0b2d99fc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -741,6 +741,7 @@ set(COMMON src/common/logging/classes.h src/common/elf_info.h src/common/endian.h src/common/enum.h + src/common/incremental_id.h src/common/io_file.cpp src/common/io_file.h src/common/lru_cache.h diff --git a/src/common/incremental_id.h b/src/common/incremental_id.h new file mode 100644 index 000000000..1891bfe72 --- /dev/null +++ b/src/common/incremental_id.h @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright 2026 shadPS4 Emulator Project +// SPDX-License-Identifier: GPL-2.0-or-later + +#pragma once +#include + +namespace Common { +template +class IncrementalIdProvider { +public: + IncrementalIdProvider() : counter(0) {} + ~IncrementalIdProvider() = default; + + CounterType Next() { + return counter.fetch_add(1, std::memory_order_relaxed) + 1; + } + +private: + std::atomic counter; +}; + +} // namespace Common \ No newline at end of file diff --git a/src/video_core/renderer_vulkan/vk_rasterizer.cpp b/src/video_core/renderer_vulkan/vk_rasterizer.cpp index 1d8e85b5f..3b734fde3 100644 --- a/src/video_core/renderer_vulkan/vk_rasterizer.cpp +++ b/src/video_core/renderer_vulkan/vk_rasterizer.cpp @@ -702,10 +702,10 @@ void Rasterizer::BindTextures(const Shader::Info& stage, Shader::Backend::Bindin image_id = texture_cache.FindImage(desc); auto* image = &texture_cache.GetImage(image_id); - if (image->depth_id) { + if (auto depth_image_id = texture_cache.GetAssociatedDepth(*image)) { // If this image has an associated depth image, it's a stencil attachment. // Redirect the access to the actual depth-stencil buffer. - image_id = image->depth_id; + image_id = depth_image_id; image = &texture_cache.GetImage(image_id); } if (image->binding.is_bound) { diff --git a/src/video_core/texture_cache/image.cpp b/src/video_core/texture_cache/image.cpp index c0ada73a8..7bb79fbd6 100644 --- a/src/video_core/texture_cache/image.cpp +++ b/src/video_core/texture_cache/image.cpp @@ -15,6 +15,8 @@ namespace VideoCore { using namespace Vulkan; +Common::IncrementalIdProvider Image::global_image_uid{}; + static vk::ImageUsageFlags ImageUsageFlags(const Vulkan::Instance* instance, const ImageInfo& info) { vk::ImageUsageFlags usage = vk::ImageUsageFlagBits::eTransferSrc | @@ -119,6 +121,7 @@ Image::Image(const Vulkan::Instance& instance_, Vulkan::Scheduler& scheduler_, if (info.pixel_format == vk::Format::eUndefined) { return; } + image_uid = global_image_uid.Next(); mip_hashes.resize(info.resources.levels); // Here we force `eExtendedUsage` as don't know all image usage cases beforehand. In normal case // the texture cache should re-create the resource with the usage requested diff --git a/src/video_core/texture_cache/image.h b/src/video_core/texture_cache/image.h index 0bf471dce..6af58cbbe 100644 --- a/src/video_core/texture_cache/image.h +++ b/src/video_core/texture_cache/image.h @@ -4,6 +4,7 @@ #pragma once #include "common/enum.h" +#include "common/incremental_id.h" #include "common/types.h" #include "video_core/renderer_vulkan/vk_common.h" #include "video_core/texture_cache/image_info.h" @@ -111,8 +112,14 @@ struct Image { return True(flags & ImageFlagBits::GpuModified) && False(flags & (ImageFlagBits::Dirty)); } - void AssociateDepth(ImageId image_id) { - depth_id = image_id; + void AssociateDepth(ImageId depth_image_id, u64 depth_image_uid) { + depth_id = depth_image_id; + depth_uid = depth_image_uid; + } + + void DisassociateDepth() { + depth_id = {}; + depth_uid = {}; } ImageView& FindView(const ImageViewInfo& view_info, bool ensure_guest_samples = true); @@ -149,6 +156,7 @@ public: VAddr track_addr = 0; VAddr track_addr_end = 0; ImageId depth_id{}; + u64 depth_uid{}; // Resource state tracking vk::ImageUsageFlags usage_flags; @@ -169,6 +177,7 @@ public: std::deque backing_images; BackingImage* backing{}; boost::container::static_vector mip_hashes{}; + u64 image_uid{}; u64 lru_id{}; u64 tick_accessed_last{}; u64 hash{}; @@ -187,6 +196,9 @@ public: u32 needs_rebind : 1; u32 force_general : 1; } binding{}; + +private: + static Common::IncrementalIdProvider global_image_uid; }; } // namespace VideoCore diff --git a/src/video_core/texture_cache/texture_cache.cpp b/src/video_core/texture_cache/texture_cache.cpp index b682196af..7c94a0176 100644 --- a/src/video_core/texture_cache/texture_cache.cpp +++ b/src/video_core/texture_cache/texture_cache.cpp @@ -708,9 +708,9 @@ ImageView& TextureCache::FindDepthTarget(ImageId image_id, const ImageDesc& desc slot_images.insert(instance, scheduler, blit_helper, slot_image_views, info); RegisterImage(stencil_id); } - Image& image = slot_images[stencil_id]; - TouchImage(image); - image.AssociateDepth(image_id); + Image& stencil_image = slot_images[stencil_id]; + TouchImage(stencil_image); + stencil_image.AssociateDepth(image_id, image.image_uid); } return image.FindView(desc.view_info, false); diff --git a/src/video_core/texture_cache/texture_cache.h b/src/video_core/texture_cache/texture_cache.h index 3741e6af7..34a8472c4 100644 --- a/src/video_core/texture_cache/texture_cache.h +++ b/src/video_core/texture_cache/texture_cache.h @@ -152,6 +152,23 @@ public: return slot_image_views[id]; } + /// Get the associated depth stencil image if it is still valid. + ImageId GetAssociatedDepth(Image& image) { + if (!image.depth_id) { + return {}; + } + if (slot_images.is_allocated(image.depth_id)) { + auto& depth_image = slot_images[image.depth_id]; + if (depth_image.image_uid == image.depth_uid && + depth_image.flags & ImageFlagBits::Registered) { + return image.depth_id; + } + } + // The linked depth image is no longer valid, disassociate it. + image.DisassociateDepth(); + return {}; + } + /// Returns true if the specified address is a metadata surface. bool IsMeta(VAddr address) const { return surface_metas.contains(address);