Fix incorrect VkBufferImageCopy offset calculations

The `VkBufferImageCopy` offset calculations were wrong inside `CopyIntoStagingBuffer` as it multiplied the mip level's linear size by `levelCount` rather than `layerCount`. This led to substantial UB in games which called this function as it led to an overflow and resulted in writing to other areas of the buffer which caused major issues such as vertex/index buffer corruption and corresponding graphical glitches alongside likely being the cause of some crashes.
This commit is contained in:
PixelyIon 2022-06-02 22:14:22 +05:30
parent 06901ef22a
commit 2712b3276b
2 changed files with 30 additions and 45 deletions

View File

@ -300,25 +300,9 @@ namespace skyline::gpu {
return stagingBuffer; return stagingBuffer;
} }
void Texture::CopyFromStagingBuffer(const vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr<memory::StagingBuffer> &stagingBuffer) { boost::container::small_vector<vk::BufferImageCopy, 10> Texture::GetBufferImageCopies() {
auto image{GetBacking()}; boost::container::small_vector<vk::BufferImageCopy, 10> bufferImageCopies;
if (layout == vk::ImageLayout::eUndefined)
commandBuffer.pipelineBarrier(vk::PipelineStageFlagBits::eHost, vk::PipelineStageFlagBits::eTransfer, {}, {}, {}, vk::ImageMemoryBarrier{
.image = image,
.srcAccessMask = vk::AccessFlagBits::eMemoryRead,
.dstAccessMask = vk::AccessFlagBits::eTransferWrite,
.oldLayout = std::exchange(layout, vk::ImageLayout::eGeneral),
.newLayout = vk::ImageLayout::eGeneral,
.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.subresourceRange = {
.aspectMask = format->vkAspect,
.levelCount = levelCount,
.layerCount = layerCount,
},
});
std::vector<vk::BufferImageCopy> bufferImageCopies;
auto pushBufferImageCopyWithAspect{[&](vk::ImageAspectFlagBits aspect) { auto pushBufferImageCopyWithAspect{[&](vk::ImageAspectFlagBits aspect) {
vk::DeviceSize bufferOffset{}; vk::DeviceSize bufferOffset{};
u32 mipLevel{}; u32 mipLevel{};
@ -345,6 +329,28 @@ namespace skyline::gpu {
if (format->vkAspect & vk::ImageAspectFlagBits::eStencil) if (format->vkAspect & vk::ImageAspectFlagBits::eStencil)
pushBufferImageCopyWithAspect(vk::ImageAspectFlagBits::eStencil); pushBufferImageCopyWithAspect(vk::ImageAspectFlagBits::eStencil);
return bufferImageCopies;
}
void Texture::CopyFromStagingBuffer(const vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr<memory::StagingBuffer> &stagingBuffer) {
auto image{GetBacking()};
if (layout == vk::ImageLayout::eUndefined)
commandBuffer.pipelineBarrier(vk::PipelineStageFlagBits::eHost, vk::PipelineStageFlagBits::eTransfer, {}, {}, {}, vk::ImageMemoryBarrier{
.image = image,
.srcAccessMask = vk::AccessFlagBits::eMemoryRead,
.dstAccessMask = vk::AccessFlagBits::eTransferWrite,
.oldLayout = std::exchange(layout, vk::ImageLayout::eGeneral),
.newLayout = vk::ImageLayout::eGeneral,
.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
.subresourceRange = {
.aspectMask = format->vkAspect,
.levelCount = levelCount,
.layerCount = layerCount,
},
});
auto bufferImageCopies{GetBufferImageCopies()};
commandBuffer.copyBufferToImage(stagingBuffer->vkBuffer, image, layout, vk::ArrayProxy(static_cast<u32>(bufferImageCopies.size()), bufferImageCopies.data())); commandBuffer.copyBufferToImage(stagingBuffer->vkBuffer, image, layout, vk::ArrayProxy(static_cast<u32>(bufferImageCopies.size()), bufferImageCopies.data()));
} }
@ -365,33 +371,7 @@ namespace skyline::gpu {
}, },
}); });
boost::container::small_vector<vk::BufferImageCopy, 10> bufferImageCopies; auto bufferImageCopies{GetBufferImageCopies()};
auto pushBufferImageCopyWithAspect{[&](vk::ImageAspectFlagBits aspect) {
vk::DeviceSize bufferOffset{};
u32 mipLevel{};
for (auto &level : mipLayouts) {
bufferImageCopies.emplace_back(
vk::BufferImageCopy{
.bufferOffset = bufferOffset,
.imageSubresource = {
.aspectMask = aspect,
.mipLevel = mipLevel++,
.layerCount = layerCount,
},
.imageExtent = level.dimensions,
}
);
bufferOffset += level.targetLinearSize * levelCount;
}
}};
if (format->vkAspect & vk::ImageAspectFlagBits::eColor)
pushBufferImageCopyWithAspect(vk::ImageAspectFlagBits::eColor);
if (format->vkAspect & vk::ImageAspectFlagBits::eDepth)
pushBufferImageCopyWithAspect(vk::ImageAspectFlagBits::eDepth);
if (format->vkAspect & vk::ImageAspectFlagBits::eStencil)
pushBufferImageCopyWithAspect(vk::ImageAspectFlagBits::eStencil);
commandBuffer.copyImageToBuffer(image, layout, stagingBuffer->vkBuffer, vk::ArrayProxy(static_cast<u32>(bufferImageCopies.size()), bufferImageCopies.data())); commandBuffer.copyImageToBuffer(image, layout, stagingBuffer->vkBuffer, vk::ArrayProxy(static_cast<u32>(bufferImageCopies.size()), bufferImageCopies.data()));
commandBuffer.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer, vk::PipelineStageFlagBits::eHost, {}, {}, vk::BufferMemoryBarrier{ commandBuffer.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer, vk::PipelineStageFlagBits::eHost, {}, {}, vk::BufferMemoryBarrier{

View File

@ -414,6 +414,11 @@ namespace skyline::gpu {
~TextureBufferCopy(); ~TextureBufferCopy();
}; };
/**
* @return A vector of all the buffer image copies that need to be done for every aspect of every level of every layer of the texture
*/
boost::container::small_vector<vk::BufferImageCopy, 10> GetBufferImageCopies();
public: public:
std::weak_ptr<FenceCycle> cycle; //!< A fence cycle for when any host operation mutating the texture has completed, it must be waited on prior to any mutations to the backing std::weak_ptr<FenceCycle> cycle; //!< A fence cycle for when any host operation mutating the texture has completed, it must be waited on prior to any mutations to the backing
std::optional<GuestTexture> guest; std::optional<GuestTexture> guest;