From 583e12de76938ce9a113c15ee030efbe3f24d1b3 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 16:19:32 -0400 Subject: [PATCH] [MG_Backend, MG_IntegrationTest, docs] (P7): the default-framebuffer blit stops refusing shapes the arm beneath it can do, and the 3D bug that hid behind the refusal is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CONTRACT-P7 §3.2's `default-color-blit-shape@P7`, retired in three pieces, because one Fatal was covering three unrelated situations. The shader pass it guarded is there for ROTATION and nothing else: it can only SAMPLE its source - sampler2D, a float result, one of two 2D view types - so an RGBA8UI attachment, a 3D slice or an array layer made it return false. The refusal then fired EVEN ON A SURFACE WITH NO ROTATION, where the ordinary vkCmdBlitImage arm twenty lines below would have done the blit and does handle those shapes. - identity: fall through to that arm; - a non-identity rotation with a source the pass cannot sample: move the region into an owned 2D single-sample image of a sampled-float format in the SAME size-compatibility class and shader-blit that; - outside the four rotations (a mirror, or a mirror-rotation): decline by name, saying which transform. Neither arm can express a reflection and this package has no device that reports one to check a result against. Reinterpretation is the point for the integer case and GL agrees: glBlitFramebuffer between an integer and a normalized format is INVALID_OPERATION (4.6 core 18.3.1), so there is no defined result to be wrong about - only a shape that used to end the session and now produces the source's bits. For a float source of the wrong view type the copy is a plain slice extraction and the result is exactly right. THE RETIREMENT UNCOVERED A REAL BUG, which is the part worth reading. The ordinary blit arm addressed a 3D source as `{aspect, level, source.layer, 1}` with z 0..1. A 3D image's slice rides Z; its only subresource is layer 0 - which is how the copy endpoint's CopyImageSliceMapping and the multisample resolve arm above already address one. So a blit from slice 1 of a 3D texture read slice 0, out of a layer the image does not have. It was invisible because that shape hit the Fatal first and never reached the arm: the new scenario's first run had the scratch arm correct and the fall-through arm wrong in all four quadrants, which is how it fell out. Both arms agree now. The scenario's source is slice 1 of a 3D texture - ordinary GL, and exactly the shape the shader pass cannot sample - with slice 0 filled in a colour that appears in no quadrant, so a scratch copy that forgot the z origin would still be reading real texels and had to be made distinguishable. It is registered twice on each arm: plain, taking the identity fall-through, and under MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH, taking the rotated arm's scratch path on an identity surface, where it must produce THE SAME PIXELS. That equality is the assertion, and it is the only way the scratch arm is reachable from a host lane at all: every surface this lane can create reports VK_SURFACE_TRANSFORM_IDENTITY, as does the Redmi pbuffer this cluster's evidence comes from. The knob does not fake a rotation - nothing downstream is told the surface turned. Red once (executed, reverted): with MagmaWireFatal("default-color-blit-shape@P7") restored, both entries die by that name; without it, 2/2 green. Gates: integration-magma-split 81 (+2), -spawn 60 (+2), -tcp 62 (+2), 0 red; ctest -L unit 2410; informational integration-magma-full-split 516, 0 red, 59 skip; fatal_census green; link_ratchet --assert-monotone unchanged at 186, p7-magma 101; G1 pull .text 0xa52203, defined-symbol name list identical (30570). Owed to a device window: the scratch arm's geometry under a REAL ROTATE_90/180/270. The knob proves it produces the fall-through's pixels on an identity surface, which exercises every line of it except the rotation constant the vertex shader applies. WireColorBlit.inc and VulkanRenderer.h are edited outside this package's named file list. Both belong to this cluster (the colour blit's implementation, and four private declarations beside the existing blit ones); flagged for the integrator. --- .../DirectVulkan/Renderer/VulkanRenderer.h | 9 ++ .../DirectVulkan/Renderer/WireColorBlit.inc | 106 ++++++++++++++++++ .../DirectVulkan/Renderer/WireFramebuffer.inc | 96 ++++++++++++++-- MobileGL/MG_IntegrationTest/CMakeLists.txt | 14 +++ .../Scenarios/F1WireScenario.cpp | 76 +++++++++++++ docs/Disaggregated/notes/p7/magma-b.md | 57 ++++++++++ 6 files changed, 349 insertions(+), 9 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 1f07213b..54422ee1 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -497,6 +497,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { Bool BlitWireColorImage(WireImage source, WireImage destination, GLint sx0, GLint sy0, GLint sx1, GLint sy1, GLint dx0, GLint dy0, GLint dx1, GLint dy1, GLenum filter, Bool mipmap); + // P7 wave 2-B, CONTRACT-P7 §3.2 (`default-color-blit-shape`): the rotated arm. Moves the + // source region into an owned single-sample 2D image of a sampled-float format in the + // same size-compatibility class, then shader-blits THAT to the rotated default + // framebuffer - the shapes BlitWireColorImage itself cannot sample (an integer format, a + // 3D or array source) become shapes it can. + Bool BlitWireColorToDefaultThroughScratch(WireImage source, WireImage destination, + GLint sx0, GLint sy0, GLint sx1, GLint sy1, + GLint dx0, GLint dy0, GLint dx1, GLint dy1, + GLenum filter); struct WireColorBlitResources; WireColorBlitResources* m_wireColorBlitResources = nullptr; void DestroyWireColorBlitResources(); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc index 530e0527..7aea149b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc @@ -43,6 +43,112 @@ return BlitWireColorImage(source, destination, sx0, sy0, sx1, sy1, dx0, dy0, dx1, dy1, filter, false); } + // P7 wave 2-B (CONTRACT-P7 §3.2, `default-color-blit-shape`). The shader blit below can only + // SAMPLE its source: sampler2D, a float result, and one of two 2D view types. A framebuffer + // whose colour attachment is RGBA8UI, or a layer of a 3D texture, is none of those, and on a + // rotated Android surface there is no other arm - an axis-aligned vkCmdBlitImage cannot + // express a quarter turn. So the source region becomes an owned 2D image first, through + // vkCmdCopyImage, which cares about the texel BLOCK SIZE and not about how the block is + // interpreted; the scratch then takes the shader blit as an ordinary 2D float source. + // + // REINTERPRETATION IS THE POINT FOR THE INTEGER CASE, and GL agrees: glBlitFramebuffer + // between an integer and a normalized format is INVALID_OPERATION (GL 4.6 core 18.3.1), so + // there is no defined result to be wrong about - only a shape that used to end the session + // and now produces the source's bits. For a float source of the wrong VIEW TYPE the copy is + // a plain slice extraction and the result is exactly right. + Bool VulkanRenderer::BlitWireColorToDefaultThroughScratch(WireImage source, WireImage destination, + GLint sx0, GLint sy0, GLint sx1, GLint sy1, + GLint dx0, GLint dy0, GLint dx1, GLint dy1, GLenum filter) { + if (!destination.isDefault || source.isDefault) return false; + if (source.samples != VK_SAMPLE_COUNT_1_BIT || destination.samples != VK_SAMPLE_COUNT_1_BIT) + return false; + if (!source.image || !destination.image || source.image == destination.image) return false; + const Int64 width = std::abs(static_cast(sx1) - sx0); + const Int64 height = std::abs(static_cast(sy1) - sy0); + if (!width || !height || dx0 == dx1 || dy0 == dy1) return true; + if (std::min(sx0, sx1) < 0 || std::min(sy0, sy1) < 0 || + static_cast(std::max(sx0, sx1)) > source.extent.width || + static_cast(std::max(sy0, sy1)) > source.extent.height) return false; + // A compressed source cannot be a colour attachment, so it cannot be a framebuffer blit's + // source either - but the block arithmetic below would silently produce nonsense for one, + // and "cannot happen" is the reason the guards this package retires were written as + // asserts. The size-compatibility classes are Vulkan's own (VkFormat texel block size); + // the destination format is whatever the swapchain uses and is always sampled-float. + if (vkuFormatIsCompressed(source.format)) return false; + VkFormat scratchFormat = VK_FORMAT_UNDEFINED; + switch (vkuFormatTexelBlockSize(source.format)) { + case 1: scratchFormat = VK_FORMAT_R8_UNORM; break; + case 2: scratchFormat = VK_FORMAT_R8G8_UNORM; break; + case 4: scratchFormat = VK_FORMAT_R8G8B8A8_UNORM; break; + case 8: scratchFormat = VK_FORMAT_R16G16B16A16_SFLOAT; break; + case 16: scratchFormat = VK_FORMAT_R32G32B32A32_SFLOAT; break; + default: return false; + } + VkFormatProperties scratchProperties{}; + vkGetPhysicalDeviceFormatProperties(m_physicalDevice.handle, scratchFormat, &scratchProperties); + const VkFormatFeatureFlags scratchRequired = VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT | + (filter == GL_LINEAR ? VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_LINEAR_BIT : 0); + if ((scratchProperties.optimalTilingFeatures & scratchRequired) != scratchRequired) return false; + + auto scratch = MakeUnique(); + VkImageCreateInfo info{VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO}; + info.imageType = VK_IMAGE_TYPE_2D; + info.format = scratchFormat; + info.extent = {static_cast(width), static_cast(height), 1}; + info.mipLevels = info.arrayLayers = 1; + info.samples = VK_SAMPLE_COUNT_1_BIT; + info.tiling = VK_IMAGE_TILING_OPTIMAL; + info.usage = VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT; + info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + VmaAllocationCreateInfo allocation{}; + allocation.usage = VMA_MEMORY_USAGE_AUTO_PREFER_DEVICE; + allocation.requiredFlags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; + if (vmaCreateImage(m_allocator, &info, &allocation, &scratch->image, &scratch->allocation, + nullptr) != VK_SUCCESS) + return false; + scratch->format = scratchFormat; + scratch->extent = {info.extent.width, info.extent.height}; + scratch->mipLevels = scratch->arrayLayers = scratch->depth = 1; + scratch->aspect = VK_IMAGE_ASPECT_COLOR_BIT; + WireImage staged{}; + staged.image = scratch->image; + staged.format = scratchFormat; + staged.aspect = VK_IMAGE_ASPECT_COLOR_BIT; + staged.extent = scratch->extent; + staged.trackedLayout = &scratch->layout; + + m_textureManager->FlushPendingUploads(); + TransitionWireImage(source, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); + TransitionWireImage(staged, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); + VkImageCopy copy{}; + // A 3D source carries its slice on z and always names layer 0; every other source + // carries it on the layer axis, which is the same split ResolveWireImage records. + copy.srcSubresource = {VK_IMAGE_ASPECT_COLOR_BIT, source.level, source.is3D ? 0u : source.layer, 1}; + copy.srcOffset = {std::min(sx0, sx1), std::min(sy0, sy1), + source.is3D ? static_cast(source.layer) : 0}; + copy.dstSubresource = {VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1}; + copy.dstOffset = {0, 0, 0}; + copy.extent = {static_cast(width), static_cast(height), 1}; + vkCmdCopyImage(m_frameContext.GetCurrent().commandBuffer, source.image, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, staged.image, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, ©); + staged.layout = scratch->layout; + // The scratch holds the region at its own origin, so a reversed source rectangle + // becomes a reversed rectangle OVER THE SCRATCH - the multisample resolve arm's shape. + const GLint rx0 = sx1 > sx0 ? 0 : static_cast(width); + const GLint rx1 = sx1 > sx0 ? static_cast(width) : 0; + const GLint ry0 = sy1 > sy0 ? 0 : static_cast(height); + const GLint ry1 = sy1 > sy0 ? static_cast(height) : 0; + const Bool blitted = BlitWireColorImage(staged, destination, rx0, ry0, rx1, ry1, + dx0, dy0, dx1, dy1, filter, false); + // Retired whether or not the blit took: the copy above is already recorded against this + // image, so the submission that consumes it owns the storage either way. + WireRetiredObjects retired{}; + retired.textures.push_back(Move(scratch)); + RetireWireObjects(Move(retired)); + return blitted; + } + Bool VulkanRenderer::BlitWireColorImage(WireImage source, WireImage destination, GLint sx0, GLint sy0, GLint sx1, GLint sy1, GLint dx0, GLint dy0, GLint dx1, GLint dy1, GLenum filter, Bool mipmap) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc index 0ac2d8c3..4d3ed87b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc @@ -87,6 +87,20 @@ return tier; } + // THE SAME PROBLEM FOR THE ROTATED DEFAULT-BLIT ARM. Every surface this lane can create is + // VK_SURFACE_TRANSFORM_IDENTITY - headless, pbuffer or X11 - and the Redmi evidence for this + // cluster is a pbuffer too, so the scratch arm that exists FOR rotation would never execute + // anywhere a test could watch it. The knob forces it on an identity surface, where it has to + // produce exactly the pixels the fall-through arm produces; that equality is the assertion. + // It does NOT fake a rotation: nothing downstream is told the surface turned. + static Bool MagmaWireForcedDefaultBlitScratch() { + static const Bool forced = [] { + const char* value = std::getenv("MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH"); + return value && *value && value[0] != '0'; + }(); + return forced; + } + VulkanRenderer::WireImage VulkanRenderer::ResolveWireImage( const MG_Pipe::MGPFramebufferState& fbo, const MG_Pipe::MGPSurface& surface, VkImageAspectFlags aspect, Bool isWriteTarget) { @@ -677,11 +691,64 @@ if (aspect == VK_IMAGE_ASPECT_COLOR_BIT && destination.isDefault && !source.isDefault && source.samples == VK_SAMPLE_COUNT_1_BIT && destination.samples == VK_SAMPLE_COUNT_1_BIT && source.image != destination.image) { - if (!BlitWireColorToDefault(source, destination, sx0, sy0, sx1, sy1, - dx0, dy0, dx1, dy1, filter)) - MagmaWireFatal("default-color-blit-shape@P7"); - m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); - return; + // P7 wave 2-B, CONTRACT-P7 §3.2: `default-color-blit-shape@P7` RETIRES, in three + // pieces, because the one Fatal covered three unrelated situations. + // + // The shader blit is here for ROTATION and for nothing else. It can only SAMPLE + // its source - sampler2D, a float result, one of two 2D view types - so an + // RGBA8UI attachment, a 3D slice or an array layer made it return false, and the + // refusal fired even on a surface with NO rotation, where the ordinary + // vkCmdBlitImage arm twenty lines down would have done the blit correctly and + // does handle those shapes. + const auto transform = m_swapchainObject.GetPreTransform(); + const Bool knownRotation = transform == VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR || + transform == VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR || + transform == VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR || + transform == VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR; + if (!knownRotation) { + // Mirrors, and mirror-rotations, are the shapes neither arm can express: the + // blit rectangle would have to be reflected, and this package does not have a + // device that reports one to check the result against. A decline (§3.2's + // "outside the four rotations"), not a Fatal, and it says which transform. + MGLOG_E_ONCE("DirectVulkan::BlitWireFramebuffers: the surface pre-transform is 0x%x, " + "which is neither identity nor one of the three rotations either blit " + "arm can express; declining the blit to the default framebuffer", + static_cast(transform)); + return; + } + if (!MagmaWireForcedDefaultBlitScratch() && + BlitWireColorToDefault(source, destination, sx0, sy0, sx1, sy1, + dx0, dy0, dx1, dy1, filter)) { + m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); + return; + } + // A source the shader pass cannot sample, on a rotated surface. Move the region + // into an owned 2D image of a sampled-float format in the same size-compatibility + // class and blit THAT. The knob forces this arm on an identity surface, where it + // must produce the same pixels the fall-through below produces - which is what + // makes it reachable from a host lane at all (every host surface here is + // identity, and the Redmi evidence is a pbuffer, so nothing else would run it). + if (transform != VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR || + MagmaWireForcedDefaultBlitScratch()) { + if (BlitWireColorToDefaultThroughScratch(source, destination, sx0, sy0, sx1, sy1, + dx0, dy0, dx1, dy1, filter)) { + m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); + return; + } + MGLOG_E_ONCE("DirectVulkan::BlitWireFramebuffers: a %ux%u source of format %d " + "(view type %d) cannot reach the rotated (0x%x) default framebuffer " + "through either the shader pass or a scratch copy; declining the blit", + source.extent.width, source.extent.height, + static_cast(source.format), static_cast(source.viewType), + static_cast(transform)); + return; + } + // Identity: fall through to the ordinary vkCmdBlitImage arm below. It already + // maps GL's bottom row onto the native image's last row for a default target, + // and vkCmdBlitImage takes integer formats and array layers natively. Its 3D + // addressing was wrong and is fixed with this package - see the srcBaseLayer / + // srcZ note down there; the bug was invisible until now because a 3D source hit + // the Fatal above and never reached that arm. } const Bool sameImage = source.image == destination.image; const VkImageLayout srcLayout = sameImage ? VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL; @@ -690,10 +757,21 @@ if (sameImage) destination.layout = source.layout; else TransitionWireImage(destination,dstLayout); VkImageBlit region{}; - region.srcSubresource = {aspect,source.level,source.layer,1}; - region.dstSubresource = {aspect,destination.level,destination.layer,1}; - region.srcOffsets[0] = {sx0,sy0,0}; region.srcOffsets[1] = {sx1,sy1,1}; - region.dstOffsets[0] = {dx0,dy0,0}; region.dstOffsets[1] = {dx1,dy1,1}; + // A 3D IMAGE CARRIES ITS SLICE ON Z, NOT ON THE LAYER AXIS. Its only subresource is + // layer 0, and the selected slice is the z of the offsets - which is how the copy + // endpoint (`CopyImageSliceMapping`) and the multisample resolve arm above already + // address one. This arm addressed it as `baseArrayLayer = source.layer` with z 0..1, + // so a blit from slice 1 of a 3D texture read slice 0 out of a layer the image does + // not have. Nothing caught it, because until this package that shape reached the + // arm's Fatal first and never got here at all. + const Uint32 srcBaseLayer = source.is3D ? 0u : source.layer; + const Uint32 dstBaseLayer = destination.is3D ? 0u : destination.layer; + const Int32 srcZ = source.is3D ? static_cast(source.layer) : 0; + const Int32 dstZ = destination.is3D ? static_cast(destination.layer) : 0; + region.srcSubresource = {aspect,source.level,srcBaseLayer,1}; + region.dstSubresource = {aspect,destination.level,dstBaseLayer,1}; + region.srcOffsets[0] = {sx0,sy0,srcZ}; region.srcOffsets[1] = {sx1,sy1,srcZ+1}; + region.dstOffsets[0] = {dx0,dy0,dstZ}; region.dstOffsets[1] = {dx1,dy1,dstZ+1}; // Headless and identity surfaces store GL's bottom row at Vulkan's last row. if (source.isDefault) { region.srcOffsets[0].y = static_cast(source.extent.height)-sy0; diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 9deb70b7..b324db35 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2734,6 +2734,20 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # strongest sense: the monolith arm declines every in-place copy, so there is no # DirectVulkan. entry to register these beside - the wire arm is the one that performs # them, which is what §3.2 asks for. + # `default-color-blit-shape@P7`'s retirement. TWO ENTRIES OVER ONE CASE, and the second + # is the only way its arm is reachable here: every surface this lane creates reports + # VK_SURFACE_TRANSFORM_IDENTITY, so the scratch path that exists FOR rotation would + # never run. Forced onto an identity surface it must produce the SAME pixels as the + # fall-through, which is what the shared assertions check. + foreach(defaultBlitScratch 0 1) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.${mglItestArm}.DefaultBlitShape${defaultBlitScratch}." + TEST_FILTER "F1WireScenario.ColorBlitToDefaultFromANonSampleableSourceDegrades" + DISCOVERY_TIMEOUT 30 + PROPERTIES LABELS "${mglItestMagmaLabels}" TIMEOUT ${MGL_ITEST_TIMEOUT} + ${mglItestArmProperties} + ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH=${defaultBlitScratch}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-defaultblit${mglItestArmLogTag}-scratch${defaultBlitScratch}.log") + endforeach() foreach(inPlaceCase CopyImageInPlaceAcrossLevelsAndLayers CopyImageInPlaceOverlapDeclinesAndLeavesTheLevelAlone) gtest_discover_tests(MobileGLIntegrationTest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp index e3228430..9e0568ca 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/F1WireScenario.cpp @@ -1554,6 +1554,82 @@ TEST_F(F1WireScenario, ColorBlitToDefaultHonorsScissorAndReversedRect) { Gl().EndFrame(); } +// P7 wave 2-B, CONTRACT-P7 §3.2: `default-color-blit-shape@P7` RETIRES. +// +// One Fatal covered three unrelated situations. The shader pass it guarded exists for ROTATION +// and can only SAMPLE its source - sampler2D, a float result, one of two 2D view types - so a +// 3D slice or an integer attachment made it return false and the refusal fired EVEN ON A SURFACE +// WITH NO ROTATION, where the ordinary vkCmdBlitImage arm below it would have done the blit +// correctly and does handle those shapes. Identity now falls through to that arm; a rotated +// surface moves the region into an owned 2D float scratch and blits that; anything outside the +// four rotations declines by name. +// +// THE SOURCE IS A 3D TEXTURE SLICE, which is ordinary defined GL (a layer of a 3D texture is a +// legal colour attachment) and is exactly the shape the shader pass cannot sample. The same case +// runs twice on each arm: plain, taking the identity fall-through, and under +// MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH, taking the rotated arm's scratch path on an identity +// surface - where it must produce THE SAME PIXELS. That equality is what makes the scratch arm +// testable at all: every surface this lane can create is identity, and so is the Redmi pbuffer +// the cluster's evidence comes from. +// +// Red once (executed, reverted): restore MagmaWireFatal("default-color-blit-shape@P7") and both +// entries die by that name. +TEST_F(F1WireScenario, ColorBlitToDefaultFromANonSampleableSourceDegrades) { + if (!Ready()) return; + ASSERT_GE(Gl().Width(), 16); + ASSERT_GE(Gl().Height(), 16); + const int width = Gl().Width(), height = Gl().Height(); + GLuint volume = 0, volumeFbo = 0; + glGenTextures(1, &volume); + glBindTexture(GL_TEXTURE_3D, volume); + glTexStorage3D(GL_TEXTURE_3D, 1, GL_RGBA8, 6, 4, 2); + glGenFramebuffers(1, &volumeFbo); + glBindFramebuffer(GL_FRAMEBUFFER, volumeFbo); + // Slice 1, not slice 0: a scratch copy that forgot the z origin would read the other slice + // and every quadrant assertion below would still be about SOME real texels. + glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, volume, 0, 1); + ASSERT_EQ(glCheckFramebufferStatus(GL_FRAMEBUFFER), GLenum(GL_FRAMEBUFFER_COMPLETE)); + constexpr GLfloat colors[4][4] = {{1, 0, 0, 1}, {0, 1, 0, 1}, {0, 0, 1, 1}, {1, 1, 0, 1}}; + glEnable(GL_SCISSOR_TEST); + for (int quadrant = 0; quadrant < 4; ++quadrant) { + glScissor((quadrant % 2) * 3, (quadrant / 2) * 2, 3, 2); + glClearColor(colors[quadrant][0], colors[quadrant][1], colors[quadrant][2], colors[quadrant][3]); + glClear(GL_COLOR_BUFFER_BIT); + } + glDisable(GL_SCISSOR_TEST); + // The OTHER slice gets one flat colour that appears in no quadrant. + glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, volume, 0, 0); + glClearColor(0, 1, 1, 1); + glClear(GL_COLOR_BUFFER_BIT); + glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, volume, 0, 1); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.DefaultBlitShape.setup"; + + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); + glClearColor(1, 0, 1, 1); + glClear(GL_COLOR_BUFFER_BIT); + glBindFramebuffer(GL_READ_FRAMEBUFFER, volumeFbo); + const auto before = PeekSplitRuntime().emitSeq; + glBlitFramebuffer(0, 0, 6, 4, 0, 0, width, height, GL_COLOR_BUFFER_BIT, GL_NEAREST); + EXPECT_GT(PeekSplitRuntime().emitSeq, before) << "F1.DefaultBlitShape.wire"; + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); + const std::string arm = SplitLane::MarkerValue("MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH"); + EXPECT_EQ(ReadOnePixel(width / 4, height / 4), (std::array{255, 0, 0, 255})) + << "F1.DefaultBlitShape bottom left (scratch arm=" << arm << ")"; + EXPECT_EQ(ReadOnePixel(3 * width / 4, height / 4), (std::array{0, 255, 0, 255})) + << "F1.DefaultBlitShape bottom right (scratch arm=" << arm << ")"; + EXPECT_EQ(ReadOnePixel(width / 4, 3 * height / 4), (std::array{0, 0, 255, 255})) + << "F1.DefaultBlitShape top left (scratch arm=" << arm << ")"; + EXPECT_EQ(ReadOnePixel(3 * width / 4, 3 * height / 4), (std::array{255, 255, 0, 255})) + << "F1.DefaultBlitShape top right (scratch arm=" << arm << ")"; + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "F1.DefaultBlitShape.error"; + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glDeleteFramebuffers(1, &volumeFbo); + glBindTexture(GL_TEXTURE_3D, 0); + glDeleteTextures(1, &volume); + glBindTexture(GL_TEXTURE_2D, texture); + Gl().EndFrame(); +} + namespace { // One 8x8x2 RGBA8 array texture with two levels, filled so that EVERY subresource this pair // of cases touches is distinguishable from every other one. A flat fill would let a copy that diff --git a/docs/Disaggregated/notes/p7/magma-b.md b/docs/Disaggregated/notes/p7/magma-b.md index f8dbd5d3..4dcc3236 100644 --- a/docs/Disaggregated/notes/p7/magma-b.md +++ b/docs/Disaggregated/notes/p7/magma-b.md @@ -20,6 +20,7 @@ | 1 | `mipmap-shader-format-or-shape` | 诊断 + 颜色 shader mip 回落（**退役**一半：可渲染格式的不可附着图像） | 见 §1 | | 2 | `depth-stencil-mipmap@P7` + 烘焙 (A)(D) | DEPTH-only **退役**（烘焙深度 mip 程序）；D\|S **decline** | 见 §2 | | 3 | `copy-image-in-place@P7` | **退役**（单次 GENERAL 转换）；同级同层矩形重叠 **decline** | 见 §3 | +| 4 | `default-color-blit-shape@P7` | **退役**（恒等回落 + 旋转 scratch）；四旋转之外 **decline** | 见 §4 | --- @@ -262,3 +263,59 @@ layer / 根本没拷」都读成对的。 > 原因是 `40613`（`~/w7/pipe/build-split` 的默认值）上已经有一个兄弟树的 `libMobileGLServer` 在监听。 > 这是本地 configure 选项，不影响车道语义；`magma-two-process-first-run.md` 里 wave 0 也为同一原因 > 用了专用端口。 + +--- + +## 4. 片 4：`default-color-blit-shape@P7` + +### 4.1 一条 Fatal 盖住了三种互不相干的情况 + +那条 shader pass **是为旋转存在的**：它只能**采样**自己的源（`sampler2D`、float 结果、两种 2D view +type 之一）。于是 RGBA8UI 附件、3D 切片、数组层都会让 `BlitWireColorToDefault` 返回 false——**而且在完全 +没有旋转的表面上也会**，尽管二十行之下那条普通的 `vkCmdBlitImage` 臂本来就能正确完成。 + +拆成三块： + +1. **恒等变换 → 回落**到普通 `vkCmdBlitImage` 臂（它已经把 GL 的底行映射到原生图像的末行）； +2. **非恒等旋转 + 采不到的源** → 把源矩形 `vkCmdCopyImage` 进一张自有的 2D、单采样、与源**同尺寸兼容类** + 的采样浮点格式 scratch，再对 scratch 做 shader blit； +3. **四个旋转之外**（镜像 / 镜像旋转）→ `MGLOG_E_ONCE` **decline** 并写明是哪个 transform：两条臂都表达 + 不了反射，而本包没有一台会报这种 transform 的设备来核对结果。 + +对整数源的**重解释就是重点**，GL 也同意：整数与归一化格式之间的 `glBlitFramebuffer` 是 +`INVALID_OPERATION`（4.6 core 18.3.1），所以没有「正确结果」可言——只有一种以前会结束会话、现在会产出源 +比特的形状。对 view type 不对的浮点源，那次 copy 就是一次切片抽取，结果**完全正确**。 + +### 4.2 退役顺带挖出一个真 bug + +普通 blit 臂把 3D 源当成层来寻址：`region.srcSubresource = {aspect, source.level, source.layer, 1}`， +`srcOffsets[].z = 0..1`。**3D 图像的切片在 z 上，它唯一的 subresource 是 layer 0**——copy 端点 +（`CopyImageSliceMapping`）与上面的 multisample resolve 臂早就是这么寻址的。所以从 3D 纹理第 1 片 blit +会去读第 0 片、并用一个该图像根本没有的层。 + +**在此之前看不见**：这种形状先撞上那条 Fatal，根本走不到这条臂。本片第一次跑场景时回落臂的四个象限全错、 +scratch 臂全对，bug 就是这么掉出来的。两条臂现在都对。 + +### 4.3 场景 + +`F1WireScenario.ColorBlitToDefaultFromANonSampleableSourceDegrades`：源是 **3D 纹理的第 1 片** +（普通合法 GL，正是 shader pass 采不到的形状），第 0 片填一个任何象限都不会出现的颜色（青）——一次忘了 z +原点的 scratch copy 会去读另一片，而那样四个象限断言**仍然是在断言某些真 texel**，所以必须让它可分辨。 + +**同一条用例、每臂注册两遍**：不带旋钮（走恒等回落臂）与 `MGITEST_MAGMA_FORCE_DEFAULT_BLIT_SCRATCH=1` +（在恒等表面上走旋转臂的 scratch 路径）。**两者必须产出同样的像素**，这条等式就是断言——也是 scratch 臂 +唯一可被主机车道到达的方式：本车道能创建的每一个表面都是 `VK_SURFACE_TRANSFORM_IDENTITY`，本簇的 Redmi +证据也是 pbuffer。 + +### 4.4 red-once（R-16，已执行并还原） + +把 `MagmaWireFatal("default-color-blit-shape@P7")` 放回去 → 两条都死于 +`Fatal{UnmigratedVerb, "Magma:default-color-blit-shape@P7"}`；还原后 2/2 绿。 + +### 4.5 门与设备债 + +`integration-magma-split` **81**（+2）、`-spawn` **60**（+2）、`-tcp` **62**（+2），三臂 0 红。 + +**欠设备核对**：非恒等旋转下的 scratch 臂只在恒等表面上被旋钮验证过（像素与回落臂相同）；真正的 +ROTATE_90/180/270 几何要在手机上看一眼。四旋转之外的 decline 无设备可核（没有会报镜像 transform 的 +设备），按 decline 收口。