diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc index 486dcc9e..906d59cd 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc @@ -654,13 +654,25 @@ // this GPU buffer cannot replace the destination's other aspect. const VkDeviceSize texelSize = source.aspect == VK_IMAGE_ASPECT_STENCIL_BIT ? 1 : (source.format == VK_FORMAT_D16_UNORM || source.format == VK_FORMAT_D16_UNORM_S8_UINT ? 2 : 4); - const VkDeviceSize bytes = static_cast(width) * height * texelSize; + // THE ROW STRIDE IS PADDED TO 4 BYTES. A depth/stencil buffer<->image copy wants + // every bufferOffset on a multiple of 4 (VUID-vkCmdCopyBufferToImage-dstImage-07978, + // pRegions-07978 in older layers), and the mirror below starts a region on every + // row - so a tight row of 1-byte stencil texels whose width is not a multiple of 4, + // or of 2-byte D16 texels with an odd width, put every row after the first on an + // illegal offset. The scissor clip in BlitWireFramebuffers produces exactly such + // widths. Both copies name the padded length as bufferRowLength, so the buffer + // layout is one layout whichever way it is walked. + const VkDeviceSize texelsPer4Bytes = 4 / texelSize; + const VkDeviceSize paddedWidth = + (static_cast(width) + texelsPer4Bytes - 1) / texelsPer4Bytes * texelsPer4Bytes; + const VkDeviceSize bytes = paddedWidth * height * texelSize; aspectBuffer = MakeUnique(); if (!aspectBuffer->Create({.allocator = m_allocator, .size = bytes, .usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT, .memoryUsage = VMA_MEMORY_USAGE_AUTO_PREFER_DEVICE})) MagmaWireFatal("multisample-depth-resolve-buffer"); VkBufferImageCopy copy{}; + copy.bufferRowLength = static_cast(paddedWidth); copy.imageSubresource = {source.aspect, 0, 0, 1}; copy.imageOffset = {sxMin, syMin, 0}; copy.imageExtent = {static_cast(width), static_cast(height), 1}; @@ -682,12 +694,11 @@ } else { // THE VERTICAL MIRROR, and it costs one region per row rather than a pass. The // buffer holds the resolved band bottom row first (the image copy above wrote it - // tightly packed from syMin upwards); writing row r of that buffer to - // destination row dyMin + height - 1 - r reverses the band. bufferRowLength / - // bufferImageHeight stay 0: each region describes exactly one tight row, so the - // default "packed to imageExtent" is what is wanted. + // from syMin upwards, one padded stride per row); writing row r of that buffer + // to destination row dyMin + height - 1 - r reverses the band. Each region + // starts on a padded row, which is what keeps its bufferOffset a multiple of 4. Vector rows(static_cast(height), copy); - const VkDeviceSize rowBytes = static_cast(width) * texelSize; + const VkDeviceSize rowBytes = paddedWidth * texelSize; for (Int64 row = 0; row < height; ++row) { auto& region = rows[static_cast(row)]; region.bufferOffset = static_cast(row) * rowBytes; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp index 2ca95d50..f20db14e 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp @@ -457,26 +457,32 @@ namespace MGITest { DestroySource(multisampled); GTEST_SKIP() << "this driver cannot host a 4x multisample DEPTH24_STENCIL8 renderbuffer"; } + constexpr int kBottomStencil = 11; + constexpr int kTopStencil = 99; + constexpr int kPrimeStencil = 3; FirstGLError(); glDisable(GL_SCISSOR_TEST); glViewport(0, 0, kWidth, kHeight); glDepthMask(GL_TRUE); + glStencilMask(0xFFu); glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); glEnable(GL_SCISSOR_TEST); glScissor(0, 0, kWidth, kHeight / 2); glClearDepth(0.25); + glClearStencil(kBottomStencil); glClearColor(1, 0, 0, 1); - glClear(GL_DEPTH_BUFFER_BIT | GL_COLOR_BUFFER_BIT); + glClear(GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT); glScissor(0, kHeight / 2, kWidth, kHeight - kHeight / 2); glClearDepth(0.75); + glClearStencil(kTopStencil); glClearColor(0, 1, 0, 1); - glClear(GL_DEPTH_BUFFER_BIT | GL_COLOR_BUFFER_BIT); + glClear(GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT); glDisable(GL_SCISSOR_TEST); ASSERT_EQ(FirstGLError(), 0u) << "banding the multisample source"; DepthSource resolved = MakeTextureSource(GL_DEPTH24_STENCIL8); ASSERT_TRUE(SourceIsUsable()); - ClearDepthStencil(GL_DEPTH24_STENCIL8, 0.5f, 3); + ClearDepthStencil(GL_DEPTH24_STENCIL8, 0.5f, kPrimeStencil); glClearColor(0, 0, 1, 1); glClear(GL_COLOR_BUFFER_BIT); ASSERT_EQ(FirstGLError(), 0u) << "priming the resolve destination"; @@ -504,6 +510,47 @@ namespace MGITest { EXPECT_EQ(colour, (std::array{0, 255, 0, 255})) << "flipped resolve: colour and depth must agree about which way up the blit landed"; + // THE STENCIL ASPECT, NARROW AND OFFSET ON BOTH SIDES. The copy-out moves one aspect + // through a buffer one row per region, and a depth/stencil buffer<->image copy wants + // every bufferOffset on a multiple of 4 (VUID-vkCmdCopyBufferToImage-pRegions-07978). + // A stencil texel is ONE byte, so a tightly packed row whose width is not a multiple + // of 4 puts every row but the first on an illegal offset - which the 64-wide depth + // leg above (4-byte texels) can never show. 29 is odd, so a 2-byte (D16) texel would + // miss the alignment too. The rectangle also sits away from the origin on both sides + // (source x 5, y 8; destination x 20, y 4), so a copy-out that forgot the min corner + // on either side lands the band somewhere else. + { + constexpr int kSx = 5, kSy = 8, kDx = 20, kDy = 4, kW = 29, kH = 32; + glBindFramebuffer(GL_READ_FRAMEBUFFER, multisampled.fbo); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolved.fbo); + glBlitFramebuffer(kSx, kSy + kH, kSx + kW, kSy, kDx, kDy, kDx + kW, kDy + kH, + GL_STENCIL_BUFFER_BIT, GL_NEAREST); + glFinish(); + EXPECT_EQ(FirstGLError(), 0u) << "a flipped, narrow multisample stencil resolve is a legal blit"; + glBindFramebuffer(GL_FRAMEBUFFER, resolved.fbo); + const std::vector stencil = ReadStencilInt(0, 0, kWidth, kHeight); + EXPECT_EQ(FirstGLError(), 0u); + size_t bad = 0; + int firstX = -1, firstY = -1, firstGot = 0, firstWant = 0; + for (int y = 0; y < kHeight; ++y) { + for (int x = 0; x < kWidth; ++x) { + int want = kPrimeStencil; + if (x >= kDx && x < kDx + kW && y >= kDy && y < kDy + kH) { + const int sourceRow = kSy + kH - 1 - (y - kDy); // the mirror + want = sourceRow < kHeight / 2 ? kBottomStencil : kTopStencil; + } + const int got = stencil[static_cast(y) * kWidth + x]; + if (got != want) { + if (bad == 0) { firstX = x; firstY = y; firstGot = got; firstWant = want; } + ++bad; + } + } + } + EXPECT_EQ(bad, 0u) << "flipped narrow stencil resolve: " << bad << " of " << stencil.size() + << " stencil values are wrong; first at (" << firstX << ", " << firstY + << "): got " << firstGot << ", want " << firstWant; + } + // A SCALE, which is INVALID_OPERATION and must leave the destination as it is. ClearDepthStencil(GL_DEPTH24_STENCIL8, 0.5f, 3); ASSERT_EQ(FirstGLError(), 0u);