From 7fe574f1b5e3fad830874623b78ca12a035f6cb3 Mon Sep 17 00:00:00 2001 From: crueter Date: Mon, 29 Dec 2025 19:41:23 -0500 Subject: [PATCH] Revert "Initial MSAA fix (Download and Upload) (#145)" This reverts commit 4758e126b863da560bf30a00deda3bb44e26b7fa. --- .../renderer_vulkan/vk_texture_cache.cpp | 305 ++++-------------- .../renderer_vulkan/vk_texture_cache.h | 3 +- 2 files changed, 67 insertions(+), 241 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_texture_cache.cpp b/src/video_core/renderer_vulkan/vk_texture_cache.cpp index f821c71b7b..eb8df35759 100644 --- a/src/video_core/renderer_vulkan/vk_texture_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_texture_cache.cpp @@ -1573,90 +1573,16 @@ void Image::UploadMemory(VkBuffer buffer, VkDeviceSize offset, if (is_rescaled) { ScaleDown(true); } - - // Handle MSAA upload if necessary - /* 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. */ - // 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 validation and runtime issues. - const bool wants_msaa_upload = info.num_samples > 1 && - (aspect_mask & VK_IMAGE_ASPECT_COLOR_BIT) != 0 && - runtime->CanUploadMSAA() && runtime->msaa_copy_pass != nullptr && - runtime->device.IsStorageImageMultisampleSupported(); - - if (wants_msaa_upload) { - // Create a temporary non-MSAA image to upload the data first - ImageInfo temp_info = info; - temp_info.num_samples = 1; - - // 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. - VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info); - 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; - - // CHANGE: The previous stack-allocated wrapper was destroyed at function exit, - // which could destroy VkImage before the GPU used it. - auto temp_wrapper = std::make_shared(*runtime, temp_info, 0, 0); - temp_wrapper->original_image = runtime->memory_allocator.CreateImage(image_ci); - temp_wrapper->current_image = &Image::original_image; - temp_wrapper->aspect_mask = aspect_mask; - temp_wrapper->initialized = true; - - // Upload to the temporary non-MSAA image - scheduler->RequestOutsideRenderPassOperationContext(); - auto vk_copies = TransformBufferImageCopies(copies, offset, temp_wrapper->aspect_mask); - const VkBuffer src_buffer = buffer; - const VkImage temp_vk_image = *temp_wrapper->original_image; - const VkImageAspectFlags vk_aspect_mask = temp_wrapper->aspect_mask; - - scheduler->Record([src_buffer, temp_vk_image, vk_aspect_mask, vk_copies, - keep = temp_wrapper](vk::CommandBuffer cmdbuf) { - CopyBufferToImage(cmdbuf, src_buffer, temp_vk_image, vk_aspect_mask, false, VideoCommon::FixSmallVectorADL(vk_copies)); - }); - - // Use MSAACopyPass to convert from non-MSAA to MSAA - std::vector 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); - - const u64 tick = scheduler->Flush(); - scheduler->Wait(tick); - - 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 was_initialized = std::exchange(initialized, true); - - scheduler->Record([src_buffer, vk_image, vk_aspect_mask, was_initialized, + 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, was_initialized, VideoCommon::FixSmallVectorADL(vk_copies)); + CopyBufferToImage(cmdbuf, src_buffer, vk_image, vk_aspect_mask, is_initialized, vk_copies); }); - if (is_rescaled) { ScaleUp(); } @@ -1678,176 +1604,75 @@ void Image::DownloadMemory(VkBuffer buffer, size_t offset, } void Image::DownloadMemory(std::span buffers_span, std::span offsets_span, - std::span copies) { + std::span copies) { const bool is_rescaled = True(flags & ImageFlagBits::Rescaled); if (is_rescaled) { ScaleDown(); } + boost::container::small_vector buffers_vector{}; + boost::container::small_vector, 8> + vk_copies; + for (size_t index = 0; index < buffers_span.size(); index++) { + buffers_vector.emplace_back(buffers_span[index]); + vk_copies.emplace_back( + TransformBufferImageCopies(copies, offsets_span[index], aspect_mask)); + } + scheduler->RequestOutsideRenderPassOperationContext(); + scheduler->Record([buffers = std::move(buffers_vector), image = *original_image, + aspect_mask_ = aspect_mask, vk_copies](vk::CommandBuffer cmdbuf) { + const VkImageMemoryBarrier read_barrier{ + .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, + .pNext = nullptr, + .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, + .dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT, + .oldLayout = VK_IMAGE_LAYOUT_GENERAL, + .newLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .image = image, + .subresourceRange{ + .aspectMask = aspect_mask_, + .baseMipLevel = 0, + .levelCount = VK_REMAINING_MIP_LEVELS, + .baseArrayLayer = 0, + .layerCount = VK_REMAINING_ARRAY_LAYERS, + }, + }; + cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, + 0, read_barrier); - // RE-USE MSAA UPLOAD CODE BUT NOW FOR DOWNLOAD - if (info.num_samples > 1 && runtime->msaa_copy_pass) { - // TODO: Depth/stencil formats need special handling - if (aspect_mask == VK_IMAGE_ASPECT_COLOR_BIT) { - ImageInfo temp_info = info; - temp_info.num_samples = 1; - - VkImageCreateInfo image_ci = MakeImageCreateInfo(runtime->device, temp_info); - image_ci.usage = original_image.UsageFlags(); - vk::Image temp_image = runtime->memory_allocator.CreateImage(image_ci); - - 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; - - std::vector image_copies; - for (const auto& copy : copies) { - VideoCommon::ImageCopy image_copy; - image_copy.src_offset = copy.image_offset; - 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(temp_wrapper, *this, image_copies, true); - - boost::container::small_vector buffers_vector{}; - boost::container::small_vector, 8> - vk_copies; - for (size_t index = 0; index < buffers_span.size(); index++) { - buffers_vector.emplace_back(buffers_span[index]); - vk_copies.emplace_back( - TransformBufferImageCopies(copies, offsets_span[index], aspect_mask)); - } - - scheduler->RequestOutsideRenderPassOperationContext(); - scheduler->Record([buffers = std::move(buffers_vector), image = *temp_wrapper.original_image, - aspect_mask_ = aspect_mask, vk_copies](vk::CommandBuffer cmdbuf) { - const VkImageMemoryBarrier read_barrier{ - .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT, - .oldLayout = VK_IMAGE_LAYOUT_GENERAL, - .newLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .image = image, - .subresourceRange{ - .aspectMask = aspect_mask_, - .baseMipLevel = 0, - .levelCount = VK_REMAINING_MIP_LEVELS, - .baseArrayLayer = 0, - .layerCount = VK_REMAINING_ARRAY_LAYERS, - }, - }; - cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, - 0, read_barrier); - - for (size_t index = 0; index < buffers.size(); index++) { - cmdbuf.CopyImageToBuffer(image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, buffers[index], - vk_copies[index]); - } - - const VkMemoryBarrier memory_write_barrier{ - .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT, - }; - const VkImageMemoryBarrier image_write_barrier{ - .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = 0, - .dstAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - .newLayout = VK_IMAGE_LAYOUT_GENERAL, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .image = image, - .subresourceRange{ - .aspectMask = aspect_mask_, - .baseMipLevel = 0, - .levelCount = VK_REMAINING_MIP_LEVELS, - .baseArrayLayer = 0, - .layerCount = VK_REMAINING_ARRAY_LAYERS, - }, - }; - cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, - 0, memory_write_barrier, nullptr, image_write_barrier); - }); - return; - } - } else { - boost::container::small_vector buffers_vector{}; - boost::container::small_vector, 8> - vk_copies; - for (size_t index = 0; index < buffers_span.size(); index++) { - buffers_vector.emplace_back(buffers_span[index]); - vk_copies.emplace_back( - TransformBufferImageCopies(copies, offsets_span[index], aspect_mask)); + for (size_t index = 0; index < buffers.size(); index++) { + cmdbuf.CopyImageToBuffer(image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, buffers[index], + vk_copies[index]); } - scheduler->RequestOutsideRenderPassOperationContext(); - scheduler->Record([buffers = std::move(buffers_vector), image = *original_image, - aspect_mask_ = aspect_mask, vk_copies](vk::CommandBuffer cmdbuf) { - const VkImageMemoryBarrier read_barrier{ - .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT, - .oldLayout = VK_IMAGE_LAYOUT_GENERAL, - .newLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .image = image, - .subresourceRange{ - .aspectMask = aspect_mask_, - .baseMipLevel = 0, - .levelCount = VK_REMAINING_MIP_LEVELS, - .baseArrayLayer = 0, - .layerCount = VK_REMAINING_ARRAY_LAYERS, - }, - }; - cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, - 0, read_barrier); - - for (size_t index = 0; index < buffers.size(); index++) { - cmdbuf.CopyImageToBuffer(image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, buffers[index], - vk_copies[index]); - } - - const VkMemoryBarrier memory_write_barrier{ - .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT, - }; - const VkImageMemoryBarrier image_write_barrier{ - .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, - .pNext = nullptr, - .srcAccessMask = 0, - .dstAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, - .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - .newLayout = VK_IMAGE_LAYOUT_GENERAL, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .image = image, - .subresourceRange{ - .aspectMask = aspect_mask_, - .baseMipLevel = 0, - .levelCount = VK_REMAINING_MIP_LEVELS, - .baseArrayLayer = 0, - .layerCount = VK_REMAINING_ARRAY_LAYERS, - }, - }; - cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, - 0, memory_write_barrier, nullptr, image_write_barrier); - }); - } + const VkMemoryBarrier memory_write_barrier{ + .sType = VK_STRUCTURE_TYPE_MEMORY_BARRIER, + .pNext = nullptr, + .srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, + .dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT, + }; + const VkImageMemoryBarrier image_write_barrier{ + .sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, + .pNext = nullptr, + .srcAccessMask = 0, + .dstAccessMask = VK_ACCESS_MEMORY_WRITE_BIT, + .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + .newLayout = VK_IMAGE_LAYOUT_GENERAL, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .image = image, + .subresourceRange{ + .aspectMask = aspect_mask_, + .baseMipLevel = 0, + .levelCount = VK_REMAINING_MIP_LEVELS, + .baseArrayLayer = 0, + .layerCount = VK_REMAINING_ARRAY_LAYERS, + }, + }; + cmdbuf.PipelineBarrier(VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, + 0, memory_write_barrier, nullptr, image_write_barrier); + }); if (is_rescaled) { ScaleUp(true); } diff --git a/src/video_core/renderer_vulkan/vk_texture_cache.h b/src/video_core/renderer_vulkan/vk_texture_cache.h index cd11cc8fc7..18d20b2db5 100644 --- a/src/video_core/renderer_vulkan/vk_texture_cache.h +++ b/src/video_core/renderer_vulkan/vk_texture_cache.h @@ -82,7 +82,8 @@ public: } bool CanUploadMSAA() const noexcept { - return msaa_copy_pass.operator bool(); + // TODO: Implement buffer to MSAA uploads + return false; } void AccelerateImageUpload(Image&, const StagingBufferRef&,