diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp index 749f353e..c1a008fe 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp @@ -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 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; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h index be6b37cd..5ad1771c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h @@ -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(); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 0b681470..c6f8caf3 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -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(level), 1, VK_REMAINING_ARRAY_LAYERS); + static_cast(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(level), 1, VK_REMAINING_ARRAY_LAYERS); + static_cast(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; } diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 8f6ae23a..5a0109eb 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -80,6 +80,7 @@ add_executable(MobileGLIntegrationTest Scenarios/VertexArrayEnableDisableScenario.cpp Scenarios/CopyImageLevelRangeScenario.cpp Scenarios/CopyImageLayeredScenario.cpp + Scenarios/LayeredAttachmentBarrierScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/LayeredAttachmentBarrierScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/LayeredAttachmentBarrierScenario.cpp new file mode 100755 index 00000000..29b6a736 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/LayeredAttachmentBarrierScenario.cpp @@ -0,0 +1,378 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/LayeredAttachmentBarrierScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - A TRANSFER OFF A NON-ZERO ATTACHMENT LAYER READS THE LAYER THE BARRIER MOVED. +// +// Every transfer DirectVulkan performs against a framebuffer attachment is three commands: a +// barrier that puts the image in TRANSFER_SRC/DST, the copy or blit itself, and a barrier that +// puts it back. The copy names the attachment's layer - glFramebufferTextureLayer(.., layer) ends +// up in `srcSubresource.baseArrayLayer` - but TransitionImageLayout used to emit `layerCount = 1` +// from `baseArrayLayer 0`, so for every attachment on a layer above zero the barrier moved layer 0 +// and the copy read layer N. The layer the transfer touched was never transitioned: it sat in +// COLOR_ATTACHMENT_OPTIMAL (or DEPTH_STENCIL_ATTACHMENT_OPTIMAL) while being read as TRANSFER_SRC. +// +// That is undefined behaviour, not a guaranteed wrong pixel: a layout is a compression/tiling +// promise, so a driver that stores both layouts identically returns the right bytes anyway. The +// software lanes (lavapipe) are exactly such a driver, which is why this scenario is paired with a +// validation-layer run - the layer names the mismatch outright +// (VUID-vkCmdCopyImageToBuffer-srcImageLayout-00189, "srcImageLayout ... doesn't match the actual +// current layout") where the pixels here cannot. On a tiler that really does re-tile per layout, +// these are the reads that come back as garbage. +// +// The four cases below are the four transfer paths that take an attachment layer from GL: +// +// glReadPixels (colour) -> VulkanRenderer::ReadPixels +// glBlitFramebuffer (colour) -> VulkanRenderer::BlitNamedFramebuffer +// glReadPixels (GL_DEPTH_COMPONENT) -> VulkanRenderer::ReadDepthStencilImageToClient +// glBlitFramebuffer (GL_DEPTH_BUFFER_BIT) -> VulkanRenderer::BlitNamedFramebuffer, depth leg +// +// Each one renders or clears INTO the non-zero layer first, so the image is genuinely sitting in +// its attachment layout when the transfer starts - a scenario that only uploaded texels would +// leave it in a transfer layout already and the mismatched barrier would be a no-op. +// +// Every case also asserts the layers it did not name still hold their own fill, so a backend that +// "fixed" the miss by transferring the whole image passes neither half. +// +// DirectGLES is the control: it hands the same calls to the driver, so a failure on both backends +// means the scenario is wrong and a failure on DirectVulkan alone means Magma is. + +#include +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr int kWidth = 8; + constexpr int kHeight = 8; + // Four layers with the subject at index 2: layers on both sides of it stay untouched, so + // "moved the whole image" and "moved layer 0" are both distinguishable from correct. + constexpr int kLayers = 4; + constexpr int kSubjectLayer = 2; + + // A value no correct read can produce, so "the backend wrote nothing" fails loudly. + constexpr float kDepthPoison = 0.2f; + + std::string Describe(const Rgba8& color) { + return "(" + std::to_string(color.r) + ", " + std::to_string(color.g) + ", " + std::to_string(color.b) + + ", " + std::to_string(color.a) + ")"; + } + + // Per-layer fill, uniform within a layer: the defect is about WHICH layer is addressed, and + // a value that also varied inside the layer would make the assertions depend on row order. + Rgba8 LayerFill(int layer) { + return {static_cast(17 + layer * 30), static_cast(200 - layer * 25), + static_cast(60 + layer * 40), 255}; + } + + // What the draw paints - matches kFS below, and is deliberately none of the LayerFill + // values so "the draw never landed" cannot read as a pass. + constexpr Rgba8 kPaintedColor{26, 51, 204, 255}; + + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +void main() { gl_Position = vec4(aPos, 0.0, 1.0); } +)"; + + constexpr const char* kFS = R"(#version 330 core +out vec4 o_color; +void main() { o_color = vec4(0.1, 0.2, 0.8, 1.0); } +)"; + + void DrawFullViewportQuad(unsigned int program) { + static const float kQuad[] = {-1.0f, -1.0f, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, 1.0f}; + GLuint vao = 0, vbo = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + glGenBuffers(1, &vbo); + glBindBuffer(GL_ARRAY_BUFFER, vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(kQuad), kQuad, GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 2 * sizeof(float), nullptr); + glUseProgram(program); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + glBindVertexArray(0); + glDeleteBuffers(1, &vbo); + glDeleteVertexArrays(1, &vao); + } + + class LayeredAttachmentBarrierScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + std::string error; + m_program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(m_program, 0u) << error; + } + + void TearDown() override { + if (!Ready()) return; + glBindFramebuffer(GL_FRAMEBUFFER, 0); + for (const GLuint fbo : m_fbos) { + glDeleteFramebuffers(1, &fbo); + } + m_fbos.clear(); + for (const GLuint texture : m_textures) { + glDeleteTextures(1, &texture); + } + m_textures.clear(); + if (m_program != 0) { + glUseProgram(0); + glDeleteProgram(m_program); + m_program = 0; + } + } + + // An RGBA8 2D array with a different uniform colour per layer. + GLuint MakeColorArray() { + GLuint texture = 0; + glGenTextures(1, &texture); + m_textures.push_back(texture); + glBindTexture(GL_TEXTURE_2D_ARRAY, texture); + glTexStorage3D(GL_TEXTURE_2D_ARRAY, 1, GL_RGBA8, kWidth, kHeight, kLayers); + glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + for (int layer = 0; layer < kLayers; ++layer) { + const std::vector texels(static_cast(kWidth) * kHeight, LayerFill(layer)); + glTexSubImage3D(GL_TEXTURE_2D_ARRAY, 0, 0, 0, layer, kWidth, kHeight, 1, GL_RGBA, + GL_UNSIGNED_BYTE, texels.data()); + } + glBindTexture(GL_TEXTURE_2D_ARRAY, 0); + return texture; + } + + // A depth 2D array. No initial upload: depth arrays are filled by clearing through an + // attachment, which is also the state the transfer paths have to cope with. + GLuint MakeDepthArray() { + GLuint texture = 0; + glGenTextures(1, &texture); + m_textures.push_back(texture); + glBindTexture(GL_TEXTURE_2D_ARRAY, texture); + glTexStorage3D(GL_TEXTURE_2D_ARRAY, 1, GL_DEPTH_COMPONENT24, kWidth, kHeight, kLayers); + glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glBindTexture(GL_TEXTURE_2D_ARRAY, 0); + return texture; + } + + // One FBO naming `layer` of the given arrays. Depth is optional (0 = colour only). + GLuint MakeLayerFbo(GLuint colorArray, GLuint depthArray, int layer) { + GLuint fbo = 0; + glGenFramebuffers(1, &fbo); + m_fbos.push_back(fbo); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, colorArray, 0, layer); + if (depthArray != 0) { + glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, depthArray, 0, layer); + } + EXPECT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), static_cast(GL_FRAMEBUFFER_COMPLETE)) + << "layer " << layer << " is not attachable"; + return fbo; + } + + // glReadPixels of one whole layer, through an FBO that names it. + Rgba8 ReadLayer(GLuint colorArray, int layer) { + const GLuint fbo = MakeLayerFbo(colorArray, 0, layer); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glReadBuffer(GL_COLOR_ATTACHMENT0); + glPixelStorei(GL_PACK_ALIGNMENT, 1); + std::vector pixels(static_cast(kWidth) * kHeight, Rgba8{}); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data()); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + // The fill is uniform within a layer, so any disagreement between texels is itself + // a failure - reported here rather than silently reduced to pixels[0]. + for (std::size_t i = 1; i < pixels.size(); ++i) { + EXPECT_TRUE(pixels[i] == pixels[0]) + << "layer " << layer << " is not uniform: texel 0 is " << Describe(pixels[0]) << ", texel " + << i << " is " << Describe(pixels[i]); + } + return pixels[0]; + } + + // Every layer but `changed` still holds its own fill. + void ExpectOtherLayersUntouched(GLuint colorArray, int changed, const char* what) { + for (int layer = 0; layer < kLayers; ++layer) { + if (layer == changed) continue; + const Rgba8 actual = ReadLayer(colorArray, layer); + EXPECT_TRUE(actual == LayerFill(layer)) + << what << ": layer " << layer << " should still hold its fill but is " << Describe(actual) + << ", expected " << Describe(LayerFill(layer)); + } + } + + float ReadDepthAt(int x, int y) const { + float depth = kDepthPoison; + glReadPixels(x, y, 1, 1, GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + return depth; + } + + std::vector m_textures; + std::vector m_fbos; + unsigned int m_program = 0; + }; + + // glReadPixels straight off a layer that was just rendered to. The image is in + // COLOR_ATTACHMENT_OPTIMAL when the readback barrier runs, so the barrier and the copy + // disagreeing about the layer is a live layout mismatch, not a bookkeeping detail. + TEST_F(LayeredAttachmentBarrierScenario, ReadPixelsOffRenderedNonZeroLayer) { + if (!Ready()) return; + + const GLuint colorArray = MakeColorArray(); + ASSERT_EQ(FirstGLError(), 0u) << "texture setup failed"; + + const GLuint fbo = MakeLayerFbo(colorArray, 0, kSubjectLayer); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glViewport(0, 0, kWidth, kHeight); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDrawBuffer(GL_COLOR_ATTACHMENT0); + DrawFullViewportQuad(m_program); + + glReadBuffer(GL_COLOR_ATTACHMENT0); + glPixelStorei(GL_PACK_ALIGNMENT, 1); + std::vector pixels(static_cast(kWidth) * kHeight, Rgba8{}); + glReadPixels(0, 0, kWidth, kHeight, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data()); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + EXPECT_EQ(FirstGLError(), 0u); + + for (std::size_t i = 0; i < pixels.size(); ++i) { + ASSERT_NEAR(pixels[i].r, kPaintedColor.r, 2) + << "texel " << i << " of the rendered layer is " << Describe(pixels[i]); + ASSERT_NEAR(pixels[i].g, kPaintedColor.g, 2) << "texel " << i; + ASSERT_NEAR(pixels[i].b, kPaintedColor.b, 2) << "texel " << i; + } + + ExpectOtherLayersUntouched(colorArray, kSubjectLayer, "readback off a rendered layer"); + } + + // glBlitFramebuffer between two non-zero layers of two different arrays. Both endpoints are + // above layer 0, so the source and destination barriers are each wrong on their own side. + TEST_F(LayeredAttachmentBarrierScenario, BlitBetweenNonZeroColorLayers) { + if (!Ready()) return; + + const GLuint sourceArray = MakeColorArray(); + const GLuint destinationArray = MakeColorArray(); + ASSERT_EQ(FirstGLError(), 0u) << "texture setup failed"; + + constexpr int kSourceLayer = 3; + constexpr int kDestinationLayer = 1; + + const GLuint sourceFbo = MakeLayerFbo(sourceArray, 0, kSourceLayer); + glBindFramebuffer(GL_FRAMEBUFFER, sourceFbo); + glViewport(0, 0, kWidth, kHeight); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDrawBuffer(GL_COLOR_ATTACHMENT0); + DrawFullViewportQuad(m_program); + + const GLuint destinationFbo = MakeLayerFbo(destinationArray, 0, kDestinationLayer); + glBindFramebuffer(GL_READ_FRAMEBUFFER, sourceFbo); + glReadBuffer(GL_COLOR_ATTACHMENT0); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, destinationFbo); + glDrawBuffer(GL_COLOR_ATTACHMENT0); + glBlitFramebuffer(0, 0, kWidth, kHeight, 0, 0, kWidth, kHeight, GL_COLOR_BUFFER_BIT, GL_NEAREST); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + EXPECT_EQ(FirstGLError(), 0u); + + const Rgba8 blitted = ReadLayer(destinationArray, kDestinationLayer); + EXPECT_NEAR(blitted.r, kPaintedColor.r, 2) << "blit destination layer is " << Describe(blitted); + EXPECT_NEAR(blitted.g, kPaintedColor.g, 2); + EXPECT_NEAR(blitted.b, kPaintedColor.b, 2); + + ExpectOtherLayersUntouched(destinationArray, kDestinationLayer, "colour blit destination"); + // The source layer was rendered, not blitted into, so it is checked separately. + const Rgba8 source = ReadLayer(sourceArray, kSourceLayer); + EXPECT_NEAR(source.r, kPaintedColor.r, 2) << "blit source layer is " << Describe(source); + ExpectOtherLayersUntouched(sourceArray, kSourceLayer, "colour blit source"); + } + + // The depth aspect of the same readback path: the depth image sits in + // DEPTH_STENCIL_ATTACHMENT_OPTIMAL after the clear, and the copy names the attached layer. + TEST_F(LayeredAttachmentBarrierScenario, ReadDepthOffClearedNonZeroLayer) { + if (!Ready()) return; + + const GLuint colorArray = MakeColorArray(); + const GLuint depthArray = MakeDepthArray(); + ASSERT_EQ(FirstGLError(), 0u) << "texture setup failed"; + + const GLuint fbo = MakeLayerFbo(colorArray, depthArray, kSubjectLayer); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glViewport(0, 0, kWidth, kHeight); + glDisable(GL_SCISSOR_TEST); + glDepthMask(GL_TRUE); + glClearDepth(0.375); + glClear(GL_DEPTH_BUFFER_BIT); + + const float centre = ReadDepthAt(kWidth / 2, kHeight / 2); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_NEAR(centre, 0.375f, 1.0f / 4096.0f) + << "glReadPixels(GL_DEPTH_COMPONENT) off layer " << kSubjectLayer << " returned " << centre + << (std::fabs(centre - kDepthPoison) < 1e-6f ? " - the destination was never written at all" : ""); + } + + // The depth leg of the blit path, both endpoints above layer 0. Verified by reading the + // destination's depth back, which is the same readback the case above pins - so a failure + // here with that one passing is the blit, not the readback. + TEST_F(LayeredAttachmentBarrierScenario, BlitDepthBetweenNonZeroLayers) { + if (!Ready()) return; + + const GLuint sourceColor = MakeColorArray(); + const GLuint sourceDepth = MakeDepthArray(); + const GLuint destinationColor = MakeColorArray(); + const GLuint destinationDepth = MakeDepthArray(); + ASSERT_EQ(FirstGLError(), 0u) << "texture setup failed"; + + constexpr int kSourceLayer = 3; + constexpr int kDestinationLayer = 1; + + const GLuint sourceFbo = MakeLayerFbo(sourceColor, sourceDepth, kSourceLayer); + glBindFramebuffer(GL_FRAMEBUFFER, sourceFbo); + glViewport(0, 0, kWidth, kHeight); + glDisable(GL_SCISSOR_TEST); + glDepthMask(GL_TRUE); + glClearDepth(0.625); + glClear(GL_DEPTH_BUFFER_BIT); + + // A destination pre-cleared to something the blit must overwrite, so "the blit did + // nothing" and "the blit landed" are different answers. + const GLuint destinationFbo = MakeLayerFbo(destinationColor, destinationDepth, kDestinationLayer); + glBindFramebuffer(GL_FRAMEBUFFER, destinationFbo); + glViewport(0, 0, kWidth, kHeight); + glDepthMask(GL_TRUE); + glClearDepth(0.125); + glClear(GL_DEPTH_BUFFER_BIT); + + glBindFramebuffer(GL_READ_FRAMEBUFFER, sourceFbo); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, destinationFbo); + glBlitFramebuffer(0, 0, kWidth, kHeight, 0, 0, kWidth, kHeight, GL_DEPTH_BUFFER_BIT, GL_NEAREST); + EXPECT_EQ(FirstGLError(), 0u); + + glBindFramebuffer(GL_FRAMEBUFFER, destinationFbo); + const float blitted = ReadDepthAt(kWidth / 2, kHeight / 2); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_NEAR(blitted, 0.625f, 1.0f / 4096.0f) + << "depth blitted onto layer " << kDestinationLayer << " reads back as " << blitted + << (std::fabs(blitted - 0.125f) < 1e-3f ? " - the destination kept its own clear" : ""); + } + + } // namespace +} // namespace MGITest