eden-emu/eden
24
3
Fork 15
Code Issues 138 Pull requests 55 Projects 1 Releases Packages Wiki Activity Actions

[vk, texture_cache] MSAA ensure no more crash (#245)

Browse source 
> The shared_ptr<Image> capture ensures the temporary image outlives all queued GPU work (both the upload/download step and the MSAA compute copy). Without this, drivers read freed memory
> The temp image always has STORAGE & TRANSFER usage, matching what the compute MSAA path actually binds.
> Since this only a temp fix ontop of the previous commit, we only use the MSAA path for color; depth/stencil still use the original safe route.
> Should still be reworked though, as seen in the other MSAA commis that are open.

Reviewed-on: #245
Reviewed-by: crueter <crueter@eden-emu.dev>
Reviewed-by: Shinmegumi <shinmegumi@eden-emu.dev>
Co-authored-by: Maufeat <sahyno1996@gmail.com>
Co-committed-by: Maufeat <sahyno1996@gmail.com>
This commit is contained in:
Maufeat 2025-08-13 01:42:56 +02:00 • committed by crueter
parent 234e41193e
commit 89d40c6302
Signed by: crueter
GPG key ID: 425ACD2D4830EBC6
1 changed files with 76 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

132
src/video_core/renderer_vulkan/vk_texture_cache.cpp
View file

@ -7,6 +7,7 @@
#include <algorithm> #include <algorithm>
#include <array> #include <array>
#include <span> #include <span>
#include <memory>
#include <vector> #include <vector>
#include <boost/container/small_vector.hpp> #include <boost/container/small_vector.hpp>
@ -1538,7 +1539,7 @@ Image::Image(const VideoCommon::NullImageParams& params) : VideoCommon::ImageBas
Image::~Image() = default; Image::~Image() = default;
void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset, void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
std::span<const VideoCommon::BufferImageCopy> copies) { std::span<const VideoCommon::BufferImageCopy> copies) {
// TODO: Move this to another API // TODO: Move this to another API
const bool is_rescaled = True(flags & ImageFlagBits::Rescaled); const bool is_rescaled = True(flags & ImageFlagBits::Rescaled);
if (is_rescaled) { if (is_rescaled) {
@ -1546,70 +1547,89 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
} }
// Handle MSAA upload if necessary // Handle MSAA upload if necessary
/* WARNING, TODO: This code uses some hacks, besides being fundamentally ugly /* WARNING, TODO: This code uses some hacks, besides being fundamentally ugly
since tropic didn't want to touch it for a long time, so it needs a rewrite from someone better than me at vulkan.*/ since tropic didn't want to touch it for a long time, so it needs a rewrite from someone
if (info.num_samples > 1 && runtime->CanUploadMSAA()) { better than me at vulkan. */
// Only use MSAA copy pass for color formats // CHANGE: Gate the MSAA path more strictly and only use it for color, when the pass and device
// TODO: Depth/stencil formats need special handling // support are available. Avoid running the MSAA path when prerequisites aren't met, preventing
if (aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT) { // validation and runtime issues.
// Create a temporary non-MSAA image to upload the data first const bool wants_msaa_upload = info.num_samples > 1 &&
ImageInfo temp_info = info; aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT &&
temp_info.num_samples = 1; runtime->CanUploadMSAA() && runtime->msaa_copy_pass != nullptr &&
runtime->device.IsStorageImageMultisampleSupported();
// Create image with same usage flags as the target image to avoid validation errors if (wants_msaa_upload) {
VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info); // Create a temporary non-MSAA image to upload the data first
image_ci.usage = original_image.UsageFlags(); ImageInfo temp_info = info;
vk::Image temp_image = runtime->memory_allocator.CreateImage(image_ci); temp_info.num_samples = 1;
// Upload to the temporary non-MSAA image // CHANGE: Build a fresh VkImageCreateInfo with robust usage flags for the temp image.
scheduler->RequestOutsideRenderPassOperationContext(); // Using the target image's usage as-is could miss STORAGE/TRANSFER bits and trigger validation errors.
auto vk_copies = TransformBufferImageCopies(copies, offset, aspect_mask); VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info);
const VkBuffer src_buffer = buffer; image_ci.usage = VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT |
const VkImage temp_vk_image = *temp_image; VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_STORAGE_BIT;
const VkImageAspectFlags vk_aspect_mask = aspect_mask;
scheduler->Record([src_buffer, temp_vk_image, vk_aspect_mask, vk_copies](vk::CommandBuffer cmdbuf) {
CopyBufferToImage(cmdbuf, src_buffer, temp_vk_image, vk_aspect_mask, false, vk_copies);
});
// Use MSAACopyPass to convert from non-MSAA to MSAA // CHANGE: The previous stack-allocated wrapper was destroyed at function exit,
std::vector<VideoCommon::ImageCopy> image_copies; // which could destroy VkImage before the GPU used it.
for (const auto& copy : copies) { auto temp_wrapper = std::make_shared<Image>(*runtime, temp_info, 0, 0);
VideoCommon::ImageCopy image_copy; temp_wrapper->original_image = runtime->memory_allocator.CreateImage(image_ci);
image_copy.src_offset = {0, 0, 0}; // Use zero offset for source temp_wrapper->current_image = &Image::original_image;
image_copy.dst_offset = copy.image_offset; temp_wrapper->aspect_mask = VK_IMAGE_ASPECT_COLOR_BIT;
image_copy.src_subresource = copy.image_subresource; temp_wrapper->initialized = true;
image_copy.dst_subresource = copy.image_subresource;
image_copy.extent = copy.image_extent;
image_copies.push_back(image_copy);
}
// wrapper image for the temporary image // Upload to the temporary non-MSAA image
Image temp_wrapper(*runtime, temp_info, 0, 0);
temp_wrapper.original_image = std::move(temp_image);
temp_wrapper.current_image = &Image::original_image;
temp_wrapper.aspect_mask = aspect_mask;
temp_wrapper.initialized = true;
// Use MSAACopyPass to convert from non-MSAA to MSAA
runtime->msaa_copy_pass->CopyImage(*this, temp_wrapper, image_copies, false);
std::exchange(initialized, true);
return;
}
// For depth/stencil formats, fall back to regular upload
} else {
// Regular non-MSAA upload
scheduler->RequestOutsideRenderPassOperationContext(); scheduler->RequestOutsideRenderPassOperationContext();
auto vk_copies = TransformBufferImageCopies(copies, offset, aspect_mask); auto vk_copies = TransformBufferImageCopies(copies, offset, temp_wrapper->aspect_mask);
const VkBuffer src_buffer = buffer; const VkBuffer src_buffer = buffer;
const VkImage vk_image = *original_image; const VkImage temp_vk_image = *temp_wrapper->original_image;
const VkImageAspectFlags vk_aspect_mask = aspect_mask; const VkImageAspectFlags vk_aspect_mask = temp_wrapper->aspect_mask;
const bool is_initialized = std::exchange(initialized, true); scheduler->Record([src_buffer, temp_vk_image, vk_aspect_mask, vk_copies,
scheduler->Record([src_buffer, vk_image, vk_aspect_mask, is_initialized, // CHANGE: capture shared_ptr to pin lifetime through submission
vk_copies](vk::CommandBuffer cmdbuf) { keep = temp_wrapper](vk::CommandBuffer cmdbuf) {
CopyBufferToImage(cmdbuf, src_buffer, vk_image, vk_aspect_mask, is_initialized, vk_copies); CopyBufferToImage(cmdbuf, src_buffer, temp_vk_image, vk_aspect_mask, false, vk_copies);
}); });
// Use MSAACopyPass to convert from non-MSAA to MSAA
// CHANGE: only lifetime and usage were fixed.
std::vector<VideoCommon::ImageCopy> image_copies;
image_copies.reserve(copies.size());
for (const auto& copy : copies) {
VideoCommon::ImageCopy image_copy;
image_copy.src_offset = {0, 0, 0}; // Use zero offset for source
image_copy.dst_offset = copy.image_offset;
image_copy.src_subresource = copy.image_subresource;
image_copy.dst_subresource = copy.image_subresource;
image_copy.extent = copy.image_extent;
image_copies.push_back(image_copy);
}
runtime->msaa_copy_pass->CopyImage(*this, *temp_wrapper, image_copies,
/*msaa_to_non_msaa=*/false);
std::exchange(initialized, true);
// CHANGE: Add a no-op recording that captures temp_wrapper to ensure it stays alive
// at least until commands are submitted/recorded.
scheduler->Record([keep = std::move(temp_wrapper)](vk::CommandBuffer) {});
// CHANGE: Restore scaling before returning from the MSAA path.
if (is_rescaled) {
ScaleUp();
}
return;
} }
// Regular non-MSAA upload (original behavior preserved)
scheduler->RequestOutsideRenderPassOperationContext();
auto vk_copies = TransformBufferImageCopies(copies, offset, aspect_mask);
const VkBuffer src_buffer = buffer;
const VkImage vk_image = *original_image;
const VkImageAspectFlags vk_aspect_mask = aspect_mask;
const bool is_initialized = std::exchange(initialized, true);
scheduler->Record([src_buffer, vk_image, vk_aspect_mask, is_initialized,
vk_copies](vk::CommandBuffer cmdbuf) {
CopyBufferToImage(cmdbuf, src_buffer, vk_image, vk_aspect_mask, is_initialized, vk_copies);
});
if (is_rescaled) { if (is_rescaled) {
ScaleUp(); ScaleUp();
} }