From c83eb1d1f5c14d0842a165de431ac6367dff2909 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:19:46 -0500 Subject: shader: tell handle-program textures apart when adding descriptors Every texture whose handle is computed per draw gets the same placeholder constant buffer address (index 0, offset 0), and Descriptors::Add did not compare handle_program. Two such textures of the same type and depth in one shader therefore shared a descriptor, so the second sampled the first, and one could also merge with a real texture bound at constant buffer 0 offset 0. Compare handle_program too. The pixel-format-integer lookup cached its last answer by (index, offset), which has the same problem for handle programs; skip that cache for them. Follow-up to 561f87abec. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index 8a72f565dc..514cd9b2b6 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -1036,7 +1036,8 @@ public: desc.secondary_cbuf_index == existing.secondary_cbuf_index && desc.secondary_cbuf_offset == existing.secondary_cbuf_offset && desc.secondary_shift_left == existing.secondary_shift_left && - desc.count == existing.count && desc.size_shift == existing.size_shift; + desc.count == existing.count && desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; // TODO: Read this from TIC texture_descriptors[index].is_multisample |= desc.is_multisample; @@ -1171,6 +1172,10 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo u32 last_cbuf_offset{}; bool last_is_integer{false}; const auto is_texture_pixel_format_integer{[&](const ConstBufferAddr& cbuf_addr) { + if (cbuf_addr.handle_program != ~0U) { + // Every handle program is at cbuf 0 offset 0: the cache below can't tell them apart. + return IsTexturePixelFormatIntegerCached(env, cbuf_addr); + } if (has_last_is_integer && last_cbuf_index == cbuf_addr.index && last_cbuf_offset == cbuf_addr.offset) { return last_is_integer; -- cgit v1.2.3 From a3d67c14c86f57ec12787db0b8bb058ec8b5f73c Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:22:23 -0500 Subject: frontend: bound the wait for the emulation thread at shutdown 426f120ee8 dropped the 10 s wait and terminate() fallback when stopping emulation, and ShutdownGame and closeEvent now wait for the emulation thread with no limit. If the thread hangs (for example a GPU or present deadlock), stopping a game, switching games or quitting never completes. Keep the asynchronous teardown, but once the thread has been asked to stop, give it 10 s and then terminate it and finish cleanup, as before. ShutdownGame's nested event loop gets the same bound. A terminated thread is logged as an error. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/yuzu/main_window.cpp | 31 +++++++++++++++++++++++++++---- src/yuzu/main_window.h | 3 +++ 2 files changed, 30 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/yuzu/main_window.cpp b/src/yuzu/main_window.cpp index e5a8f81424..02b36cf9cf 100644 --- a/src/yuzu/main_window.cpp +++ b/src/yuzu/main_window.cpp @@ -320,6 +320,7 @@ __declspec(dllexport) int AmdPowerXpressRequestHighPerformance = 1; constexpr int default_mouse_hide_timeout = 2500; constexpr int default_input_update_timeout = 1; +constexpr int EmuThreadStopTimeoutMs = 10'000; constexpr size_t CopyBufferSize = 1_MiB; @@ -2209,6 +2210,23 @@ void MainWindow::OnShutdownBeginDialog() { void MainWindow::OnEmulationStopTimeExpired() { if (QtCommon::emu_thread) { QtCommon::emu_thread->ForceStop(); + emu_terminate_timer.setSingleShot(true); + emu_terminate_timer.start(EmuThreadStopTimeoutMs); + connect(&emu_terminate_timer, &QTimer::timeout, this, &MainWindow::OnEmulationStopTimeout, + Qt::UniqueConnection); + } +} + +void MainWindow::OnEmulationStopTimeout() { + TerminateEmulationThread(); + OnEmulationStopped(); +} + +void MainWindow::TerminateEmulationThread() { + if (QtCommon::emu_thread && QtCommon::emu_thread->isRunning()) { + LOG_ERROR(Frontend, "Emulation thread failed to stop, terminating it"); + QtCommon::emu_thread->terminate(); + QtCommon::emu_thread->wait(); } } @@ -2220,6 +2238,7 @@ void MainWindow::OnEmulationStopped() { return; } shutdown_timer.stop(); + emu_terminate_timer.stop(); if (QtCommon::emu_thread) { QtCommon::emu_thread->disconnect(); QtCommon::emu_thread->wait(); @@ -2309,13 +2328,17 @@ void MainWindow::ShutdownGame() { play_time_manager->Stop(); OnShutdownBegin(); OnEmulationStopTimeExpired(); - if (QtCommon::emu_thread) { + emu_terminate_timer.stop(); + if (QtCommon::emu_thread && QtCommon::emu_thread->isRunning()) { QEventLoop loop; + QTimer timeout; + timeout.setSingleShot(true); connect(QtCommon::emu_thread.get(), &QThread::finished, &loop, &QEventLoop::quit); - if (QtCommon::emu_thread->isRunning()) { - loop.exec(QEventLoop::ExcludeUserInputEvents); - } + connect(&timeout, &QTimer::timeout, &loop, &QEventLoop::quit); + timeout.start(EmuThreadStopTimeoutMs); + loop.exec(QEventLoop::ExcludeUserInputEvents); } + TerminateEmulationThread(); OnEmulationStopped(); } diff --git a/src/yuzu/main_window.h b/src/yuzu/main_window.h index e2c5a0f6db..a3b030a6a6 100644 --- a/src/yuzu/main_window.h +++ b/src/yuzu/main_window.h @@ -279,6 +279,7 @@ private: StartGameType with_config = StartGameType::Normal); void BootGameFromList(const QString& filename, StartGameType with_config); void ShutdownGame(); + void TerminateEmulationThread(); void SetDiscordEnabled(bool state); void LoadAmiibo(const QString& filename); @@ -437,6 +438,7 @@ private slots: void OnShutdownBeginDialog(); void OnEmulationStopped(); void OnEmulationStopTimeExpired(); + void OnEmulationStopTimeout(); #ifdef ENABLE_UPDATE_CHECKER void OnEmulatorUpdateAvailable(); @@ -506,6 +508,7 @@ private: GameList* game_list = nullptr; LoadingScreen* loading_screen = nullptr; QTimer shutdown_timer; + QTimer emu_terminate_timer; OverlayDialog* shutdown_dialog{}; PerformanceOverlay* perf_overlay = nullptr; #ifdef HAS_RESHADE -- cgit v1.2.3 From 8748fbd542a00ab7bbfbf17210b29f8d51c3c4b5 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:23:37 -0500 Subject: file_sys: decompress large NUP zstd members once 31b901b93b only cached a zstd member while it fit the 512 MiB cache. Anything larger was never cached, so every Read() allocated the full uncompressed size and decompressed the whole extent again, which makes a multi-GB RomFS read in small chunks unusably slow. Members between 256 and 512 MiB evicted each other on alternate reads, and Insert copied the vector so memory briefly doubled. Two threads decompressing the same member also left a duplicate LRU entry and a wrong byte count. The writer emits one zstd frame per member, so there are no frame boundaries to seek by. Members larger than half the cache are now streamed once into a temporary file under the cache directory and read from there, using bounded memory. The file is removed when the container is released. Smaller members stay in the LRU, which now moves the vector in and ignores a duplicate insert. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/file_sys/nup_container.cpp | 114 +++++++++++++++++++++++++++++++++--- src/core/file_sys/nup_container.h | 53 +++++++++++++---- src/core/file_sys/nup_writer.h | 7 +-- src/tests/core/nup_writer.cpp | 52 ++++++++++++++++ 4 files changed, 205 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/core/file_sys/nup_container.cpp b/src/core/file_sys/nup_container.cpp index 0e485b44ec..28e4a6ac89 100644 --- a/src/core/file_sys/nup_container.cpp +++ b/src/core/file_sys/nup_container.cpp @@ -8,11 +8,15 @@ #include #include #include +#include +#include #include +#include #include #include +#include "common/fs/path_util.h" #include "common/logging.h" #include "common/swap.h" #include "core/file_sys/vfs/vfs_offset.h" @@ -142,8 +146,8 @@ bool Sha256(const u8* data, std::size_t size, std::array& out) { return length == out.size(); } -/// A member file whose extent is zstd-compressed. Decompresses on first read -/// and caches the result (LRU-evicted) unless the file exceeds the cache cap. +/// A member file whose extent is zstd-compressed. Decompresses on first read and caches the +/// result (LRU-evicted); a file over half the cache cap is decompressed once to a temporary file. class NupVfsFile final : public VfsFile { public: NupVfsFile(VirtualFile backing_, u64 extent_offset_, u64 extent_size_, u64 uncompressed_size_, @@ -151,7 +155,7 @@ public: std::size_t cache_index_) : backing(std::move(backing_)), extent_offset(extent_offset_), extent_size(extent_size_), uncompressed_size(uncompressed_size_), name(std::move(name_)), cache(std::move(cache_)), - cache_index(cache_index_), cache_enabled(uncompressed_size <= NupDecompressCache::DecompressCacheCap) {} + cache_index(cache_index_), cache_enabled(uncompressed_size <= cache->cap / 2) {} std::string GetName() const override { return name; @@ -183,6 +187,10 @@ public: } length = std::min(length, static_cast(uncompressed_size) - offset); + if (!cache_enabled) { + return ReadSpilled(data, length, offset); + } + { std::scoped_lock lock{cache->mutex}; if (const auto* cached = cache->Find(cache_index)) { @@ -198,9 +206,7 @@ public: } { std::scoped_lock lock{cache->mutex}; - if (cache_enabled) { - cache->Insert(cache_index, scratch); - } + cache->Insert(cache_index, scratch); if (const auto* cached = cache->Find(cache_index)) { std::memcpy(data, cached->data() + offset, length); return length; @@ -220,6 +226,88 @@ public: } private: + std::size_t ReadSpilled(u8* data, std::size_t length, std::size_t offset) const { + std::shared_ptr spill; + { + std::scoped_lock lock{cache->mutex}; + spill = cache->GetSpill(cache_index); + } + std::scoped_lock lock{spill->mutex}; + if (!spill->attempted) { + spill->attempted = true; + DecompressToFile(*spill); + } + if (!spill->file.IsOpen() || !spill->file.Seek(static_cast(offset))) { + return 0; + } + return spill->file.ReadSpan(std::span{data, length}); + } + + void DecompressToFile(NupSpillFile& spill) const { + const auto fail = [&](const char* reason) { + LOG_ERROR(Loader, "NUP zstd decompression of {} to a temporary file failed: {}", name, + reason); + spill.file.Close(); + if (!spill.path.empty()) { + std::error_code ec; + std::filesystem::remove(spill.path, ec); + } + }; + if (extent_size > backing->GetSize() || extent_offset > backing->GetSize() - extent_size) { + return fail("extent out of range"); + } + + std::random_device random; + std::error_code ec; + std::filesystem::create_directories(cache->spill_dir, ec); + spill.path = + cache->spill_dir / fmt::format("{:08x}{:08x}-{}.tmp", random(), random(), cache_index); + Common::FS::IOFile out{spill.path, Common::FS::FileAccessMode::Write}; + if (!out.IsOpen()) { + return fail("cannot create the temporary file"); + } + + std::unique_ptr dctx{ZSTD_createDCtx(), + &ZSTD_freeDCtx}; + std::vector in_buffer(ZSTD_DStreamInSize()); + std::vector out_buffer(ZSTD_DStreamOutSize()); + u64 read = 0; + u64 written = 0; + bool frame_done = false; + while (!frame_done && read < extent_size) { + const auto chunk = + static_cast(std::min(in_buffer.size(), extent_size - read)); + if (backing->Read(in_buffer.data(), chunk, + static_cast(extent_offset + read)) != chunk) { + return fail("short read of the compressed data"); + } + read += chunk; + ZSTD_inBuffer input{in_buffer.data(), chunk, 0}; + ZSTD_outBuffer output{}; + do { + output = {out_buffer.data(), out_buffer.size(), 0}; + const std::size_t result = ZSTD_decompressStream(dctx.get(), &output, &input); + if (ZSTD_isError(result) || output.pos > uncompressed_size - written) { + return fail("corrupt or oversized data"); + } + if (out.WriteSpan(std::span{out_buffer.data(), output.pos}) != + output.pos) { + return fail("cannot write the temporary file"); + } + written += output.pos; + frame_done = result == 0; + } while (!frame_done && (input.pos < input.size || output.pos == output.size)); + } + if (!frame_done || written != uncompressed_size) { + return fail("size mismatch"); + } + out.Close(); + spill.file.Open(spill.path, Common::FS::FileAccessMode::Read); + if (!spill.file.IsOpen()) { + fail("cannot reopen the temporary file"); + } + } + std::vector Decompress() const { if (extent_size > backing->GetSize() || extent_offset > backing->GetSize() - extent_size) { return {}; @@ -251,6 +339,13 @@ private: } // namespace +NupSpillFile::~NupSpillFile() { + file.Close(); + if (!path.empty()) { + std::error_code ec; + std::filesystem::remove(path, ec); + } +} bool NupContainer::IsNupFile(const VirtualFile& file) { if (file == nullptr || file->GetSize() < SuperblockSize) { @@ -261,8 +356,13 @@ bool NupContainer::IsNupFile(const VirtualFile& file) { return magic == HeaderMagic; } -NupContainer::NupContainer(VirtualFile file_) +NupContainer::NupContainer(VirtualFile file_, std::size_t memory_cache_cap, + std::filesystem::path spill_dir_) : file(std::move(file_)), cache(std::make_shared()) { + cache->cap = memory_cache_cap; + cache->spill_dir = spill_dir_.empty() + ? Common::FS::GetEdenPath(Common::FS::EdenPath::CacheDir) / "nup" + : std::move(spill_dir_); if (file == nullptr) { return; } diff --git a/src/core/file_sys/nup_container.h b/src/core/file_sys/nup_container.h index 118b137a7b..0980c25517 100644 --- a/src/core/file_sys/nup_container.h +++ b/src/core/file_sys/nup_container.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include #include @@ -14,19 +15,36 @@ #include #include "common/common_types.h" +#include "common/fs/file.h" #include "core/file_sys/vfs/vfs_types.h" #include "core/file_sys/vfs/vfs_vector.h" #include "core/loader/loader.h" namespace FileSys { +/// A zstd member too large for the in-memory cache, decompressed once into a temporary file. +struct NupSpillFile { + ~NupSpillFile(); + + std::mutex mutex; + std::filesystem::path path; + Common::FS::IOFile file; + bool attempted{}; +}; + /// LRU over decompressed zstd extents, shared by the files of one container. struct NupDecompressCache { + static constexpr std::size_t DefaultCap = 512 * 1024 * 1024; + + using Entry = std::pair>; + std::mutex mutex; - std::list>> lru; - std::unordered_map>>::iterator> - index; + std::list lru; + std::unordered_map::iterator> index; std::size_t bytes{}; + std::size_t cap{DefaultCap}; + std::filesystem::path spill_dir; + std::unordered_map> spills; std::vector* Find(std::size_t file_index) { const auto it = index.find(file_index); @@ -37,21 +55,31 @@ struct NupDecompressCache { return &it->second->second; } + /// Takes ownership of `data` only when it is inserted. void Insert(std::size_t file_index, std::vector& data) { - while (bytes + data.size() > DecompressCacheCap && !lru.empty()) { + if (index.contains(file_index)) { + return; + } + while (bytes + data.size() > cap && !lru.empty()) { bytes -= lru.back().second.size(); index.erase(lru.back().first); lru.pop_back(); } - if (bytes + data.size() > DecompressCacheCap) { + if (bytes + data.size() > cap) { return; } - lru.emplace_front(file_index, data); - index.emplace(file_index, lru.begin()); bytes += data.size(); + lru.emplace_front(file_index, std::move(data)); + index.emplace(file_index, lru.begin()); } - static constexpr std::size_t DecompressCacheCap = 512 * 1024 * 1024; + std::shared_ptr GetSpill(std::size_t file_index) { + auto& spill = spills[file_index]; + if (!spill) { + spill = std::make_shared(); + } + return spill; + } }; /** @@ -101,7 +129,11 @@ public: /// Recursively merges overlay into a copy of base; overlay paths win. static VirtualDir MergeShadowDirs(const VirtualDir& base, const VirtualDir& overlay); - explicit NupContainer(VirtualFile file_); + /// Zstd members larger than half of `memory_cache_cap` are decompressed to a temporary file + /// in `spill_dir` (the cache directory by default) instead of being held in memory. + explicit NupContainer(VirtualFile file_, + std::size_t memory_cache_cap = NupDecompressCache::DefaultCap, + std::filesystem::path spill_dir_ = {}); ~NupContainer(); Loader::ResultStatus GetStatus() const { @@ -119,7 +151,8 @@ public: std::optional FindBestUpdatePackage() const; /// Opens a member file by node index; raw extents are zero-copy views into - /// the container, zstd extents are lazily decompressed and cached. + /// the container, zstd extents are lazily decompressed, cached in memory + /// or, when large, in a temporary file. VirtualFile OpenFile(std::size_t file_index); VirtualDir GetExeFSDir(std::size_t package_index); diff --git a/src/core/file_sys/nup_writer.h b/src/core/file_sys/nup_writer.h index d2423dd1f4..dd7b7e0883 100644 --- a/src/core/file_sys/nup_writer.h +++ b/src/core/file_sys/nup_writer.h @@ -25,10 +25,9 @@ struct PackProgress { enum class PackResult { Success, Cancelled, InvalidSource, IoError }; -// NupContainer keeps a zstd file in memory only while it fits its 512 MiB decompression cache; -// a larger one is decompressed again in full on every read, which makes big compressed RomFS -// files unusable. Files above this size are therefore stored raw, with headroom below the cap so -// that one file does not evict everything else from the cache. +// NupContainer keeps a zstd file in memory only while it fits half of its 512 MiB decompression +// cache; a larger one is decompressed to a temporary file on first read, which costs time and +// disk space. Files above this size are therefore stored raw. constexpr u64 DefaultRawThreshold = 256ULL * 1024 * 1024; // Application metadata written to the base package (or the first package when there is no base). diff --git a/src/tests/core/nup_writer.cpp b/src/tests/core/nup_writer.cpp index c0230d8694..5dec0134cb 100644 --- a/src/tests/core/nup_writer.cpp +++ b/src/tests/core/nup_writer.cpp @@ -253,6 +253,58 @@ TEST_CASE("NUP packs base, updates and add-ons and reads back through NupContain REQUIRE_FALSE(by_name.contains(".listing")); } +TEST_CASE("NUP reads zstd members above the memory cache through a temporary file", "[nup]") { + TempDir t; + const auto root = t.path / "Game"; + MakeDump(root); + std::string large(300 * 1024, '\0'); + u32 x = 7; + for (auto& c : large) { + x = x * 1664525 + 1013904223; + c = static_cast('a' + (x >> 28)); + } + Put(root / "Base/RomFS/large.bin", large); + const auto archive = t.path / "Game.nup"; + FileSys::Nup::PackOptions options{.raw_threshold = 1024 * 1024}; + REQUIRE(FileSys::Nup::PackDump(root, archive, options) == FileSys::Nup::PackResult::Success); + for (const auto& file : RawFiles(ReadAll(archive))) { + if (file.name == "large.bin") { + REQUIRE(file.codec == 1); + } + } + + const auto spill_dir = t.path / "spill"; + FileSys::RealVfsFilesystem real; + { + // Members above 32 KiB do not fit the 64 KiB cache budget and are spilled to disk. + NupContainer nup{real.OpenFile(archive.string(), FileSys::OpenMode::Read), 64 * 1024, + spill_dir}; + REQUIRE(nup.GetStatus() == Loader::ResultStatus::Success); + const auto file = nup.GetRomFSDir(0)->GetFile("large.bin"); + REQUIRE(file != nullptr); + REQUIRE(file->GetSize() == large.size()); + + std::string chunk(4096, '\0'); + for (size_t offset = 0; offset < large.size(); offset += chunk.size()) { + const size_t count = + file->Read(reinterpret_cast(chunk.data()), chunk.size(), offset); + REQUIRE(count == std::min(chunk.size(), large.size() - offset)); + REQUIRE(chunk.compare(0, count, large, offset, count) == 0); + } + for (const size_t offset : {size_t{0}, size_t{123456}, size_t{77}, large.size() - 5}) { + const size_t count = file->Read(reinterpret_cast(chunk.data()), 100, offset); + REQUIRE(count == std::min(100, large.size() - offset)); + REQUIRE(chunk.compare(0, count, large, offset, count) == 0); + } + REQUIRE(file->Read(reinterpret_cast(chunk.data()), 10, large.size()) == 0); + + REQUIRE(std::distance(fs::directory_iterator(spill_dir), fs::directory_iterator{}) > 0); + REQUIRE(PackedTree(nup.GetRomFSDir(0)) == SourceTree(root / "Base/RomFS")); + REQUIRE(nup.VerifyIntegrity({}) == Loader::ResultStatus::Success); + } + REQUIRE(std::distance(fs::directory_iterator(spill_dir), fs::directory_iterator{}) == 0); +} + TEST_CASE("NUP marks updates standalone when the dump has no base", "[nup]") { TempDir t; const auto root = t.path / "Game"; -- cgit v1.2.3 From 089105ab44c87f1ef0526221144a1979c56558e6 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:25:26 -0500 Subject: file_sys: reject NUP tables with wrapping bounds or cyclic directories 31b901b93b checked the TOC with unchecked arithmetic on counts and offsets read from the file. The record-table size (u64 count times record size) and u32 first_child + child_count can wrap, so the bounds checks pass and later reads go out of bounds; the same holds for the TOC range, name ranges, file extents and the meta range. A directory that lists its own ancestor as a child also made BuildDirTree recurse until the stack overflowed. Bound the counts by the TOC size before multiplying, compare ranges by subtraction, and widen the child range to u64. Walk the directory graph once after parsing with an explicit stack and reject cycles and nesting deeper than 128, so BuildDirTree can no longer recurse without bound. The file is rejected with ErrorNUPBadFormat. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/file_sys/nup_container.cpp | 96 +++++++++++++++++++++++++++++++------ src/core/file_sys/nup_container.h | 1 + src/tests/core/nup_writer.cpp | 62 ++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/core/file_sys/nup_container.cpp b/src/core/file_sys/nup_container.cpp index 28e4a6ac89..27ac4486ca 100644 --- a/src/core/file_sys/nup_container.cpp +++ b/src/core/file_sys/nup_container.cpp @@ -35,6 +35,7 @@ constexpr u32 TocMagic = 0x01434F54; // "TOC\x01" little-endian constexpr u8 CodecRaw = 0; constexpr u8 CodecZstd = 1; constexpr u8 PackageFlagStandaloneUpdate = 0x01; +constexpr u32 MaxDirDepth = 128; #pragma pack(push, 1) struct Superblock { @@ -393,9 +394,9 @@ bool NupContainer::Parse() { toc_size = super.toc_size; blob_offset = super.blob_offset; file_size = super.file_size; - if (file_size != file->GetSize() || toc_offset < SuperblockSize || - blob_offset < toc_offset + toc_size || blob_offset % 0x10 != 0 || - blob_offset > file_size) { + if (file_size != file->GetSize() || toc_offset < SuperblockSize || blob_offset > file_size || + toc_offset > blob_offset || toc_size > blob_offset - toc_offset || + blob_offset % 0x10 != 0) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -425,15 +426,22 @@ bool NupContainer::Parse() { return false; } + // The counts come from the file, so bound them by the TOC size before multiplying. + if (header.dir_count > toc.size() / DirRecordSize || + header.file_count > toc.size() / FileRecordSize) { + status = Loader::ResultStatus::ErrorNUPBadFormat; + return false; + } const u64 needed = TocHeaderSize + static_cast(header.package_count) * PackageRecordSize + header.dir_count * DirRecordSize + header.file_count * FileRecordSize; - const u64 children_offset = needed; - const u64 children_bytes = header.pool_offset - needed; - if (toc.size() < needed || header.pool_offset < needed || - header.pool_offset + header.pool_size > toc.size() || children_bytes % sizeof(u32) != 0) { + if (needed > toc.size() || header.pool_offset < needed || header.pool_offset > toc.size() || + header.pool_size > toc.size() - header.pool_offset || + (header.pool_offset - needed) % sizeof(u32) != 0) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } + const u64 children_offset = needed; + const u64 children_bytes = header.pool_offset - needed; const u8* base = toc.data(); std::size_t pos = TocHeaderSize; @@ -458,8 +466,8 @@ bool NupContainer::Parse() { package.root_dir = record.root_dir; package.exefs_dir = record.exefs_dir; package.romfs_dir = record.romfs_dir; - if (record.meta_size > 0 && record.meta_offset != 0 && - record.meta_offset + record.meta_size <= file_size) { + if (record.meta_size > 0 && record.meta_offset != 0 && record.meta_offset <= file_size && + record.meta_size <= file_size - record.meta_offset) { package.meta_offset = record.meta_offset; package.meta_size = record.meta_size; } @@ -471,8 +479,9 @@ bool NupContainer::Parse() { DirRecord record{}; std::memcpy(&record, base + pos, sizeof(record)); pos += DirRecordSize; - if (record.first_child + record.child_count > children_bytes / sizeof(u32) || - record.name_offset + record.name_length > header.pool_size) { + if (static_cast(record.first_child) + record.child_count > + children_bytes / sizeof(u32) || + static_cast(record.name_offset) + record.name_length > header.pool_size) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -492,9 +501,9 @@ bool NupContainer::Parse() { (record.size == 0 && (record.extent_offset != 0 || record.extent_size != 0)) || (record.size != 0 && (record.extent_size == 0 || record.extent_offset < blob_offset || - record.extent_offset % 0x10 != 0 || - record.extent_offset + record.extent_size > file_size)) || - record.name_offset + record.name_length > header.pool_size) { + record.extent_offset % 0x10 != 0 || record.extent_offset > file_size || + record.extent_size > file_size - record.extent_offset)) || + static_cast(record.name_offset) + record.name_length > header.pool_size) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -517,6 +526,11 @@ bool NupContainer::Parse() { } } + if (!ValidateDirTree()) { + status = Loader::ResultStatus::ErrorNUPBadFormat; + return false; + } + for (auto& package : packages) { if (!ParseMeta(package)) { status = Loader::ResultStatus::ErrorNUPBadFormat; @@ -526,6 +540,57 @@ bool NupContainer::Parse() { return true; } +bool NupContainer::ValidateDirTree() const { + enum : u8 { Unvisited, InProgress, Done }; + struct Frame { + u32 dir; + u32 next; + }; + std::vector state(dirs.size(), Unvisited); + std::vector height(dirs.size(), 0); + std::vector stack; + for (u32 start = 0; start < dirs.size(); ++start) { + if (state[start] != Unvisited) { + continue; + } + state[start] = InProgress; + stack.push_back({start, 0}); + while (!stack.empty()) { + const Frame frame = stack.back(); + const DirNode& node = dirs[frame.dir]; + if (frame.next == node.child_count) { + state[frame.dir] = Done; + stack.pop_back(); + if (!stack.empty()) { + u32& parent_height = height[stack.back().dir]; + parent_height = std::max(parent_height, height[frame.dir] + 1); + } + continue; + } + ++stack.back().next; + const u32 child = children[static_cast(node.first_child) + frame.next]; + if (child >= dirs.size()) { + continue; + } + if (state[child] == InProgress) { + LOG_ERROR(Loader, "NUP directory tree contains a cycle"); + return false; + } + if (state[child] == Done) { + height[frame.dir] = std::max(height[frame.dir], height[child] + 1); + } else { + state[child] = InProgress; + stack.push_back({child, 0}); + } + if (stack.size() + height[child] > MaxDirDepth) { + LOG_ERROR(Loader, "NUP directory tree is nested too deeply"); + return false; + } + } + } + return true; +} + bool NupContainer::ParseMeta(PackageInfo& package) { if (package.meta_offset == 0) { return true; @@ -647,7 +712,8 @@ VirtualFile NupContainer::OpenFile(std::size_t file_index) { void NupContainer::BuildDirTree(u32 dir_index, VectorVfsDirectory& out) { const DirNode& node = dirs[dir_index]; - for (u32 slot = node.first_child; slot < node.first_child + node.child_count; ++slot) { + const u64 end = static_cast(node.first_child) + node.child_count; + for (u64 slot = node.first_child; slot < end; ++slot) { const u32 child = children[slot]; if (child >= dirs.size()) { out.AddFile(OpenFile(child - static_cast(dirs.size()))); diff --git a/src/core/file_sys/nup_container.h b/src/core/file_sys/nup_container.h index 0980c25517..8787717d47 100644 --- a/src/core/file_sys/nup_container.h +++ b/src/core/file_sys/nup_container.h @@ -184,6 +184,7 @@ private: bool Parse(); bool ParseMeta(PackageInfo& package); + bool ValidateDirTree() const; void BuildDirTree(u32 dir_index, VectorVfsDirectory& out); VirtualDir BuildPackageDir(u32 dir_index); std::vector ReadExtentRaw(const FileNode& node) const; diff --git a/src/tests/core/nup_writer.cpp b/src/tests/core/nup_writer.cpp index 5dec0134cb..48e14daaef 100644 --- a/src/tests/core/nup_writer.cpp +++ b/src/tests/core/nup_writer.cpp @@ -19,6 +19,7 @@ #include "core/file_sys/nup_container.h" #include "core/file_sys/nup_writer.h" #include "core/file_sys/vfs/vfs_real.h" +#include "core/file_sys/vfs/vfs_vector.h" namespace { @@ -173,6 +174,23 @@ void MakeDump(const fs::path& root) { Put(root / "dlc/0400ABCD00011001/0/romfs/addon.bin", "addon data"); } +void SetLe(std::string& buf, size_t at, u64 value, size_t bytes) { + for (size_t i = 0; i < bytes; ++i) { + buf[at + i] = static_cast(value >> (8 * i)); + } +} + +// Applies `patch` to a valid archive, fixes up the TOC hash and opens the result. +template +Loader::ResultStatus StatusAfterPatch(std::string nup, Patch&& patch) { + const u64 toc = Le(nup, 0x08, 8); + patch(nup, toc); + const auto hash = Sha256(std::string_view{nup}.substr(toc, Le(nup, 0x10, 8))); + std::copy(hash.begin(), hash.end(), nup.begin() + 0x18); + const auto data = std::vector(nup.begin(), nup.end()); + return NupContainer{std::make_shared(data, "bad.nup")}.GetStatus(); +} + } // namespace TEST_CASE("NUP packs base, updates and add-ons and reads back through NupContainer", "[nup]") { @@ -305,6 +323,50 @@ TEST_CASE("NUP reads zstd members above the memory cache through a temporary fil REQUIRE(std::distance(fs::directory_iterator(spill_dir), fs::directory_iterator{}) == 0); } +TEST_CASE("NUP rejects a TOC whose counts or offsets wrap", "[nup]") { + TempDir t; + const auto root = t.path / "Game"; + MakeDump(root); + const auto archive = t.path / "Game.nup"; + REQUIRE(FileSys::Nup::PackDump(root, archive) == FileSys::Nup::PackResult::Success); + const auto nup = ReadAll(archive); + const auto no_patch = [](std::string&, u64) {}; + REQUIRE(StatusAfterPatch(nup, no_patch) == Loader::ResultStatus::Success); + + // 2^61 directories wrap dir_count * 0x18 back to the real record size. + REQUIRE(StatusAfterPatch(nup, [](std::string& buf, u64 toc) { + SetLe(buf, toc + 0x08, Le(buf, toc + 0x08, 8) + (1ULL << 61), 8); + }) == Loader::ResultStatus::ErrorNUPBadFormat); + + const auto first_dir = [](const std::string& buf, u64 toc) { + return toc + 0x30 + Le(buf, toc + 0x04, 4) * 0x40; + }; + // first_child + child_count wraps to a small value. + REQUIRE(StatusAfterPatch(nup, [&](std::string& buf, u64 toc) { + SetLe(buf, first_dir(buf, toc) + 0x08, 0xFFFFFFFF, 4); + SetLe(buf, first_dir(buf, toc) + 0x0C, 2, 4); + }) == Loader::ResultStatus::ErrorNUPBadFormat); +} + +TEST_CASE("NUP rejects a directory that contains its own ancestor", "[nup]") { + TempDir t; + const auto root = t.path / "Game"; + MakeDump(root); + const auto archive = t.path / "Game.nup"; + REQUIRE(FileSys::Nup::PackDump(root, archive) == FileSys::Nup::PackResult::Success); + + // Give the base RomFS directory the same children as the package root, which lists it. + REQUIRE(StatusAfterPatch(ReadAll(archive), [](std::string& buf, u64 toc) { + const u64 dirs = toc + 0x30 + Le(buf, toc + 0x04, 4) * 0x40; + const u64 root_dir = Le(buf, toc + 0x30 + 0x08, 4); + const u64 romfs_dir = Le(buf, toc + 0x30 + 0x10, 4); + const u64 root_record = dirs + root_dir * 0x18; + const u64 romfs_record = dirs + romfs_dir * 0x18; + SetLe(buf, romfs_record + 0x08, Le(buf, root_record + 0x08, 4), 4); + SetLe(buf, romfs_record + 0x0C, Le(buf, root_record + 0x0C, 4), 4); + }) == Loader::ResultStatus::ErrorNUPBadFormat); +} + TEST_CASE("NUP marks updates standalone when the dump has no base", "[nup]") { TempDir t; const auto root = t.path / "Game"; -- cgit v1.2.3 From e277b4ba61694f229b38ec0dc62f31c35d0cbff3 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:27:20 -0500 Subject: nx2: make the 216 sparse request range checks overflow-safe 4d6a13e3d9 and 3d6156434a added bounds checks to the nx2:gpu 216 sparse map path, but the region checks still compute gpu_address + size, which wraps for a guest-controlled size. 3d6156434a only guards size == 0 and the memory offset on the map path, so an unmap (memory handle 0) with a huge size matches a region through the wrapped sum and reaches Nx2SparseTile, whose own check wraps the same way, and then smmu.Unmap(device, size) with a giant page count. Reject a request whose end wraps the address space before it is used, for map and unmap alike, with a warning like the existing unknown-memory check. Also compare by subtraction in the region lookups here and in Nx2SparseTile and Nx2BackSparse, so none of them depends on the sum. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp | 4 ++-- src/core/hle/service/nx2_gsv.h | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp b/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp index 40313a10fb..843d31caf1 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp @@ -270,7 +270,7 @@ void nvhost_as_gpu::Nx2BackSparse(u64 gpu_address, u64 cpu_address, u64 size) { --it; const auto& [start, range] = *it; const auto& [region_size, device_address] = range; - if (gpu_address + size > start + region_size) { + if (size > region_size || gpu_address - start > region_size - size) { return; } system.Host1x().MemoryManager().Map(device_address + (gpu_address - start), cpu_address, size, @@ -287,7 +287,7 @@ void nvhost_as_gpu::Nx2SparseTile(u64 gpu_address, u64 cpu_address, u64 size) { --it; const auto& [start, range] = *it; const auto& [region_size, device_address] = range; - if (gpu_address + size > start + region_size) { + if (size > region_size || gpu_address - start > region_size - size) { return; } // In GPU-thread order: command lists submitted before the request still see the old tile. diff --git a/src/core/hle/service/nx2_gsv.h b/src/core/hle/service/nx2_gsv.h index 37c797fa3f..bc354b4d8f 100644 --- a/src/core/hle/service/nx2_gsv.h +++ b/src/core/hle/service/nx2_gsv.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1076,13 +1077,18 @@ private: } const u64 gpu_address = request.gpu_address; const u64 size = request.size; + if (size > std::numeric_limits::max() - gpu_address) { + LOG_WARNING(Service, "nx2:gpu 216 [{:#x}, +{:#x}] wraps the address space", gpu_address, + size); + return; + } auto& registry = Nvidia::Devices::Nx2GpuMemoryRegistry::Get(); auto it = m_sparse_regions.upper_bound(gpu_address); SparseRegion* region = nullptr; u64 start = 0; if (it != m_sparse_regions.begin()) { --it; - if (gpu_address + size <= it->first + it->second.size) { + if (size <= it->second.size && gpu_address - it->first <= it->second.size - size) { start = it->first; region = &it->second; } @@ -1145,7 +1151,7 @@ private: return std::nullopt; } --it; - if (gpu_address + size > it->first + it->second) { + if (size > it->second || gpu_address - it->first > it->second - size) { return std::nullopt; } return it->first; @@ -1240,7 +1246,7 @@ private: return std::nullopt; } --it; - if (gpu_address + size > it->first + it->second.size) { + if (size > it->second.size || gpu_address - it->first > it->second.size - size) { return std::nullopt; } return it->second.cpu_address + (gpu_address - it->first); -- cgit v1.2.3 From bda49e810b5955bff8474e7b8f2b09eab51e9be2 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:28:52 -0500 Subject: shader: evaluate handle programs for image and texture buffer descriptors 561f87abec only gave TextureDescriptor a handle_program. Image load/store/atomics and texture/image buffers whose handle comes from a storage buffer still got the placeholder constant buffer address (index 0, offset 0), so the pipelines read cbuf 0 word 0 as the handle (and asserted if cbuf 0 was unbound). Add handle_program to TextureBufferDescriptor, ImageBufferDescriptor and ImageDescriptor, compare it when merging, and let all four pipelines evaluate it for every descriptor kind instead of only TextureDescriptor. ImageQueryDimensions on such a handle was always declared Color2D, so array, cube and 3D views got a 2D declaration. The instruction carries no type, so take it from another instruction using the same handle program, falling back to Color2D when there is none. Follow-up to 561f87abec. The OpenGL pipeline edits are the same change as the Vulkan ones but were not compiled here. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 34 ++++++++++++++++++---- src/shader_recompiler/shader_info.h | 9 ++++++ .../renderer_opengl/gl_compute_pipeline.cpp | 25 ++++++++-------- .../renderer_opengl/gl_graphics_pipeline.cpp | 22 +++++++------- .../renderer_vulkan/vk_compute_pipeline.cpp | 25 ++++++++-------- .../renderer_vulkan/vk_graphics_pipeline.cpp | 22 +++++++------- 6 files changed, 82 insertions(+), 55 deletions(-) (limited to 'src') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index 514cd9b2b6..db8f1ac716 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -345,7 +345,8 @@ static inline u32 GetTextureHandleCached(Environment& env, const ConstBufferAddr // Cached variants of existing helpers // A handle computed per draw has no descriptor to read at translation time: assume an ordinary -// floating-point 2D texture, the instruction's own type deciding the rest. +// floating-point 2D texture, the instruction's own type deciding the rest (see the TexturePass +// ImageQueryDimensions case, which has none). static inline TextureType ReadTextureTypeCached(Environment& env, const ConstBufferAddr& cbuf) { if (cbuf.handle_program != ~0U) return TextureType::Color2D; return env.ReadTextureType(GetTextureHandleCached(env, cbuf)); @@ -1010,7 +1011,8 @@ public: desc.secondary_cbuf_offset == existing.secondary_cbuf_offset && desc.secondary_shift_left == existing.secondary_shift_left && desc.count == existing.count && desc.size_shift == existing.size_shift && - desc.has_secondary == existing.has_secondary; + desc.has_secondary == existing.has_secondary && + desc.handle_program == existing.handle_program; }); } @@ -1018,7 +1020,8 @@ public: const u32 index{Add(image_buffer_descriptors, desc, [&desc](const auto& existing) { return desc.format == existing.format && desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && desc.count == existing.count && - desc.size_shift == existing.size_shift; + desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; image_buffer_descriptors[index].is_written |= desc.is_written; image_buffer_descriptors[index].is_read |= desc.is_read; @@ -1050,7 +1053,8 @@ public: return desc.type == existing.type && desc.format == existing.format && desc.cbuf_index == existing.cbuf_index && desc.cbuf_offset == existing.cbuf_offset && desc.count == existing.count && - desc.size_shift == existing.size_shift; + desc.size_shift == existing.size_shift && + desc.handle_program == existing.handle_program; })}; image_descriptors[index].is_written |= desc.is_written; image_descriptors[index].is_read |= desc.is_read; @@ -1186,6 +1190,18 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo has_last_is_integer = true; return last_is_integer; }}; + // A handle program has no descriptor to read a type from, and ImageQueryDimensions carries + // none: it takes the type of another instruction using the same handle. + std::unordered_map handle_program_types; + for (const TextureInst& texture_inst : to_replace) { + const IR::Opcode opcode{IndexedInstruction(*texture_inst.inst)}; + if (texture_inst.cbuf.handle_program != ~0U && opcode != IR::Opcode::ImageQueryDimensions && + !IsStorageImageOpcode(opcode)) { + handle_program_types.try_emplace( + texture_inst.cbuf.handle_program, + texture_inst.inst->Flags().type.Value()); + } + } for (TextureInst& texture_inst : to_replace) { // TODO: Handle arrays IR::Inst* const inst{texture_inst.inst}; @@ -1196,7 +1212,12 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo bool is_multisample{false}; switch (inst->GetOpcode()) { case IR::Opcode::ImageQueryDimensions: - flags.type.Assign(ReadTextureTypeCached(env, cbuf)); + if (const auto it{handle_program_types.find(cbuf.handle_program)}; + it != handle_program_types.end()) { + flags.type.Assign(it->second); + } else { + flags.type.Assign(ReadTextureTypeCached(env, cbuf)); + } inst->SetFlags(flags); break; case IR::Opcode::ImageSampleImplicitLod: @@ -1261,6 +1282,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .cbuf_offset = cbuf.offset, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } else { index = descriptors.Add(ImageDescriptor{ @@ -1273,6 +1295,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .cbuf_offset = cbuf.offset, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } break; @@ -1289,6 +1312,7 @@ void TexturePass(Environment& env, IR::Program& program, const HostTranslateInfo .secondary_shift_left = cbuf.secondary_shift_left, .count = count, .size_shift = size_shift, + .handle_program = cbuf.handle_program, }); } else { count = std::min(count, sampled_dynamic_cap); diff --git a/src/shader_recompiler/shader_info.h b/src/shader_recompiler/shader_info.h index eeb8d768d7..e2b341b0c3 100644 --- a/src/shader_recompiler/shader_info.h +++ b/src/shader_recompiler/shader_info.h @@ -189,6 +189,9 @@ struct TextureBufferDescriptor { u32 secondary_shift_left; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const TextureBufferDescriptor&) const = default; }; @@ -203,6 +206,9 @@ struct ImageBufferDescriptor { u32 cbuf_offset; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const ImageBufferDescriptor&) const = default; }; @@ -240,6 +246,9 @@ struct ImageDescriptor { u32 cbuf_offset; u32 count; u32 size_shift; + /// Index in Info::handle_programs when the handle is computed at draw time (read from a + /// storage buffer), ~0 when it is the constant buffer word. + u32 handle_program{~0U}; auto operator<=>(const ImageDescriptor&) const = default; }; diff --git a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp index eb134e86b5..5fc10cfce2 100644 --- a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp +++ b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp @@ -106,19 +106,18 @@ void ComputePipeline::Configure() { const auto& cbufs{qmd.const_buffer_config}; const bool via_header_index{qmd.linked_tsc != 0}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this dispatch. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && - ((qmd.const_buffer_enable_mask >> cbuf) & 1) != 0 - ? cbufs[cbuf].Address() - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this dispatch. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && + ((qmd.const_buffer_enable_mask >> cbuf) & + 1) != 0 + ? cbufs[cbuf].Address() + : GPUVAddr{}; + }), + via_header_index); } ASSERT(((qmd.const_buffer_enable_mask >> desc.cbuf_index) & 1) != 0); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp index 81950279d5..4e02f0569b 100644 --- a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp +++ b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp @@ -309,18 +309,16 @@ bool GraphicsPipeline::ConfigureImpl(bool is_indexed) { } const auto& cbufs{maxwell3d->state.shader_stages[stage].const_buffers}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this draw. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && cbufs[cbuf].enabled - ? cbufs[cbuf].address - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this draw. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && cbufs[cbuf].enabled + ? cbufs[cbuf].address + : GPUVAddr{}; + }), + via_header_index); } ASSERT(cbufs[desc.cbuf_index].enabled); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp index 1cfe391ba6..3a3861930c 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp @@ -176,19 +176,18 @@ bool ComputePipeline::Configure(Tegra::Engines::KeplerCompute& kepler_compute, const auto& cbufs{qmd.const_buffer_config}; const bool via_header_index{qmd.linked_tsc != 0}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this dispatch. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && - ((qmd.const_buffer_enable_mask >> cbuf) & 1) != 0 - ? cbufs[cbuf].Address() - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this dispatch. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && + ((qmd.const_buffer_enable_mask >> cbuf) & + 1) != 0 + ? cbufs[cbuf].Address() + : GPUVAddr{}; + }), + via_header_index); } ASSERT(((qmd.const_buffer_enable_mask >> desc.cbuf_index) & 1) != 0); const u32 index_offset{index << desc.size_shift}; diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp index fa273f6ef9..4bd2e2735e 100644 --- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp @@ -400,18 +400,16 @@ bool GraphicsPipeline::ConfigureImpl(bool is_indexed) { } const auto& cbufs{maxwell3d->state.shader_stages[stage].const_buffers}; const auto read_handle{[&](const auto& desc, u32 index) { - if constexpr (std::is_same_v) { - if (desc.handle_program != ~0U) { - // Read by the shader from a storage buffer: computed for this draw. - return TexturePair(VideoCommon::EvaluateTextureHandle( - info, desc.handle_program, *gpu_memory, - [&](u32 cbuf) -> GPUVAddr { - return cbuf < cbufs.size() && cbufs[cbuf].enabled - ? cbufs[cbuf].address - : GPUVAddr{}; - }), - via_header_index); - } + if (desc.handle_program != ~0U) { + // Read by the shader from a storage buffer: computed for this draw. + return TexturePair(VideoCommon::EvaluateTextureHandle( + info, desc.handle_program, *gpu_memory, + [&](u32 cbuf) -> GPUVAddr { + return cbuf < cbufs.size() && cbufs[cbuf].enabled + ? cbufs[cbuf].address + : GPUVAddr{}; + }), + via_header_index); } ASSERT(cbufs[desc.cbuf_index].enabled); const u32 index_offset{index << desc.size_shift}; -- cgit v1.2.3 From 8b0666b74868da8221d9e3ef3995f3a862ee24b6 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:29:14 -0500 Subject: shader: restore the handle build depth when a build throws BuildHandleProgram incremented g_handle_depth and decremented it at the end, so a NotImplementedException from BuildHandleNode left the thread-local depth above zero. That thread never cleared g_handle_slots again, and stale IR::Inst* keys from an earlier program could match instructions of a later one (a freed instruction's address can be reused). Do the decrement and the outermost clear in SCOPE_EXIT. Follow-up to e23b0f2fab. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index db8f1ac716..b33700cabd 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -16,6 +16,7 @@ #include #include +#include "common/scope_exit.h" #include "common/settings.h" #include "shader_recompiler/environment.h" #include "shader_recompiler/frontend/ir/basic_block.h" @@ -455,6 +456,12 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud if (outermost) { g_handle_slots.clear(); } + SCOPE_EXIT { + --g_handle_depth; + if (outermost) { + g_handle_slots.clear(); + } + }; const IR::Inst* const inst{value.InstRecursive()}; bool result; if (const auto it = g_handle_slots.find(inst); it != g_handle_slots.end()) { @@ -484,10 +491,6 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud code.push_back(slot); } } - --g_handle_depth; - if (outermost) { - g_handle_slots.clear(); - } return result; } -- cgit v1.2.3 From 0ed4a3a9e795896a4751d0330c78958a309d1aa0 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:29:42 -0500 Subject: shader: drop the slots of a failed handle subtree with its code When BuildHandleProgram failed on a subtree it truncated the code back to where the subtree started, which removed the Tee ops of the values the subtree had kept, but their entries stayed in g_handle_slots. A Phi tries its other inputs after the first one fails; if one of them reused a value from the dropped subtree, it emitted a Load of a slot that nothing wrote, so the handle read 0. Number slots with a counter instead of counting map entries, and on failure erase the entries numbered from the subtree's first slot on and rewind the counter, so such a value is built again. Follow-up to 561f87abec. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/shader_recompiler/ir_opt/texture_pass.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/shader_recompiler/ir_opt/texture_pass.cpp b/src/shader_recompiler/ir_opt/texture_pass.cpp index b33700cabd..b6aca0a118 100644 --- a/src/shader_recompiler/ir_opt/texture_pass.cpp +++ b/src/shader_recompiler/ir_opt/texture_pass.cpp @@ -438,9 +438,10 @@ bool IsUndefined(IR::Opcode opcode) { opcode == IR::Opcode::UndefU64; } -// Values already emitted by the program being built, by the slot that keeps them, and how deep -// in nested BuildHandleProgram calls the build is. +// Values already emitted by the program being built, by the slot that keeps them, how many slots +// are in use, and how deep in nested BuildHandleProgram calls the build is. thread_local std::unordered_map g_handle_slots; +thread_local u32 g_handle_slot_count{}; thread_local int g_handle_depth{}; bool BuildHandleNode(const IR::Value& value, std::vector& code, int& budget, @@ -455,11 +456,13 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud const bool outermost = g_handle_depth++ == 0; if (outermost) { g_handle_slots.clear(); + g_handle_slot_count = 0; } SCOPE_EXIT { --g_handle_depth; if (outermost) { g_handle_slots.clear(); + g_handle_slot_count = 0; } }; const IR::Inst* const inst{value.InstRecursive()}; @@ -478,14 +481,20 @@ bool BuildHandleProgram(const IR::Value& value, std::vector& code, int& bud } else { g_handle_slots.emplace(inst, ~0U); const size_t start = code.size(); + const u32 first_slot = g_handle_slot_count; result = BuildHandleNode(value, code, budget, block); g_handle_slots.erase(inst); if (!result) { + // The Tee of every value the dropped code kept goes with it: a later use of one of + // them (another Phi input) must build it again, not load a slot nothing wrote. code.resize(start); + std::erase_if(g_handle_slots, [first_slot](const auto& entry) { + return entry.second != ~0U && entry.second >= first_slot; + }); + g_handle_slot_count = first_slot; } if (result && inst->UseCount() > 1 && g_handle_slots.size() < 256) { - const u32 slot = static_cast( - std::ranges::count_if(g_handle_slots, [](const auto& entry) { return entry.second != ~0U; })); + const u32 slot = g_handle_slot_count++; g_handle_slots.emplace(inst, slot); code.push_back(static_cast(Op::Tee)); code.push_back(slot); -- 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') 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 From a1f781fa24cc16e6d636213a5596cfed4123f493 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:31:05 -0500 Subject: video_core: flush GPU-written code when capturing native shaders 10e7063905 moved native program capture from one ReadBlock per instruction to ReadBlockUnsafe per 4 KiB page. ReadBlockUnsafe skips the rasterizer flush, so code that a GPU copy had written and not yet flushed to guest memory was captured stale. The speed-up comes from reading a page at a time, not from skipping the flush: read each page (and the odd single instruction outside a fully mapped page) with ReadBlock, which flushes once per page instead of once per instruction. Follow-up to 10e7063905. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/video_core/shader_environment.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/video_core/shader_environment.cpp b/src/video_core/shader_environment.cpp index 8d340ba881..0c7d8a0d42 100644 --- a/src/video_core/shader_environment.cpp +++ b/src/video_core/shader_environment.cpp @@ -328,9 +328,9 @@ Tegra::Texture::TICEntry GenericEnvironment::ReadTextureInfo(GPUVAddr tic_addr, namespace { // Reads a native program a 4 KiB page at a time instead of one 16-byte instruction at a time. -// The reads do not flush the GPU caches first (ReadBlockUnsafe): the previous per-instruction -// ReadBlock did, once per instruction. An instruction that is not inside a fully mapped page is -// left to the caller. +// Each page is read with ReadBlock, so GPU-written code is flushed first, once per page instead +// of once per instruction. An instruction that is not inside a fully mapped page is left to the +// caller. class NativeInstructionReader { public: using Instruction = Shader::Ampere::Instruction; @@ -365,7 +365,7 @@ private: page_begin = address; page_end = address; if (length >= sizeof(Instruction) && gpu_memory.IsFullyMappedRange(address, length)) { - gpu_memory.ReadBlockUnsafe(address, page.data(), length); + gpu_memory.ReadBlock(address, page.data(), length); page_end = next_page; } } @@ -415,7 +415,7 @@ std::shared_ptr GenericEnvironment::LowerNativeCode(u32 if (!native.empty()) break; throw Shader::RuntimeError("Unmapped native shader instruction at {:#x}", address); } - gpu_memory->ReadBlockUnsafe(address, &instruction, sizeof(instruction)); + gpu_memory->ReadBlock(address, &instruction, sizeof(instruction)); } native.push_back(instruction); const auto status = capture.Observe(instruction, index); -- cgit v1.2.3 From 690f80e15f411d8fa46a2eecdfc50ddadc8ed5e0 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:34:01 -0500 Subject: nx2: don't stall presents on a current-frame wait the GPU can't finish 0f14696044 made PresentClientBuffer also wait for the current frame's fence whenever the previous frame's wasn't reached. Mario Kart World's GPU waits on semaphores its CPU releases only after the present returns, so in that state the current fence can't be reached until the present returns, and each such present stalled up to the 4 s deadline. Wait for the current frame by default, but give up after a few ms once a GPU channel was seen suspended on a semaphore acquire and wait for the previous frame instead. EDEN_NX2_FRAMES_AHEAD=1 always waits for the previous frame (with 0f14696044's current-frame wait when the GPU is behind), =0 always for the current one. The scheduler gains a lock-free flag so the present thread can see suspended channels. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/hle/service/nx2_gsv.h | 65 ++++++++++++++++++++++++------------ src/video_core/control/scheduler.cpp | 2 ++ src/video_core/control/scheduler.h | 6 ++++ src/video_core/gpu.cpp | 4 +++ src/video_core/gpu.h | 3 ++ 5 files changed, 58 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/core/hle/service/nx2_gsv.h b/src/core/hle/service/nx2_gsv.h index bc354b4d8f..259681a2a2 100644 --- a/src/core/hle/service/nx2_gsv.h +++ b/src/core/hle/service/nx2_gsv.h @@ -1711,37 +1711,41 @@ private: LOG_INFO(Service, "nx2:vi2 229 present #{} token={:#x} slot={} image={}", m_presents, u64{w[2]} | (u64{w[3]} << 32), w[4], w[5]); } - // Wait for the previous frame's fence instead of this one, so the game prepares frame N+1 - // while the GPU thread runs frame N (the console keeps several frames in flight). Waiting - // for this frame serialized the two: 8-56 ms of every 34-105 ms frame on title 1. - // EDEN_NX2_FRAMES_AHEAD=0 restores the wait for the current frame. - static const bool frames_ahead = [] { + // Wait for this frame's fence, so the game doesn't write the next frame's data (constant + // buffers holding texture handles, vertex data, Pokopia's glyph atlas) while the GPU + // thread still reads this frame's. But title 1's GPU waits on semaphores its CPU releases + // only after this present returns: while a channel stays suspended on one, wait for the + // previous frame's fence instead and let the game prepare frame N+1 meanwhile (waiting + // for this frame there cost 8-56 ms of every 34-105 ms frame, or never finished). + // EDEN_NX2_FRAMES_AHEAD=1 always waits for the previous frame, =0 always for this one. + static const int frames_ahead = [] { const char* value = std::getenv("EDEN_NX2_FRAMES_AHEAD"); - return value == nullptr || value[0] != '0'; + return value == nullptr ? -1 : value[0] == '1' ? 1 : 0; }(); const auto entered = std::chrono::steady_clock::now(); - if (frames_ahead) { + if (frames_ahead == 1 || (frames_ahead == -1 && !WaitForFrame(~w[22], w[24], true))) { LOG_DEBUG(Service, "nx2:vi2 present #{} fence ({}, {:#x}), waiting for ({}, {:#x}); now {:#x}", m_presents, ~w[22], w[24], m_previous_fence_id, m_previous_fence_value, ~w[22] < 192 ? system.Host1x().GetSyncpointManager().GetGuestSyncpointValue(~w[22]) : 0U); - // Only run a frame ahead while the GPU keeps up. With the previous frame still on the - // GPU here, the GPU is the limit and running ahead gains nothing; and Splatoon Raiders, - // which reuses its command memory as if the GPU were at most a frame behind, rewrote - // command lists Eden had not read yet (misread commands, then crashes, with a slow - // GPU). Wait for this frame too then. - const bool gpu_behind = - m_previous_fence_id < 192 && - !system.Host1x().GetSyncpointManager().IsReadyGuest(m_previous_fence_id, - m_previous_fence_value); + // With EDEN_NX2_FRAMES_AHEAD=1, only run a frame ahead while the GPU keeps up. With + // the previous frame still on the GPU here, the GPU is the limit and running ahead + // gains nothing; and Splatoon Raiders, which reuses its command memory as if the GPU + // were at most a frame behind, rewrote command lists Eden had not read yet (misread + // commands, then crashes, with a slow GPU). Wait for this frame too then. Not when a + // suspended channel brought us here: this frame can't finish before the present + // returns. + const bool gpu_behind = frames_ahead == 1 && m_previous_fence_id < 192 && + !system.Host1x().GetSyncpointManager().IsReadyGuest( + m_previous_fence_id, m_previous_fence_value); WaitForFrame(m_previous_fence_id, m_previous_fence_value); if (gpu_behind) { WaitForFrame(~w[22], w[24]); } - m_previous_fence_id = ~w[22]; - m_previous_fence_value = w[24]; - } else { + } else if (frames_ahead == 0) { WaitForFrame(~w[22], w[24]); } + m_previous_fence_id = ~w[22]; + m_previous_fence_value = w[24]; const auto waited = std::chrono::steady_clock::now(); Composite(w[5], ~w[22], w[24], w[7]); const auto composited = std::chrono::steady_clock::now(); @@ -1838,9 +1842,12 @@ private: m_next_vblank += interval; } - void WaitForFrame(u32 fence_id, u32 fence_value) { + /// Waits until the fence is reached. With `unless_gpu_waits`, gives up after a few ms once a + /// GPU channel was seen suspended on a semaphore (it may be one this thread releases later). + /// Returns whether the fence was reached. + bool WaitForFrame(u32 fence_id, u32 fence_value, bool unless_gpu_waits = false) { if (fence_id >= 192) { - return; + return true; } auto& syncpoints = system.Host1x().GetSyncpointManager(); const auto start = std::chrono::steady_clock::now(); @@ -1865,11 +1872,24 @@ private: wake->cv.notify_all(); }); std::unique_lock lk{wake->mutex}; + // Title 1's channels resume and suspend again while they wait on this thread, so any + // suspension counts, not only an unbroken one. + bool gpu_waited = false; while (!wake->done && system.IsPoweredOn() && std::chrono::steady_clock::now() < deadline) { - wake->cv.wait_for(lk, std::chrono::milliseconds{50}); + if (unless_gpu_waits) { + gpu_waited |= system.GPU().HasSuspendedChannels(); + if (gpu_waited && + std::chrono::steady_clock::now() - start > std::chrono::milliseconds{3}) { + return false; + } + } + wake->cv.wait_for(lk, std::chrono::milliseconds{unless_gpu_waits ? 1 : 50}); } } + if (unless_gpu_waits && !syncpoints.IsReadyGuest(fence_id, fence_value)) { + return false; + } if (!syncpoints.IsReadyGuest(fence_id, fence_value) && m_wait_timeouts++ < 64) { LOG_WARNING(Service, "nx2:vi2 present #{} fence ({}, {}) not reached in 4 s (now {})", m_presents, fence_id, fence_value, syncpoints.GetGuestSyncpointValue(fence_id)); @@ -1881,6 +1901,7 @@ private: std::chrono::steady_clock::now() - start) .count()); } + return syncpoints.IsReadyGuest(fence_id, fence_value); } // Shows a presented native image: word 5 is the image id, ~word 22 / word 24 the GPU fence diff --git a/src/video_core/control/scheduler.cpp b/src/video_core/control/scheduler.cpp index dc42906752..97d2dcd482 100644 --- a/src/video_core/control/scheduler.cpp +++ b/src/video_core/control/scheduler.cpp @@ -48,6 +48,7 @@ void Scheduler::Push(GPU& gpu, s32 channel, CommandList&& entries) { std::find(pending_channels.begin(), pending_channels.end(), channel_state) == pending_channels.end()) { pending_channels.push_back(channel_state); + any_pending.store(true, std::memory_order_release); } if (!pending_channels.empty()) { ResumePending(gpu); @@ -77,6 +78,7 @@ void Scheduler::ResumePending(GPU& gpu) { std::erase_if(pending_channels, [](const auto& state) { return !state->payload->dma_pusher.HasPendingAcquire(); }); + any_pending.store(!pending_channels.empty(), std::memory_order_release); } void Scheduler::DeclareChannel(std::shared_ptr new_channel) { diff --git a/src/video_core/control/scheduler.h b/src/video_core/control/scheduler.h index 9a0f1b919f..b1aaf46b60 100644 --- a/src/video_core/control/scheduler.h +++ b/src/video_core/control/scheduler.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -30,6 +31,10 @@ public: [[nodiscard]] bool HasPendingChannels() const { return !pending_channels.empty(); } + /// HasPendingChannels for threads other than the GPU thread. + [[nodiscard]] bool HasPendingChannelsAnyThread() const { + return any_pending.load(std::memory_order_acquire); + } void DeclareChannel(std::shared_ptr new_channel); @@ -39,6 +44,7 @@ private: ::Common::unordered_map> channels; std::mutex scheduling_guard; std::vector> pending_channels; ///< Suspended on an acquire + std::atomic any_pending{}; }; } // namespace Control diff --git a/src/video_core/gpu.cpp b/src/video_core/gpu.cpp index 53f8f063f7..a6f31359a3 100644 --- a/src/video_core/gpu.cpp +++ b/src/video_core/gpu.cpp @@ -505,6 +505,10 @@ bool GPU::IsAsync() const { return impl->is_async; } +bool GPU::HasSuspendedChannels() const { + return impl->scheduler.HasPendingChannelsAnyThread(); +} + bool GPU::UseNvdec() const { return impl->use_nvdec; } diff --git a/src/video_core/gpu.h b/src/video_core/gpu.h index 3a6546b41b..4afc89f0e6 100644 --- a/src/video_core/gpu.h +++ b/src/video_core/gpu.h @@ -217,6 +217,9 @@ public: [[nodiscard]] bool IsAsync() const; + /// Whether a channel is suspended on a semaphore acquire (callable from any thread). + [[nodiscard]] bool HasSuspendedChannels() const; + [[nodiscard]] bool UseNvdec() const; void RendererFrameEndNotify(); -- cgit v1.2.3 From 256b1dc346e19bc60c2884d4af875fc53ef0a99a Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:34:40 -0500 Subject: video_core: don't execute command lists copied before their channel waited cce6d44af2 executes each command list entry from a copy of its words taken at submit. Games submit command lists before writing them: the lists wait behind a semaphore acquire, and the game writes their commands just before it releases the acquire (Mario Kart World after present). The copies of those lists were stale. Pokopia stopped after its first frame on macOS: its GPU never reached the fence the game spun on (EDEN_NX2_PB_SNAPSHOT=0 made it run again). Count each channel's suspensions on an acquire and the resumes after them, and record the count with each copy. A list copied while its channel was suspended, or whose channel suspended since, reads guest memory instead. Lists submitted and run without a wait in between keep the copy. EDEN_NX2_PB_SNAPSHOT=0 still disables it altogether. Assisted-by: Claude Sonnet 5.5 (first version); Claude Opus 5.5 (bisect, suspension count, testing and commit message) --- src/core/hle/service/nvdrv/devices/nvhost_gpu.cpp | 5 ++++- src/video_core/dma_pusher.cpp | 3 +++ src/video_core/dma_pusher.h | 9 +++++++++ src/video_core/engines/puller.cpp | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/hle/service/nvdrv/devices/nvhost_gpu.cpp b/src/core/hle/service/nvdrv/devices/nvhost_gpu.cpp index 848c15d6f8..79586446d5 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_gpu.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_gpu.cpp @@ -445,7 +445,10 @@ NvResult nvhost_gpu::SubmitGPFIFOImpl(IoctlSubmitGpfifo& params, Tegra::CommandL const char* value = std::getenv("EDEN_NX2_PB_SNAPSHOT"); return value == nullptr || value[0] != '0'; }(); - if (snapshot && channel_state->memory_manager) { + if (snapshot && channel_state->memory_manager && channel_state->payload) { + // Before the copy: a suspension during it must count as one after it. + entries.snapshot_suspensions = + channel_state->payload->dma_pusher.suspensions.load(std::memory_order_acquire); size_t words = 0; for (const auto& entry : entries.command_lists) { words += entry.size; diff --git a/src/video_core/dma_pusher.cpp b/src/video_core/dma_pusher.cpp index 6b93bb7f60..6a79e0570e 100644 --- a/src/video_core/dma_pusher.cpp +++ b/src/video_core/dma_pusher.cpp @@ -162,6 +162,9 @@ bool DmaPusher::Step() { // read: their words at submit time are an older frame's. const bool submitted = command_resume_offset == 0 && dma_pushbuffer_subindex < command_list.submitted_offsets.size() && + (command_list.snapshot_suspensions & 1) == 0 && + command_list.snapshot_suspensions == + suspensions.load(std::memory_order_acquire) && !IsGpuWrittenCommands(dma_state.dma_get, size_t{header.size} * sizeof(u32)) && !(use_safe && memory_manager.IsMemoryDirty(dma_state.dma_get, diff --git a/src/video_core/dma_pusher.h b/src/video_core/dma_pusher.h index 289786bb9e..feab652bb5 100644 --- a/src/video_core/dma_pusher.h +++ b/src/video_core/dma_pusher.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -121,6 +122,10 @@ struct CommandList final { /// executed instead of the memory, which the game may already be refilling for a later frame. std::vector submitted_words; std::vector submitted_offsets; + /// DmaPusher::suspensions when the words were copied. A list copied during a suspension, or + /// that waited behind one, reads memory instead: games write the commands it runs before they + /// release the acquire. + u64 snapshot_suspensions = 0; }; /// FNV-1a over pushbuffer words (EDEN_NX2_PB_CHECK). @@ -139,6 +144,10 @@ inline u64 HashPushbufferWords(std::span words) { /// details on this implementation. class DmaPusher final { public: + /// Counts suspensions on a semaphore acquire and the resumes after them, so it is odd while + /// the channel is suspended (see CommandList::snapshot_suspensions). + std::atomic suspensions{0}; + explicit DmaPusher(Core::System& system_, MemoryManager& memory_manager_, Control::ChannelState& channel_state_); ~DmaPusher(); diff --git a/src/video_core/engines/puller.cpp b/src/video_core/engines/puller.cpp index 0bb1809aa3..37ac3ec851 100644 --- a/src/video_core/engines/puller.cpp +++ b/src/video_core/engines/puller.cpp @@ -116,6 +116,7 @@ bool Puller::ResumeAcquire(DmaPusher& dma_pusher) { return false; } pending_acquire.reset(); + dma_pusher.suspensions.fetch_add(1, std::memory_order_release); Nx2SemaphoreWaiters::Get().Remove(this); return true; } @@ -257,6 +258,7 @@ void Puller::ProcessSemaphoreTriggerMethod(DmaPusher& dma_pusher) { dma_pusher.rasterizer->ReleaseFences(); if (!waiters.Latched(this) && !acquire.Matches(ReadSemaphoreWord(dma_pusher, acquire.address))) { pending_acquire = acquire; + dma_pusher.suspensions.fetch_add(1, std::memory_order_release); VideoCommon::Nx2Profile::Add(VideoCommon::Nx2Profile::Counter::AcquireWaits, 1); } else { waiters.Remove(this); -- cgit v1.2.3 From 13fc5fda26e6b5a9af67f5a654e831187bfa1962 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:38:11 -0500 Subject: vulkan: repack 69-byte vertices only when needed SetVertexRepackLayout returned old_mask | new_mask, which stays non-zero on every draw that uses a 69-byte binding. PrepareDraw then re-dirtied the vertex buffers and vertex input each time, so the repack compute pass ran and the render pass ended once per draw, which is costly on tiled GPUs. Return old_mask ^ new_mask so only a layout change re-dirties the bindings. The repacked copy only stays valid while the source data is unchanged, so the buffer cache now flags the vertex buffers overlapping a CPU write, inline or DMA write, or GPU write, and re-dirties the repacked ones before binding. Command buffer submission already invalidates all vertex buffer state, which keeps the staging copy within its tick. Also pass the source buffer size to the repack pass and clamp the word-rounded storage descriptor range to it; it could run up to three bytes past the end of the buffer. A binding that does not fit is skipped like one over the device limits. Follow-up to e38b738b84. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/tests/video_core/vertex_repack.cpp | 26 ++++++++++----- src/video_core/buffer_cache/buffer_cache.h | 37 ++++++++++++++++++++++ src/video_core/buffer_cache/buffer_cache_base.h | 8 +++++ src/video_core/renderer_vulkan/vertex_repack.h | 14 +++++--- src/video_core/renderer_vulkan/vk_buffer_cache.cpp | 8 +++-- src/video_core/renderer_vulkan/vk_buffer_cache.h | 1 + src/video_core/renderer_vulkan/vk_compute_pass.cpp | 5 +-- src/video_core/renderer_vulkan/vk_compute_pass.h | 7 ++-- 8 files changed, 86 insertions(+), 20 deletions(-) (limited to 'src') diff --git a/src/tests/video_core/vertex_repack.cpp b/src/tests/video_core/vertex_repack.cpp index 6409be3ca4..3705a97926 100644 --- a/src/tests/video_core/vertex_repack.cpp +++ b/src/tests/video_core/vertex_repack.cpp @@ -85,7 +85,8 @@ TEST_CASE("Static vertex repacking requires explicit experimental opt-in", "[ver TEST_CASE("Vertex repack planning keeps descriptor offsets aligned", "[vertex_repack]") { for (const u32 alignment : {4U, 16U, 64U, 256U, 4096U}) { for (const u32 offset : {0U, 1U, 3U, 255U, 4099U}) { - const auto plan = Vulkan::MakeVertexRepackParameters(offset, 70, alignment, 1U << 20, 65535); + const auto plan = Vulkan::MakeVertexRepackParameters(offset, 70, alignment, 1U << 20, + 65535, 1U << 20); REQUIRE(plan.has_value()); REQUIRE(plan->source_base % alignment == 0); REQUIRE(plan->source_base + plan->source_bias == offset); @@ -99,13 +100,22 @@ TEST_CASE("Vertex repack planning keeps descriptor offsets aligned", "[vertex_re } TEST_CASE("Vertex repack planning rejects overflow and device-limit violations", "[vertex_repack]") { - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0, 256, 1U << 20, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 0, 1U << 20, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 71, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(255, 69, 256, 323, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 0)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0xffffffffU, 256, ~u64{0}, 0xffffffffU)); - const auto one = Vulkan::MakeVertexRepackParameters(0, 69, 256, 72, 1); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0, 256, 1U << 20, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 0, 1U << 20, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 71, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(255, 69, 256, 323, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 0, 1U << 20)); + REQUIRE_FALSE( + Vulkan::MakeVertexRepackParameters(0, 0xffffffffU, 256, ~u64{0}, 0xffffffffU, ~u64{0})); + const auto one = Vulkan::MakeVertexRepackParameters(0, 69, 256, 72, 1, 72); REQUIRE(one.has_value()); REQUIRE(one->source_range == 72); } + +TEST_CASE("Vertex repack planning keeps the source range inside the buffer", "[vertex_repack]") { + const auto clamped = Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 65535, 69); + REQUIRE(clamped.has_value()); + REQUIRE(clamped->source_range == 69); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 65535, 68)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(300, 69, 256, 1U << 20, 65535, 256)); +} diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h index cd176dec27..6878780866 100644 --- a/src/video_core/buffer_cache/buffer_cache.h +++ b/src/video_core/buffer_cache/buffer_cache.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -180,6 +181,7 @@ void BufferCache

::WriteMemory(DAddr device_addr, u64 size) { gpu_modified_ranges.Subtract(device_addr, size); } memory_tracker.MarkRegionAsCpuModified(device_addr, size); + MarkVertexBuffersStale(device_addr, size); } template @@ -265,6 +267,7 @@ bool BufferCache

::DMACopy(GPUVAddr src_address, GPUVAddr dest_address, u64 am } ClearDownload(*cpu_dest_address, amount); + MarkVertexBuffersStale(*cpu_dest_address, amount); BufferId buffer_a; BufferId buffer_b; @@ -892,8 +895,40 @@ void BufferCache

::UpdateVertexBufferSlot(u32 index, const Binding& binding) { } } +template +void BufferCache

::MarkVertexBuffersStale(DAddr device_addr, u64 size) { + if constexpr (requires { runtime.IsVertexRepacked(u32{}); }) { + u32 mask = enabled_vertex_buffers_mask; + while (mask != 0) { + const u32 index = std::countr_zero(mask); + mask &= mask - 1; + const Binding& binding = v_buffer[index]; + if (device_addr < binding.device_addr + binding.size && + binding.device_addr < device_addr + size) { + stale_vertex_buffers_mask |= 1U << index; + } + } + } +} + +template +void BufferCache

::RefreshRepackedVertexBuffers() { + if constexpr (requires { runtime.IsVertexRepacked(u32{}); }) { + u32 mask = std::exchange(stale_vertex_buffers_mask, 0) & enabled_vertex_buffers_mask; + auto& flags = maxwell3d->dirty.flags; + while (mask != 0) { + const u32 index = std::countr_zero(mask); + mask &= mask - 1; + if (runtime.IsVertexRepacked(index)) { + flags[Dirty::VertexBuffer0 + index] = true; + } + } + } +} + template void BufferCache

::BindHostVertexBuffers() { + RefreshRepackedVertexBuffers(); #ifdef __ANDROID__ const bool use_optimized_vertex_buffers = Settings::values.use_optimized_vertex_buffers.GetValue(); @@ -1655,6 +1690,7 @@ void BufferCache

::MarkWrittenBuffer(BufferId buffer_id, DAddr device_addr, u3 } } memory_tracker.MarkRegionAsGpuModified(device_addr, size); + MarkVertexBuffersStale(device_addr, size); gpu_modified_ranges.Add(device_addr, size); if (DownloadsAtFence(device_addr, size)) { uncommitted_gpu_modified_ranges.Add(device_addr, size); @@ -2205,6 +2241,7 @@ void BufferCache

::InlineMemoryImplementation(DAddr dest_address, size_t copy_ std::span inlined_buffer) { ClearDownload(dest_address, copy_size); gpu_modified_ranges.Subtract(dest_address, copy_size); + MarkVertexBuffersStale(dest_address, copy_size); BufferId buffer_id = FindBuffer(dest_address, static_cast(copy_size), false); auto& buffer = slot_buffers[buffer_id]; diff --git a/src/video_core/buffer_cache/buffer_cache_base.h b/src/video_core/buffer_cache/buffer_cache_base.h index 1f0c6cf10f..59be418030 100644 --- a/src/video_core/buffer_cache/buffer_cache_base.h +++ b/src/video_core/buffer_cache/buffer_cache_base.h @@ -525,6 +525,13 @@ private: void UpdateVertexBufferSlot(u32 index, const Binding& binding); + /// Flags the vertex buffers overlapping a written range so host copies derived from them are + /// rebuilt. + void MarkVertexBuffersStale(DAddr device_addr, u64 size); + + /// Re-dirties repacked vertex buffers whose source data was written since they were bound. + void RefreshRepackedVertexBuffers(); + void ClearDownload(DAddr base_addr, u64 size); void InlineMemoryImplementation(DAddr dest_address, size_t copy_size, @@ -545,6 +552,7 @@ private: u32 last_index_count = 0; u32 enabled_vertex_buffers_mask = 0; + u32 stale_vertex_buffers_mask = 0; u64 vertex_buffers_serial = 0; std::array v_buffer{}; diff --git a/src/video_core/renderer_vulkan/vertex_repack.h b/src/video_core/renderer_vulkan/vertex_repack.h index b0849c25c7..456d1c6f20 100644 --- a/src/video_core/renderer_vulkan/vertex_repack.h +++ b/src/video_core/renderer_vulkan/vertex_repack.h @@ -2,6 +2,7 @@ #pragma once +#include #include #include #include @@ -31,8 +32,11 @@ struct VertexRepackParameters { u32 dispatch_groups; }; -inline std::optional MakeVertexRepackParameters( - u32 offset, u32 size, u64 alignment, u64 max_storage_range, u32 max_dispatch_groups) { +inline std::optional MakeVertexRepackParameters(u32 offset, u32 size, + u64 alignment, + u64 max_storage_range, + u32 max_dispatch_groups, + u64 buffer_size) { if (size == 0 || alignment == 0) { return std::nullopt; } @@ -40,9 +44,11 @@ inline std::optional MakeVertexRepackParameters( const u64 output_size = vertices * 72; const u64 base = u64{offset} / alignment * alignment; const u64 bias = offset - base; - const u64 range = (bias + size + 3) / 4 * 4; + // The word-rounded range must not run past the end of the source buffer + const u64 available = buffer_size > base ? buffer_size - base : 0; + const u64 range = (std::min)((bias + size + 3) / 4 * 4, available); const u64 groups = (vertices * 18 + 127) / 128; - if (output_size > std::numeric_limits::max() || + if (range < bias + size || output_size > std::numeric_limits::max() || range > std::numeric_limits::max() || output_size > max_storage_range || range > max_storage_range || groups > max_dispatch_groups) { return std::nullopt; diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp index 2449265509..4d5ccb6520 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp @@ -394,7 +394,7 @@ u32 BufferCacheRuntime::SetVertexRepackLayout(const Tegra::Engines::Maxwell3D::R LOG_DEBUG(Render_Vulkan, "Vertex repack selection: mask={:#x}, dynamic_input={}", vertex_repack_mask, dynamic_input); } - return old_mask | vertex_repack_mask; + return old_mask ^ vertex_repack_mask; } StagingBufferRef BufferCacheRuntime::UploadStagingBuffer(size_t size) { @@ -657,7 +657,8 @@ void BufferCacheRuntime::BindVertexBuffer(u32 index, VkBuffer buffer, u32 offset return; } if (buffer != VK_NULL_HANDLE && size != 0 && IsVertexRepacked(index)) { - const auto repacked = vertex_repack_pass->Assemble(buffer, offset, size); + const auto repacked = + vertex_repack_pass->Assemble(buffer, offset, size, u64{offset} + size); if (!repacked) { return; } @@ -702,7 +703,8 @@ void BufferCacheRuntime::BindVertexBuffers(VideoCommon::HostBindings& bi if (handle != VK_NULL_HANDLE && IsVertexRepacked(binding) && bindings.strides[i] == 69 && bindings.sizes[i] > 0) { const auto repacked = vertex_repack_pass->Assemble( - handle, static_cast(bindings.offsets[i]), static_cast(bindings.sizes[i])); + handle, static_cast(bindings.offsets[i]), static_cast(bindings.sizes[i]), + bindings.buffers[i]->SizeBytes()); if (!repacked) { continue; } diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.h b/src/video_core/renderer_vulkan/vk_buffer_cache.h index d3295ec6ed..a8d0274016 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.h +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.h @@ -110,6 +110,7 @@ public: void TickFrame(Common::SlotVector& slot_buffers) noexcept; + /// Returns the mask of bindings whose repacked state changed. u32 SetVertexRepackLayout(const Tegra::Engines::Maxwell3D::Regs& regs, bool dynamic_input); bool IsVertexRepacked(u32 binding) const { diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.cpp b/src/video_core/renderer_vulkan/vk_compute_pass.cpp index cb8dcde42f..5902b7d8f5 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.cpp +++ b/src/video_core/renderer_vulkan/vk_compute_pass.cpp @@ -335,10 +335,11 @@ VertexRepackPass::VertexRepackPass(const Device& device_, Scheduler& scheduler_, scheduler{scheduler_}, staging_pool{staging_pool_}, descriptor_queue{descriptor_queue_} {} std::optional> VertexRepackPass::Assemble(VkBuffer source, - u32 offset, u32 size) { + u32 offset, u32 size, + u64 buffer_size) { const auto parameters = MakeVertexRepackParameters( offset, size, device.GetStorageBufferAlignment(), device.GetMaxStorageBufferRange(), - device.GetMaxComputeWorkGroupCount()[0]); + device.GetMaxComputeWorkGroupCount()[0], buffer_size); if (!parameters) { LOG_ERROR(Render_Vulkan, "Vertex repack exceeds device limits: offset={}, size={}", offset, size); return std::nullopt; diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.h b/src/video_core/renderer_vulkan/vk_compute_pass.h index 27fffbbbb7..9687bfff91 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.h +++ b/src/video_core/renderer_vulkan/vk_compute_pass.h @@ -60,9 +60,10 @@ class VertexRepackPass final : public ComputePass { public: VertexRepackPass(const Device& device, Scheduler& scheduler, DescriptorPool& descriptor_pool, StagingBufferPool& staging_pool, ComputePassDescriptorQueue& descriptor_queue); - /// Returns nothing when the range exceeds the device limits; the binding is then left as is. - std::optional> Assemble(VkBuffer source, u32 offset, - u32 size); + /// Returns nothing when the range exceeds the device limits or the source buffer; the binding + /// is then left as is. + std::optional> Assemble(VkBuffer source, u32 offset, u32 size, + u64 buffer_size); private: Scheduler& scheduler; -- cgit v1.2.3 From 8556e27015ded665ca9a5522071cdeeec20a1575 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:39:27 -0500 Subject: buffer_cache: fix the already-disabled check for uniform buffers DisableGraphicsUniformBuffer tested !current.buffer_id to detect a binding that is already NULL_BINDING. SlotId converts to true unless it is the invalid id, and NULL_BUFFER_ID is BufferId{0}, so the check was false for NULL_BINDING and the early return never triggered. Compare against NULL_BUFFER_ID instead. Follow-up to 46b2577bf7. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/video_core/buffer_cache/buffer_cache.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h index 6878780866..2d90a949c3 100644 --- a/src/video_core/buffer_cache/buffer_cache.h +++ b/src/video_core/buffer_cache/buffer_cache.h @@ -411,7 +411,8 @@ void BufferCache

::BindGraphicsUniformBuffer(size_t stage, u32 index, GPUVAddr template void BufferCache

::DisableGraphicsUniformBuffer(size_t stage, u32 index) { auto& current = channel_state->uniform_buffers[stage][index]; - if (current.device_addr == 0 && current.size == 0 && !current.buffer_id) [[likely]] { + if (current.device_addr == 0 && current.size == 0 && current.buffer_id == NULL_BUFFER_ID) + [[likely]] { return; } current = NULL_BINDING; -- cgit v1.2.3 From 427b9abfdc4898f1aeef978ee6edbdeb336db280 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:41:26 -0500 Subject: video_core: continue a split instance loop at its own instance index Writing any register outside the instance loop's own methods now draws the waiting instances first, so the batch sees the base instance it was begun with. When a title writes another register in the middle of a loop, such as a per-instance constant buffer, the loop is drawn in several batches, and each later batch started again at global_base_instance_index with one extra instance, so it used the instance IDs of the first records. Count the instances already drawn for the current loop and start the next batch after them, without the extra first-instance draw. A loop that is not split is drawn exactly as before. Follow-up to 2ad0830d2f and eb99da6072. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/video_core/engines/draw_manager.cpp | 16 ++++++++++++++-- src/video_core/engines/maxwell_3d.cpp | 4 ++-- src/video_core/engines/maxwell_3d.h | 5 ++++- 3 files changed, 20 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/video_core/engines/draw_manager.cpp b/src/video_core/engines/draw_manager.cpp index 038e5a2e69..974a9a6e9a 100644 --- a/src/video_core/engines/draw_manager.cpp +++ b/src/video_core/engines/draw_manager.cpp @@ -72,11 +72,19 @@ void Maxwell3D::DrawManager::Clear(Maxwell3D& maxwell3d, u32 layer_count) { } } -void Maxwell3D::DrawManager::DrawDeferred(Maxwell3D& maxwell3d) { +void Maxwell3D::DrawManager::DrawDeferred(Maxwell3D& maxwell3d, bool loop_continues) { if (draw_state.draw_mode != DrawMode::Instance || draw_state.instance_count == 0) { + if (!loop_continues) { + draw_state.drawn_instances = 0; + } return; } - DrawEnd(maxwell3d, draw_state.instance_count + 1, true); + // The first draw of a batch also covers the loop's first instance; a batch that continues a + // loop split by a register write draws only the instances after the ones already drawn. + const bool continued = draw_state.drawn_instances != 0; + DrawEnd(maxwell3d, draw_state.instance_count + (continued ? 0 : 1), true); + draw_state.drawn_instances = + loop_continues ? draw_state.drawn_instances + draw_state.instance_count : 0; draw_state.instance_count = 0; } @@ -121,6 +129,7 @@ void Maxwell3D::DrawManager::DrawArrayInstanced(Maxwell3D& maxwell3d, Maxwell3D: draw_state.vertex_buffer.count = vertex_count; if (!subsequent) { draw_state.instance_count = 1; + draw_state.drawn_instances = 0; } draw_state.base_instance = draw_state.instance_count - 1; draw_state.draw_mode = DrawMode::Instance; @@ -220,6 +229,9 @@ void Maxwell3D::DrawManager::DrawEnd(Maxwell3D& maxwell3d, u32 instance_count, b [[fallthrough]]; case DrawMode::General: draw_state.base_instance = maxwell3d.regs.global_base_instance_index; + if (draw_state.draw_mode == DrawMode::Instance && draw_state.drawn_instances != 0) { + draw_state.base_instance += draw_state.drawn_instances + 1; + } draw_state.base_index = maxwell3d.regs.global_base_vertex_index; if (draw_state.draw_indexed) { draw_state.index_buffer = maxwell3d.regs.index_buffer; diff --git a/src/video_core/engines/maxwell_3d.cpp b/src/video_core/engines/maxwell_3d.cpp index 9731f54584..4ef7d42979 100644 --- a/src/video_core/engines/maxwell_3d.cpp +++ b/src/video_core/engines/maxwell_3d.cpp @@ -342,7 +342,7 @@ void Maxwell3D::ProcessDirtyRegisters(u32 method, u32 argument) { // the state they were begun with: NVN's draw macros reset the base instance (and its copy in // c[0][0x644]) right after the loop, before the macro returns. if (draw_manager.HasDeferredDraw() && !DrawManager::IsInstanceLoopMethod(method)) [[unlikely]] { - draw_manager.DrawDeferred(*this); + draw_manager.DrawDeferred(*this, true); } regs.reg_array[method] = argument; for (auto const& table : dirty.tables) @@ -537,7 +537,7 @@ void Maxwell3D::CallMultiMethod(Core::System& system, u32 method, const u32* bas } // As in ProcessDirtyRegisters: the paths below write without it. if (draw_manager.HasDeferredDraw() && !DrawManager::IsInstanceLoopMethod(method)) [[unlikely]] { - draw_manager.DrawDeferred(*this); + draw_manager.DrawDeferred(*this, true); } switch (method) { case MAXWELL3D_REG_INDEX(const_buffer.buffer): diff --git a/src/video_core/engines/maxwell_3d.h b/src/video_core/engines/maxwell_3d.h index af0e37978e..c5d1eb3230 100644 --- a/src/video_core/engines/maxwell_3d.h +++ b/src/video_core/engines/maxwell_3d.h @@ -3069,6 +3069,8 @@ public: Maxwell3D::Regs::IndexBuffer index_buffer; u32 base_instance{}; u32 instance_count{}; + /// Deferred instances already drawn by flushes in the middle of the current loop. + u32 drawn_instances{}; std::vector inline_index_draw_indexes; }; struct DrawTextureState { @@ -3095,7 +3097,8 @@ public: }; void ProcessMethodCall(Maxwell3D& maxwell3d, u32 method, u32 argument); void Clear(Maxwell3D& maxwell3d, u32 layer_count); - void DrawDeferred(Maxwell3D& maxwell3d); + /// Draws the waiting instances. loop_continues is set when the loop goes on afterwards. + void DrawDeferred(Maxwell3D& maxwell3d, bool loop_continues = false); /// Instances after the first wait to be drawn together (DrawDeferred). [[nodiscard]] bool HasDeferredDraw() const noexcept { return draw_state.draw_mode == DrawMode::Instance && draw_state.instance_count != 0; -- cgit v1.2.3 From 46d1554186cdfd2255cc3745e3e2cfe9130c5a84 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:43:07 -0500 Subject: vulkan: back a sparse tiled image whole when its tiles can't be set up In the aliased mode (AMD) a SparseTiles image is created unbound, and only InitSparseTiles binds every block to the shared zeroed one. When it fails (an unusual block shape, no memory for the zeroed block or the mip tail, a failed bind), the constructor went on to the next branch and flagged the image UnwrittenSparse with nothing bound. AMD's drivers can't sample such an image, and it is sampled before its first write replaces it, which is the hang a855e8373f avoided for large sparse images. Create the image again as an ordinary backed image in that case, as before sparse tiles, and drop the Unbacked flag. The other modes keep falling back to UnwrittenSparse, since their drivers sample unbound memory. Follow-up to 130c255ed9 (with a855e8373f and 1c794af564). Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- .../renderer_vulkan/vk_texture_cache.cpp | 45 +++++++++++++++------- 1 file changed, 32 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/video_core/renderer_vulkan/vk_texture_cache.cpp b/src/video_core/renderer_vulkan/vk_texture_cache.cpp index 30981fe413..928146027d 100644 --- a/src/video_core/renderer_vulkan/vk_texture_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_texture_cache.cpp @@ -2350,23 +2350,31 @@ void TextureCacheRuntime::TickFrame() { }); } +namespace { + +// The host image of a texture; unbacked images are created without memory (see MakeImage). +[[nodiscard]] vk::Image MakeHostImage(TextureCacheRuntime& runtime, const ImageInfo& info, + u32 host_level_offset, GPUVAddr gpu_addr, bool backed) { + const Device& device = runtime.device; + const bool sparse_tiles = !backed && UsesSparseTiles(device, info, gpu_addr); + return MakeImage( + device, runtime.memory_allocator, TrimmedHostInfo(device, info, host_level_offset), + IsHugeAstcArray(device, info) ? std::span{HugeAstcViewFormats} + : IsOpaqueBc1Astc(device, info) ? std::span{OpaqueAstcViewFormats} + : runtime.ViewFormats(info.format), + AstcHostFormat(device, info), + !backed && (UsesSparseLayers(device, info, gpu_addr) || sparse_tiles || + IsUnwrittenSparse(device, info, gpu_addr)), + sparse_tiles && SparseTilesModeOf(device) == SparseTilesMode::Aliased); +} + +} // Anonymous namespace + Image::Image(TextureCacheRuntime& runtime_, const ImageInfo& info_, GPUVAddr gpu_addr_, VAddr cpu_addr_) : VideoCommon::ImageBase(info_, gpu_addr_, cpu_addr_), scheduler{&runtime_.scheduler}, runtime{&runtime_}, - original_image(MakeImage(runtime_.device, runtime_.memory_allocator, - TrimmedHostInfo(runtime_.device, info, host_level_offset), - IsHugeAstcArray(runtime_.device, info) - ? std::span{HugeAstcViewFormats} - : IsOpaqueBc1Astc(runtime_.device, info) - ? std::span{OpaqueAstcViewFormats} - : runtime->ViewFormats(info.format), - AstcHostFormat(runtime_.device, info), - UsesSparseLayers(runtime_.device, info, gpu_addr_) || - UsesSparseTiles(runtime_.device, info, gpu_addr_) || - IsUnwrittenSparse(runtime_.device, info, gpu_addr_), - UsesSparseTiles(runtime_.device, info, gpu_addr_) && - SparseTilesModeOf(runtime_.device) == SparseTilesMode::Aliased)), + original_image(MakeHostImage(runtime_, info, host_level_offset, gpu_addr_, false)), aspect_mask(ImageAspectMask(info.format)) { if (original_image.IsUnbacked()) { flags |= VideoCommon::ImageFlagBits::Unbacked; @@ -2377,6 +2385,17 @@ Image::Image(TextureCacheRuntime& runtime_, const ImageInfo& info_, GPUVAddr gpu flags |= VideoCommon::ImageFlagBits::SparseLayers; } else if (UsesSparseTiles(runtime_.device, info, gpu_addr_) && InitSparseTiles()) { flags |= VideoCommon::ImageFlagBits::SparseTiles; + } else if (UsesSparseTiles(runtime_.device, info, gpu_addr_) && + SparseTilesModeOf(runtime_.device) == SparseTilesMode::Aliased) { + // The driver can't sample unbound memory (that is why the blocks were to share one), + // so an image whose tiles could not be set up is backed whole like any other. + LOG_WARNING(Render_Vulkan, "Sparse tiled image {:#x}: backing it whole", gpu_addr_); + original_image = MakeHostImage(runtime_, info, host_level_offset, gpu_addr_, true); + if (!original_image.IsUnbacked()) { + flags &= ~VideoCommon::ImageFlagBits::Unbacked; + } else if (texels <= (u64{1} << 28)) { + flags |= VideoCommon::ImageFlagBits::UnwrittenSparse; + } } else if (texels <= (u64{1} << 28)) { // Not too big to back (see MemoryAllocator::CreateImage): unbacked because nothing // was written there yet, so the first write gets it replaced. -- cgit v1.2.3 From b45f4c6d2c79cbb60eeda4d3ad887f9964a297e3 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:44:14 -0500 Subject: vk_pipeline_cache: keep the pipelines on MoltenVK and preload eight sessions 5de94bdd1d cut the boot preload to the last two sessions and 2f94fb9680 evicts pipelines unused for 9000 frames, both because rebuilding one costs 2-3 ms through the driver's pipeline cache and each holds ~0.45 MB of driver memory. On MoltenVK a pipeline built while playing is translated from SPIR-V to MSL and compiled by Metal, which takes far longer even with a VkPipelineCache and shows as a stutter. On MoltenVK, keep the previous eight sessions' pipelines in vulkan_used.bin (be76402ab5's window) so they are built at boot, and do not evict. The memory is bounded by what a title builds; the stutter is not. Other drivers are unchanged and EDEN_NX2_PRELOAD_ALL_PIPELINES=1 still builds every cached pipeline. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- .../renderer_vulkan/vk_pipeline_cache.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp index ff8e84b019..4df940bf71 100644 --- a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp @@ -600,9 +600,17 @@ constexpr u32 UsedPipelinesMagic = 0x31505545; // "EUP1" // Pipelines the last two sessions used are built at boot. Each holds ~0.45 MB of driver memory: // title 1 built 4,800 from eight sessions, 1.5 GB more than building them on first use (but that // stalled frames 4x as often), while one session of its title screen uses ~1,500. +// MoltenVK keeps the previous eight sessions: a pipeline built while playing goes through +// SPIR-V -> MSL -> Metal, far more than the 2-3 ms the shorter window assumes, so each one +// built at boot is a stutter avoided. constexpr u8 UsedPipelinesMaxAge = 2; +constexpr u8 UsedPipelinesMaxAgeMoltenVK = 8; -std::unordered_map LoadUsedPipelines(const std::filesystem::path& path) { +u8 MaxUsedPipelineAge(const Device& device) { + return device.IsMoltenVK() ? UsedPipelinesMaxAgeMoltenVK : UsedPipelinesMaxAge; +} + +std::unordered_map LoadUsedPipelines(const std::filesystem::path& path, u8 max_age) { std::unordered_map used; std::ifstream file{path, std::ios::binary}; u32 magic{}, count{}; @@ -618,7 +626,7 @@ std::unordered_map LoadUsedPipelines(const std::filesystem::path& path) !file.read(reinterpret_cast(&age), sizeof(age))) { break; } - if (age < UsedPipelinesMaxAge) { // lists saved with a longer window + if (age < max_age) { // lists saved with a longer window used.emplace(hash, age); } } @@ -669,7 +677,7 @@ void PipelineCache::SaveUsedPipelines() { next.emplace(hash, u8{0}); } for (const auto& [hash, age] : used_pipelines) { - if (age + 1 < UsedPipelinesMaxAge) { + if (age + 1 < MaxUsedPipelineAge(device)) { next.try_emplace(hash, static_cast(age + 1)); } } @@ -744,7 +752,7 @@ void PipelineCache::LoadDiskResources(u64 title_id, std::stop_token stop_loading } pipeline_cache_filename = base_dir / "vulkan.bin"; used_pipelines_filename = base_dir / "vulkan_used.bin"; - used_pipelines = LoadUsedPipelines(used_pipelines_filename); + used_pipelines = LoadUsedPipelines(used_pipelines_filename, MaxUsedPipelineAge(device)); used_pipelines_saved = std::chrono::steady_clock::now(); size_t skipped_pipelines = 0; size_t queued_graphics = 0; @@ -1004,6 +1012,11 @@ void PipelineCache::TickFrame() { } void PipelineCache::EvictUnusedPipelines() { + // Building a pipeline again on MoltenVK means a Metal compile mid-game, far dearer than the + // 2-3 ms below, so none are evicted there. + if (device.IsMoltenVK()) { + return; + } // Each built pipeline holds ~0.45 MB of driver memory and none was ever destroyed, so it grew // with every place a session went (title 1: 1,500 on its title screen alone). One unused for // this long is destroyed; it takes 2-3 ms to build again from the driver's pipeline cache. -- cgit v1.2.3 From cd9b19c69d7eedc2d2d3557c27a0a06c4420213d Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:56:47 -0500 Subject: vulkan: repack 69-byte vertices without dynamic vertex input on MoltenVK e38b738b84 repacks the 69-byte vertex stride to 72 only with dynamic vertex input, unless EGLANTINE_EXPERIMENTAL_STATIC_VERTEX_REPACK=1. MoltenVK has no VK_EXT_vertex_input_dynamic_state and Metal needs strides that are a multiple of 4, so the stride was never fixed on Macs. Enable the static path automatically when the device's minimum vertex binding stride alignment does not divide 69. The device reports 4 for MoltenVK, matching its portability subset limit. The environment variable still overrides it: 1 forces the repack on, 0 forces it off. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/tests/video_core/vertex_repack.cpp | 7 ++++++- .../renderer_vulkan/fixed_pipeline_state.cpp | 2 +- .../renderer_vulkan/fixed_pipeline_state.h | 1 + src/video_core/renderer_vulkan/vertex_repack.h | 21 +++++++++++++++------ src/video_core/renderer_vulkan/vk_buffer_cache.cpp | 9 ++++++--- src/video_core/renderer_vulkan/vk_buffer_cache.h | 1 + .../renderer_vulkan/vk_pipeline_cache.cpp | 3 +++ src/video_core/vulkan_common/vulkan_device.h | 6 ++++++ 8 files changed, 39 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/tests/video_core/vertex_repack.cpp b/src/tests/video_core/vertex_repack.cpp index 3705a97926..5e21e3714c 100644 --- a/src/tests/video_core/vertex_repack.cpp +++ b/src/tests/video_core/vertex_repack.cpp @@ -76,10 +76,15 @@ TEST_CASE("Vertex repacking requires the captured tail and stride", "[vertex_rep REQUIRE(Vulkan::VertexRepackMask(regs) == 0); } -TEST_CASE("Static vertex repacking requires explicit experimental opt-in", "[vertex_repack]") { +TEST_CASE("Static vertex repacking follows the device stride alignment", "[vertex_repack]") { REQUIRE(Vulkan::VertexRepackInputSupported(true, false)); REQUIRE_FALSE(Vulkan::VertexRepackInputSupported(false, false)); REQUIRE(Vulkan::VertexRepackInputSupported(false, true)); + REQUIRE_FALSE(Vulkan::StaticVertexRepackSelected(nullptr, 1)); + REQUIRE(Vulkan::StaticVertexRepackSelected(nullptr, 4)); + REQUIRE(Vulkan::StaticVertexRepackSelected("1", 1)); + REQUIRE_FALSE(Vulkan::StaticVertexRepackSelected("0", 4)); + REQUIRE(Vulkan::StaticVertexRepackSelected("other", 4)); } TEST_CASE("Vertex repack planning keeps descriptor offsets aligned", "[vertex_repack]") { diff --git a/src/video_core/renderer_vulkan/fixed_pipeline_state.cpp b/src/video_core/renderer_vulkan/fixed_pipeline_state.cpp index d1f78c0034..76a564aae9 100644 --- a/src/video_core/renderer_vulkan/fixed_pipeline_state.cpp +++ b/src/video_core/renderer_vulkan/fixed_pipeline_state.cpp @@ -269,7 +269,7 @@ void FixedPipelineState::Refresh(Tegra::Engines::Maxwell3D& maxwell3d, DynamicFe if (xfb_enabled) { RefreshXfbState(xfb_state, regs); } - if (!dynamic_vertex_input && ExperimentalStaticVertexRepackEnabled()) { + if (!dynamic_vertex_input && features.has_static_vertex_repack) { const u32 repack_mask = VertexRepackMask(regs); // Refresh from guest values even when only the stride changed: offsets must not accumulate. for (size_t index = 0; index < attributes.size(); ++index) { diff --git a/src/video_core/renderer_vulkan/fixed_pipeline_state.h b/src/video_core/renderer_vulkan/fixed_pipeline_state.h index 36a4b4b7a2..1fe5cd75f3 100644 --- a/src/video_core/renderer_vulkan/fixed_pipeline_state.h +++ b/src/video_core/renderer_vulkan/fixed_pipeline_state.h @@ -33,6 +33,7 @@ struct DynamicFeatures { bool has_dynamic_state3_logic_op_enable; bool has_dynamic_state3_line_stipple_enable; bool has_dynamic_vertex_input; + bool has_static_vertex_repack; bool has_color_write_enable; bool has_provoking_vertex; bool has_provoking_vertex_first_mode; diff --git a/src/video_core/renderer_vulkan/vertex_repack.h b/src/video_core/renderer_vulkan/vertex_repack.h index 456d1c6f20..493eb41bd1 100644 --- a/src/video_core/renderer_vulkan/vertex_repack.h +++ b/src/video_core/renderer_vulkan/vertex_repack.h @@ -15,12 +15,21 @@ constexpr bool VertexRepackInputSupported(bool dynamic_input, bool experimental_ return dynamic_input || experimental_static; } -inline bool ExperimentalStaticVertexRepackEnabled() { - static const bool enabled = [] { - const char* value = std::getenv("EGLANTINE_EXPERIMENTAL_STATIC_VERTEX_REPACK"); - return value && std::strcmp(value, "1") == 0; - }(); - return enabled; +// Pipelines without dynamic vertex input repack when the device cannot fetch the 69-byte stride. +// EGLANTINE_EXPERIMENTAL_STATIC_VERTEX_REPACK=1 forces the repack on, =0 forces it off. +inline bool StaticVertexRepackSelected(const char* override_value, u32 stride_alignment) { + if (override_value && std::strcmp(override_value, "1") == 0) { + return true; + } + if (override_value && std::strcmp(override_value, "0") == 0) { + return false; + } + return stride_alignment > 1 && 69 % stride_alignment != 0; +} + +inline bool StaticVertexRepackEnabled(u32 stride_alignment) { + return StaticVertexRepackSelected(std::getenv("EGLANTINE_EXPERIMENTAL_STATIC_VERTEX_REPACK"), + stride_alignment); } struct VertexRepackParameters { diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp index 4d5ccb6520..43cb7d9414 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp @@ -365,6 +365,10 @@ BufferCacheRuntime::BufferCacheRuntime(const Device& device_, MemoryAllocator& m LOG_INFO(Render_Vulkan, "69-byte vertex layout repacking {} ({})", enable_vertex_repack ? "enabled" : "disabled", "portable Vulkan layout fallback"); + static_vertex_repack = + StaticVertexRepackEnabled(device.GetMinVertexInputBindingStrideAlignment()); + LOG_INFO(Render_Vulkan, "Vertex repacking without dynamic vertex input {}", + static_vertex_repack ? "enabled" : "disabled"); if (enable_vertex_repack) { vertex_repack_pass = std::make_unique( device, scheduler, descriptor_pool, staging_pool, compute_pass_descriptor_queue); @@ -387,8 +391,7 @@ BufferCacheRuntime::BufferCacheRuntime(const Device& device_, MemoryAllocator& m u32 BufferCacheRuntime::SetVertexRepackLayout(const Tegra::Engines::Maxwell3D::Regs& regs, bool dynamic_input) { const u32 old_mask = vertex_repack_mask; - const bool input_supported = VertexRepackInputSupported( - dynamic_input, ExperimentalStaticVertexRepackEnabled()); + const bool input_supported = VertexRepackInputSupported(dynamic_input, static_vertex_repack); vertex_repack_mask = vertex_repack_pass && input_supported ? VertexRepackMask(regs) : 0; if (vertex_repack_mask != old_mask) { LOG_DEBUG(Render_Vulkan, "Vertex repack selection: mask={:#x}, dynamic_input={}", @@ -714,7 +717,7 @@ void BufferCacheRuntime::BindVertexBuffers(VideoCommon::HostBindings& bi bindings.strides[i] = 72; static bool reported = false; if (!reported) { - LOG_WARNING(Render_Vulkan, "Experimental GPU vertex repacking active: 69 -> 72 bytes"); + LOG_INFO(Render_Vulkan, "GPU vertex repacking active: 69 -> 72 bytes"); reported = true; } } diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.h b/src/video_core/renderer_vulkan/vk_buffer_cache.h index a8d0274016..f65b6daf2c 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.h +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.h @@ -271,6 +271,7 @@ private: std::unique_ptr uint8_pass; std::unique_ptr vertex_repack_pass; u32 vertex_repack_mask{}; + bool static_vertex_repack{}; QuadIndexedPass quad_index_pass; MultiRangeBufferCache multi_range_buffers; diff --git a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp index 4df940bf71..a3cd93484e 100644 --- a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp @@ -34,6 +34,7 @@ #include "video_core/renderer_vulkan/maxwell_to_vk.h" #include "video_core/renderer_vulkan/pipeline_helper.h" #include "video_core/renderer_vulkan/pipeline_statistics.h" +#include "video_core/renderer_vulkan/vertex_repack.h" #include "video_core/renderer_vulkan/vk_compute_pipeline.h" #include "video_core/renderer_vulkan/vk_descriptor_pool.h" #include "video_core/renderer_vulkan/vk_pipeline_cache.h" @@ -583,6 +584,8 @@ PipelineCache::PipelineCache(Tegra::MaxwellDeviceMemoryManager& device_memory_, dynamic_features.has_dynamic_vertex_input = device.IsExtVertexInputDynamicStateSupported() && Settings::values.vertex_input_dynamic_state.GetValue(); + dynamic_features.has_static_vertex_repack = + StaticVertexRepackEnabled(device.GetMinVertexInputBindingStrideAlignment()); dynamic_features.has_provoking_vertex = device.IsExtProvokingVertexSupported(); dynamic_features.has_provoking_vertex_first_mode = diff --git a/src/video_core/vulkan_common/vulkan_device.h b/src/video_core/vulkan_common/vulkan_device.h index 622d792210..5f75bcc18e 100644 --- a/src/video_core/vulkan_common/vulkan_device.h +++ b/src/video_core/vulkan_common/vulkan_device.h @@ -1161,6 +1161,12 @@ FN_MAX_LIMIT_LIST return properties.driver.driverID == VK_DRIVER_ID_MOLTENVK; } + /// The multiple vertex input binding strides must be. Metal needs 4 bytes, which MoltenVK + /// reports as minVertexInputBindingStrideAlignment through VK_KHR_portability_subset. + u32 GetMinVertexInputBindingStrideAlignment() const noexcept { + return IsMoltenVK() ? 4 : 1; + } + NvidiaArchitecture GetNvidiaArch() const noexcept { return nvidia_arch; } -- cgit v1.2.3 From 9f4f01f76e8556941c0c0c3f57edc1d600ed23ea Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:18:13 -0500 Subject: nx2: map GPU memory before later command lists are read 837133792d queues the GMMU maps on the GPU thread (GPU::RunInOrder) so the service thread never waits for the texture cache. With the GPU split (Low GPU accuracy) the GPU thread defers queued functions to its back, but its front reads the command lists: it read the lists submitted after a map through GPU addresses that weren't mapped yet. Pokopia stopped after its first frame. Add GPU::MapInOrder for the GMMU maps and the sparse tile remaps. It runs in order like RunInOrder, and with the split it runs on the front after draining the back, before the front reads further command lists. The service thread still never takes the texture cache lock, and the back is idle while the map changes the caches. Unmaps and the other queued work still run on the back. Assisted-by: Claude Opus 5.5 (bisect, fix, testing and commit message) --- src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp | 6 +++--- src/video_core/gpu.cpp | 8 ++++++++ src/video_core/gpu.h | 4 ++++ src/video_core/gpu_thread.cpp | 11 ++++++++--- src/video_core/gpu_thread.h | 5 ++++- 5 files changed, 27 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp b/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp index 843d31caf1..c5b23bef3e 100644 --- a/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp +++ b/src/core/hle/service/nvdrv/devices/nvhost_as_gpu.cpp @@ -176,7 +176,7 @@ void nvhost_as_gpu::Nx2Map(u64 gpu_address, u64 cpu_address, u64 size) { Nx2GpuMemoryRegistry::Get().RecordDeviceRange(cpu_address, size, device_address); // On the GPU thread, like unmaps: a GMMU map takes the texture cache lock, and the callers hold // nx2:gpu locks that the GPU thread takes under that lock (sparse blocks backed on write). - system.GPU().RunInOrder([gmmu = gmmu.get(), gpu_address, device_address, size] { + system.GPU().MapInOrder([gmmu = gmmu.get(), gpu_address, device_address, size] { gmmu->Map(gpu_address, device_address, size, Tegra::PTEKind::PITCH, false); }); LOG_DEBUG(Service_NVDRV, "Switch 2 GPU mapping {:#x}+{:#x} -> CPU {:#x}", gpu_address, size, @@ -253,7 +253,7 @@ void nvhost_as_gpu::Nx2Sparse(u64 gpu_address, u64 size, bool create) { } nx2_sparse_ranges[gpu_address] = {size, device_address}; // On the GPU thread, as in Nx2Map. - system.GPU().RunInOrder([gmmu = gmmu.get(), gpu_address, device_address, size] { + system.GPU().MapInOrder([gmmu = gmmu.get(), gpu_address, device_address, size] { gmmu->Map(gpu_address, device_address, size, Tegra::PTEKind::PITCH, false); }); LOG_DEBUG(Service_NVDRV, "Switch 2 sparse region {:#x}+{:#x} -> device {:#x}", gpu_address, @@ -295,7 +295,7 @@ void nvhost_as_gpu::Nx2SparseTile(u64 gpu_address, u64 cpu_address, u64 size) { // whose contents are now what the game's memory holds there. const DAddr device = device_address + (gpu_address - start); auto& smmu = system.Host1x().MemoryManager(); - system.GPU().RunInOrder([&smmu, device, cpu_address, size, asid = session->asid] { + system.GPU().MapInOrder([&smmu, device, cpu_address, size, asid = session->asid] { smmu.Unmap(device, size); if (cpu_address != 0) { smmu.Map(device, cpu_address, size, asid, true); diff --git a/src/video_core/gpu.cpp b/src/video_core/gpu.cpp index a6f31359a3..444dc6d015 100644 --- a/src/video_core/gpu.cpp +++ b/src/video_core/gpu.cpp @@ -210,6 +210,10 @@ struct GPU::Impl { gpu_thread.Run(std::move(func), is_async); } + void MapInOrder(std::function&& func) { + gpu_thread.Run(std::move(func), is_async, true); + } + /// Notify rasterizer that any caches of the specified region should be flushed to Switch memory void FlushRegion(DAddr addr, u64 size) { gpu_thread.FlushRegion(addr, size, is_async); @@ -553,6 +557,10 @@ void GPU::RunInOrder(std::function&& func) { impl->RunInOrder(std::move(func)); } +void GPU::MapInOrder(std::function&& func) { + impl->MapInOrder(std::move(func)); +} + void GPU::InvalidateRegion(DAddr addr, u64 size, bool preserve_gpu_writes) { impl->InvalidateRegion(addr, size, preserve_gpu_writes); } diff --git a/src/video_core/gpu.h b/src/video_core/gpu.h index 4afc89f0e6..d37dfa5990 100644 --- a/src/video_core/gpu.h +++ b/src/video_core/gpu.h @@ -257,6 +257,10 @@ public: /// Runs func on the GPU thread after the command lists submitted so far. void RunInOrder(std::function&& func); + /// RunInOrder for GPU memory maps: also done before the GPU thread reads the command lists + /// submitted later (with the GPU split, on its front rather than its back). + void MapInOrder(std::function&& func); + /// Notify rasterizer that any caches of the specified region should be invalidated void InvalidateRegion(DAddr addr, u64 size, bool preserve_gpu_writes = false); diff --git a/src/video_core/gpu_thread.cpp b/src/video_core/gpu_thread.cpp index 55200b0647..3af8cb65a8 100644 --- a/src/video_core/gpu_thread.cpp +++ b/src/video_core/gpu_thread.cpp @@ -99,7 +99,12 @@ void ThreadManager::StartThread(VideoCore::RendererBase& renderer, Core::Fronten } else if (std::holds_alternative(next.data)) { split.Defer([gpu = &system.GPU()] { gpu->TickWork(); }); } else if (auto* run = std::get_if(&next.data)) { - split.DeferFunction(std::move(run->func)); + if (run->before_later_reads) { + split.Drain(); + run->func(); + } else { + split.DeferFunction(std::move(run->func)); + } } else if (const auto* flush = std::get_if(&next.data)) { split.Defer([rasterizer_ptr, addr = flush->addr, size = flush->size] { rasterizer_ptr->FlushRegion(addr, size); @@ -155,8 +160,8 @@ void ThreadManager::TickGPU(bool is_async) { PushCommand(GPUTickCommand(), false, is_async); } -void ThreadManager::Run(std::function&& func, bool is_async) { - PushCommand(RunCommand{std::move(func)}, false, is_async); +void ThreadManager::Run(std::function&& func, bool is_async, bool before_later_reads) { + PushCommand(RunCommand{std::move(func), before_later_reads}, false, is_async); } void ThreadManager::InvalidateRegion(DAddr addr, u64 size) { diff --git a/src/video_core/gpu_thread.h b/src/video_core/gpu_thread.h index 796c0d89f6..927453dab0 100644 --- a/src/video_core/gpu_thread.h +++ b/src/video_core/gpu_thread.h @@ -80,6 +80,9 @@ struct GPUTickCommand final {}; /// Command to run a function on the GPU thread after the commands queued before it struct RunCommand final { std::function func; + /// With the GPU split, run on its front (after draining the back) instead of deferring to the + /// back, so command lists read after this one see what func did (GMMU maps). + bool before_later_reads{}; }; using CommandData = @@ -132,7 +135,7 @@ public: void TickGPU(bool is_async); /// Runs func on the GPU thread once the commands queued before it have been processed. - void Run(std::function&& func, bool is_async); + void Run(std::function&& func, bool is_async, bool before_later_reads = false); void NotifyShutdown(); -- cgit v1.2.3