diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index fda938d4..91f89dd0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -442,41 +442,94 @@ namespace MobileGL::MG_Backend::DirectVulkan { for (Uint32 location = 0; location < elements.AttributeCount; ++location) { const auto& attr = elements.Attributes[location]; if (!attr.Enabled || !(activeMask & (1u << location))) continue; - const auto reject = [&](const char* reason, VkFormat format = VK_FORMAT_UNDEFINED) { - MGLOG_E("Magma wire vertex layout: %s location=%u type=%u size=%u normalized=%u integer=%u long=%u bgra=%u stride=%d offset=%llu format=%d bufferWindow=%u+%u activeMask=0x%x", + // P7 wave 2 package C, CONTRACT-P7 §3.2 `vertex-layout`: THE ONE STRING BECAME TWO + // VERDICTS, and the split is by what the reason is ABOUT rather than by severity. + // + // Every reason here used to return false, and the single caller answered that with + // one P7-marked MagmaWireFatal - an abort, bypassing Session::Fail, invisible + // to the census gate and with no equivalent on the monolith arm at all. But the + // reasons are not one kind of thing: + // + // * `buffer-window` is a statement about the RECORD: an enabled attribute the + // program reads names a slot outside the window set_vertex_buffers published. + // Nothing about the device or the format is involved; the two halves of the + // protocol disagree about what crossed. It stays a named Fatal through the hook + // (see the caller), and it is load-bearing rather than defensive - it is what + // caught the Redmi VAO hash collision fixed in 376c04be. + // + // * every other reason is a statement about what VULKAN CAN EXPRESS on this + // device: a GL type with no VkFormat, a format without + // VK_FORMAT_FEATURE_VERTEX_BUFFER_BIT, a component size we cannot compute, an + // offset sum that does not fit. The monolith arm has had an answer for these + // since it was written, and it is not an abort: mask the attribute out of the + // vertex input state and carry on (the `unsupportedAttribMask |= ...; continue;` + // sites in GetOrCreateVertexInputState above). The draw path then decides, + // loudly and once, whether the masked attribute was one the program actually + // reads. This arm now does exactly that, which is what "same observable as the + // monolith lane" means for this row. + // + // THE COUNT: the contract says "the other six"; there are SEVEN return sites below + // it, because `offset-overflow` was added after the audit's count and is an + // arithmetic guard rather than a shape one. It is masked with the rest: an + // attribute whose base offsets cannot be added is one Vulkan cannot fetch, the + // monolith arm never computes that sum at all, and masking is strictly safer than + // a Fatal for a number no application can reach on purpose. Recorded in + // notes/p7/magma-c.md rather than silently reconciled. + const auto describe = [&](const char* reason, VkFormat format) { + MGLOG_E_ONCE("Magma wire vertex layout: %s location=%u type=%u size=%u normalized=%u integer=%u long=%u bgra=%u stride=%d offset=%llu format=%d bufferWindow=%u+%u activeMask=0x%x", reason, location, attr.Type, static_cast(attr.Size), static_cast(attr.Normalized), static_cast(attr.IsInteger), static_cast(attr.IsLong), static_cast(attr.IsBgra), attr.Stride, static_cast(attr.Offset), static_cast(format), state.VertexBufferStart, state.VertexBufferCount, activeMask); + }; + // The protocol verdict: the caller turns a false into the named Fatal. + const auto reject = [&](const char* reason, VkFormat format = VK_FORMAT_UNDEFINED) { + describe(reason, format); return false; }; + // The device verdict: MGLOG_E_ONCE + mask + continue, the monolith arm's shape. + // MGLOG_E_ONCE and not MGLOG_E, also the monolith arm's: an unmappable attribute is + // a property of the VAO and the device, so it repeats every draw, and the per-draw + // line is what made the old abort look preferable to whoever wrote it. + const auto maskOut = [&](const char* reason, VkFormat format = VK_FORMAT_UNDEFINED) { + describe(reason, format); + out.unsupportedAttribMask |= 1u << location; + }; // set_vertex_buffers is flattened PER ATTRIBUTE (VertexInputEmit.h), not // indexed by the original ARB binding point in attr.BindingIndex. if (location < state.VertexBufferStart || location - state.VertexBufferStart >= state.VertexBufferCount) return reject("buffer-window"); const auto& buffer = state.VertexBuffers[location]; const auto type = static_cast(attr.Type); - if (attr.Stride < 0 || attr.Size < 1 || attr.Size > 4) return reject("attribute-shape"); + if (attr.Stride < 0 || attr.Size < 1 || attr.Size > 4) { maskOut("attribute-shape"); continue; } VkFormat format = ToVkVertexFormat(type, attr.Size, attr.Normalized, attr.IsInteger, attr.IsBgra, attr.IsLong); auto conversion = VertexStreamConversion::None; if (format == VK_FORMAT_UNDEFINED && type == DataType::Float64) { const auto* backend = MG_Remote::Server::ServerLoopInstance().Backend(); - if (backend && backend->GetDynamicParameters().SupportsFloat64VertexAttributes) return reject("native-fp64-format"); + if (backend && backend->GetDynamicParameters().SupportsFloat64VertexAttributes) { + // Exactly the monolith arm's answer for the same state: with native fp64 + // the module KEPT its 64-bit inputs (DemoteFloat64Pass did not run), so + // narrowing the stream would feed float32 to a Float64 input - and the + // monolith build therefore leaves sourceVkFormat UNDEFINED and falls into + // its own mask-out below. Same place, same mask. + maskOut("native-fp64-format"); + continue; + } format = ToFloat32VertexFormat(attr.Size); conversion = VertexStreamConversion::Float64ToFloat32; } - if (format == VK_FORMAT_UNDEFINED) return reject("format-map"); + if (format == VK_FORMAT_UNDEFINED) { maskOut("format-map"); continue; } if (!SupportsVertexBufferFormat(format)) { - if (!IsScaledIntegerVertexFormat(format)) return reject("native-format-feature", format); + if (!IsScaledIntegerVertexFormat(format)) { maskOut("native-format-feature", format); continue; } format = ToFloat32VertexFormat(attr.Size); conversion = VertexStreamConversion::ScaledIntegerToFloat32; - if (!SupportsVertexBufferFormat(format)) return reject("converted-format-feature", format); + if (!SupportsVertexBufferFormat(format)) { maskOut("converted-format-feature", format); continue; } } const SizeT elementSize = GetAttributeByteSize(type, attr.Size, attr.IsBgra); - if (!elementSize) return reject("element-size", format); + if (!elementSize) { maskOut("element-size", format); continue; } const Uint64 offset = attr.Offset + buffer.Offset; - if (offset < attr.Offset) return reject("offset-overflow", format); + if (offset < attr.Offset) { maskOut("offset-overflow", format); continue; } const SizeT alignment = (type == DataType::Int2101010Rev || type == DataType::Uint2101010Rev) ? elementSize : GetComponentSize(type); if (conversion == VertexStreamConversion::None && alignment > 1 && @@ -500,8 +553,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { // resolved binding/attribute/divisor values, including a legal zero stride. Uint64 hash = XXH64(out.bindings.data(), out.bindings.size() * sizeof(out.bindings[0]), 0); hash = XXH64(out.attributes.data(), out.attributes.size() * sizeof(out.attributes[0]), hash); - out.layoutHash = XXH64(out.bindingDivisors.data(), + hash = XXH64(out.bindingDivisors.data(), out.bindingDivisors.size() * sizeof(out.bindingDivisors[0]), hash); + // THE MASK IS PART OF THE LAYOUT NOW, for the same reason the monolith entry's hash + // carries it (the XXH64_update over unsupportedAttribMask in GetOrCreateVertexInputState + // above). Before this package the mask was always zero here, so leaving it out of the + // hash was free; now two VAOs can produce the SAME bindings, attributes and divisors + // and differ only in which enabled attribute was masked out, and a hash blind to that + // would serve one of them the other's pipeline. + out.layoutHash = XXH64(&out.unsupportedAttribMask, sizeof(out.unsupportedAttribMask), hash); out.state.vertexBindingDescriptionCount = static_cast(out.bindings.size()); out.state.pVertexBindingDescriptions = out.bindings.data(); out.state.vertexAttributeDescriptionCount = static_cast(out.attributes.size()); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc index 9a186394..cb33ac78 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc @@ -245,8 +245,40 @@ const auto& vao = state.VertexElementsCsos[vaoHandle.Slot]; if (!vao.Live || vao.Gen != vaoHandle.Gen) MagmaWireFatal("vertex-elements-generation"); VertexInputStateFactory::BackendVertexInputState input{}; + // P7 wave 2 package C, CONTRACT-P7 §3.2: THE `vertex-layout` ROW IS RETIRED AND SPLIT. + // + // A false is now ONE reason and only one - `buffer-window`, an enabled attribute the + // program reads whose slot lies outside the window set_vertex_buffers published. That + // is a disagreement about what crossed, not about what this device can express, so it + // keeps a named Fatal through the hook (MGPipeSessionFail -> Session::Fail, which is + // what rule I asks of a refusal that must stay a refusal). It is the check that caught + // the Redmi VAO hash collision fixed in 376c04be, which is why it is not softened. + // The P7 marker is gone from the detail string with the abort: this is a permanent + // protocol error, not a shape the split path has yet to migrate, so it must not keep + // counting against the census of unmigrated refusals (scripts/ci/fatal_census.py + // counts the marker literally, which is also why no comment in this package spells it). if (!m_vertexInputStateFactory->BuildWireVertexInput(vao, state, - programObj.activeVertexInputLocationMask, input)) MagmaWireFatal("vertex-layout@P7"); + programObj.activeVertexInputLocationMask, input)) + MagmaWireFatal("vertex-layout-buffer-window"); + // The seven device/format reasons are now MASKED OUT of the vertex input state, and + // this is the monolith arm's pre-flight for them, statement for statement + // (VulkanRenderer.cpp's SetupDraw: "Fail loudly rather than render wrong pixels"). An + // enabled array with no usable VkFormat never reaches the vertex input state, which + // makes it indistinguishable from a DISABLED one - and a disabled one is answered + // below by uploading the current attribute value, so without this check the draw would + // quietly feed the shader a constant in place of the application's vertex data. + // + // A DECLINE, NOT A FATAL (rule I (a)): the draw is skipped, the reason is named once, + // and the observable is the monolith lane's for the same VAO on the same device. + // BuildWireVertexInput only considers attributes the program reads, so a masked bit is + // always an active one and this predicate is the whole of the difference. + if ((input.unsupportedAttribMask & programObj.activeVertexInputLocationMask) != 0) { + MGLOG_E_ONCE("Magma wire draw skipped: program=%u reads vertex attribute location " + "mask 0x%x whose enabled array has no supported vertex format", + program.GetExternalIndex(), + input.unsupportedAttribMask & programObj.activeVertexInputLocationMask); + return false; + } Vector vertexSlices(input.bindings.size()); for (SizeT binding = 0; binding < input.bindings.size(); ++binding) { const Uint32 location = input.bindingAttributeLocations[binding]; @@ -272,8 +304,22 @@ Vector source((count - 1) * stride + elementSize); if (!m_bufferManager.ReadWireBuffer(resource.Res, offset, source.size(), source.data())) return false; Vector converted; - if (!ConvertWireVertexStream(attr, conversion, source.data(), stride, count, converted)) - MagmaWireFatal("vertex-format-conversion@P7"); + // P7 wave 2 package C, CONTRACT-P7 §3.2 `vertex-format-conversion`: DECLINE. + // The converter is asked only for a conversion BuildWireVertexInput already + // chose, so a refusal here means the two disagree about what is convertible - + // a MobileGL-internal defect, not a peer's malformed record and not a verb the + // split path has yet to migrate. Killing the session for it told the peer + // "protocol corruption" about a bug on this side; skipping the draw and naming + // it once is the honest answer and is what the row asks for. The packed + // converter that would make the refusal unreachable is the row's optional half + // and is not taken here. + if (!ConvertWireVertexStream(attr, conversion, source.data(), stride, count, converted)) { + MGLOG_E_ONCE("Magma wire draw skipped: no vertex stream converter for " + "location=%u type=%u size=%u conversion=%d", + location, attr.Type, static_cast(attr.Size), + static_cast(conversion)); + return false; + } if (!m_bufferManager.UploadTransient(BufferKind::Vertex, m_frameContext.GetCurrentFrameIndex(), converted.data(), converted.size(), 16, vertexSlices[binding])) return false; } else { diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 4d4b9f11..a9402271 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2826,6 +2826,49 @@ if (MOBILEGL_BUILD_DISAGGREGATED) endforeach() endif() +# ===================================================================================== +# P7 WAVE 2 PACKAGE C: THE `vertex-layout` RETIREMENT (CONTRACT-P7 §3.2) +# ===================================================================================== +# +# The source is NOT inside the `if (MOBILEGL_BUILD_DISAGGREGATED)` above, and that is the +# point of the case pair: the claim is "the wire arm now does what the MONOLITH arm does", +# so the monolith DirectVulkan arm has to run the very same two cases. The ambient +# `TEST_PREFIX "DirectVulkan."` discovery near the top of this file picks them up from the +# target sources with no entry of its own; the three transports are added below. +target_sources(MobileGLIntegrationTest PRIVATE Scenarios/MagmaVertexLayoutScenario.cpp) + +if (MOBILEGL_BUILD_DISAGGREGATED) + # The flavour block is the buffers lane's minus the persistent-map knobs, which nothing + # here touches. MOBILEGL_BACKEND_TYPE and the ICD pin are the load-bearing pair: without + # them the entries run DirectGLES under a DirectVulkan prefix and skip themselves in + # SetUp, which reads as six green skips and proves nothing (measured, first cut). + set(MGL_ITEST_MAGMA_VERTEXLAYOUT_FLAVOUR_ENV + "MOBILEGL_BACKEND_TYPE=DirectVulkan" + "MOBILEGL_IPC_ROLE_SPLIT_STATE=1" "MOBILEGL_IPC_STRICT_ERRORS=1" + "MOBILEGL_ITEST_REQUIRE_GPU=1" "MGITEST_SPLIT_LANE=1" + ${MGL_ITEST_CAPABILITY_ENV} ${MGL_ITEST_VULKAN_ENV}) + # ALL THREE ARMS, unlike the caches and run-ahead lanes above: nothing here reads this + # process's server loop, so there is no inproc-only property to declare. The scenario's own + # SetUp skips DirectGLES, so a DirectGLES registration would only add three skips. + foreach(mglItestArm IN LISTS MGL_ITEST_MAGMA_ARMS) + mgl_itest_magma_arm_shape(${mglItestArm} vertexlayout) + mgl_itest_join_environment(MGL_ITEST_MAGMA_VERTEXLAYOUT_ENVIRONMENT + ${MGL_ITEST_MAGMA_VERTEXLAYOUT_FLAVOUR_ENV} ${mglItestArmTransportEnv}) + foreach(vertexLayoutCase + AnUnmappableVertexFormatDeclinesTheDrawInsteadOfEndingTheSession + AnUnreadUnmappableArrayDoesNotStopTheRestOfTheDraw) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.${mglItestArm}.VertexLayout." + TEST_FILTER "MagmaVertexLayoutScenario.${vertexLayoutCase}" + DISCOVERY_TIMEOUT 30 + PROPERTIES LABELS "${mglItestMagmaLabels}" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ${mglItestArmProperties} + ENVIRONMENT "${MGL_ITEST_MAGMA_VERTEXLAYOUT_ENVIRONMENT}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/magma-vertexlayout${mglItestArmLogTag}-${vertexLayoutCase}.log") + endforeach() + endforeach() +endif() + # P5c ct (MG_Remote/CONTRACT-P5C.md §5): the two control records, split-only source for the # same reason as f1's - the scenario reads the SERVER sink's tallies, which a monolith build # has no symbol for. One block per case with a LOG PATH OF ITS OWN, the F1 pattern: diff --git a/MobileGL/MG_IntegrationTest/Scenarios/MagmaVertexLayoutScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/MagmaVertexLayoutScenario.cpp new file mode 100644 index 00000000..7647e792 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/MagmaVertexLayoutScenario.cpp @@ -0,0 +1,220 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/MagmaVertexLayoutScenario.cpp +// Copyright (c) 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 +// +// P7 wave 2 package C, CONTRACT-P7 §3.2's `vertex-layout` row: THE ATTRIBUTE VULKAN CANNOT MAP. +// +// WHAT THIS EXISTS TO CATCH, and it is not a picture. Until this package +// VertexInputStateFactory::BuildWireVertexInput answered SEVEN different device/format +// conditions and one protocol condition with the same `false`, and the single caller answered +// that `false` with a P7-marked MagmaWireFatal - which, before wave 0 funnelled it, was a bare +// std::abort(). So a Magma split session that met a GL_FIXED array, a dvec on a device with +// native fp64, or any format without VK_FORMAT_FEATURE_VERTEX_BUFFER_BIT did not decline the +// draw: it KILLED THE PROCESS. The monolith arm on the very same device has always masked the +// attribute out of the vertex input state and then declined the draw loudly and once, which is +// the answer GL asks for. These cases pin that the two arms now agree. +// +// WHY GL_FIXED IS THE VEHICLE. It is the one unmappable shape that needs no device to cooperate: +// DataType::Fixed32 has no arm in ToVkVertexFormat at all, so the format map answers +// VK_FORMAT_UNDEFINED on every ICD including the lavapipe the host lanes run on. That matters +// because CONTRACT-P7 §2.5 records that host lavapipe reaches no other P7-marked refusal - +// the readback matrices and odd-offset uniform ranges that reach the rest need real hardware - +// so without this shape the whole `vertex-layout` retirement would have no host gate at all. +// +// DIRECTVULKAN ONLY, deliberately. The claim is about what Magma does with a format Vulkan +// cannot express; Espryt hands GL_FIXED straight to a driver that supports it and renders, so +// registering these on DirectGLES would assert a different fact under the same name. + +#include "../Harness/ScenarioFixture.h" +#include "../Harness/SplitRuntimePeek.h" + +#include +#include + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { +namespace { + + constexpr int kWidth = 16, kHeight = 16; + + // Reads BOTH attributes, so an array masked out of the vertex input state is one the + // program actually consumes and the draw must decline rather than silently substitute the + // current attribute value. + constexpr const char* kVertexReadsBoth = R"(#version 430 core +layout(location=0) in vec2 inPos; +layout(location=1) in vec3 inTint; +out vec3 vTint; +void main() { + vTint = inTint; + gl_Position = vec4(inPos, 0.0, 1.0); +})"; + + // Reads ONLY location 0. An enabled-but-unread array is not part of this program's vertex + // input at all, so neither arm has anything to mask and the draw must land. + constexpr const char* kVertexReadsPositionOnly = R"(#version 430 core +layout(location=0) in vec2 inPos; +out vec3 vTint; +void main() { + vTint = vec3(0.0, 1.0, 0.0); + gl_Position = vec4(inPos, 0.0, 1.0); +})"; + + constexpr const char* kFragment = R"(#version 430 core +in vec3 vTint; +layout(location=0) out vec4 color; +void main() { color = vec4(vTint, 1.0); })"; + + class MagmaVertexLayoutScenario : public ScenarioTest { + protected: + GLuint m_vao = 0, m_positions = 0, m_fixedAttrib = 0, m_program = 0; + + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + if (Gl().BackendName() != "DirectVulkan") { + GTEST_SKIP() << "the format map under test is Magma's; Espryt hands GL_FIXED to a " + "driver that supports it"; + } + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glDisable(GL_STENCIL_TEST); + glViewport(0, 0, kWidth, kHeight); + } + + void TearDown() override { + if (m_program) glDeleteProgram(m_program); + if (m_vao) glDeleteVertexArrays(1, &m_vao); + if (m_positions) glDeleteBuffers(1, &m_positions); + if (m_fixedAttrib) glDeleteBuffers(1, &m_fixedAttrib); + ScenarioTest::TearDown(); + } + + GLuint MakeProgram(const char* vertexSource) { + const GLuint vs = glCreateShader(GL_VERTEX_SHADER); + glShaderSource(vs, 1, &vertexSource, nullptr); + glCompileShader(vs); + GLint ok = GL_FALSE; + glGetShaderiv(vs, GL_COMPILE_STATUS, &ok); + EXPECT_EQ(ok, GL_TRUE) << "vertex shader did not compile"; + const GLuint fs = glCreateShader(GL_FRAGMENT_SHADER); + glShaderSource(fs, 1, &kFragment, nullptr); + glCompileShader(fs); + glGetShaderiv(fs, GL_COMPILE_STATUS, &ok); + EXPECT_EQ(ok, GL_TRUE) << "fragment shader did not compile"; + const GLuint program = glCreateProgram(); + glAttachShader(program, vs); + glAttachShader(program, fs); + glLinkProgram(program); + glGetProgramiv(program, GL_LINK_STATUS, &ok); + EXPECT_EQ(ok, GL_TRUE) << "program did not link"; + glDeleteShader(vs); + glDeleteShader(fs); + return program; + } + + // A full-viewport triangle in location 0 and a GL_FIXED size-3 array in location 1. + // GL_FIXED is 16.16 fixed point; the bytes are never fetched on the arm under test + // (the attribute is masked out before any binding is built), so their VALUE matters + // only for the unread case, where nothing reads them either. + void BuildVaoWithAFixedAttribute() { + const float positions[6] = {-1.0f, -1.0f, 3.0f, -1.0f, -1.0f, 3.0f}; + const GLint fixedTint[9] = {0, 1 << 16, 0, 0, 1 << 16, 0, 0, 1 << 16, 0}; + + glGenVertexArrays(1, &m_vao); + glBindVertexArray(m_vao); + + glGenBuffers(1, &m_positions); + glBindBuffer(GL_ARRAY_BUFFER, m_positions); + glBufferData(GL_ARRAY_BUFFER, sizeof(positions), positions, GL_STATIC_DRAW); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 0, nullptr); + glEnableVertexAttribArray(0); + + glGenBuffers(1, &m_fixedAttrib); + glBindBuffer(GL_ARRAY_BUFFER, m_fixedAttrib); + glBufferData(GL_ARRAY_BUFFER, sizeof(fixedTint), fixedTint, GL_STATIC_DRAW); + // THE SHAPE UNDER TEST: size 3, GL_FIXED. ToVkVertexFormat has no Fixed32 arm, so + // this resolves to VK_FORMAT_UNDEFINED and the wire builder's `format-map` reason + // fires - the reason that used to abort the session. + glVertexAttribPointer(1, 3, GL_FIXED, GL_FALSE, 0, nullptr); + glEnableVertexAttribArray(1); + glBindBuffer(GL_ARRAY_BUFFER, 0); + } + + std::array ClearDrawRead(GLuint program) { + glClearColor(0.0f, 0.0f, 1.0f, 1.0f); // blue: the "the draw did not land" colour + glClear(GL_COLOR_BUFFER_BIT); + glUseProgram(program); + glBindVertexArray(m_vao); + glDrawArrays(GL_TRIANGLES, 0, 3); + std::array pixel{}; + glReadPixels(kWidth / 2, kHeight / 2, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, pixel.data()); + glBindVertexArray(0); + glUseProgram(0); + return pixel; + } + }; + + // ------------------------------------------------------------------------------------ + // THE RETIREMENT ITSELF. The program READS the unmappable array, so the attribute is + // masked out of the vertex input state and the draw is then declined - loudly, once, and + // without touching the session. Before this package the same GL killed the process. + // + // THE ASSERTION IS THAT THE PROCESS IS STILL HERE and that the framebuffer still holds the + // clear, which together say "declined" rather than "rendered wrong pixels". A gtest case + // cannot assert the absence of an abort any more directly than by reaching its next + // statement: the old behaviour took the whole binary down, so every case after this one in + // the lane was equally the control. + TEST_F(MagmaVertexLayoutScenario, AnUnmappableVertexFormatDeclinesTheDrawInsteadOfEndingTheSession) { + if (!Ready() || IsSkipped()) return; + m_program = MakeProgram(kVertexReadsBoth); + BuildVaoWithAFixedAttribute(); + const auto pixel = ClearDrawRead(m_program); + + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) + << "a declined draw must not invent a GL error: GL has no error for 'this " + "implementation cannot fetch that array', and the monolith arm raises none"; + const int blue[4] = {0, 0, 255, 255}; + for (int i = 0; i < 4; ++i) { + EXPECT_NEAR(pixel[i], blue[i], 1) + << "component " << i << ": the draw was expected to DECLINE (leaving the clear " + "colour) because location 1 is an enabled GL_FIXED array the program reads " + "and Vulkan has no format for it. A non-clear pixel means the attribute was " + "silently replaced by the current vertex attribute value, which is the exact " + "wrong-pixels outcome the monolith arm's pre-flight refuses."; + } + } + + // ------------------------------------------------------------------------------------ + // THE OTHER HALF, and it is what keeps the case above from being satisfied by "decline + // every draw that mentions GL_FIXED". The array is still enabled and still unmappable; the + // program simply does not read it, so it is not part of this program's vertex input on + // either arm and the draw must land normally. + TEST_F(MagmaVertexLayoutScenario, AnUnreadUnmappableArrayDoesNotStopTheRestOfTheDraw) { + if (!Ready() || IsSkipped()) return; + m_program = MakeProgram(kVertexReadsPositionOnly); + BuildVaoWithAFixedAttribute(); + const auto pixel = ClearDrawRead(m_program); + + EXPECT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "the draw should have been ordinary"; + const int green[4] = {0, 255, 0, 255}; + for (int i = 0; i < 4; ++i) { + EXPECT_NEAR(pixel[i], green[i], 1) + << "component " << i << ": an enabled but UNREAD GL_FIXED array must not cost " + "the draw. Blue here means the whole draw was declined for an attribute " + "that is not part of this program's vertex input at all."; + } + } + +} // namespace +} // namespace MGITest diff --git a/docs/Disaggregated/notes/p7/magma-c.md b/docs/Disaggregated/notes/p7/magma-c.md index 8e6b298a..0c818614 100644 --- a/docs/Disaggregated/notes/p7/magma-c.md +++ b/docs/Disaggregated/notes/p7/magma-c.md @@ -151,3 +151,84 @@ case did no GL work and failed on `ScenarioFixture.h:90` — an armed split lane whose encoder ordinal did not move, because "emitted nothing" and "resolved the transport and then put nothing on the wire" are the same observation. The case now clears and reads back first. This is the same F1 arming rule package L hit on tier 3. + +--- + +## Slice 4 — the `vertex-layout` split and `vertex-format-conversion` (§3.2) + +**What landed.** `BuildWireVertexInput` answered eight conditions with one `false`, and the +caller answered that with a P7-marked `MagmaWireFatal` — a session kill where the monolith arm +masks the attribute out and declines the draw. Split by what the reason is ABOUT: + +| reason | before | after | +|---|---|---| +| `buffer-window` | `false` → P7-marked Fatal | `false` → named Fatal `Magma:vertex-layout-buffer-window` (no P7 marker: it is a permanent protocol error, not an unmigrated shape) | +| `attribute-shape`, `native-fp64-format`, `format-map`, `native-format-feature`, `converted-format-feature`, `element-size`, `offset-overflow` | `false` → Fatal | `MGLOG_E_ONCE` + `unsupportedAttribMask \|= 1<