[texture_cache] fix msaa upload again, (#2550)

temproary non-msaa images used for msaa uploads were being destroyed before the GPU finished using them, causing validation errors/crashes.
This keeps the temp image alive until the GPU finishes and switches the aspect-mask check to bitwise so the MSAA path only runs when needed.

Reviewed-on: #2550
Reviewed-by: MaranBr <maranbr@eden-emu.dev>
Reviewed-by: CamilleLaVey <camillelavey99@gmail.com>
Co-authored-by: Maufeat <sahyno1996@gmail.com>
Co-committed-by: Maufeat <sahyno1996@gmail.com>
This commit is contained in:
Maufeat 2025-09-23 03:49:36 +02:00 committed by crueter
parent 7050b92d61
commit aec7f19b7e
Signed by: crueter
GPG key ID: 425ACD2D4830EBC6

View file

@ -1377,17 +1377,13 @@ void TextureCacheRuntime::CopyImage(Image& dst, Image& src,
// As per the size-compatible formats section of vulkan, copy manually via ReinterpretImage // As per the size-compatible formats section of vulkan, copy manually via ReinterpretImage
// these images that aren't size-compatible // these images that aren't size-compatible
if (BytesPerBlock(src.info.format) != BytesPerBlock(dst.info.format)) { if (BytesPerBlock(src.info.format) != BytesPerBlock(dst.info.format)) {
auto oneCopy = VideoCommon::ImageCopy{
if (src.info.type == ImageType::Linear || dst.info.type == ImageType::Linear) { .src_offset = VideoCommon::Offset3D(0, 0, 0),
return; .dst_offset = VideoCommon::Offset3D(0, 0, 0),
} .extent = dst.info.size
};
auto oneCopy = VideoCommon::ImageCopy{.src_offset = VideoCommon::Offset3D(0, 0, 0),
.dst_offset = VideoCommon::Offset3D(0, 0, 0),
.extent = dst.info.size};
return ReinterpretImage(dst, src, std::span{&oneCopy, 1}); return ReinterpretImage(dst, src, std::span{&oneCopy, 1});
} }
boost::container::small_vector<VkImageCopy, 16> vk_copies(copies.size()); boost::container::small_vector<VkImageCopy, 16> vk_copies(copies.size());
const VkImageAspectFlags aspect_mask = dst.AspectMask(); const VkImageAspectFlags aspect_mask = dst.AspectMask();
ASSERT(aspect_mask == src.AspectMask()); ASSERT(aspect_mask == src.AspectMask());
@ -1565,10 +1561,10 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
since tropic didn't want to touch it for a long time, so it needs a rewrite from someone since tropic didn't want to touch it for a long time, so it needs a rewrite from someone
better than me at vulkan. */ better than me at vulkan. */
// CHANGE: Gate the MSAA path more strictly and only use it for color, when the pass and device // CHANGE: Gate the MSAA path more strictly and only use it for color, when the pass and device
// support are available. Avoid running the MSAA path when prerequisites aren't met, preventing // support are available. Avoid running the MSAA path when prerequisites aren't met,
// validation and runtime issues. // preventing validation and runtime issues.
const bool wants_msaa_upload = info.num_samples > 1 && const bool wants_msaa_upload = info.num_samples > 1 &&
aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT && (aspect_mask & VK_IMAGE_ASPECT_COLOR_BIT) != 0 &&
runtime->CanUploadMSAA() && runtime->msaa_copy_pass != nullptr && runtime->CanUploadMSAA() && runtime->msaa_copy_pass != nullptr &&
runtime->device.IsStorageImageMultisampleSupported(); runtime->device.IsStorageImageMultisampleSupported();
@ -1578,7 +1574,8 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
temp_info.num_samples = 1; temp_info.num_samples = 1;
// CHANGE: Build a fresh VkImageCreateInfo with robust usage flags for the temp image. // CHANGE: Build a fresh VkImageCreateInfo with robust usage flags for the temp image.
// Using the target image's usage as-is could miss STORAGE/TRANSFER bits and trigger validation errors. // Using the target image's usage as-is could miss STORAGE/TRANSFER bits and trigger
// validation errors.
VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info); VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info);
image_ci.usage = VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT | image_ci.usage = VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT |
VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_STORAGE_BIT; VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_STORAGE_BIT;
@ -1588,7 +1585,7 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
auto temp_wrapper = std::make_shared<Image>(*runtime, temp_info, 0, 0); auto temp_wrapper = std::make_shared<Image>(*runtime, temp_info, 0, 0);
temp_wrapper->original_image = runtime->memory_allocator.CreateImage(image_ci); temp_wrapper->original_image = runtime->memory_allocator.CreateImage(image_ci);
temp_wrapper->current_image = &Image::original_image; temp_wrapper->current_image = &Image::original_image;
temp_wrapper->aspect_mask = VK_IMAGE_ASPECT_COLOR_BIT; temp_wrapper->aspect_mask = aspect_mask;
temp_wrapper->initialized = true; temp_wrapper->initialized = true;
// Upload to the temporary non-MSAA image // Upload to the temporary non-MSAA image
@ -1597,18 +1594,17 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
const VkBuffer src_buffer = buffer; const VkBuffer src_buffer = buffer;
const VkImage temp_vk_image = *temp_wrapper->original_image; const VkImage temp_vk_image = *temp_wrapper->original_image;
const VkImageAspectFlags vk_aspect_mask = temp_wrapper->aspect_mask; const VkImageAspectFlags vk_aspect_mask = temp_wrapper->aspect_mask;
scheduler->Record([src_buffer, temp_vk_image, vk_aspect_mask, vk_copies, scheduler->Record([src_buffer, temp_vk_image, vk_aspect_mask, vk_copies,
// CHANGE: capture shared_ptr to pin lifetime through submission
keep = temp_wrapper](vk::CommandBuffer cmdbuf) { keep = temp_wrapper](vk::CommandBuffer cmdbuf) {
CopyBufferToImage(cmdbuf, src_buffer, temp_vk_image, vk_aspect_mask, false, vk_copies); CopyBufferToImage(cmdbuf, src_buffer, temp_vk_image, vk_aspect_mask, false, vk_copies);
}); });
// Use MSAACopyPass to convert from non-MSAA to MSAA // Use MSAACopyPass to convert from non-MSAA to MSAA
// CHANGE: only lifetime and usage were fixed.
std::vector<VideoCommon::ImageCopy> image_copies; std::vector<VideoCommon::ImageCopy> image_copies;
image_copies.reserve(copies.size()); image_copies.reserve(copies.size());
for (const auto& copy : copies) { for (const auto& copy : copies) {
VideoCommon::ImageCopy image_copy; VideoCommon::ImageCopy image_copy{};
image_copy.src_offset = {0, 0, 0}; // Use zero offset for source image_copy.src_offset = {0, 0, 0}; // Use zero offset for source
image_copy.dst_offset = copy.image_offset; image_copy.dst_offset = copy.image_offset;
image_copy.src_subresource = copy.image_subresource; image_copy.src_subresource = copy.image_subresource;
@ -1621,11 +1617,9 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
/*msaa_to_non_msaa=*/false); /*msaa_to_non_msaa=*/false);
std::exchange(initialized, true); std::exchange(initialized, true);
// CHANGE: Add a no-op recording that captures temp_wrapper to ensure it stays alive const u64 tick = scheduler->Flush();
// at least until commands are submitted/recorded. scheduler->Wait(tick);
scheduler->Record([keep = std::move(temp_wrapper)](vk::CommandBuffer) {});
// CHANGE: Restore scaling before returning from the MSAA path.
if (is_rescaled) { if (is_rescaled) {
ScaleUp(); ScaleUp();
} }
@ -1638,10 +1632,11 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset,
const VkBuffer src_buffer = buffer; const VkBuffer src_buffer = buffer;
const VkImage vk_image = *original_image; const VkImage vk_image = *original_image;
const VkImageAspectFlags vk_aspect_mask = aspect_mask; const VkImageAspectFlags vk_aspect_mask = aspect_mask;
const bool is_initialized = std::exchange(initialized, true); const bool was_initialized = std::exchange(initialized, true);
scheduler->Record([src_buffer, vk_image, vk_aspect_mask, is_initialized,
scheduler->Record([src_buffer, vk_image, vk_aspect_mask, was_initialized,
vk_copies](vk::CommandBuffer cmdbuf) { vk_copies](vk::CommandBuffer cmdbuf) {
CopyBufferToImage(cmdbuf, src_buffer, vk_image, vk_aspect_mask, is_initialized, vk_copies); CopyBufferToImage(cmdbuf, src_buffer, vk_image, vk_aspect_mask, was_initialized, vk_copies);
}); });
if (is_rescaled) { if (is_rescaled) {