From 1e46a6709190d5264a2e0540f43480f99a603038 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 20:25:51 -0400 Subject: [PATCH] [MG_Backend, MG_IntegrationTest] (P7): the multisample depth/stencil copy-out pads its row stride to 4 bytes, so a narrow flipped stencil resolve puts every row region on a legal offset - ResolveWireDepthStencil moved the resolved aspect through a tightly packed buffer and, for a vertical mirror, started one region per row at row * width * texelSize. A depth/stencil buffer->image copy needs every bufferOffset on a multiple of 4 (VUID-vkCmdCopyBufferToImage-dstImage-07978), so a stencil aspect (1-byte texel) whose width is not a multiple of 4, or a D16 aspect with an odd width, was illegal on every row after the first; scissor clipping produces such widths. - paddedWidth = width rounded up to 4 / texelSize texels; the buffer is paddedWidth * height * texelSize, both copies name it as bufferRowLength, and each row region starts at row * paddedWidth * texelSize. - The flipped-resolve case gains a stencil leg: stencil bands 11/99 in the multisample source, a 29x32 flipped STENCIL_BUFFER_BIT blit from (5,8) to (20,4), and every stencil value of the destination checked against the mirrored band or the untouched prime. - Red-once (split + spawn, under VK_LOADER_LAYERS_ENABLE=*khronos_validation): lavapipe does not enforce the VU and the stencil bytes came back right on the unpadded code, so the red is the validation layer: 10 x "pRegions[1].bufferOffset (29) must be a multiple 4 if using a depth/stencil format (VK_FORMAT_D24_UNORM_S8_UINT)" on each arm before, 0 validation errors of any kind after. --- .../DirectVulkan/Renderer/WireFramebuffer.inc | 23 +++++--- .../DepthStencilReadbackMatrixScenario.cpp | 53 +++++++++++++++++-- 2 files changed, 67 insertions(+), 9 deletions(-) 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);