swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_Backend, MG_IntegrationTest] (P7): a flipped multisample depth or stencil resolve lands mirrored and a scaled one declines, and neither ends the session

Browse Source 
ID-P7-31 review, should-fix 2.

ResolveWireDepthStencil required sx1 > sx0, sy1 > sy0 and identical signed spans, and
took the session with Magma:multisample-depth-resolve-region otherwise - so
glBlitFramebuffer(0, h, w, 0, 0, 0, w, h, COLOR|DEPTH) out of a multisample framebuffer
resolved its colour correctly and then died on its depth. GL 4.6 core 18.3.1 asks for
identical DIMENSIONS, not identical corners.

  * spans are signed; everything downstream (renderArea, the shader arm's rectangle, the
    buffer copy offsets, BlitDepthAcrossFormats' corners) works off the min corner;
  * a SCALE is INVALID_OPERATION in 18.3.1 and now declines by rule I (a) with the colour
    arm's three lines (MGLOG_E_ONCE, RecordTextureCopyError, return);
  * a same-format Y mirror is done in the copy-out: one VkBufferImageCopy per row,
    buffer row r written to destination row dyMin + height - 1 - r;
  * an X mirror (a region per texel is not a copy) and a Y mirror across two formats
    (BlitDepthAcrossFormats, shared with the monolith, has no flip) decline by name.

The region Fatal stays for a rectangle outside either image.

Scenario DepthStencilReadbackMatrixScenario.AFlippedMultisampleResolveMirrorsTheBandsAnd-
AScaleDeclines, registered as MsFlip. on all three magma arms (no knob): unequal depth
bands, COLOR|DEPTH in one flipped call with colour and depth asserted the same way up,
then a scaled depth blit that must be INVALID_OPERATION and leave the destination alone.

Red once (R-16, executed and reverted, split and spawn each):
  * the old guard restored -> both arms die by
    Fatal{UnmigratedVerb, "Magma:multisample-depth-resolve-region"};
  * the scale decline turned back into that Fatal -> the flip half passes, both arms die
    at the scale;
  * the per-row copy-out bypassed (if (!mirrorY) forced true) -> both arms live and fail
    "flipped resolve: the destination's bottom band: 768 of 768 depth values differ from
    0.75; first bad value 0.25" (and the top band the other way).
This commit is contained in:
swung0x48
2026-09-22 20:05:28 -04:00
parent 9fed478dfd
commit 645dd96f69
3 changed files with 188 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc
+84 -14
View File
@@ -434,15 +434,65 @@
void VulkanRenderer::ResolveWireDepthStencil(WireImage source, WireImage destination,
GLint sx0, GLint sy0, GLint sx1, GLint sy1, GLint dx0, GLint dy0, GLint dx1, GLint dy1) {
const Int64 width = static_cast<Int64>(sx1) - sx0;
const Int64 height = static_cast<Int64>(sy1) - sy0;
if (!width || !height) return;
// P7 wave 2-B2 (review round): A REVERSED RECTANGLE IS NOT A MALFORMED ONE. This guard
// used to require `sx1 > sx0` and `sy1 > sy0` and take the session with a named Fatal
// otherwise - so `glBlitFramebuffer(0, h, w, 0, 0, 0, w, h, COLOR|DEPTH)` out of a
// multisample framebuffer blitted its COLOUR correctly (the colour arm has handled a
// flip since P5f and now handles a scale too) and then died on its DEPTH. GL 4.6 core
// 18.3.1 asks for identical DIMENSIONS, not identical corners - Mesa compares absolute
// spans - so a flip is a legal blit that has to produce a mirrored picture.
//
// Spans are therefore signed and everything downstream works off the min corner.
const Int64 spanX = static_cast<Int64>(sx1) - sx0;
const Int64 spanY = static_cast<Int64>(sy1) - sy0;
const Int64 dstSpanX = static_cast<Int64>(dx1) - dx0;
const Int64 dstSpanY = static_cast<Int64>(dy1) - dy0;
if (!spanX || !spanY || !dstSpanX || !dstSpanY) return;
const Int64 width = spanX < 0 ? -spanX : spanX;
const Int64 height = spanY < 0 ? -spanY : spanY;
// A SCALE has no arm here and is not supposed to: 18.3.1 makes a multisample blit whose
// rectangles differ in size INVALID_OPERATION. Rule I (a), the same three lines the
// colour arm's multisample-destination decline uses.
if (width != (dstSpanX < 0 ? -dstSpanX : dstSpanX) ||
height != (dstSpanY < 0 ? -dstSpanY : dstSpanY)) {
MGLOG_E_ONCE("DirectVulkan::ResolveWireDepthStencil: a %lldx%lld source rectangle "
"cannot reach a %lldx%lld destination rectangle - a multisample resolve "
"carries no scale; declining the blit",
static_cast<long long>(width), static_cast<long long>(height),
static_cast<long long>(dstSpanX < 0 ? -dstSpanX : dstSpanX),
static_cast<long long>(dstSpanY < 0 ? -dstSpanY : dstSpanY));
RecordTextureCopyError("ResolveWireDepthStencil", ErrorCode::InvalidOperation,
"glBlitFramebuffer from a multisampled read framebuffer requires "
"identical source and destination rectangle dimensions.");
return;
}
const Bool mirrorX = (spanX < 0) != (dstSpanX < 0);
const Bool mirrorY = (spanY < 0) != (dstSpanY < 0);
const GLint sxMin = std::min(sx0, sx1), syMin = std::min(sy0, sy1);
const GLint dxMin = std::min(dx0, dx1), dyMin = std::min(dy0, dy1);
if (source.isDefault || destination.isDefault || source.is3D || destination.is3D ||
width <= 0 || height <= 0 || width != static_cast<Int64>(dx1) - dx0 ||
height != static_cast<Int64>(dy1) - dy0 || sx0 < 0 || sy0 < 0 || dx0 < 0 || dy0 < 0 ||
static_cast<Uint64>(sx1) > source.extent.width || static_cast<Uint64>(sy1) > source.extent.height ||
static_cast<Uint64>(dx1) > destination.extent.width || static_cast<Uint64>(dy1) > destination.extent.height)
sxMin < 0 || syMin < 0 || dxMin < 0 || dyMin < 0 ||
static_cast<Uint64>(sxMin + width) > source.extent.width ||
static_cast<Uint64>(syMin + height) > source.extent.height ||
static_cast<Uint64>(dxMin + width) > destination.extent.width ||
static_cast<Uint64>(dyMin + height) > destination.extent.height)
MagmaWireFatal("multisample-depth-resolve-region");
// WHICH MIRRORS HAVE AN ARM. The copy-out below moves the resolved aspect through a
// buffer, and a buffer copy addresses ROWS: one VkBufferImageCopy per row, walked
// backwards, is a vertical mirror for free. A HORIZONTAL one would need a region per
// TEXEL, which is not a copy any more - and re-encoding across formats
// (BlitDepthAcrossFormats, shared with the monolith arm) has no flip either. Both
// decline by name; §12 in notes/p7/magma-b2.md carries them.
if (mirrorX || (mirrorY && source.format != destination.format)) {
MGLOG_E_ONCE("DirectVulkan::ResolveWireDepthStencil: a multisample resolve mirrored on "
"%s (source format %d, destination format %d) has no arm - the copy-out "
"reverses whole rows and nothing smaller; declining the blit",
mirrorX ? "X" : "Y across two formats",
static_cast<Int>(source.format), static_cast<Int>(destination.format));
RecordTextureCopyError("ResolveWireDepthStencil", ErrorCode::InvalidOperation,
"This device cannot mirror that multisample depth/stencil blit.");
return;
}
VkImageAspectFlags formatAspects = VK_IMAGE_ASPECT_DEPTH_BIT;
if (source.format == VK_FORMAT_S8_UINT) formatAspects = VK_IMAGE_ASPECT_STENCIL_BIT;
else if (source.format == VK_FORMAT_D16_UNORM_S8_UINT || source.format == VK_FORMAT_D24_UNORM_S8_UINT ||
@@ -554,10 +604,10 @@
VkRenderPassBeginInfo begin{VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO};
begin.renderPass = pass;
begin.framebuffer = framebuffer;
begin.renderArea = {{sx0, sy0}, {static_cast<Uint32>(width), static_cast<Uint32>(height)}};
begin.renderArea = {{sxMin, syMin}, {static_cast<Uint32>(width), static_cast<Uint32>(height)}};
vkCmdBeginRenderPass(m_frameContext.GetCurrent().commandBuffer, &begin, VK_SUBPASS_CONTENTS_INLINE);
vkCmdEndRenderPass(m_frameContext.GetCurrent().commandBuffer);
} else if (!ResolveWireDepthStencilWithShader(source, resolved, source.aspect, sx0, sy0,
} else if (!ResolveWireDepthStencilWithShader(source, resolved, source.aspect, sxMin, syMin,
static_cast<Uint32>(width), static_cast<Uint32>(height))) {
// NEITHER ARM, and rule I (a) says what that is: this device has no
// VK_KHR_depth_stencil_resolve and the pass cannot stand in for it either - the
@@ -612,7 +662,7 @@
MagmaWireFatal("multisample-depth-resolve-buffer");
VkBufferImageCopy copy{};
copy.imageSubresource = {source.aspect, 0, 0, 1};
copy.imageOffset = {sx0, sy0, 0};
copy.imageOffset = {sxMin, syMin, 0};
copy.imageExtent = {static_cast<Uint32>(width), static_cast<Uint32>(height), 1};
vkCmdCopyImageToBuffer(frame.commandBuffer, resolved.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
aspectBuffer->GetHandle(), 1, &copy);
@@ -625,16 +675,36 @@
vkCmdPipelineBarrier(frame.commandBuffer, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
0, 0, nullptr, 1, &barrier, 0, nullptr);
copy.imageSubresource = {source.aspect, destination.level, destination.layer, 1};
copy.imageOffset = {dx0, dy0, 0};
vkCmdCopyBufferToImage(frame.commandBuffer, aspectBuffer->GetHandle(), destination.image,
VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &copy);
copy.imageOffset = {dxMin, dyMin, 0};
if (!mirrorY) {
vkCmdCopyBufferToImage(frame.commandBuffer, aspectBuffer->GetHandle(), destination.image,
VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &copy);
} 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.
Vector<VkBufferImageCopy> rows(static_cast<SizeT>(height), copy);
const VkDeviceSize rowBytes = static_cast<VkDeviceSize>(width) * texelSize;
for (Int64 row = 0; row < height; ++row) {
auto& region = rows[static_cast<SizeT>(row)];
region.bufferOffset = static_cast<VkDeviceSize>(row) * rowBytes;
region.imageOffset = {dxMin, static_cast<Int32>(dyMin + height - 1 - row), 0};
region.imageExtent = {static_cast<Uint32>(width), 1, 1};
}
vkCmdCopyBufferToImage(frame.commandBuffer, aspectBuffer->GetHandle(), destination.image,
VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
static_cast<Uint32>(rows.size()), rows.data());
}
} else {
// A logical GL depth format may use different native renderbuffer
// and texture formats. Re-encode only after producing real resolved
// samples. Keep all scratch objects local across this helper's flush.
if (!BlitDepthAcrossFormats(frame, resolved.image, resolved.format, &scratch->layout, 0, 0,
destination.image, destination.format, destination.trackedLayout, destination.level, destination.layer,
sx0, sy0, dx0, dy0, static_cast<GLint>(width), static_cast<GLint>(height),
sxMin, syMin, dxMin, dyMin, static_cast<GLint>(width), static_cast<GLint>(height),
VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
source.aspect == VK_IMAGE_ASPECT_STENCIL_BIT))
MagmaWireFatal("multisample-depth-resolve-conversion");
MobileGL/MG_IntegrationTest/CMakeLists.txt
+11
View File
@@ -3206,6 +3206,17 @@ if (MOBILEGL_BUILD_DISAGGREGATED)
${mglItestArmProperties}
ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MGITEST_MAGMA_FORCE_SHADER_DEPTH_RESOLVE=${shaderDepthResolve}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-msresolve${mglItestArmLogTag}-shader${shaderDepthResolve}.log")
endforeach()
# P7 wave 2-B2 (review round): a FLIPPED multisample depth/stencil resolve is a legal blit
# that used to end the session, and a SCALED one is INVALID_OPERATION that used to end it
# too. Both shapes are decided on the rectangles, before any capability question, so this
# entry needs NO knob and runs on all three arms.
gtest_discover_tests(MobileGLIntegrationTest
TEST_PREFIX "DirectVulkan.${mglItestArm}.MsFlip."
TEST_FILTER "DepthStencilReadbackMatrixScenario.AFlippedMultisampleResolveMirrorsTheBandsAndAScaleDeclines"
DISCOVERY_TIMEOUT 30
PROPERTIES LABELS "${mglItestMagmaLabels}" TIMEOUT ${MGL_ITEST_TIMEOUT}
${mglItestArmProperties}
ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-msflip${mglItestArmLogTag}.log")
foreach(inPlaceCase CopyImageInPlaceAcrossLevelsAndLayers
CopyImageInPlaceOverlapDeclinesAndLeavesTheLevelAlone)
gtest_discover_tests(MobileGLIntegrationTest
MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp
+93
View File
@@ -35,6 +35,7 @@
// only checked "no GL error" would pass against a readback that never touched the buffer,
// which is precisely how this whole cluster hid for so long.
#include <array>
#include <algorithm>
#include <cmath>
#include <cstring>
@@ -404,6 +405,98 @@ namespace MGITest {
Gl().EndFrame();
}
// P7 wave 2-B2 (review round): A MULTISAMPLE DEPTH RESOLVE THAT FLIPS, AND ONE THAT SCALES.
//
// GL 4.6 core 18.3.1 asks a multisample blit for identical rectangle DIMENSIONS, not for
// identical corners - Mesa compares absolute spans - so `glBlitFramebuffer(0, h, w, 0, ...)`
// out of a multisample framebuffer is legal and owes a mirrored picture, while a blit whose
// sizes differ is INVALID_OPERATION and owes nothing. The wire arm used to end the SESSION
// on both: `Magma:multisample-depth-resolve-region` covered the reversed rectangle and the
// scaled one under one Fatal.
//
// COLOUR AND DEPTH IN ONE CALL, because that is the shape that made this visible: the colour
// arm has handled a flip since P5f and handles a scale as of this package, so
// GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT resolved its colour correctly and then died on its
// depth. Asserting both aspects of the same blit is what pins them together.
//
// THE BANDS ARE UNEQUAL ON PURPOSE (0.25 bottom / 0.75 top): a flip that did not happen
// returns the bands the right way up, and a "flip" that merely reordered the readback would
// move the colour too - so colour and depth are checked to agree about which way up they are.
TEST_F(DepthStencilReadbackMatrixScenario, AFlippedMultisampleResolveMirrorsTheBandsAndAScaleDeclines) {
if (!Ready()) return;
DepthSource multisampled = MakeRenderbufferSource(GL_DEPTH24_STENCIL8, 4);
if (!SourceIsUsable()) {
DestroySource(multisampled);
GTEST_SKIP() << "this driver cannot host a 4x multisample DEPTH24_STENCIL8 renderbuffer";
}
FirstGLError();
glDisable(GL_SCISSOR_TEST);
glViewport(0, 0, kWidth, kHeight);
glDepthMask(GL_TRUE);
glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
glEnable(GL_SCISSOR_TEST);
glScissor(0, 0, kWidth, kHeight / 2);
glClearDepth(0.25);
glClearColor(1, 0, 0, 1);
glClear(GL_DEPTH_BUFFER_BIT | GL_COLOR_BUFFER_BIT);
glScissor(0, kHeight / 2, kWidth, kHeight - kHeight / 2);
glClearDepth(0.75);
glClearColor(0, 1, 0, 1);
glClear(GL_DEPTH_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);
glClearColor(0, 0, 1, 1);
glClear(GL_COLOR_BUFFER_BIT);
ASSERT_EQ(FirstGLError(), 0u) << "priming the resolve destination";
// Y REVERSED ON THE SOURCE SIDE, identical dimensions.
glBindFramebuffer(GL_READ_FRAMEBUFFER, multisampled.fbo);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolved.fbo);
glDisable(GL_SCISSOR_TEST);
glBlitFramebuffer(0, kHeight, kWidth, 0, 0, 0, kWidth, kHeight,
GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT, GL_NEAREST);
glFinish();
EXPECT_EQ(FirstGLError(), 0u) << "a flipped multisample resolve is a legal blit";
glBindFramebuffer(GL_FRAMEBUFFER, resolved.fbo);
// The bands come back swapped: the destination's bottom rows hold what was the top.
const std::vector<float> bottom = ReadDepthFloat(0, 0, kWidth, kHeight / 4);
EXPECT_EQ(FirstGLError(), 0u);
ExpectAllDepth(bottom, 0.75f, "flipped resolve: the destination's bottom band");
const std::vector<float> top = ReadDepthFloat(0, kHeight - kHeight / 4, kWidth, kHeight / 4);
EXPECT_EQ(FirstGLError(), 0u);
ExpectAllDepth(top, 0.25f, "flipped resolve: the destination's top band");
// Colour went the same way up, through the arm that could already do this.
std::array<GLubyte, 4> colour{};
glReadPixels(kWidth / 2, 1, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, colour.data());
EXPECT_EQ(colour, (std::array<GLubyte, 4>{0, 255, 0, 255}))
<< "flipped resolve: colour and depth must agree about which way up the blit landed";
// 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);
glBindFramebuffer(GL_READ_FRAMEBUFFER, multisampled.fbo);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolved.fbo);
glBlitFramebuffer(0, 0, kWidth, kHeight, 0, 0, kWidth / 2, kHeight / 2,
GL_DEPTH_BUFFER_BIT, GL_NEAREST);
glFinish(); // the decline's error rides a later reply under a transport
EXPECT_EQ(FirstGLError(), GLenum(GL_INVALID_OPERATION))
<< "a scaled multisample resolve is INVALID_OPERATION, not a picture";
glBindFramebuffer(GL_FRAMEBUFFER, resolved.fbo);
ExpectAllDepth(ReadDepthFloat(0, 0, kWidth, kHeight), 0.5f,
"the declined scale must have left the destination alone");
EXPECT_EQ(FirstGLError(), 0u) << "the session survived the decline";
DestroySource(resolved);
DestroySource(multisampled);
glBindFramebuffer(GL_FRAMEBUFFER, 0);
Gl().EndFrame();
}
// A read whose rectangle is NOT the whole attachment. The staging copy has to carry
// the requested rect (not the origin) and hand back its rows bottom-up, which a
// full-extent uniform read is a fixed point of and therefore cannot see.