From 93adb2bb95b34c0e8fde640a3794926a05a428e6 Mon Sep 17 00:00:00 2001 From: "Herman S." <429230+has207@users.noreply.github.com> Date: Thu, 6 Nov 2025 09:42:28 +0900 Subject: [PATCH 1/5] [Config] Gracefully handle type mismatches in cvars. Detect cvar type mismatch and reset to default value, as well as show message to the user that the cvar from the config has been updated. --- src/xenia/base/cvar.cc | 1 + src/xenia/base/cvar.h | 25 +++++++++++++++++++++++-- src/xenia/config.cc | 21 +++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/xenia/base/cvar.cc b/src/xenia/base/cvar.cc index ad092a78b..3289df94f 100644 --- a/src/xenia/base/cvar.cc +++ b/src/xenia/base/cvar.cc @@ -30,6 +30,7 @@ cxxopts::Options options("xenia", "Xbox 360 Emulator"); std::map* CmdVars; std::map* ConfigVars; std::multimap* IConfigVarUpdate::updates_; +std::vector* config_type_mismatch_warnings = nullptr; void PrintHelpAndExit() { std::cout << options.help({""}) << std::endl; diff --git a/src/xenia/base/cvar.h b/src/xenia/base/cvar.h index 6c3357836..6b139bda7 100644 --- a/src/xenia/base/cvar.h +++ b/src/xenia/base/cvar.h @@ -33,6 +33,9 @@ namespace toml_internal { std::string EscapeString(const std::string_view str); } +// Track config values that had type mismatches during loading +extern std::vector* config_type_mismatch_warnings; + class ICommandVar { public: virtual ~ICommandVar() = default; @@ -147,7 +150,16 @@ inline void CommandVar::LoadFromLaunchOptions( } template void ConfigVar::LoadConfigValue(const toml::node* result) { - SetConfigValue(result->value().value()); + auto value_opt = result->value(); + if (value_opt) { + SetConfigValue(value_opt.value()); + } else { + // Type mismatch - track for warning + if (!config_type_mismatch_warnings) { + config_type_mismatch_warnings = new std::vector(); + } + config_type_mismatch_warnings->push_back(this->name_); + } } template <> inline void ConfigVar::LoadConfigValue( @@ -157,7 +169,16 @@ inline void ConfigVar::LoadConfigValue( } template void ConfigVar::LoadGameConfigValue(const toml::node* result) { - SetGameConfigValue(result->value().value()); + auto value_opt = result->value(); + if (value_opt) { + SetGameConfigValue(value_opt.value()); + } else { + // Type mismatch - track for warning + if (!config_type_mismatch_warnings) { + config_type_mismatch_warnings = new std::vector(); + } + config_type_mismatch_warnings->push_back(this->name_); + } } template <> inline void ConfigVar::LoadGameConfigValue( diff --git a/src/xenia/config.cc b/src/xenia/config.cc index a0b32e652..e64b8922c 100644 --- a/src/xenia/config.cc +++ b/src/xenia/config.cc @@ -16,6 +16,7 @@ #include "xenia/base/logging.h" #include "xenia/base/string.h" #include "xenia/base/string_buffer.h" +#include "xenia/base/system.h" toml::parse_result ParseFile(const std::filesystem::path& filename) { return toml::parse_file(xe::path_to_utf8(filename)); @@ -118,6 +119,26 @@ void ReadConfig(const std::filesystem::path& file_path, cvar::IConfigVarUpdate::ApplyUpdates(config_defaults_date); } + // Check for type mismatch warnings + if (cvar::config_type_mismatch_warnings && + !cvar::config_type_mismatch_warnings->empty()) { + std::string warning_message = + "The following config values had invalid types and have been reset to " + "defaults:\n\n"; + for (const auto& name : *cvar::config_type_mismatch_warnings) { + warning_message += " - " + name + "\n"; + } + warning_message += + "\nPlease check your config file. The config will be saved with the " + "correct types."; + + xe::ShowSimpleMessageBox(xe::SimpleMessageBoxType::Warning, + warning_message); + + // Clear warnings + cvar::config_type_mismatch_warnings->clear(); + } + XELOGI("Loaded config: {}", file_path); } From e2c33686ccf6137fa40d215e3235f797ce3db6b8 Mon Sep 17 00:00:00 2001 From: "Herman S." <429230+has207@users.noreply.github.com> Date: Fri, 31 Oct 2025 21:20:37 +0900 Subject: [PATCH 2/5] [GPU] Double-buffer readback_resolve. Introduce "fast" readback resolve that reads from previous frame's resolve buffers, quick swap between buffers each frame to avoid copies so minimal performance impact and mostly correct bahavior. Checks if previous frame had a buffer for the current resolve and falls through to the slow path, which also allows to support "screenshot" features in the games that do that without stalling on normal resolve operations. Re-enabled readback_memexport as separate feature, was previously bundled with readback_resolve (probably not intentionally) and ensures destination address is writeable to avoid memory access related crashes and unnecessary work. readback_resolve cvar changes from bool to string ternary with "none", "fast" and "full" options, defaulting to the new "fast" mode. --- src/xenia/app/emulator_window.cc | 25 +- src/xenia/app/emulator_window.h | 1 + src/xenia/gpu/command_processor.cc | 32 +- src/xenia/gpu/command_processor.h | 33 +- .../gpu/d3d12/d3d12_command_processor.cc | 187 ++++++++--- src/xenia/gpu/d3d12/d3d12_command_processor.h | 15 +- .../gpu/vulkan/vulkan_command_processor.cc | 294 ++++++++++++++---- .../gpu/vulkan/vulkan_command_processor.h | 19 +- 8 files changed, 482 insertions(+), 124 deletions(-) diff --git a/src/xenia/app/emulator_window.cc b/src/xenia/app/emulator_window.cc index dc00c9d5e..f9548ba5c 100644 --- a/src/xenia/app/emulator_window.cc +++ b/src/xenia/app/emulator_window.cc @@ -50,7 +50,7 @@ DECLARE_bool(guide_button); DECLARE_bool(clear_memory_page_state); -DECLARE_bool(readback_resolve); +DECLARE_string(readback_resolve); DECLARE_bool(readback_memexport); @@ -1832,10 +1832,10 @@ EmulatorWindow::ControllerHotKey EmulatorWindow::ProcessControllerHotkey( xe::threading::Sleep(delay); break; case ButtonFunctions::ReadbackResolve: - ToggleGPUSetting(GPUSetting::ReadbackResolve); + CycleReadbackResolve(); - notificationTitle = "Toggle Readback Resolve"; - notificationDesc = cvars::readback_resolve ? "Enabled" : "Disabled"; + notificationTitle = "Readback Resolve Mode"; + notificationDesc = cvars::readback_resolve; // Extra Sleep xe::threading::Sleep(delay); @@ -2019,15 +2019,23 @@ void EmulatorWindow::ToggleGPUSetting(gpu::GPUSetting setting) { SaveGPUSetting(GPUSetting::ClearMemoryPageState, !cvars::clear_memory_page_state); break; - case GPUSetting::ReadbackResolve: - SaveGPUSetting(GPUSetting::ReadbackResolve, !cvars::readback_resolve); - break; case GPUSetting::ReadbackMemexport: SaveGPUSetting(GPUSetting::ReadbackMemexport, !cvars::readback_memexport); break; } } +void EmulatorWindow::CycleReadbackResolve() { + const std::string& current = cvars::readback_resolve; + if (current == "fast") { + gpu::SetReadbackResolveMode("full"); + } else if (current == "full") { + gpu::SetReadbackResolveMode("none"); + } else { + gpu::SetReadbackResolveMode("fast"); + } +} + void EmulatorWindow::DisplayHotKeysConfig() { std::string msg = ""; std::string msg_passthru = ""; @@ -2066,8 +2074,7 @@ void EmulatorWindow::DisplayHotKeysConfig() { msg.insert(0, msg_passthru); msg += "\n"; - msg += "Readback Resolve: " + - xe::string_util::BoolToString(cvars::readback_resolve); + msg += "Readback Resolve: " + cvars::readback_resolve; msg += "\n"; msg += "Clear Memory Page State: " + diff --git a/src/xenia/app/emulator_window.h b/src/xenia/app/emulator_window.h index de0958902..e748b91a7 100644 --- a/src/xenia/app/emulator_window.h +++ b/src/xenia/app/emulator_window.h @@ -280,6 +280,7 @@ class EmulatorWindow { bool vibrate = true); void GamepadHotKeys(); void ToggleGPUSetting(gpu::GPUSetting setting); + void CycleReadbackResolve(); void DisplayHotKeysConfig(); static std::string CanonicalizeFileExtension( diff --git a/src/xenia/gpu/command_processor.cc b/src/xenia/gpu/command_processor.cc index 64d556f7a..6e31cca24 100644 --- a/src/xenia/gpu/command_processor.cc +++ b/src/xenia/gpu/command_processor.cc @@ -48,11 +48,12 @@ DEFINE_bool(clear_memory_page_state, false, "for 'Team Ninja' Games to fix missing character models)", "GPU"); -DEFINE_bool( - readback_resolve, false, - "Read render-to-texture results on the CPU. This may be " - "needed in some games, for instance, for screenshots in saved games, but " - "causes mid-frame synchronization, so it has a huge performance impact.", +DEFINE_string( + readback_resolve, "fast", + "Controls CPU readback of render-to-texture resolve results.\n" + " fast: Read from previous frame (1 frame delay, no GPU stall - default)\n" + " full: Wait for GPU to finish (accurate but slow, GPU-CPU sync stall)\n" + " none: Disable readback completely (some games render better without it)", "GPU"); DEFINE_bool( @@ -73,9 +74,6 @@ void SaveGPUSetting(GPUSetting setting, uint64_t value) { case GPUSetting::ClearMemoryPageState: OVERRIDE_bool(clear_memory_page_state, static_cast(value)); break; - case GPUSetting::ReadbackResolve: - OVERRIDE_bool(readback_resolve, static_cast(value)); - break; case GPUSetting::ReadbackMemexport: OVERRIDE_bool(readback_memexport, static_cast(value)); break; @@ -86,14 +84,28 @@ bool GetGPUSetting(GPUSetting setting) { switch (setting) { case GPUSetting::ClearMemoryPageState: return cvars::clear_memory_page_state; - case GPUSetting::ReadbackResolve: - return cvars::readback_resolve; case GPUSetting::ReadbackMemexport: return cvars::readback_memexport; } return false; } +ReadbackResolveMode GetReadbackResolveMode() { + const std::string& mode = cvars::readback_resolve; + if (mode == "full") { + return ReadbackResolveMode::kFull; + } else if (mode == "none") { + return ReadbackResolveMode::kDisabled; + } else { + // Default to "fast" for any unrecognized value + return ReadbackResolveMode::kFast; + } +} + +void SetReadbackResolveMode(const std::string& mode) { + OVERRIDE_string(readback_resolve, mode); +} + using namespace xe::gpu::xenos; CommandProcessor::CommandProcessor(GraphicsSystem* graphics_system, diff --git a/src/xenia/gpu/command_processor.h b/src/xenia/gpu/command_processor.h index b8f7d4d56..f2bf000d4 100644 --- a/src/xenia/gpu/command_processor.h +++ b/src/xenia/gpu/command_processor.h @@ -33,14 +33,18 @@ class ByteStream; namespace gpu { -enum class GPUSetting { - ClearMemoryPageState, - ReadbackResolve, - ReadbackMemexport +enum class GPUSetting { ClearMemoryPageState, ReadbackMemexport }; + +enum class ReadbackResolveMode { + kDisabled, // No readback (none) + kFast, // Delayed sync, 1 frame behind (fast) + kFull // Immediate sync with GPU stall (full) }; void SaveGPUSetting(GPUSetting setting, uint64_t value); bool GetGPUSetting(GPUSetting setting); +ReadbackResolveMode GetReadbackResolveMode(); +void SetReadbackResolveMode(const std::string& mode); class GraphicsSystem; class Shader; @@ -162,6 +166,27 @@ class CommandProcessor { static constexpr uint32_t kReadbackBufferSizeIncrement = 16 * 1024 * 1024; + // Eviction policy constants for readback buffer cache + static constexpr size_t kMaxReadbackBuffers = 64; + static constexpr uint64_t kReadbackBufferEvictionAgeFrames = 60; + + // Progressive alignment for readback buffers to avoid wasting memory + static inline uint32_t AlignReadbackBufferSize(uint32_t size) { + if (size < 1 * 1024 * 1024) { + return xe::align(size, 256u * 1024u); // 256KB for < 1MB + } else if (size < 4 * 1024 * 1024) { + return xe::align(size, 1u * 1024u * 1024u); // 1MB for < 4MB + } else { + return xe::align(size, kReadbackBufferSizeIncrement); // 16MB for >= 4MB + } + } + + // Generate a cache key for a specific resolve operation + static inline uint64_t MakeReadbackResolveKey(uint32_t address, + uint32_t length) { + return (uint64_t(address) << 32) | uint64_t(length); + } + void WorkerThreadMain(); virtual bool SetupContext() = 0; virtual void ShutdownContext() = 0; diff --git a/src/xenia/gpu/d3d12/d3d12_command_processor.cc b/src/xenia/gpu/d3d12/d3d12_command_processor.cc index 0d917fde8..b6fed8a12 100644 --- a/src/xenia/gpu/d3d12/d3d12_command_processor.cc +++ b/src/xenia/gpu/d3d12/d3d12_command_processor.cc @@ -1613,8 +1613,14 @@ bool D3D12CommandProcessor::SetupContext() { void D3D12CommandProcessor::ShutdownContext() { AwaitAllQueueOperationsCompletion(); - ui::d3d12::util::ReleaseAndNull(readback_buffer_); - readback_buffer_size_ = 0; + for (auto& pair : readback_buffers_) { + ui::d3d12::util::ReleaseAndNull(pair.second.buffers[0]); + ui::d3d12::util::ReleaseAndNull(pair.second.buffers[1]); + } + readback_buffers_.clear(); + + ui::d3d12::util::ReleaseAndNull(memexport_readback_buffer_); + memexport_readback_buffer_size_ = 0; ui::d3d12::util::ReleaseAndNull(scratch_buffer_); scratch_buffer_size_ = 0; @@ -2987,7 +2993,7 @@ bool D3D12CommandProcessor::IssueDraw(xenos::PrimitiveType primitive_type, memexport_range.base_address_dwords << 2, memexport_range.size_bytes, false); } - if (GetGPUSetting(GPUSetting::ReadbackResolve)) { + if (GetGPUSetting(GPUSetting::ReadbackMemexport)) { // Read the exported data on the CPU. uint32_t memexport_total_size = 0; for (const draw_util::MemExportRange& memexport_range : @@ -3067,7 +3073,8 @@ bool D3D12CommandProcessor::IssueCopy() { if (!BeginSubmission(true)) { return false; } - if (!GetGPUSetting(GPUSetting::ReadbackResolve)) { + ReadbackResolveMode readback_mode = GetReadbackResolveMode(); + if (readback_mode == ReadbackResolveMode::kDisabled) { uint32_t written_address, written_length; return render_target_cache_->Resolve(*memory_, *shared_memory_, *texture_cache_, written_address, @@ -3075,7 +3082,6 @@ bool D3D12CommandProcessor::IssueCopy() { } else { return IssueCopy_ReadbackResolvePath(); } - return true; } XE_NOINLINE bool D3D12CommandProcessor::IssueCopy_ReadbackResolvePath() { @@ -3083,32 +3089,113 @@ bool D3D12CommandProcessor::IssueCopy_ReadbackResolvePath() { if (render_target_cache_->Resolve(*memory_, *shared_memory_, *texture_cache_, written_address, written_length)) { if (!texture_cache_->IsDrawResolutionScaled() && written_length) { - // Read the resolved data on the CPU. - ID3D12Resource* readback_buffer = RequestReadbackBuffer(written_length); - if (readback_buffer != nullptr) { - shared_memory_->UseAsCopySource(); - SubmitBarriers(); - ID3D12Resource* shared_memory_buffer = shared_memory_->GetBuffer(); - deferred_command_list_.D3DCopyBufferRegion( - readback_buffer, 0, shared_memory_buffer, written_address, - written_length); - if (AwaitAllQueueOperationsCompletion()) { - D3D12_RANGE readback_range; - readback_range.Begin = 0; - readback_range.End = written_length; - void* readback_mapping; - if (SUCCEEDED(readback_buffer->Map(0, &readback_range, - &readback_mapping))) { - // chrispy: this memcpy needs to be optimized as much as possible - - auto physaddr = memory_->TranslatePhysical(written_address); - memory::vastcpy(physaddr, (uint8_t*)readback_mapping, - written_length); - D3D12_RANGE readback_write_range = {}; - readback_buffer->Unmap(0, &readback_write_range); + // Early check: if destination memory is not accessible, skip all the + // expensive GPU readback work. + VirtualHeap* physical_heap = memory_->GetPhysicalHeap(); + bool memory_accessible = false; + if (physical_heap) { + HeapAllocationInfo alloc_info; + if (physical_heap->QueryRegionInfo(written_address, &alloc_info) && + (alloc_info.state & kMemoryAllocationCommit) && + (alloc_info.protect & kMemoryProtectWrite)) { + uint32_t end_address = written_address + written_length; + uint32_t region_end = + alloc_info.base_address + alloc_info.region_size; + if (end_address <= region_end) { + memory_accessible = true; } } } + + if (!memory_accessible) { + // Destination memory not accessible, skip readback entirely + return true; + } + + // Create a key for this specific resolve operation + uint64_t resolve_key = + MakeReadbackResolveKey(written_address, written_length); + ReadbackBuffer& rb = readback_buffers_[resolve_key]; + rb.last_used_frame = frame_current_; + + uint32_t write_index = rb.current_index; + uint32_t size = AlignReadbackBufferSize(written_length); + + // Allocate/resize write buffer if needed + if (size > rb.sizes[write_index]) { + const ui::d3d12::D3D12Provider& provider = GetD3D12Provider(); + ID3D12Device* device = provider.GetDevice(); + D3D12_RESOURCE_DESC buffer_desc; + ui::d3d12::util::FillBufferResourceDesc(buffer_desc, size, + D3D12_RESOURCE_FLAG_NONE); + ID3D12Resource* buffer; + if (SUCCEEDED(device->CreateCommittedResource( + &ui::d3d12::util::kHeapPropertiesReadback, + provider.GetHeapFlagCreateNotZeroed(), &buffer_desc, + D3D12_RESOURCE_STATE_COPY_DEST, nullptr, + IID_PPV_ARGS(&buffer)))) { + if (rb.buffers[write_index] != nullptr) { + rb.buffers[write_index]->Release(); + } + rb.buffers[write_index] = buffer; + rb.sizes[write_index] = size; + } else { + XELOGE("Failed to create a {} MB readback buffer", size >> 20); + return true; + } + } + + // Copy resolved data to current frame's buffer + shared_memory_->UseAsCopySource(); + SubmitBarriers(); + ID3D12Resource* shared_memory_buffer = shared_memory_->GetBuffer(); + deferred_command_list_.D3DCopyBufferRegion( + rb.buffers[write_index], 0, shared_memory_buffer, written_address, + written_length); + + ReadbackResolveMode readback_mode = GetReadbackResolveMode(); + bool use_delayed_sync = (readback_mode == ReadbackResolveMode::kFast); + uint32_t read_index = write_index; + + if (use_delayed_sync) { + // Use previous frame's data (avoid stall) + read_index = 1 - write_index; + } else { + // Wait for GPU to finish (accurate but slow) + if (!AwaitAllQueueOperationsCompletion()) { + return true; + } + } + + // Read from the appropriate buffer + ID3D12Resource* read_source = rb.buffers[read_index]; + + // If using delayed sync but previous buffer doesn't exist, use current + // buffer with sync as fallback + if (use_delayed_sync && + (read_source == nullptr || written_length > rb.sizes[read_index])) { + read_source = rb.buffers[write_index]; + read_index = write_index; + if (!AwaitAllQueueOperationsCompletion()) { + return true; + } + } + + if (read_source != nullptr && written_length <= rb.sizes[read_index]) { + D3D12_RANGE readback_range; + readback_range.Begin = 0; + readback_range.End = written_length; + void* readback_mapping; + if (SUCCEEDED( + read_source->Map(0, &readback_range, &readback_mapping))) { + // Memory accessibility already checked at the start of this function + // chrispy: this memcpy needs to be optimized as much as possible + auto physaddr = memory_->TranslatePhysical(written_address); + memory::vastcpy(physaddr, (uint8_t*)readback_mapping, written_length); + D3D12_RANGE readback_write_range = {}; + read_source->Unmap(0, &readback_write_range); + } + } } } else { return false; @@ -3298,6 +3385,34 @@ bool D3D12CommandProcessor::BeginSubmission(bool is_guest_command) { if (is_opening_frame) { frame_open_ = true; + // Swap all readback buffers for delayed sync (one frame behind) + for (auto& pair : readback_buffers_) { + pair.second.current_index = 1 - pair.second.current_index; + } + + // Evict old readback buffers only when map gets too large to prevent + // unbounded memory growth. Don't do this every frame as it's expensive. + if (readback_buffers_.size() > kMaxReadbackBuffers) { + for (auto it = readback_buffers_.begin(); + it != readback_buffers_.end();) { + // Evict if not used recently + if (frame_current_ > kReadbackBufferEvictionAgeFrames && + it->second.last_used_frame < + frame_current_ - kReadbackBufferEvictionAgeFrames) { + // Release both buffers + if (it->second.buffers[0] != nullptr) { + it->second.buffers[0]->Release(); + } + if (it->second.buffers[1] != nullptr) { + it->second.buffers[1]->Release(); + } + it = readback_buffers_.erase(it); + } else { + ++it; + } + } + } + // Reset bindings that depend on the data stored in the pools. std::memset(current_float_constant_map_vertex_, 0, sizeof(current_float_constant_map_vertex_)); @@ -5077,8 +5192,10 @@ ID3D12Resource* D3D12CommandProcessor::RequestReadbackBuffer(uint32_t size) { if (size == 0) { return nullptr; } - size = xe::align(size, kReadbackBufferSizeIncrement); - if (size > readback_buffer_size_) { + + size = AlignReadbackBufferSize(size); + + if (size > memexport_readback_buffer_size_) { const ui::d3d12::D3D12Provider& provider = GetD3D12Provider(); ID3D12Device* device = provider.GetDevice(); D3D12_RESOURCE_DESC buffer_desc; @@ -5092,13 +5209,13 @@ ID3D12Resource* D3D12CommandProcessor::RequestReadbackBuffer(uint32_t size) { XELOGE("Failed to create a {} MB readback buffer", size >> 20); return nullptr; } - if (readback_buffer_ != nullptr) { - readback_buffer_->Release(); + if (memexport_readback_buffer_ != nullptr) { + memexport_readback_buffer_->Release(); } - readback_buffer_ = buffer; - readback_buffer_size_ = size; + memexport_readback_buffer_ = buffer; + memexport_readback_buffer_size_ = size; } - return readback_buffer_; + return memexport_readback_buffer_; } void D3D12CommandProcessor::WriteGammaRampSRV( diff --git a/src/xenia/gpu/d3d12/d3d12_command_processor.h b/src/xenia/gpu/d3d12/d3d12_command_processor.h index d2c8e7054..ca015cc49 100644 --- a/src/xenia/gpu/d3d12/d3d12_command_processor.h +++ b/src/xenia/gpu/d3d12/d3d12_command_processor.h @@ -692,8 +692,19 @@ class D3D12CommandProcessor final : public CommandProcessor { D3D12_RESOURCE_STATES scratch_buffer_state_; bool scratch_buffer_used_ = false; - ID3D12Resource* readback_buffer_ = nullptr; - uint32_t readback_buffer_size_ = 0; + // Per-resolve double-buffered readback for delayed sync + struct ReadbackBuffer { + ID3D12Resource* buffers[2] = {nullptr, nullptr}; + uint32_t sizes[2] = {0, 0}; + uint32_t current_index = 0; + uint64_t last_used_frame = 0; + }; + // Map: (written_address << 32 | written_length) -> ReadbackBuffer + std::unordered_map readback_buffers_; + + // Simple single buffer for memexport (always syncs, no double-buffering) + ID3D12Resource* memexport_readback_buffer_ = nullptr; + uint32_t memexport_readback_buffer_size_ = 0; // The current fixed-function drawing state. D3D12_VIEWPORT ff_viewport_; diff --git a/src/xenia/gpu/vulkan/vulkan_command_processor.cc b/src/xenia/gpu/vulkan/vulkan_command_processor.cc index 0a262f065..91229391b 100644 --- a/src/xenia/gpu/vulkan/vulkan_command_processor.cc +++ b/src/xenia/gpu/vulkan/vulkan_command_processor.cc @@ -1087,12 +1087,24 @@ void VulkanCommandProcessor::ShutdownContext() { ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, gamma_ramp_buffer_memory_); - // Clean up readback buffer. + // Clean up all readback buffers. + for (auto& pair : readback_buffers_) { + ui::vulkan::util::DestroyAndNullHandle(dfn.vkDestroyBuffer, device, + pair.second.buffers[0]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, + pair.second.memories[0]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkDestroyBuffer, device, + pair.second.buffers[1]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, + pair.second.memories[1]); + } + readback_buffers_.clear(); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkDestroyBuffer, device, - readback_buffer_); + memexport_readback_buffer_); ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, - readback_buffer_memory_); - readback_buffer_size_ = 0; + memexport_readback_buffer_memory_); + memexport_readback_buffer_size_ = 0; ui::vulkan::util::DestroyAndNullHandle( dfn.vkDestroyDescriptorPool, device, @@ -2690,7 +2702,7 @@ bool VulkanCommandProcessor::IssueDraw(xenos::PrimitiveType prim_type, if (AwaitAllQueueOperationsCompletion()) { // Map staging buffer and copy to guest memory. void* mapped_data; - if (dfn.vkMapMemory(device, readback_buffer_memory_, 0, + if (dfn.vkMapMemory(device, memexport_readback_buffer_memory_, 0, memexport_total_size, 0, &mapped_data) == VK_SUCCESS) { if (mapped_data) { @@ -2708,7 +2720,7 @@ bool VulkanCommandProcessor::IssueDraw(xenos::PrimitiveType prim_type, "VulkanCommandProcessor: Failed to map readback buffer " "(mapped_data is null)"); } - dfn.vkUnmapMemory(device, readback_buffer_memory_); + dfn.vkUnmapMemory(device, memexport_readback_buffer_memory_); } else { XELOGE( "VulkanCommandProcessor: Failed to map readback buffer memory " @@ -2741,56 +2753,183 @@ bool VulkanCommandProcessor::IssueCopy() { return false; } - // CPU readback resolve path (if enabled). - if (GetGPUSetting(GPUSetting::ReadbackResolve) && + // CPU readback resolve path (if not disabled). + ReadbackResolveMode readback_mode = GetReadbackResolveMode(); + if (readback_mode != ReadbackResolveMode::kDisabled && !texture_cache_->IsDrawResolutionScaled() && written_length > 0) { - VkBuffer readback_buffer = RequestReadbackBuffer(written_length); - if (readback_buffer != VK_NULL_HANDLE) { - const ui::vulkan::VulkanDevice* const vulkan_device = GetVulkanDevice(); - const ui::vulkan::VulkanDevice::Functions& dfn = - vulkan_device->functions(); - const VkDevice device = vulkan_device->device(); - - VkBuffer shared_memory_buffer = shared_memory_->buffer(); - - // Ensure shared memory is ready for transfer. - shared_memory_->Use(VulkanSharedMemory::Usage::kRead); - - // Copy GPU buffer → staging buffer. - VkBufferCopy copy_region = {}; - copy_region.srcOffset = written_address; - copy_region.dstOffset = 0; - copy_region.size = written_length; - - deferred_command_buffer_.CmdVkCopyBuffer( - shared_memory_buffer, readback_buffer, 1, ©_region); - - // Wait for GPU to finish (SYNCHRONIZATION STALL - major performance - // hit!). - if (AwaitAllQueueOperationsCompletion()) { - // Map staging buffer and copy to guest memory. - void* mapped_data; - if (dfn.vkMapMemory(device, readback_buffer_memory_, 0, written_length, - 0, &mapped_data) == VK_SUCCESS) { - if (mapped_data) { - memory::vastcpy(memory_->TranslatePhysical(written_address), - static_cast(mapped_data), written_length); - } else { - XELOGE( - "VulkanCommandProcessor: Failed to map readback buffer " - "(mapped_data is null)"); - } - dfn.vkUnmapMemory(device, readback_buffer_memory_); - } else { - XELOGE( - "VulkanCommandProcessor: Failed to map readback buffer memory " - "for " - "resolve"); + // Early check: if destination memory is not accessible, skip all the + // expensive GPU readback work. + VirtualHeap* physical_heap = memory_->GetPhysicalHeap(); + bool memory_accessible = false; + if (physical_heap) { + HeapAllocationInfo alloc_info; + if (physical_heap->QueryRegionInfo(written_address, &alloc_info) && + (alloc_info.state & kMemoryAllocationCommit) && + (alloc_info.protect & kMemoryProtectWrite)) { + uint32_t end_address = written_address + written_length; + uint32_t region_end = alloc_info.base_address + alloc_info.region_size; + if (end_address <= region_end) { + memory_accessible = true; } - } else { + } + } + + if (!memory_accessible) { + // Destination memory not accessible, skip readback entirely + return true; + } + + // Create a key for this specific resolve operation + uint64_t resolve_key = + MakeReadbackResolveKey(written_address, written_length); + ReadbackBuffer& rb = readback_buffers_[resolve_key]; + rb.last_used_frame = frame_current_; + + const ui::vulkan::VulkanDevice* const vulkan_device = GetVulkanDevice(); + const ui::vulkan::VulkanDevice::Functions& dfn = vulkan_device->functions(); + const VkDevice device = vulkan_device->device(); + + uint32_t write_index = rb.current_index; + uint32_t size = AlignReadbackBufferSize(written_length); + + // Allocate/resize write buffer if needed + if (size > rb.sizes[write_index]) { + // Create buffer with TRANSFER_DST usage for copying from GPU. + VkBufferCreateInfo buffer_info = {}; + buffer_info.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; + buffer_info.size = size; + buffer_info.usage = VK_BUFFER_USAGE_TRANSFER_DST_BIT; + buffer_info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + + VkBuffer new_buffer; + if (dfn.vkCreateBuffer(device, &buffer_info, nullptr, &new_buffer) != + VK_SUCCESS) { + XELOGE( + "VulkanCommandProcessor: Failed to create readback buffer of {} MB", + size >> 20); + return true; + } + + // Get memory requirements. + VkMemoryRequirements memory_requirements; + dfn.vkGetBufferMemoryRequirements(device, new_buffer, + &memory_requirements); + + // Allocate HOST_VISIBLE | HOST_CACHED | HOST_COHERENT memory for + // readback. + const uint32_t memory_type_index = ui::vulkan::util::ChooseMemoryType( + vulkan_device->memory_types(), memory_requirements.memoryTypeBits, + ui::vulkan::util::MemoryPurpose::kReadback); + + if (memory_type_index == UINT32_MAX) { + XELOGE( + "VulkanCommandProcessor: Failed to find memory type for readback " + "buffer"); + dfn.vkDestroyBuffer(device, new_buffer, nullptr); + return true; + } + + VkMemoryAllocateInfo memory_info = {}; + memory_info.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; + memory_info.allocationSize = memory_requirements.size; + memory_info.memoryTypeIndex = memory_type_index; + + VkDeviceMemory new_memory; + if (dfn.vkAllocateMemory(device, &memory_info, nullptr, &new_memory) != + VK_SUCCESS) { + XELOGE( + "VulkanCommandProcessor: Failed to allocate readback buffer " + "memory"); + dfn.vkDestroyBuffer(device, new_buffer, nullptr); + return true; + } + + // Bind memory to buffer. + if (dfn.vkBindBufferMemory(device, new_buffer, new_memory, 0) != + VK_SUCCESS) { + XELOGE("VulkanCommandProcessor: Failed to bind readback buffer memory"); + dfn.vkFreeMemory(device, new_memory, nullptr); + dfn.vkDestroyBuffer(device, new_buffer, nullptr); + return true; + } + + // Clean up old buffer if exists + if (rb.buffers[write_index] != VK_NULL_HANDLE) { + dfn.vkDestroyBuffer(device, rb.buffers[write_index], nullptr); + } + if (rb.memories[write_index] != VK_NULL_HANDLE) { + dfn.vkFreeMemory(device, rb.memories[write_index], nullptr); + } + + rb.buffers[write_index] = new_buffer; + rb.memories[write_index] = new_memory; + rb.sizes[write_index] = size; + } + + VkBuffer shared_memory_buffer = shared_memory_->buffer(); + + // Ensure shared memory is ready for transfer. + shared_memory_->Use(VulkanSharedMemory::Usage::kRead); + + // Copy GPU buffer → staging buffer. + VkBufferCopy copy_region = {}; + copy_region.srcOffset = written_address; + copy_region.dstOffset = 0; + copy_region.size = written_length; + + deferred_command_buffer_.CmdVkCopyBuffer( + shared_memory_buffer, rb.buffers[write_index], 1, ©_region); + + bool use_delayed_sync = (readback_mode == ReadbackResolveMode::kFast); + uint32_t read_index = write_index; + + if (use_delayed_sync) { + // Use previous frame's data (avoid stall) + read_index = 1 - write_index; + } else { + // Wait for GPU to finish (accurate but slow) + if (!AwaitAllQueueOperationsCompletion()) { XELOGE( "VulkanCommandProcessor: Failed to complete queue operations for " "resolve readback"); + return true; + } + } + + // Read from the appropriate buffer + // If using delayed sync but previous buffer doesn't exist, use current + // buffer with sync as fallback + if (use_delayed_sync && (rb.buffers[read_index] == VK_NULL_HANDLE || + written_length > rb.sizes[read_index])) { + read_index = write_index; + if (!AwaitAllQueueOperationsCompletion()) { + XELOGE( + "VulkanCommandProcessor: Failed to complete queue operations for " + "resolve readback fallback"); + return true; + } + } + + if (rb.buffers[read_index] != VK_NULL_HANDLE && + written_length <= rb.sizes[read_index]) { + void* mapped_data; + if (dfn.vkMapMemory(device, rb.memories[read_index], 0, written_length, 0, + &mapped_data) == VK_SUCCESS) { + if (mapped_data) { + // Memory accessibility already checked at the start of this function + uint8_t* dest_ptr = memory_->TranslatePhysical(written_address); + memory::vastcpy(dest_ptr, static_cast(mapped_data), + written_length); + } else { + XELOGE( + "VulkanCommandProcessor: Failed to map readback buffer " + "(mapped_data is null)"); + } + dfn.vkUnmapMemory(device, rb.memories[read_index]); + } else { + XELOGE( + "VulkanCommandProcessor: Failed to map readback buffer memory for " + "resolve"); } } } @@ -2803,9 +2942,9 @@ VkBuffer VulkanCommandProcessor::RequestReadbackBuffer(uint32_t size) { return VK_NULL_HANDLE; } - size = xe::align(size, kReadbackBufferSizeIncrement); + size = AlignReadbackBufferSize(size); - if (size > readback_buffer_size_) { + if (size > memexport_readback_buffer_size_) { const ui::vulkan::VulkanDevice* const vulkan_device = GetVulkanDevice(); const ui::vulkan::VulkanDevice::Functions& dfn = vulkan_device->functions(); const VkDevice device = vulkan_device->device(); @@ -2868,17 +3007,17 @@ VkBuffer VulkanCommandProcessor::RequestReadbackBuffer(uint32_t size) { } // Destroy old buffer if it exists. - if (readback_buffer_ != VK_NULL_HANDLE) { - dfn.vkDestroyBuffer(device, readback_buffer_, nullptr); - dfn.vkFreeMemory(device, readback_buffer_memory_, nullptr); + if (memexport_readback_buffer_ != VK_NULL_HANDLE) { + dfn.vkDestroyBuffer(device, memexport_readback_buffer_, nullptr); + dfn.vkFreeMemory(device, memexport_readback_buffer_memory_, nullptr); } - readback_buffer_ = new_buffer; - readback_buffer_memory_ = new_memory; - readback_buffer_size_ = size; + memexport_readback_buffer_ = new_buffer; + memexport_readback_buffer_memory_ = new_memory; + memexport_readback_buffer_size_ = size; } - return readback_buffer_; + return memexport_readback_buffer_; } void VulkanCommandProcessor::InitializeTrace() { @@ -3107,6 +3246,41 @@ bool VulkanCommandProcessor::BeginSubmission(bool is_guest_command) { if (is_opening_frame) { frame_open_ = true; + // Swap all readback buffers for delayed sync (one frame behind) + for (auto& pair : readback_buffers_) { + pair.second.current_index = 1 - pair.second.current_index; + } + + // Evict old readback buffers only when map gets too large to prevent + // unbounded memory growth. Don't do this every frame as it's expensive. + if (readback_buffers_.size() > kMaxReadbackBuffers) { + const ui::vulkan::VulkanDevice* const vulkan_device = GetVulkanDevice(); + const ui::vulkan::VulkanDevice::Functions& dfn = + vulkan_device->functions(); + const VkDevice device = vulkan_device->device(); + + for (auto it = readback_buffers_.begin(); + it != readback_buffers_.end();) { + // Evict if not used recently + if (frame_current_ > kReadbackBufferEvictionAgeFrames && + it->second.last_used_frame < + frame_current_ - kReadbackBufferEvictionAgeFrames) { + // Release both buffers and memories + ui::vulkan::util::DestroyAndNullHandle(dfn.vkDestroyBuffer, device, + it->second.buffers[0]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, + it->second.memories[0]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkDestroyBuffer, device, + it->second.buffers[1]); + ui::vulkan::util::DestroyAndNullHandle(dfn.vkFreeMemory, device, + it->second.memories[1]); + it = readback_buffers_.erase(it); + } else { + ++it; + } + } + } + // Reset bindings that depend on transient data. std::memset(current_float_constant_map_vertex_, 0, sizeof(current_float_constant_map_vertex_)); diff --git a/src/xenia/gpu/vulkan/vulkan_command_processor.h b/src/xenia/gpu/vulkan/vulkan_command_processor.h index 2aec81f9b..29a1f88ac 100644 --- a/src/xenia/gpu/vulkan/vulkan_command_processor.h +++ b/src/xenia/gpu/vulkan/vulkan_command_processor.h @@ -754,10 +754,21 @@ class VulkanCommandProcessor final : public CommandProcessor { // Temporary storage for memexport stream constants used in the draw. std::vector memexport_ranges_; - // Readback buffer for CPU access to resolved data - VkBuffer readback_buffer_ = VK_NULL_HANDLE; - VkDeviceMemory readback_buffer_memory_ = VK_NULL_HANDLE; - uint32_t readback_buffer_size_ = 0; + // Per-resolve double-buffered readback for delayed sync + struct ReadbackBuffer { + VkBuffer buffers[2] = {VK_NULL_HANDLE, VK_NULL_HANDLE}; + VkDeviceMemory memories[2] = {VK_NULL_HANDLE, VK_NULL_HANDLE}; + uint32_t sizes[2] = {0, 0}; + uint32_t current_index = 0; + uint64_t last_used_frame = 0; + }; + // Map: (written_address << 32 | written_length) -> ReadbackBuffer + std::unordered_map readback_buffers_; + + // Simple single buffer for memexport (always syncs, no double-buffering) + VkBuffer memexport_readback_buffer_ = VK_NULL_HANDLE; + VkDeviceMemory memexport_readback_buffer_memory_ = VK_NULL_HANDLE; + uint32_t memexport_readback_buffer_size_ = 0; }; } // namespace vulkan From 016797d19c8a629081c80475723a1f3233e56c9b Mon Sep 17 00:00:00 2001 From: Gliniak Date: Mon, 1 Dec 2025 18:36:12 +0100 Subject: [PATCH 3/5] [Emulator] Return error when device initialization fails This should prevent "Install Content" option to freeze in specific scenarios --- src/xenia/emulator.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/xenia/emulator.cc b/src/xenia/emulator.cc index 730a07c02..c0ad9655f 100644 --- a/src/xenia/emulator.cc +++ b/src/xenia/emulator.cc @@ -866,6 +866,10 @@ X_STATUS Emulator::InstallContentPackage( vfs::XContentContainerDevice::CreateContentDevice("", path); if (!device || !device->Initialize()) { + installation_info.installation_state_ = InstallState::failed; + installation_info.installation_error_message_ = + "Device initialization failed!"; + installation_info.installation_result_ = X_STATUS_ACCESS_DENIED; XELOGE("Failed to initialize device"); return X_STATUS_INVALID_PARAMETER; } From dfa70b3677cfee4e538e9f0b8d10b339bc9a4501 Mon Sep 17 00:00:00 2001 From: Anton Dorozhkin Date: Sun, 30 Nov 2025 13:40:34 +0700 Subject: [PATCH 4/5] [Kernel] Change _vsnprintf overflow behavior. --- src/xenia/kernel/xboxkrnl/xboxkrnl_strings.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_strings.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_strings.cc index 1554e30ff..4e5c9c8f3 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_strings.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_strings.cc @@ -1025,8 +1025,9 @@ SHIM_CALL _vsnprintf_entry(PPCContext* ppc_context) { buffer[count] = '\0'; } } else { - // Overflowed buffer. We still return the count we would have written. + // Overflowed buffer. std::memcpy(buffer, data.str().c_str(), buffer_count); + count = -1; } SHIM_SET_RETURN_32(count); } From dd29365970e06a1e40b06714a8fed0fcfffbbd1d Mon Sep 17 00:00:00 2001 From: Gliniak Date: Mon, 1 Dec 2025 23:57:53 +0100 Subject: [PATCH 5/5] [APU] Switched to New XMA decoder as default - Added codepath for "consume only" pass. This should resolve constant clicking in Source engine games. - Added smaller performance improvements to decoder --- src/xenia/apu/xma_context.h | 6 ++++++ src/xenia/apu/xma_context_new.cc | 12 +++++++++++- src/xenia/apu/xma_context_new.h | 3 ++- src/xenia/apu/xma_decoder.cc | 4 +++- src/xenia/base/cvar.h | 2 +- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/xenia/apu/xma_context.h b/src/xenia/apu/xma_context.h index d52f68897..d7dfa39c8 100644 --- a/src/xenia/apu/xma_context.h +++ b/src/xenia/apu/xma_context.h @@ -146,6 +146,12 @@ struct XMA_CONTEXT_DATA { const uint32_t GetCurrentInputBufferPacketCount() const { return GetInputBufferPacketCount(current_buffer); } + const bool IsStreamingContext() const { + return (input_buffer_0_packet_count | input_buffer_1_packet_count) == 1; + } + const bool IsConsumeOnlyContext() const { + return (input_buffer_0_packet_count | input_buffer_1_packet_count) == 0; + } }; static_assert_size(XMA_CONTEXT_DATA, 64); diff --git a/src/xenia/apu/xma_context_new.cc b/src/xenia/apu/xma_context_new.cc index 4d7034c6c..6dad9704c 100644 --- a/src/xenia/apu/xma_context_new.cc +++ b/src/xenia/apu/xma_context_new.cc @@ -128,6 +128,15 @@ bool XmaContextNew::Work() { RingBuffer output_rb = PrepareOutputRingBuffer(&data); + if (data.IsConsumeOnlyContext()) { + Consume(&output_rb, &data); + if (data.output_buffer_read_offset == data.output_buffer_write_offset) { + Clear(); + } + data.Store(context_ptr); + return true; + } + const int32_t minimum_subframe_decode_count = (data.subframe_decode_count * 2) - 1; @@ -257,7 +266,8 @@ void XmaContextNew::SwapInputBuffer(XMA_CONTEXT_DATA* data) { data->input_buffer_read_offset = kBitsPerPacketHeader; } -void XmaContextNew::Consume(RingBuffer* output_rb, XMA_CONTEXT_DATA* data) { +void XmaContextNew::Consume(RingBuffer* XE_RESTRICT output_rb, + const XMA_CONTEXT_DATA* const XE_RESTRICT data) { if (!current_frame_remaining_subframes_) { return; } diff --git a/src/xenia/apu/xma_context_new.h b/src/xenia/apu/xma_context_new.h index 1d86c7af4..985b2ca9a 100644 --- a/src/xenia/apu/xma_context_new.h +++ b/src/xenia/apu/xma_context_new.h @@ -106,7 +106,8 @@ class XmaContextNew : public XmaContext { static uint32_t GetCurrentInputBufferSize(XMA_CONTEXT_DATA* data); void Decode(XMA_CONTEXT_DATA* data); - void Consume(RingBuffer* output_rb, XMA_CONTEXT_DATA* data); + void Consume(RingBuffer* XE_RESTRICT output_rb, + const XMA_CONTEXT_DATA* const XE_RESTRICT data); void UpdateLoopStatus(XMA_CONTEXT_DATA* data); int PrepareDecoder(int sample_rate, bool is_two_channel); diff --git a/src/xenia/apu/xma_decoder.cc b/src/xenia/apu/xma_decoder.cc index 07ff4c659..114885edd 100644 --- a/src/xenia/apu/xma_decoder.cc +++ b/src/xenia/apu/xma_decoder.cc @@ -54,7 +54,7 @@ extern "C" { DEFINE_bool(ffmpeg_verbose, false, "Verbose FFmpeg output (debug and above)", "APU"); -DEFINE_bool(use_new_decoder, false, +DEFINE_bool(use_new_decoder, true, "Enables usage of new experimental XMA audio decoder.", "APU"); DEFINE_bool(use_dedicated_xma_thread, true, @@ -62,6 +62,8 @@ DEFINE_bool(use_dedicated_xma_thread, true, "better results, but decrease performance a bit.", "APU"); +UPDATE_from_bool(use_new_decoder, 2025, 12, 01, 23, false); + namespace xe { namespace apu { diff --git a/src/xenia/base/cvar.h b/src/xenia/base/cvar.h index 6b139bda7..3c9581ba7 100644 --- a/src/xenia/base/cvar.h +++ b/src/xenia/base/cvar.h @@ -511,7 +511,7 @@ class IConfigVarUpdate { // If you're reviewing a pull request with a change here, check if 1) has been // done by the submitter before merging. static constexpr uint32_t kLastCommittedUpdateDate = - MakeConfigVarUpdateDate(2024, 9, 23, 9); + MakeConfigVarUpdateDate(2025, 12, 1, 23); virtual ~IConfigVarUpdate() = default;