diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/NarrowIntegerVertexFormats.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/NarrowIntegerVertexFormats.h new file mode 100644 index 00000000..8f02a1a6 --- /dev/null +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/NarrowIntegerVertexFormats.h @@ -0,0 +1,133 @@ +// MobileGL - MobileGL/MG_Backend/DirectVulkan/Renderer/NarrowIntegerVertexFormats.h +// Copyright (c) 2025-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 + +#pragma once + +#include "../VkIncludes.h" +#include + +#include + +namespace MobileGL::MG_Backend::DirectVulkan { + using MG_Util::ShaderTranspiler::NarrowIntegerVertexInputs; + + // Which vertex inputs need the ExtendNarrowIntegerVertexInputs program variant, decided from + // two halves that are each computed once and cached with the object they describe. + // + // The pipeline gives an integer attribute the fetch format of the shader input's signedness + // (VUID-VkGraphicsPipelineCreateInfo-Input-08733). For 32-bit data that swap only renames the + // bits; for 8/16-bit data it changes the extension rule, so 240 in GL_UNSIGNED_BYTE data + // reaches an `int` input as -16. These are exactly the (data, input) pairs that need it. + enum class NarrowIntegerVertexFormat : Uint8 { + None = 0, + Unsigned8, + Unsigned16, + Signed8, + Signed16, + }; + + constexpr NarrowIntegerVertexFormat ClassifyNarrowIntegerVertexFormat(VkFormat format) { + switch (format) { + case VK_FORMAT_R8_UINT: + case VK_FORMAT_R8G8_UINT: + case VK_FORMAT_R8G8B8_UINT: + case VK_FORMAT_R8G8B8A8_UINT: + return NarrowIntegerVertexFormat::Unsigned8; + case VK_FORMAT_R16_UINT: + case VK_FORMAT_R16G16_UINT: + case VK_FORMAT_R16G16B16_UINT: + case VK_FORMAT_R16G16B16A16_UINT: + return NarrowIntegerVertexFormat::Unsigned16; + case VK_FORMAT_R8_SINT: + case VK_FORMAT_R8G8_SINT: + case VK_FORMAT_R8G8B8_SINT: + case VK_FORMAT_R8G8B8A8_SINT: + return NarrowIntegerVertexFormat::Signed8; + case VK_FORMAT_R16_SINT: + case VK_FORMAT_R16G16_SINT: + case VK_FORMAT_R16G16B16_SINT: + case VK_FORMAT_R16G16B16A16_SINT: + return NarrowIntegerVertexFormat::Signed16; + default: + return NarrowIntegerVertexFormat::None; + } + } + + // The vertex-input half: the locations whose attribute DATA is narrow integer, by width and + // signedness. Filled from the source formats when a vertex input layout is built, so a draw + // reads four words instead of walking its attributes. + struct NarrowIntegerAttributeMasks { + Uint32 unsigned8 = 0; + Uint32 unsigned16 = 0; + Uint32 signed8 = 0; + Uint32 signed16 = 0; + + constexpr void Note(Uint32 location, VkFormat format) { + if (location >= 32) return; + const Uint32 bit = 1u << location; + switch (ClassifyNarrowIntegerVertexFormat(format)) { + case NarrowIntegerVertexFormat::Unsigned8: + unsigned8 |= bit; + break; + case NarrowIntegerVertexFormat::Unsigned16: + unsigned16 |= bit; + break; + case NarrowIntegerVertexFormat::Signed8: + signed8 |= bit; + break; + case NarrowIntegerVertexFormat::Signed16: + signed16 |= bit; + break; + case NarrowIntegerVertexFormat::None: + break; + } + } + }; + + // The program half: the locations whose shader input is a 32-bit int or uint scalar/vector. + // Both masks are zero for a program without integer vertex inputs, which is what keeps the + // common draw at four AND instructions. + struct IntegerVertexInputMasks { + Uint32 signedInputs = 0; + Uint32 unsignedInputs = 0; + + constexpr void Note(Uint32 location, GLenum shaderInputType) { + if (location >= 32) return; + const Uint32 bit = 1u << location; + switch (shaderInputType) { + case GL_INT: + case GL_INT_VEC2: + case GL_INT_VEC3: + case GL_INT_VEC4: + signedInputs |= bit; + break; + case GL_UNSIGNED_INT: + case GL_UNSIGNED_INT_VEC2: + case GL_UNSIGNED_INT_VEC3: + case GL_UNSIGNED_INT_VEC4: + unsignedInputs |= bit; + break; + default: + break; + } + } + }; + + // Unsigned narrow data into an int input is fetched sign-extended and must be zero-extended; + // signed narrow data into a uint input is fetched zero-extended and must be sign-extended. + // Matching signedness, 32-bit data and float inputs need nothing. + constexpr NarrowIntegerVertexInputs ResolveNarrowIntegerVertexInputs(const IntegerVertexInputMasks& inputs, + const NarrowIntegerAttributeMasks& data) { + NarrowIntegerVertexInputs result; + result.zeroExtend8 = inputs.signedInputs & data.unsigned8; + result.zeroExtend16 = inputs.signedInputs & data.unsigned16; + result.signExtend8 = inputs.unsignedInputs & data.signed8; + result.signExtend16 = inputs.unsignedInputs & data.signed16; + return result; + } +} // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp index c285209c..95302868 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.cpp @@ -2736,6 +2736,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkProgramObject& entry) const { entry.activeVertexInputLocationMask = 0; entry.vertexInputTypes.fill(0); + entry.integerVertexInputs = {}; entry.readsBaseVertexBuiltin = false; for (SizeT moduleIndex = 0; moduleIndex < stages.size() && moduleIndex < spirv.size(); ++moduleIndex) { @@ -2802,6 +2803,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { entry.activeVertexInputLocationMask |= (1u << expandedLocation); entry.vertexInputTypes[expandedLocation] = locationType; + entry.integerVertexInputs.Note(expandedLocation, locationType); } } } @@ -3553,8 +3555,80 @@ namespace MobileGL::MG_Backend::DirectVulkan { ++m_cacheStructureEpoch; } - const ProgramFactory::VkProgramObject& ProgramFactory::GetOrCreateProgram(const MagmaProgramSource& program, - CompileOptionFlags flags) { + namespace { + // The vertex stage's own transforms, in the order they have to run: the ZeroBaseVertex + // variant, the unconditional 64-bit input packing, then the narrow-integer variant. + void TransformVertexStageForVulkan(Vector& module, ProgramFactory::CompileOptionFlags flags, + const NarrowIntegerVertexInputs& narrowIntegerInputs, + Bool enableSpirvValidation, Uint programIndex) { + // The non-indexed variant of a vertex stage that reads gl_BaseVertex: GL wants zero + // there, Vulkan's builtin would hand it the draw's firstVertex. Requested per draw + // through CompileOptionBit::ZeroBaseVertex, so the indexed variant of the same + // program keeps the native builtin and stays correct for glDrawElementsBaseVertex + // and for the baseVertex word of an indexed indirect command. + if (flags & ProgramFactory::CompileOptionBit::ZeroBaseVertex) { + Vector zeroedSpirv; + if (MG_Util::ShaderTranspiler::ShaderCompiler::ZeroBaseVertexForVulkan(module, zeroedSpirv, + enableSpirvValidation)) { + module = std::move(zeroedSpirv); + } else { + // Failing open keeps the native builtin, which is the pre-fix behavior: + // gl_BaseVertex reads firstVertex on a DrawArrays instead of zero. + MGLOG_E("ProgramFactory: failed to zero gl_BaseVertex for program %u; non-indexed " + "draws will read the draw's first vertex from it instead of zero", + programIndex); + } + } + + // A 64-bit vertex input has to arrive as its 32-bit word pair: VK_FORMAT_R64*_SFLOAT is + // optional and lavapipe advertises none of them at all. The pass is unconditional so it + // always agrees with the Float64 case in VertexInputStateFactory::ToVkVertexFormat, and + // ReflectVertexInputs then sees an ordinary uvec2/uvec4 input. + // + // Failure here is not recoverable and must not be swallowed: ToVkVertexFormat has already + // committed to R32G32{,B32A32}_UINT for the attribute, so a module still declaring + // `in double` would reconcile to Unknown and build a pipeline with a UINT format under a + // double input - garbage with no diagnostic anywhere. + { + Vector packedSpirv; + const Bool packOk = MG_Util::ShaderTranspiler::ShaderCompiler::PackDoubleVertexInputsForVulkan( + module, packedSpirv, enableSpirvValidation); + MOBILEGL_ASSERT(packOk, + "ProgramFactory: 64-bit vertex input packing failed for program %u; the " + "vertex-input format and the shader input type now disagree", + programIndex); + if (packOk) { + module = std::move(packedSpirv); + } else { + MGLOG_E("ProgramFactory: failed to pack 64-bit vertex inputs for program %u; " + "double-typed vertex attributes will be fetched as uint32 words and not " + "reinterpreted", + programIndex); + } + } + + // The narrow-integer variant: the pipeline fetches these attributes with the format of + // the input's signedness, and this restores the extension rule of the data's own (see + // ResolveNarrowIntegerVertexInputs). After the 64-bit packing so it sees the inputs in + // the shape the pipeline's vertex input state and ReflectVertexInputs describe. + if (narrowIntegerInputs.Any()) { + Vector extendedSpirv; + if (MG_Util::ShaderTranspiler::ShaderCompiler::ExtendNarrowIntegerVertexInputsForVulkan( + module, extendedSpirv, narrowIntegerInputs, enableSpirvValidation)) { + module = std::move(extendedSpirv); + } else { + MGLOG_E("ProgramFactory: failed to extend narrow integer vertex inputs for program %u; " + "8/16-bit integer attributes read through an input of the other signedness keep " + "the fetch format's extension", + programIndex); + } + } + } + } // namespace + + const ProgramFactory::VkProgramObject& ProgramFactory::GetOrCreateProgram( + const MagmaProgramSource& program, CompileOptionFlags flags, + const NarrowIntegerVertexInputs& narrowIntegerInputs) { // Hashing the full SPIR-V of every stage is far too expensive to repeat per draw; // reuse the program's memoized hash while its backend state version is unchanged. // The memo keys on the flags word, which ComputeHash is no longer a pure function of: @@ -3575,6 +3649,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { hash = ComputeHash(program, flags); program.SetBackendHashMemo(memoKey, hash); } + // Mixed in after the memo, which keys on the flags word alone: the memo keeps answering + // for the plain program, and the variant costs one 16-byte hash on top of it. + if (narrowIntegerInputs.Any()) hash = MixNarrowIntegerVertexInputs(hash, narrowIntegerInputs); auto it = m_cache.find(hash); if (it != m_cache.end()) { // Every draw/dispatch funnels through this lookup (the renderer memos only @@ -3827,50 +3904,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - // The non-indexed variant of a vertex stage that reads gl_BaseVertex: GL wants zero - // there, Vulkan's builtin would hand it the draw's firstVertex. Requested per draw - // through CompileOptionBit::ZeroBaseVertex, so the indexed variant of the same - // program keeps the native builtin and stays correct for glDrawElementsBaseVertex - // and for the baseVertex word of an indexed indirect command. - if (stages[i] == ShaderStage::Vertex && (flags & CompileOptionBit::ZeroBaseVertex)) { - Vector zeroedSpirv; - if (MG_Util::ShaderTranspiler::ShaderCompiler::ZeroBaseVertexForVulkan(moduleSpirvs[i], zeroedSpirv, - enableSpirvValidation)) { - moduleSpirvs[i] = std::move(zeroedSpirv); - } else { - // Failing open keeps the native builtin, which is the pre-fix behavior: - // gl_BaseVertex reads firstVertex on a DrawArrays instead of zero. - MGLOG_E("ProgramFactory: failed to zero gl_BaseVertex for program %u; non-indexed " - "draws will read the draw's first vertex from it instead of zero", - program.GetExternalIndex()); - } - } - - // A 64-bit vertex input has to arrive as its 32-bit word pair: VK_FORMAT_R64*_SFLOAT is - // optional and lavapipe advertises none of them at all. The pass is unconditional so it - // always agrees with the Float64 case in VertexInputStateFactory::ToVkVertexFormat, and - // ReflectVertexInputs below then sees an ordinary uvec2/uvec4 input. - // - // Failure here is not recoverable and must not be swallowed: ToVkVertexFormat has already - // committed to R32G32{,B32A32}_UINT for the attribute, so a module still declaring - // `in double` would reconcile to Unknown and build a pipeline with a UINT format under a - // double input - garbage with no diagnostic anywhere. if (stages[i] == ShaderStage::Vertex) { - Vector packedSpirv; - const Bool packOk = MG_Util::ShaderTranspiler::ShaderCompiler::PackDoubleVertexInputsForVulkan( - moduleSpirvs[i], packedSpirv, enableSpirvValidation); - MOBILEGL_ASSERT(packOk, - "ProgramFactory: 64-bit vertex input packing failed for program %u; the " - "vertex-input format and the shader input type now disagree", - program.GetExternalIndex()); - if (packOk) { - moduleSpirvs[i] = std::move(packedSpirv); - } else { - MGLOG_E("ProgramFactory: failed to pack 64-bit vertex inputs for program %u; " - "double-typed vertex attributes will be fetched as uint32 words and not " - "reinterpreted", - program.GetExternalIndex()); - } + TransformVertexStageForVulkan(moduleSpirvs[i], flags, narrowIntegerInputs, enableSpirvValidation, + program.GetExternalIndex()); } // When Vulkan can legally access storage images without a statically declared diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h index a1178993..743ec60b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/ProgramFactory.h @@ -12,6 +12,7 @@ #include "../MagmaSession.h" #include "PipelineFactory.h" #include "MagmaProgramSource.h" +#include "NarrowIntegerVertexFormats.h" #include "MG_State/GLState/ProgramState/ShaderObject.h" #include "MG_State/GLState/TextureState/TextureEnum.h" @@ -200,6 +201,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { Int globalUboBinding = -1; Uint32 activeVertexInputLocationMask = 0; Array vertexInputTypes{}; + // The locations of vertexInputTypes that are int / uint, for the per-draw narrow-integer + // variant decision (ResolveNarrowIntegerVertexInputs). Identical in every variant. + IntegerVertexInputMasks integerVertexInputs; Uint32 activeFragmentOutputLocationMask = 0; Array fragmentOutputTypes{}; ShaderStage rasterizationProducerStage = ShaderStage::Unknown; @@ -328,6 +332,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { globalUboBinding = other.globalUboBinding; activeVertexInputLocationMask = other.activeVertexInputLocationMask; vertexInputTypes = other.vertexInputTypes; + integerVertexInputs = other.integerVertexInputs; activeFragmentOutputLocationMask = other.activeFragmentOutputLocationMask; fragmentOutputTypes = other.fragmentOutputTypes; rasterizationProducerStage = other.rasterizationProducerStage; @@ -351,6 +356,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { other.pointSizeCapabilityUnsupported = false; other.globalUboBinding = -1; other.activeVertexInputLocationMask = 0; + other.integerVertexInputs = {}; other.activeFragmentOutputLocationMask = 0; other.rasterizationProducerStage = ShaderStage::Unknown; other.producerOutputComponentCount = 0; @@ -402,6 +408,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { globalUboBinding = other.globalUboBinding; activeVertexInputLocationMask = other.activeVertexInputLocationMask; vertexInputTypes = other.vertexInputTypes; + integerVertexInputs = other.integerVertexInputs; activeFragmentOutputLocationMask = other.activeFragmentOutputLocationMask; fragmentOutputTypes = other.fragmentOutputTypes; rasterizationProducerStage = other.rasterizationProducerStage; @@ -425,6 +432,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { other.pointSizeCapabilityUnsupported = false; other.globalUboBinding = -1; other.activeVertexInputLocationMask = 0; + other.integerVertexInputs = {}; other.activeFragmentOutputLocationMask = 0; other.rasterizationProducerStage = ShaderStage::Unknown; other.producerOutputComponentCount = 0; @@ -518,7 +526,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { ProgramFactory(const ProgramFactory&) = delete; HashType ComputeHash(const MagmaProgramSource& program, CompileOptionFlags flags) const; - const VkProgramObject& GetOrCreateProgram(const MagmaProgramSource& program, CompileOptionFlags flags); + // `narrowIntegerInputs` selects the variant whose vertex stage widens narrow integer + // attribute data GL's way (ExtendNarrowIntegerVertexInputsPass). It is draw-time state like + // the flags, and the draw path asks for it only when ResolveNarrowIntegerVertexInputs finds + // a location that needs it, so every other program keeps a single entry. + const VkProgramObject& GetOrCreateProgram(const MagmaProgramSource& program, CompileOptionFlags flags, + const NarrowIntegerVertexInputs& narrowIntegerInputs = {}); + // The cache identity of a narrow-integer variant: the hash of the same program and flags + // without it, mixed with the corrected locations. Content in, content out, so the variant's + // pipelines key apart from the plain program's through programHash alone. + static HashType MixNarrowIntegerVertexInputs(HashType hash, const NarrowIntegerVertexInputs& inputs) { + return XXH64(&inputs, sizeof(inputs), hash); + } // The default framebuffer's current image height, baked as a literal into every // FragCoordYFlip variant (there is no push-constant or specialization channel here, and diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index 84be9077..158394c6 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -214,6 +214,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { out.bindings.push_back( {binding, stride, buffer.Divisor ? VK_VERTEX_INPUT_RATE_INSTANCE : VK_VERTEX_INPUT_RATE_VERTEX}); out.attributes.push_back({location, binding, format, 0}); + out.narrowIntegerAttributes.Note(location, format); out.bindingAttributeLocations.push_back(location); out.bindingBaseOffsets.push_back(static_cast(offset)); out.bindingConversions.push_back(conversion); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h index f234cf2b..31c4d32a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.h @@ -14,6 +14,7 @@ #include "Config.h" #include "MagmaPipeArms.h" +#include "NarrowIntegerVertexFormats.h" #include "VertexInputStateBuilder.h" #include "MG_State/GLState/VertexArrayState/VertexArrayObject.h" #include @@ -63,6 +64,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // Bitmask of `attributes[i].location` - the draw path needs it up to // three times per draw, so it is baked once at build time. Uint32 attributeLocationMask = 0; + // The locations of `attributes` whose source data is 8/16-bit integer, by width and + // signedness - the vertex-input half of the narrow-integer program variant decision + // (NarrowIntegerVertexFormats.h). A pure function of `attributes`, so `layoutHash` + // already covers it. + NarrowIntegerAttributeMasks narrowIntegerAttributes; // Per-binding glVertexAttribDivisor values other than 1. Vulkan's instance input // rate advances once per instance and nothing else, so anything else has to be // stated through VK_EXT_vertex_attribute_divisor. Empty when every instanced diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRendererPipeline.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRendererPipeline.cpp index f70887c6..f6dd8454 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRendererPipeline.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRendererPipeline.cpp @@ -918,6 +918,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { const Uint32 vertexInputAttribMask = vis.attributeLocationMask; const Uint32 activeAttribMask = programObj.activeVertexInputLocationMask; const Uint32 missingAttribMask = activeAttribMask & ~vertexInputAttribMask; + // The fetch format takes the shader input's signedness (VUID-VkGraphicsPipelineCreateInfo- + // Input-08733). That renames the bits of 32-bit data, but extends 8/16-bit data by the + // input's rule instead of the data's; the program variant SetupWireDraw picks for exactly + // those attributes (ResolveNarrowIntegerVertexInputs) puts GL's value back in the shader. auto& patchedAttributes = m_patchedAttributesScratch; patchedAttributes.assign(vis.attributes.begin(), vis.attributes.end()); Bool hasPatchedVertexAttributes = false; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.cpp index 3f0af3e6..a31af943 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.cpp @@ -739,6 +739,43 @@ namespace MobileGL::MG_Backend::DirectVulkan { flags |= ProgramFactory::CompileOptionBit::ZeroBaseVertex; programObject = &m_programFactory->GetOrCreateProgram(program, flags); } + const auto vaoHandle = state.BoundVertexElements; + if (MG_Pipe::MGPipeHandleIsNull(vaoHandle)) MagmaWireFatal("vertex-elements-record"); + if (vaoHandle.Slot >= state.VertexElementsCsos.size()) MagmaWireFatal("vertex-elements-record"); + const auto& vao = state.VertexElementsCsos[vaoHandle.Slot]; + if (!vao.Live || vao.Gen != vaoHandle.Gen) MagmaWireFatal("vertex-elements-generation"); + // 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). + // P14: resolved through the record's own memo (ResolveWireVertexInput), except in a nested + // preparation, which builds into its own scratch rather than under the outer draw's feet. + const VertexInputStateFactory::BackendVertexInputState* resolvedInput = nullptr; + if (&scratch == &m_wireDrawScratch) { + resolvedInput = m_vertexInputStateFactory->ResolveWireVertexInput( + vao, state, programObject->activeVertexInputLocationMask); + } else if (m_vertexInputStateFactory->BuildWireVertexInput( + vao, state, programObject->activeVertexInputLocationMask, scratch.input)) { + resolvedInput = &scratch.input; + } + if (resolvedInput == nullptr) MagmaWireFatal("vertex-layout-buffer-window"); + const auto& input = *resolvedInput; + // The narrow-integer variant (NarrowIntegerVertexFormats.h), chosen from two masks cached on + // the program entry and on the resolved layout: a program without integer vertex inputs pays + // four ANDs here. Every variant reflects the same inputs, so the layout resolved above with + // the plain program's active mask is this variant's too. + const NarrowIntegerVertexInputs narrowIntegerInputs = + ResolveNarrowIntegerVertexInputs(programObject->integerVertexInputs, input.narrowIntegerAttributes); + if (narrowIntegerInputs.Any()) + programObject = &m_programFactory->GetOrCreateProgram(program, flags, narrowIntegerInputs); if (programObject->invalidSpirvDeclined) { MGL_WIRE_DECLINE_AT(ProgramInvalidSpirv, "a module of the program fails SPIR-V validation and was never built"); @@ -776,35 +813,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { (void)ResolveWireImage(*fbo, fbo->Stencil, VK_IMAGE_ASPECT_STENCIL_BIT); } m_textureManager->FlushPendingUploads(); - const auto vaoHandle = state.BoundVertexElements; - if (MG_Pipe::MGPipeHandleIsNull(vaoHandle)) MagmaWireFatal("vertex-elements-record"); - if (vaoHandle.Slot >= state.VertexElementsCsos.size()) MagmaWireFatal("vertex-elements-record"); - const auto& vao = state.VertexElementsCsos[vaoHandle.Slot]; - if (!vao.Live || vao.Gen != vaoHandle.Gen) MagmaWireFatal("vertex-elements-generation"); - // 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). - // P14: resolved through the record's own memo (ResolveWireVertexInput), except in a nested - // preparation, which builds into its own scratch rather than under the outer draw's feet. - const VertexInputStateFactory::BackendVertexInputState* resolvedInput = nullptr; - if (&scratch == &m_wireDrawScratch) { - resolvedInput = - m_vertexInputStateFactory->ResolveWireVertexInput(vao, state, programObj.activeVertexInputLocationMask); - } else if (m_vertexInputStateFactory->BuildWireVertexInput(vao, state, programObj.activeVertexInputLocationMask, - scratch.input)) { - resolvedInput = &scratch.input; - } - if (resolvedInput == nullptr) MagmaWireFatal("vertex-layout-buffer-window"); - const auto& input = *resolvedInput; // 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 diff --git a/MobileGL/MG_Test/Pipeline/CMakeLists.txt b/MobileGL/MG_Test/Pipeline/CMakeLists.txt index deb5b1ef..c6e131fd 100644 --- a/MobileGL/MG_Test/Pipeline/CMakeLists.txt +++ b/MobileGL/MG_Test/Pipeline/CMakeLists.txt @@ -14,6 +14,7 @@ add_executable( PassthroughTessControlTest.cpp ViewportIndexReflectionTest.cpp DriverModuleScreenTest.cpp + NarrowIntegerVertexFormatsTest.cpp ) target_include_directories(PipelineQuirkTest PRIVATE diff --git a/MobileGL/MG_Test/Pipeline/NarrowIntegerVertexFormatsTest.cpp b/MobileGL/MG_Test/Pipeline/NarrowIntegerVertexFormatsTest.cpp new file mode 100644 index 00000000..84cc7524 --- /dev/null +++ b/MobileGL/MG_Test/Pipeline/NarrowIntegerVertexFormatsTest.cpp @@ -0,0 +1,157 @@ +// MobileGL - MobileGL/MG_Test/Pipeline/NarrowIntegerVertexFormatsTest.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 +// +// Which vertex inputs take Magma's narrow-integer program variant. The pipeline fetches an integer +// attribute with the format of the shader input's signedness, which re-extends 8/16-bit data by the +// wrong rule exactly when the data and the input disagree in signedness; every other pairing - +// 32-bit data, matching signedness, float data or float inputs - must keep today's program. + +#include + +#include +#include + +using namespace MobileGL; +using MobileGL::MG_Backend::DirectVulkan::ClassifyNarrowIntegerVertexFormat; +using MobileGL::MG_Backend::DirectVulkan::IntegerVertexInputMasks; +using MobileGL::MG_Backend::DirectVulkan::NarrowIntegerAttributeMasks; +using MobileGL::MG_Backend::DirectVulkan::NarrowIntegerVertexFormat; +using MobileGL::MG_Backend::DirectVulkan::NarrowIntegerVertexInputs; +using MobileGL::MG_Backend::DirectVulkan::ProgramFactory; +using MobileGL::MG_Backend::DirectVulkan::ResolveNarrowIntegerVertexInputs; + +namespace { + // The correction one (data format, shader input type) pair gets, alone at `location`. + NarrowIntegerVertexInputs Resolve(VkFormat format, GLenum inputType, Uint32 location = 3) { + IntegerVertexInputMasks inputs; + inputs.Note(location, inputType); + NarrowIntegerAttributeMasks data; + data.Note(location, format); + return ResolveNarrowIntegerVertexInputs(inputs, data); + } + + NarrowIntegerVertexInputs Only(Uint32 NarrowIntegerVertexInputs::* field, Uint32 location = 3) { + NarrowIntegerVertexInputs expected; + expected.*field = 1u << location; + return expected; + } +} // namespace + +TEST(NarrowIntegerVertexFormats, ClassifiesExactlyThe8And16BitIntegerFormats) { + for (const VkFormat format : + {VK_FORMAT_R8_UINT, VK_FORMAT_R8G8_UINT, VK_FORMAT_R8G8B8_UINT, VK_FORMAT_R8G8B8A8_UINT}) + EXPECT_EQ(ClassifyNarrowIntegerVertexFormat(format), NarrowIntegerVertexFormat::Unsigned8) << format; + for (const VkFormat format : + {VK_FORMAT_R16_UINT, VK_FORMAT_R16G16_UINT, VK_FORMAT_R16G16B16_UINT, VK_FORMAT_R16G16B16A16_UINT}) + EXPECT_EQ(ClassifyNarrowIntegerVertexFormat(format), NarrowIntegerVertexFormat::Unsigned16) << format; + for (const VkFormat format : + {VK_FORMAT_R8_SINT, VK_FORMAT_R8G8_SINT, VK_FORMAT_R8G8B8_SINT, VK_FORMAT_R8G8B8A8_SINT}) + EXPECT_EQ(ClassifyNarrowIntegerVertexFormat(format), NarrowIntegerVertexFormat::Signed8) << format; + for (const VkFormat format : + {VK_FORMAT_R16_SINT, VK_FORMAT_R16G16_SINT, VK_FORMAT_R16G16B16_SINT, VK_FORMAT_R16G16B16A16_SINT}) + EXPECT_EQ(ClassifyNarrowIntegerVertexFormat(format), NarrowIntegerVertexFormat::Signed16) << format; + // 32-bit integers, float/normalized/scaled data and packed or reversed formats never qualify. + for (const VkFormat format : + {VK_FORMAT_R32_UINT, VK_FORMAT_R32G32B32A32_UINT, VK_FORMAT_R32_SINT, VK_FORMAT_R32G32B32A32_SINT, + VK_FORMAT_R8G8B8A8_UNORM, VK_FORMAT_R8G8B8A8_SNORM, VK_FORMAT_R8G8B8A8_USCALED, VK_FORMAT_R16G16_SSCALED, + VK_FORMAT_R16_SFLOAT, VK_FORMAT_R32G32_SFLOAT, VK_FORMAT_B8G8R8A8_UNORM, VK_FORMAT_A2B10G10R10_UINT_PACK32, + VK_FORMAT_A2B10G10R10_SNORM_PACK32, VK_FORMAT_UNDEFINED}) + EXPECT_EQ(ClassifyNarrowIntegerVertexFormat(format), NarrowIntegerVertexFormat::None) << format; +} + +TEST(NarrowIntegerVertexFormats, CorrectsOnlyTheSignednessMismatches) { + // Sodium 0.5's a_LightAndData: GL_UNSIGNED_BYTE x4 into `in ivec4`. + EXPECT_EQ(Resolve(VK_FORMAT_R8G8B8A8_UINT, GL_INT_VEC4), Only(&NarrowIntegerVertexInputs::zeroExtend8)); + EXPECT_EQ(Resolve(VK_FORMAT_R8_UINT, GL_INT), Only(&NarrowIntegerVertexInputs::zeroExtend8)); + EXPECT_EQ(Resolve(VK_FORMAT_R16G16_UINT, GL_INT_VEC2), Only(&NarrowIntegerVertexInputs::zeroExtend16)); + EXPECT_EQ(Resolve(VK_FORMAT_R16G16B16_UINT, GL_INT_VEC3), Only(&NarrowIntegerVertexInputs::zeroExtend16)); + EXPECT_EQ(Resolve(VK_FORMAT_R8G8B8A8_SINT, GL_UNSIGNED_INT_VEC4), Only(&NarrowIntegerVertexInputs::signExtend8)); + EXPECT_EQ(Resolve(VK_FORMAT_R8G8_SINT, GL_UNSIGNED_INT_VEC2), Only(&NarrowIntegerVertexInputs::signExtend8)); + EXPECT_EQ(Resolve(VK_FORMAT_R16_SINT, GL_UNSIGNED_INT), Only(&NarrowIntegerVertexInputs::signExtend16)); + + // Matching signedness: the fetch already extends by the data's rule. + EXPECT_FALSE(Resolve(VK_FORMAT_R8G8B8A8_UINT, GL_UNSIGNED_INT_VEC4).Any()); + EXPECT_FALSE(Resolve(VK_FORMAT_R16_SINT, GL_INT).Any()); + // 32-bit data: the format swap renames bits, which is GL's reinterpretation already. + EXPECT_FALSE(Resolve(VK_FORMAT_R32_UINT, GL_INT).Any()); + EXPECT_FALSE(Resolve(VK_FORMAT_R32G32B32A32_SINT, GL_UNSIGNED_INT_VEC4).Any()); + // Float data into an integer input and integer data into a float input are not this rule. + EXPECT_FALSE(Resolve(VK_FORMAT_R8G8B8A8_USCALED, GL_INT_VEC4).Any()); + EXPECT_FALSE(Resolve(VK_FORMAT_R8_UNORM, GL_UNSIGNED_INT).Any()); + EXPECT_FALSE(Resolve(VK_FORMAT_R8G8B8A8_UINT, GL_FLOAT_VEC4).Any()); + EXPECT_FALSE(Resolve(VK_FORMAT_R16_SINT, GL_FLOAT).Any()); +} + +// Locations are independent bits, a location the program does not read as an integer is ignored +// even when its data is narrow, and the top location still fits. +TEST(NarrowIntegerVertexFormats, KeepsLocationsApart) { + IntegerVertexInputMasks inputs; + inputs.Note(0, GL_FLOAT_VEC3); + inputs.Note(1, GL_INT_VEC4); + inputs.Note(2, GL_UNSIGNED_INT_VEC2); + inputs.Note(5, GL_UNSIGNED_INT_VEC4); + inputs.Note(31, GL_INT); + inputs.Note(32, GL_INT); // beyond the masks: ignored, not wrapped + EXPECT_EQ(inputs.signedInputs, (1u << 1) | (1u << 31)); + EXPECT_EQ(inputs.unsignedInputs, (1u << 2) | (1u << 5)); + + NarrowIntegerAttributeMasks data; + data.Note(0, VK_FORMAT_R8G8B8A8_UINT); // a float input: not corrected + data.Note(1, VK_FORMAT_R8G8B8A8_UINT); + data.Note(2, VK_FORMAT_R16G16_UINT); // matching signedness + data.Note(5, VK_FORMAT_R8G8B8A8_SINT); + data.Note(7, VK_FORMAT_R16_SINT); // no shader input + data.Note(31, VK_FORMAT_R16_UINT); + data.Note(40, VK_FORMAT_R8_UINT); + + const NarrowIntegerVertexInputs resolved = ResolveNarrowIntegerVertexInputs(inputs, data); + EXPECT_EQ(resolved.zeroExtend8, 1u << 1); + EXPECT_EQ(resolved.zeroExtend16, 1u << 31); + EXPECT_EQ(resolved.signExtend8, 1u << 5); + EXPECT_EQ(resolved.signExtend16, 0u); + + // A program without integer inputs resolves to nothing whatever the data is. + EXPECT_FALSE(ResolveNarrowIntegerVertexInputs(IntegerVertexInputMasks{}, data).Any()); +} + +// The variant's program-cache identity: distinct from the plain program and from every other +// correction set, and a pure function of its inputs. +TEST(NarrowIntegerVertexFormats, VariantHashKeysEveryCorrectionApart) { + constexpr ProgramFactory::HashType kBase = 0x0123456789abcdefull; + const NarrowIntegerVertexInputs sodium = Only(&NarrowIntegerVertexInputs::zeroExtend8, 3); + const auto hash = ProgramFactory::MixNarrowIntegerVertexInputs(kBase, sodium); + EXPECT_NE(hash, kBase); + EXPECT_EQ(ProgramFactory::MixNarrowIntegerVertexInputs(kBase, sodium), hash); + EXPECT_NE(ProgramFactory::MixNarrowIntegerVertexInputs(kBase + 1, sodium), hash); + for (const auto field : {&NarrowIntegerVertexInputs::zeroExtend16, &NarrowIntegerVertexInputs::signExtend8, + &NarrowIntegerVertexInputs::signExtend16}) + EXPECT_NE(ProgramFactory::MixNarrowIntegerVertexInputs(kBase, Only(field, 3)), hash); + EXPECT_NE(ProgramFactory::MixNarrowIntegerVertexInputs(kBase, Only(&NarrowIntegerVertexInputs::zeroExtend8, 4)), + hash); +} + +// The program half lives on the cache entry, and the cache's robin-hood insert moves entries +// around: a mask left behind by a move would pick another program's variant. +TEST(NarrowIntegerVertexFormats, IntegerInputMasksTravelWithTheProgramEntry) { + ProgramFactory::VkProgramObject source; + source.integerVertexInputs.signedInputs = 1u << 3; + source.integerVertexInputs.unsignedInputs = 1u << 7; + + ProgramFactory::VkProgramObject moved(std::move(source)); + EXPECT_EQ(moved.integerVertexInputs.signedInputs, 1u << 3); + EXPECT_EQ(moved.integerVertexInputs.unsignedInputs, 1u << 7); + EXPECT_EQ(source.integerVertexInputs.signedInputs, 0u); + EXPECT_EQ(source.integerVertexInputs.unsignedInputs, 0u); + + ProgramFactory::VkProgramObject assigned; + assigned = std::move(moved); + EXPECT_EQ(assigned.integerVertexInputs.signedInputs, 1u << 3); + EXPECT_EQ(assigned.integerVertexInputs.unsignedInputs, 1u << 7); + EXPECT_EQ(moved.integerVertexInputs.signedInputs, 0u); + EXPECT_EQ(moved.integerVertexInputs.unsignedInputs, 0u); +}