swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-13 22:58:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix, Test] (MG_Backend/DirectVulkan, MG_IntegrationTest): the barrier before every attachment transfer only ever moved layer 0, so a copy off a non-zero layer read a layout nothing had transitioned

Browse Source
This commit is contained in:
swung0x48
2026-08-13 04:01:20 -04:00
parent 91475a7b6f
commit b8233f9c4e
5 changed files with 437 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp
+18 -17
View File
@@ -300,8 +300,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
Bool ok = VkTextureManager::TransitionImageLayout(
commandBuffer, newResource.image, newResource.layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
0, VK_ACCESS_TRANSFER_WRITE_BIT, newResource.aspect, 0, newResource.mipLevels,
newResource.arrayLayers);
0, VK_ACCESS_TRANSFER_WRITE_BIT, newResource.aspect, 0, newResource.mipLevels);
MOBILEGL_ASSERT(ok, "PreserveTextureContentsOnRecreate: failed to prepare destination image");
VkImageLayout srcTrackedLayout = oldResource.layout;
@@ -311,8 +310,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
ok = VkTextureManager::TransitionImageLayout(
commandBuffer, oldResource.image, srcTrackedLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, oldResource.aspect, 0, preservedMipLevels,
oldResource.arrayLayers);
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, oldResource.aspect, 0, preservedMipLevels);
MOBILEGL_ASSERT(ok, "PreserveTextureContentsOnRecreate: failed to prepare source image");
Vector<VkImageCopy> copyRegions;
@@ -344,8 +342,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
ok = VkTextureManager::TransitionImageLayout(
commandBuffer, newResource.image, newResource.layout, oldResource.layout,
VK_PIPELINE_STAGE_TRANSFER_BIT, dstStageMask,
VK_ACCESS_TRANSFER_WRITE_BIT, dstAccessMask, newResource.aspect, 0, newResource.mipLevels,
newResource.arrayLayers);
VK_ACCESS_TRANSFER_WRITE_BIT, dstAccessMask, newResource.aspect, 0, newResource.mipLevels);
MOBILEGL_ASSERT(ok, "PreserveTextureContentsOnRecreate: failed to restore destination layout");
VK_VERIFY(vkEndCommandBuffer(commandBuffer), "vkEndCommandBuffer(texture preserve)");
@@ -1191,7 +1188,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
const Bool lowerTransitioned = TransitionImageLayout(
commandBuffer, resource.image, lowerMipLayout, newLayout,
srcStageMask, dstStageMask, srcAccessMask, dstAccessMask,
resource.aspect, 0, writtenMipLevel, resource.arrayLayers);
resource.aspect, 0, writtenMipLevel);
MOBILEGL_ASSERT(lowerTransitioned,
"UpdateTrackedImageLayoutAfterAttachmentWrite: failed to transition lower mip levels for textureId=%d",
texture->GetExternalIndex());
@@ -1203,8 +1200,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
const Bool upperTransitioned = TransitionImageLayout(
commandBuffer, resource.image, upperMipLayout, newLayout,
srcStageMask, dstStageMask, srcAccessMask, dstAccessMask,
resource.aspect, upperBaseMipLevel, resource.mipLevels - upperBaseMipLevel,
resource.arrayLayers);
resource.aspect, upperBaseMipLevel, resource.mipLevels - upperBaseMipLevel);
MOBILEGL_ASSERT(upperTransitioned,
"UpdateTrackedImageLayoutAfterAttachmentWrite: failed to transition upper mip levels for textureId=%d",
texture->GetExternalIndex());
@@ -1257,8 +1253,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
const Bool ok = TransitionImageLayout(commandBuffer, resource->image, resource->layout, targetLayout, srcStageMask,
s_sampledReadStages, srcAccessMask,
VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels,
resource->arrayLayers);
VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels);
MOBILEGL_ASSERT(ok, "TransitionTextureForSampling: transition failed for textureId=%d", texture.GetExternalIndex());
// Pre-pass stream bookkeeping: a command referencing the image was recorded.
StampResourceRecordingUse(*resource);
@@ -1288,7 +1283,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
VK_IMAGE_LAYOUT_GENERAL, srcStageMask,
VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, srcAccessMask,
VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT,
resource->aspect, 0, resource->mipLevels, resource->arrayLayers);
resource->aspect, 0, resource->mipLevels);
MOBILEGL_ASSERT(ok, "TransitionTextureForStorageImage: transition failed for textureId=%d",
texture.GetExternalIndex());
// Pre-pass stream bookkeeping: a command referencing the image was recorded.
@@ -1355,8 +1350,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
VkImageLayout& trackedLayout, VkImageLayout newLayout,
VkPipelineStageFlags srcStageMask, VkPipelineStageFlags dstStageMask,
VkAccessFlags srcAccessMask, VkAccessFlags dstAccessMask,
VkImageAspectFlags aspectMask, Uint32 baseMipLevel, Uint32 levelCount,
Uint32 layerCount) {
VkImageAspectFlags aspectMask, Uint32 baseMipLevel,
Uint32 levelCount) {
MOBILEGL_ASSERT(image != VK_NULL_HANDLE, "TransitionImageLayout: m_image == VK_NULL_HANDLE");
MOBILEGL_ASSERT(!((dstAccessMask & VK_ACCESS_TRANSFER_READ_BIT) != 0 &&
(dstStageMask & VK_PIPELINE_STAGE_TRANSFER_BIT) == 0),
@@ -1381,7 +1376,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
barrier.subresourceRange.baseMipLevel = baseMipLevel;
barrier.subresourceRange.levelCount = levelCount;
barrier.subresourceRange.baseArrayLayer = 0;
barrier.subresourceRange.layerCount = layerCount;
// Every layer, always - see the declaration for why layout tracking leaves no other
// correct answer. VK_REMAINING_ARRAY_LAYERS rather than the image's own `arrayLayers`
// because those are not the same number for a 3D image: MobileGL creates 3D images
// 2D_ARRAY_COMPATIBLE and their arrayLayers is 1, which today Vulkan reads as "all depth
// slices" but will read as "depth slice 0" once VK_KHR_maintenance9 is enabled. The
// validation layer warns about that literal 1 by name.
barrier.subresourceRange.layerCount = VK_REMAINING_ARRAY_LAYERS;
vkCmdPipelineBarrier(commandBuffer, srcStageMask, dstStageMask, 0, 0, nullptr, 0, nullptr, 1, &barrier);
trackedLayout = newLayout;
@@ -2605,7 +2606,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
VK_PIPELINE_STAGE_TRANSFER_BIT,
uploadSrcAccessMask,
VK_ACCESS_TRANSFER_WRITE_BIT,
aspectMask, 0, outResource.mipLevels, outResource.arrayLayers);
aspectMask, 0, outResource.mipLevels);
MOBILEGL_ASSERT(ok, "TransitionImageLayout to VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL failed");
// Array textures keep their GL "depth" in VkImage array layers, so the
@@ -2709,7 +2710,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
s_sampledReadStages,
VK_ACCESS_TRANSFER_WRITE_BIT,
VK_ACCESS_SHADER_READ_BIT,
aspectMask, 0, outResource.mipLevels, outResource.arrayLayers);
aspectMask, 0, outResource.mipLevels);
MOBILEGL_ASSERT(ok, "TransitionImageLayout to sampled read-only layout failed");
outResource.layout = finalLayout;
MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h
+14 -2
View File
@@ -388,12 +388,24 @@ public:
static Bool AreSampledImageViewFormatsCompatible(VkFormat imageFormat, VkFormat viewFormat);
static Bool AreStorageImageViewFormatsCompatible(VkFormat imageFormat, VkFormat viewFormat);
// Moves `image` to `newLayout` and writes the new layout back through `trackedLayout`.
//
// The barrier covers EVERY array layer of the image, and there is deliberately no layer
// parameter to say otherwise: layout here is tracked per IMAGE (one `TextureResource::layout`,
// or one caller-owned variable), so a barrier narrower than the image would leave the layers it
// skipped in the old layout while the tracker claims they moved. Every transfer against a
// framebuffer attachment above layer 0 - glReadPixels, glBlitFramebuffer, glCopyTexSubImage,
// glCopyImageSubData - then ran its copy on a layer no barrier had transitioned.
//
// The mip range IS a parameter, because mip levels really are transitioned piecewise (see
// UpdateTrackedImageLayoutAfterAttachmentWrite and the mipmap generation loops): those callers
// move the complement of the level they wrote so the whole image converges on one layout again.
// Nothing does, or can, do that per layer.
static Bool TransitionImageLayout(VkCommandBuffer commandBuffer, VkImage image, VkImageLayout& trackedLayout,
VkImageLayout newLayout, VkPipelineStageFlags srcStageMask,
VkPipelineStageFlags dstStageMask, VkAccessFlags srcAccessMask,
VkAccessFlags dstAccessMask, VkImageAspectFlags aspectMask,
Uint32 baseMipLevel = 0, Uint32 levelCount = 1,
Uint32 layerCount = 1);
Uint32 baseMipLevel = 0, Uint32 levelCount = 1);
SizeT CollectGarbage();
MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp
+26 -30
View File
@@ -7113,7 +7113,7 @@ void main() {
Bool ok = VkTextureManager::TransitionImageLayout(
commandBuffer, resource->image, resource->layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT,
resource->aspect, 0, resource->mipLevels, resource->arrayLayers);
resource->aspect, 0, resource->mipLevels);
MOBILEGL_ASSERT(ok,
"MaterializePendingClearForTexture: failed to transition textureId=%d to TRANSFER_DST",
texture.GetExternalIndex());
@@ -7231,8 +7231,7 @@ void main() {
ok = VkTextureManager::TransitionImageLayout(
commandBuffer, resource->image, clearLayout, sampledLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT,
VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels,
resource->arrayLayers);
VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels);
MOBILEGL_ASSERT(ok,
"MaterializePendingClearForTexture: failed to transition textureId=%d to sampled layout",
texture.GetExternalIndex());
@@ -7270,7 +7269,7 @@ void main() {
Bool ok = VkTextureManager::TransitionImageLayout(
commandBuffer, resource->image, resource->layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT,
resource->aspect, 0, 1, 1);
resource->aspect, 0, 1);
MOBILEGL_ASSERT(ok,
"MaterializePendingClearForRenderbuffer: failed to transition renderbuffer %u to TRANSFER_DST",
renderbuffer->GetExternalIndex());
@@ -7321,7 +7320,7 @@ void main() {
VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT |
VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT |
VK_ACCESS_TRANSFER_READ_BIT,
resource->aspect, 0, 1, 1);
resource->aspect, 0, 1);
MOBILEGL_ASSERT(ok,
"MaterializePendingClearForRenderbuffer: failed to transition renderbuffer %u to steady layout",
renderbuffer->GetExternalIndex());
@@ -7928,6 +7927,9 @@ void main() {
VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
VkAccessFlags srcAccessMask = 0;
GetImageTransitionSourceState(srcOriginalLayout, srcStageMask, srcAccessMask);
// Both blit regions below name `baseArrayLayer` from their binding, and a layered depth
// attachment puts that above 0. These barriers carry a mip range only - their layer
// range is every layer (see VkTextureManager::TransitionImageLayout).
if (readIsDefaultFbo) {
VkImageLayout srcTrackedLayout = srcOriginalLayout;
Bool ok = VkTextureManager::TransitionImageLayout(
@@ -8755,30 +8757,22 @@ void main() {
VkAccessFlags srcAccessMask = 0;
GetImageTransitionSourceState(srcOriginalLayout, srcStageMask, srcAccessMask);
VkImageLayout srcCopyLayout = srcOriginalLayout;
// The barrier has to name every layer the copy touches, not just layer 0 - otherwise the
// slice fix above lands the copy on layers the barrier never transitioned, which is the
// same defect one level down. TransitionImageLayout always starts its range at
// baseArrayLayer 0, so VK_REMAINING_ARRAY_LAYERS is the whole range and a superset of
// [baseSlice, baseSlice + depth).
//
// Not `arrayLayers`, which is 1 for a 3D image: MobileGL creates 3D images
// 2D_ARRAY_COMPATIBLE, and a literal 1 on one of those means "every depth slice" today but
// "depth slice 0" once VK_KHR_maintenance9 is enabled - i.e. it would silently become a
// single-slice barrier again on a newer driver. The validation layer says so by name.
static constexpr Uint32 kAllLayers = VK_REMAINING_ARRAY_LAYERS;
// The barriers below name a MIP range only. Their layer range is not a parameter:
// TransitionImageLayout always covers every layer of the image, which is a superset of the
// [baseSlice, baseSlice + depth) the slice mapping above hands the copy.
if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) {
Bool srcReady = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, srcResource->image, srcResource->layout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT,
srcResource->aspect, 0, srcResource->mipLevels, kAllLayers);
srcResource->aspect, 0, srcResource->mipLevels);
MOBILEGL_ASSERT(srcReady, "%s: failed to transition undefined source image", __func__);
srcCopyLayout = srcResource->layout;
} else {
Bool srcReady = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, srcResource->image, srcCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, copyAspectMask, srcMipLevel, 1, kAllLayers);
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, copyAspectMask, srcMipLevel, 1);
MOBILEGL_ASSERT(srcReady, "%s: failed to transition source image", __func__);
}
@@ -8791,14 +8785,14 @@ void main() {
frame.commandBuffer, dstResource->image, dstResource->layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
dstStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT,
dstResource->aspect, 0, dstResource->mipLevels, kAllLayers);
dstResource->aspect, 0, dstResource->mipLevels);
MOBILEGL_ASSERT(dstReady, "%s: failed to transition undefined destination image", __func__);
dstCopyLayout = dstResource->layout;
} else {
Bool dstReady = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, dstResource->image, dstCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
dstStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, copyAspectMask, dstMipLevel, 1, kAllLayers);
dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, copyAspectMask, dstMipLevel, 1);
MOBILEGL_ASSERT(dstReady, "%s: failed to transition destination image", __func__);
}
@@ -8840,13 +8834,13 @@ void main() {
frame.commandBuffer, srcResource->image, srcResource->layout, srcRestoreLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask,
VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask,
srcResource->aspect, 0, srcResource->mipLevels, kAllLayers);
srcResource->aspect, 0, srcResource->mipLevels);
MOBILEGL_ASSERT(srcRestored, "%s: failed to restore undefined source image layout", __func__);
} else {
Bool srcRestored = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, srcResource->image, srcCopyLayout, srcRestoreLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask,
VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, copyAspectMask, srcMipLevel, 1, kAllLayers);
VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, copyAspectMask, srcMipLevel, 1);
MOBILEGL_ASSERT(srcRestored, "%s: failed to restore source image layout", __func__);
}
@@ -8858,13 +8852,13 @@ void main() {
frame.commandBuffer, dstResource->image, dstResource->layout, dstRestoreLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask,
VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask,
dstResource->aspect, 0, dstResource->mipLevels, kAllLayers);
dstResource->aspect, 0, dstResource->mipLevels);
MOBILEGL_ASSERT(dstRestored, "%s: failed to restore undefined destination image layout", __func__);
} else {
Bool dstRestored = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, dstResource->image, dstCopyLayout, dstRestoreLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask,
VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, copyAspectMask, dstMipLevel, 1, kAllLayers);
VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, copyAspectMask, dstMipLevel, 1);
MOBILEGL_ASSERT(dstRestored, "%s: failed to restore destination image layout", __func__);
}
@@ -9023,6 +9017,9 @@ void main() {
VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
VkAccessFlags srcAccessMask = 0;
GetImageTransitionSourceState(srcOriginalLayout, srcStageMask, srcAccessMask);
// The copy below reads `srcBinding.baseArrayLayer`, which for a glFramebufferTextureLayer
// attachment is any layer of the array - the barrier covers all of them (see
// VkTextureManager::TransitionImageLayout), so the layer being read is one it moved.
if (readIsDefaultFbo) {
VkImageLayout trackedLayout = srcOriginalLayout;
Bool ok = VkTextureManager::TransitionImageLayout(
@@ -9825,14 +9822,13 @@ void main() {
VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
VkAccessFlags srcAccessMask = 0;
GetImageTransitionSourceState(originalLayout, srcStageMask, srcAccessMask);
// The copy below reads EVERY layer of the level, so the barrier has to name every layer
// too; a layerCount of 1 left an array texture's layers 1.. in whatever layout they were
// last left in while the transfer read them.
// The copy below reads EVERY layer of the level, which is exactly the range
// TransitionImageLayout barriers cover.
Bool ok = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, resource->image, resource->layout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, resource->aspect,
static_cast<Uint32>(level), 1, VK_REMAINING_ARRAY_LAYERS);
static_cast<Uint32>(level), 1);
MOBILEGL_ASSERT(ok, "%s: failed to transition texture image", __func__);
VkBufferImageCopy copyRegion{};
@@ -9852,7 +9848,7 @@ void main() {
frame.commandBuffer, resource->image, resource->layout, originalLayout,
VK_PIPELINE_STAGE_TRANSFER_BIT, restoreStageMask,
VK_ACCESS_TRANSFER_READ_BIT, restoreAccessMask, resource->aspect,
static_cast<Uint32>(level), 1, VK_REMAINING_ARRAY_LAYERS);
static_cast<Uint32>(level), 1);
MOBILEGL_ASSERT(ok, "%s: failed to restore texture image layout", __func__);
if (!SubmitReadbackCommandsAndWait(frame)) {
@@ -9960,7 +9956,7 @@ void main() {
Bool transitioned = VkTextureManager::TransitionImageLayout(
frame.commandBuffer, resource->image, resource->layout, finalLayout,
VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT,
0, VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels, resource->arrayLayers);
0, VK_ACCESS_SHADER_READ_BIT, resource->aspect, 0, resource->mipLevels);
MOBILEGL_ASSERT(transitioned, "GenerateMipmap: failed to transition uninitialized mip chain");
return;
}