[Fix] (Review): scope the sample mask to a multisample target, version the sampled-set memo on completeness, clamp the mask word count, give the multisample placeholder every numeric domain, unwind the fixup revert one pass at a time, and bound the validator log

MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp

@@ -9,6 +9,7 @@
 #include "Loader.h"
 
 #include <Config.h>
+#include <algorithm>
 #include <cmath>
 #include <limits>
 
@@ -177,7 +178,15 @@ namespace MobileGL::MG_Util::BackendLoader {
         caps.MaxFramebufferSamples = ResolveConservativeFramebufferSampleLimit(p.limits);
         caps.MaxIntegerSamples = MaxSampleCountFromFlags(p.limits.sampledImageIntegerSampleCounts);
         caps.MaxSamples = caps.MaxFramebufferSamples;
-        caps.MaxSampleMaskWords = SaturateToInt(p.limits.maxSampleMaskWords);
+        // Clamped to one word, exactly as the GLES loader clamps the driver's value and for the
+        // same reason: MobileGL's sample-mask state IS a single 32-bit word
+        // (RenderState::SampleMaskValue) and SampleMaski_State() raises GL_INVALID_VALUE for any
+        // maskNumber other than 0. dEQP's per-case gluStateReset issues glSampleMaski up to
+        // GL_MAX_SAMPLE_MASK_WORDS, so advertising a device's real 2 would abort the whole glcts
+        // process after every single case - the failure da6f75dbd added the GLES clamp to stop,
+        // reproduced on this backend. One word is the spec minimum and therefore always legal.
+        // It is also what PipelineCreatePayload::sampleMask is sized for.
+        caps.MaxSampleMaskWords = std::min(SaturateToInt(p.limits.maxSampleMaskWords), 1);
         caps.MaxTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages);
         caps.MaxVertexTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages);
         caps.MaxComputeTextureImageUnits = SaturateToInt(p.limits.maxPerStageDescriptorSampledImages);
@@ -300,7 +309,15 @@ namespace MobileGL::MG_Util::BackendLoader {
         caps.MaxFramebufferSamples = ResolveConservativeFramebufferSampleLimit(properties.limits);
         caps.MaxIntegerSamples = MaxSampleCountFromFlags(properties.limits.sampledImageIntegerSampleCounts);
         caps.MaxSamples = caps.MaxFramebufferSamples;
-        caps.MaxSampleMaskWords = SaturateToInt(properties.limits.maxSampleMaskWords);
+        // Clamped to one word, exactly as the GLES loader clamps the driver's value and for the
+        // same reason: MobileGL's sample-mask state IS a single 32-bit word
+        // (RenderState::SampleMaskValue) and SampleMaski_State() raises GL_INVALID_VALUE for any
+        // maskNumber other than 0. dEQP's per-case gluStateReset issues glSampleMaski up to
+        // GL_MAX_SAMPLE_MASK_WORDS, so advertising a device's real 2 would abort the whole glcts
+        // process after every single case - the failure da6f75dbd added the GLES clamp to stop,
+        // reproduced on this backend. One word is the spec minimum and therefore always legal.
+        // It is also what PipelineCreatePayload::sampleMask is sized for.
+        caps.MaxSampleMaskWords = std::min(SaturateToInt(properties.limits.maxSampleMaskWords), 1);
         caps.MaxTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages);
         caps.MaxVertexTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages);
         caps.MaxComputeTextureImageUnits = SaturateToInt(properties.limits.maxPerStageDescriptorSampledImages);

MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp

@@ -758,6 +758,33 @@ namespace MobileGL {
                 return false;
             }
 
+            Bool ShaderCompiler::ModuleDeclaresTransformFeedback(const Vector<Uint32>& spirv) {
+                if (spirv.empty()) {
+                    return false;
+                }
+                std::unique_ptr<spvtools::opt::IRContext> context = spvtools::BuildModule(
+                    SPV_ENV_VULKAN_1_1, MakeSpirvMessageConsumer("ModuleDeclaresTransformFeedback"),
+                    spirv.data(), spirv.size());
+                if (!context) {
+                    // Unparseable is not a capture verdict; say no, which makes the caller decline
+                    // the span rather than issue transform-feedback commands against it.
+                    return false;
+                }
+                // The exact question VUID-vkCmdBeginTransformFeedbackEXT-None-04128 asks of the
+                // bound pipeline's last pre-rasterization stage: was it declared with the Xfb
+                // execution mode. Reading the execution modes rather than the TransformFeedback
+                // capability because the capability can legally be declared by a module that has
+                // no Xfb entry point, and the VUID is about the mode.
+                for (const spvtools::opt::Instruction& mode : context->module()->execution_modes()) {
+                    if (mode.NumInOperands() >= 2 &&
+                        static_cast<spv::ExecutionMode>(mode.GetSingleWordInOperand(1)) ==
+                            spv::ExecutionMode::Xfb) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+
             Bool ShaderCompiler::ModuleDeclaresFloat64(const Vector<Uint32>& spirv) {
                 if (spirv.empty()) {
                     // Same reasoning as ModuleDeclaresBufferTextureSampler: a stage that produced

MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h

@@ -532,6 +532,12 @@ namespace MobileGL {
                 // check exists so that failure can be reported as the missing capability it is,
                 // naming the shader, rather than as a driver info log nobody sees.
                 static Bool ModuleDeclaresBufferTextureSampler(const Vector<Uint32>& spirv);
+                // Does this module carry the Xfb execution mode - i.e. would a
+                // vkCmdBeginTransformFeedbackEXT against a pipeline whose last pre-rasterization
+                // stage is this module satisfy VUID-vkCmdBeginTransformFeedbackEXT-None-04128?
+                // Asked of the FINAL bytes, so it answers for whatever the backend transform
+                // chain actually produced rather than for what it was asked to produce.
+                static Bool ModuleDeclaresTransformFeedback(const Vector<Uint32>& spirv);
 
                 // True when the module still declares a 64-bit float type. After
                 // SanitizeAndOptimizeBinary that can only mean DemoteFloat64Pass declined the