From 8af82558bb25bb2ab46f3d750d3da55b82512e41 Mon Sep 17 00:00:00 2001 From: rdenno Date: Mon, 2 Nov 2026 06:51:36 -0500 Subject: texture_cache: look up X8Z24 with UNORM, UINT, UNORM, UNORM components X8Z24 was only known with UINT in the last three components. Mario Tennis Fever samples its depth buffer as X8Z24 with components UNORM, UINT, UNORM, UNORM (as S8Z24 already is in the table), which hit the "texture format=42" fallback to A8B8G8R8. The depth image was then copied into a colour image of the wrong format every frame (with "Unimplemented" layer-count asserts) and read as colour, and distant scenery around the court (the arch and the castle stadium) wasn't drawn. Map it to X8_D24_UNORM like the other X8Z24 entry. Assisted-by: Claude Opus 5.5 (diagnosis, fix and testing) --- src/video_core/texture_cache/format_lookup_table.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/video_core/texture_cache') diff --git a/src/video_core/texture_cache/format_lookup_table.cpp b/src/video_core/texture_cache/format_lookup_table.cpp index 20bb96aae3..18be2dcbf3 100644 --- a/src/video_core/texture_cache/format_lookup_table.cpp +++ b/src/video_core/texture_cache/format_lookup_table.cpp @@ -150,6 +150,7 @@ PixelFormat PixelFormatFromTextureInfo(TextureFormat format, ComponentType red, case Hash(TextureFormat::X8Z24, UNORM): return PixelFormat::X8_D24_UNORM; case Hash(TextureFormat::X8Z24, UNORM, UINT, UINT, UINT, LINEAR): + case Hash(TextureFormat::X8Z24, UNORM, UINT, UNORM, UNORM, LINEAR): return PixelFormat::X8_D24_UNORM; case Hash(TextureFormat::Z24S8, UINT, UNORM, UNORM, UNORM, LINEAR): return PixelFormat::S8_UINT_D24_UNORM; -- cgit v1.2.3 From 1e37847a805e87a0cac9e78755fe103571329eaa Mon Sep 17 00:00:00 2001 From: snowstormxd <214507868+snowstormxd@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:32:04 +0000 Subject: tests: verify deferred descriptors against eager invalidation Exercise 40,000 mutations and table switches plus repeated deletion across five channels. Make the existing descriptor read work with a memory reader fixture and add an explicit CPU benchmark with opaque invalidation calls. Assisted-by: Codex (implementation, review, commit text and automated validation) (cherry picked from commit 3b62a551da8ec0b4edf19a1c4e14fa2519dd20e2) --- src/tests/CMakeLists.txt | 1 + src/tests/video_core/descriptor_table.cpp | 134 ++++++++++++++++++++++++ src/video_core/texture_cache/descriptor_table.h | 5 +- 3 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 src/tests/video_core/descriptor_table.cpp (limited to 'src/video_core/texture_cache') diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index be5528ef46..e064792e19 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -24,6 +24,7 @@ add_executable(tests video_core/ampere_returns.cpp video_core/native_macro_fifo.cpp video_core/shader_invalidation.cpp + video_core/descriptor_table.cpp input_common/calibration_configuration_job.cpp ) diff --git a/src/tests/video_core/descriptor_table.cpp b/src/tests/video_core/descriptor_table.cpp new file mode 100644 index 0000000000..b4c226241d --- /dev/null +++ b/src/tests/video_core/descriptor_table.cpp @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include +#include +#include +#include +#include "video_core/texture_cache/descriptor_table.h" + +namespace { +struct DescriptorMemory { + std::array, 2> banks{}; + + void ReadBlockUnsafe(GPUVAddr address, void* output, size_t size) const { + const auto bank = (address - 0x1000) / 0x1000; + const auto offset = (address - 0x1000) % 0x1000; + std::memcpy(output, reinterpret_cast(banks[bank].data()) + offset, size); + } +}; +} // namespace + +TEST_CASE("Descriptor invalidation matches eager refresh across mutations", "[descriptor_table]") { + DescriptorMemory memory; + VideoCommon::DescriptorTable table; + REQUIRE(table.Synchronize(0x1000, 127)); + std::array seen{}; + std::array previous{}; + std::mt19937 random{0x5080}; + unsigned bank{}; + u32 limit = 127; + for (unsigned i = 0; i < 40000; ++i) { + switch (random() % 5) { + case 0: + table.Invalidate(); + seen.fill(false); + break; + case 1: + memory.banks[random() % 2][random() % 128] = random(); + break; + case 2: { + const auto next_bank = random() % 2; + const u32 next_limit = random() % 2 ? 63 : 127; + const bool changed = next_bank != bank || next_limit != limit; + REQUIRE(table.Synchronize(0x1000 + next_bank * 0x1000, next_limit) == changed); + if (changed) { + seen.fill(false); + } + bank = next_bank; + limit = next_limit; + break; + } + default: { + const u32 index = random() % (limit + 1); + const u64 expected = memory.banks[bank][index]; + const bool changed = !seen[index] || previous[index] != expected; + const auto actual = table.Read(memory, index); + REQUIRE(actual.first == expected); + REQUIRE(actual.second == changed); + seen[index] = true; + previous[index] = expected; + } + } + } +} + +TEST_CASE("Repeated image deletion cannot reuse stale descriptor view IDs", "[descriptor_table]") { + DescriptorMemory memory; + std::array, 5> channels; + std::array, 5> view_ids{}; + unsigned live_generation = 1; + const auto visit = [&](size_t channel, u32 index) { + if (channels[channel].Read(memory, index).second) { + view_ids[channel][index] = live_generation; + } + return view_ids[channel][index]; + }; + for (size_t channel = 0; channel < channels.size(); ++channel) { + REQUIRE(channels[channel].Synchronize(0x1000, 127)); + for (u32 index = 0; index < 128; ++index) { + REQUIRE(visit(channel, index) == live_generation); + } + } + for (unsigned generation = 0; generation < 32; ++generation) { + ++live_generation; + for (unsigned deletion = 0; deletion < 50; ++deletion) { + for (auto& table : channels) { + table.Invalidate(); + } + } + for (size_t channel = 0; channel < channels.size(); ++channel) { + for (u32 index = 0; index < 128; ++index) { + REQUIRE(visit(channel, index) == live_generation); + REQUIRE_FALSE(channels[channel].Read(memory, index).second); + } + } + } +} + +TEST_CASE("Descriptor invalidation CPU benchmark", "[.][cache_bench]") { + DescriptorMemory memory; + const auto measure = [&](bool deferred) { + std::array, 5> tables; + for (auto& table : tables) { + REQUIRE(table.Synchronize(0x1000, 127)); + } + void (*volatile invalidate)(VideoCommon::DescriptorTable&) = + deferred ? +[](VideoCommon::DescriptorTable& table) { table.Invalidate(); } + : +[](VideoCommon::DescriptorTable& table) { + std::ranges::fill(table.read_descriptors, 0); + }; + size_t changed{}; + const auto start = std::chrono::steady_clock::now(); + for (unsigned batch = 0; batch < 2000; ++batch) { + for (unsigned deletion = 0; deletion < 50; ++deletion) { + for (auto& table : tables) { + invalidate(table); + } + } + for (auto& table : tables) { + changed += table.Read(memory, 0).second; + } + } + const auto ms = + std::chrono::duration(std::chrono::steady_clock::now() - start) + .count(); + REQUIRE(changed == 10000); + return ms; + }; + const double eager = measure(false); + const double deferred = measure(true); + std::cout << "Descriptor bitmap invalidation: eager=" << eager << " ms deferred=" << deferred + << " ms (CPU only, not gameplay FPS)\n"; +} diff --git a/src/video_core/texture_cache/descriptor_table.h b/src/video_core/texture_cache/descriptor_table.h index 75383809c5..32ce01eeed 100644 --- a/src/video_core/texture_cache/descriptor_table.h +++ b/src/video_core/texture_cache/descriptor_table.h @@ -35,9 +35,10 @@ public: invalidated = true; } - [[nodiscard]] std::pair Read(Tegra::MemoryManager const& gpu_memory, u32 index) noexcept { + template + [[nodiscard]] std::pair Read(const MemoryManager& gpu_memory, u32 index) noexcept { DEBUG_ASSERT(index <= current_limit); - if (invalidated) [[unlikely]] { + if (invalidated) { std::ranges::fill(read_descriptors, 0); invalidated = false; } -- cgit v1.2.3 From 24da16703dd2a9f883d80f3c5c0280d12767b5ea Mon Sep 17 00:00:00 2001 From: snowstormxd <214507868+snowstormxd@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:04:01 +0000 Subject: texture_cache: clear only exposed descriptor bitmap words Avoid zeroing unused preallocation during invalidation and pool refresh. Clear newly exposed words whenever limits grow, bound clearing to allocated storage, and test word/allocation boundaries, shrink/regrowth and malformed large limits. Assisted-by: Codex (implementation, review, commit text and automated validation) (cherry picked from commit 4bc1ae01f79cef43be016504c850ca8a6998457a) --- src/tests/video_core/descriptor_table.cpp | 36 +++++++++++++++++++++++++ src/video_core/texture_cache/descriptor_table.h | 13 ++++++--- 2 files changed, 45 insertions(+), 4 deletions(-) (limited to 'src/video_core/texture_cache') diff --git a/src/tests/video_core/descriptor_table.cpp b/src/tests/video_core/descriptor_table.cpp index b4c226241d..8b20564949 100644 --- a/src/tests/video_core/descriptor_table.cpp +++ b/src/tests/video_core/descriptor_table.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include "video_core/texture_cache/descriptor_table.h" @@ -97,6 +98,41 @@ TEST_CASE("Repeated image deletion cannot reuse stale descriptor view IDs", "[de } } +TEST_CASE("Descriptor limits clear exposed slots after shrink and growth", "[descriptor_table]") { + struct AddressMemory { + void ReadBlockUnsafe(GPUVAddr address, void* output, size_t size) const { + std::memcpy(output, &address, size); + } + } memory; + constexpr std::array limits{0, 63, 64, 127, 128, 0x7ffff, 0x80000, 0x80001, 0x100000}; + for (const u32 limit : limits) { + VideoCommon::DescriptorTable table; + REQUIRE(table.Synchronize(0x1000, limit)); + CHECK(table.Read(memory, limit).second); + CHECK_FALSE(table.Read(memory, limit).second); + table.Refresh(0x1000, 0); + table.Invalidate(); + CHECK(table.Read(memory, 0).second); + CHECK_FALSE(table.Read(memory, 0).second); + table.Refresh(0x1000, limit); + CHECK(table.Read(memory, limit).second); + CHECK_FALSE(table.Read(memory, limit).second); + table.Invalidate(); + table.Invalidate(); + CHECK(table.Read(memory, limit).second); + CHECK_FALSE(table.Read(memory, limit).second); + } + for (const u32 limit : + {std::numeric_limits::max(), std::numeric_limits::max() - 0x1000}) { + VideoCommon::DescriptorTable table; + table.Refresh(0x1000, limit); + CHECK(table.Read(memory, 0).second); + table.Invalidate(); + CHECK(table.Read(memory, 0).second); + CHECK_FALSE(table.Read(memory, 0).second); + } +} + TEST_CASE("Descriptor invalidation CPU benchmark", "[.][cache_bench]") { DescriptorMemory memory; const auto measure = [&](bool deferred) { diff --git a/src/video_core/texture_cache/descriptor_table.h b/src/video_core/texture_cache/descriptor_table.h index 32ce01eeed..a441e572de 100644 --- a/src/video_core/texture_cache/descriptor_table.h +++ b/src/video_core/texture_cache/descriptor_table.h @@ -39,7 +39,7 @@ public: [[nodiscard]] std::pair Read(const MemoryManager& gpu_memory, u32 index) noexcept { DEBUG_ASSERT(index <= current_limit); if (invalidated) { - std::ranges::fill(read_descriptors, 0); + ClearReadDescriptors(); invalidated = false; } const GPUVAddr gpu_addr = current_gpu_addr + index * sizeof(T); @@ -63,10 +63,8 @@ public: // Mario Brothership reallocates a lot of times, so use aggressive pre-alloc sizes // std::vector by default uses quadratic growth, but that isn't even enough to satisfy brothership const size_t num_descriptors = ((limit + 0x80000) & (~0x7ffff)) + 1; - size_t old_size = read_descriptors.size(); read_descriptors.resize(Common::DivCeil(num_descriptors, 64U)); - old_size = (std::min)(old_size, read_descriptors.size()); - std::fill(read_descriptors.begin(), read_descriptors.begin() + old_size, 0); + ClearReadDescriptors(); invalidated = false; // descriptors.resize(num_descriptors); @@ -77,6 +75,13 @@ public: GPUVAddr current_gpu_addr{}; u32 current_limit{}; bool invalidated{}; ///< read_descriptors is to be cleared before the next read + +private: + void ClearReadDescriptors() noexcept { + // Reserved capacity is not addressable. A later limit increase clears its new range. + const size_t words = (std::min)(read_descriptors.size(), size_t{current_limit} / 64 + 1); + std::fill_n(read_descriptors.begin(), words, u64{0}); + } }; } // namespace VideoCommon -- cgit v1.2.3 From e46c6cbfb0f6735ca35f1150338c300f3c9c18fe Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:30:32 -0500 Subject: video_core: read texture handles as header offsets only on Switch 2 049c9633ed made TextureHeaderIndex take index / 32 for any texture index past the pool that is a multiple of 32, in VisitImageView and in the shader environment. That was written for Switch 2 handles that hold a header byte offset, but it ran for Switch 1 games too, where such an out-of-range index used to give a null view (and fail the shader environment's range check) and now binds an unrelated header. Pass whether the memory manager uses native tiling, which is how Switch 2 titles are set up, and keep the previous behaviour otherwise. Follow-up to 049c9633ed. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/video_core/shader_environment.cpp | 3 ++- src/video_core/texture_cache/texture_cache.h | 3 ++- src/video_core/textures/texture.h | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'src/video_core/texture_cache') diff --git a/src/video_core/shader_environment.cpp b/src/video_core/shader_environment.cpp index 22e661c849..8d340ba881 100644 --- a/src/video_core/shader_environment.cpp +++ b/src/video_core/shader_environment.cpp @@ -316,7 +316,8 @@ std::optional GenericEnvironment::TryFindSize() { Tegra::Texture::TICEntry GenericEnvironment::ReadTextureInfo(GPUVAddr tic_addr, u32 tic_limit, bool via_header_index, u32 raw) { const auto handle{Tegra::Texture::TexturePair(raw, via_header_index)}; - const u32 index{Tegra::Texture::TextureHeaderIndex(handle.first, tic_limit)}; + const u32 index{Tegra::Texture::TextureHeaderIndex(handle.first, tic_limit, + gpu_memory->UsesNativeTiling())}; ASSERT(index <= tic_limit); const GPUVAddr descriptor_addr{tic_addr + index * sizeof(Tegra::Texture::TICEntry)}; Tegra::Texture::TICEntry entry; diff --git a/src/video_core/texture_cache/texture_cache.h b/src/video_core/texture_cache/texture_cache.h index 3f3ae9cafd..4929c2df44 100644 --- a/src/video_core/texture_cache/texture_cache.h +++ b/src/video_core/texture_cache/texture_cache.h @@ -809,7 +809,8 @@ typename P::Framebuffer* TextureCache

::GetFramebuffer() { template ImageViewId TextureCache

::VisitImageView(u32 index, bool compute) { auto& table = compute ? channel_state->compute_image_table : channel_state->graphics_image_table; - index = Tegra::Texture::TextureHeaderIndex(index, table.current_limit); + index = Tegra::Texture::TextureHeaderIndex(index, table.current_limit, + gpu_memory->UsesNativeTiling()); if (index > table.current_limit) { LOG_DEBUG(HW_GPU, "Invalid image view index={}", index); return NULL_IMAGE_VIEW_ID; diff --git a/src/video_core/textures/texture.h b/src/video_core/textures/texture.h index 79396d8a68..35a4c8eecf 100644 --- a/src/video_core/textures/texture.h +++ b/src/video_core/textures/texture.h @@ -167,10 +167,10 @@ static_assert(sizeof(TextureHandle) == 4, "TextureHandle has wrong size"); /// Some Switch 2 bindless handles (Switch Sports' scenery, read from a uniform register) hold /// the texture header's byte offset in the pool, index * 32, where the handle normally has the -/// index. Read it as an offset only when the plain index falls past the pool and the offset -/// lands inside it, so handles that already resolve are left alone. -[[nodiscard]] constexpr u32 TextureHeaderIndex(u32 index, u32 limit) { - if (index > limit && index % 32 == 0 && index / 32 <= limit) { +/// index. Read it as an offset only on Switch 2 (native tiling), when the plain index falls past +/// the pool and the offset lands inside it, so handles that already resolve are left alone. +[[nodiscard]] constexpr u32 TextureHeaderIndex(u32 index, u32 limit, bool switch2) { + if (switch2 && index > limit && index % 32 == 0 && index / 32 <= limit) { return index / 32; } return index; -- cgit v1.2.3