[Diagnostic, Test] (DirectGLES): name the image-uniform split as a cause when the backend link fails

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -4754,6 +4754,22 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 return reflectionName;
             }
 
+            // GL_MAX_<stage>_IMAGE_UNIFORMS as the ES driver reports it, which is also exactly what
+            // MobileGL advertises for it (GL_Getter answers from the same DynamicBackendParameters).
+            // -1 for a stage the ES side has no such limit for, which is the "cannot say" answer
+            // the diagnostic that reads it prints rather than a made-up number. [[maybe_unused]]
+            // because its only caller is an MGLOG_E argument, and MGLOG_E compiles to nothing in a
+            // build whose MOBILEGL_LOG_ACTIVE_LEVEL is above ERROR.
+            [[maybe_unused]] Int AdvertisedStageImageUniformLimit(ShaderStage stage) {
+                switch (stage) {
+                case ShaderStage::Vertex: return g_GLESCapabilities.MaxVertexImageUniforms;
+                case ShaderStage::Geometry: return g_GLESCapabilities.MaxGeometryImageUniforms;
+                case ShaderStage::Fragment: return g_GLESCapabilities.MaxFragmentImageUniforms;
+                case ShaderStage::Compute: return g_GLESCapabilities.MaxComputeImageUniforms;
+                default: return -1;
+                }
+            }
+
             // Whether a glslang layout format is one GLSL ES has in core; the rest reach ES only
             // through GL_NV_image_formats. Asked of DECLARED formats, which this backend passes
             // through untouched - the emitted ESSL still has to be legal for the driver.
@@ -5026,6 +5042,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 }
             }
             std::set<String> flattenedXfbBlockNames;
+            // Stages whose ESSL had a read+write image declaration doubled into a coherent
+            // read/write pair, and by how many. Empty for every program but a handful; consulted
+            // ONLY when the link then fails, because the doubling spends the driver's per-stage
+            // GL_MAX_*_IMAGE_UNIFORMS budget that MobileGL keeps advertising unadjusted (halving
+            // the advertised value would fail basic-api and NotSupported-out cases that never pay
+            // the doubling, so the limit must stay honest and the connection has to be made here
+            // instead). See the budget note on SplitReadWriteImageUniforms.
+            struct SplitImageUniformStage {
+                ShaderStage stage;
+                Uint splitCount;
+            };
+            Vector<SplitImageUniformStage> splitImageUniformStages;
 
             // Desktop GLSL keeps SEPARATE name namespaces for input and output interface
             // blocks, so ONE stage may legally declare `in FOO {...}` and `out FOO {...}` at
@@ -5475,7 +5503,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 //    declaration and preserves its binding - an image unit cannot be set from
                 //    the API in ES, so the qualifier is the only binding mechanism there is,
                 //    and both halves of the pair have to still be carrying theirs when it runs.
-                source = SplitReadWriteImageUniforms(source);
+                Uint splitImageUniformCount = 0;
+                source = SplitReadWriteImageUniforms(source, &splitImageUniformCount);
+                if (splitImageUniformCount != 0) {
+                    splitImageUniformStages.push_back({shader->GetShaderStage(), splitImageUniformCount});
+                }
                 source = RemoveLayoutBinding(source);
                 source = ProcessOutColorLocations(source);
                 source = ForceFlatIntegerVaryings(source, glShaderType);
@@ -5620,6 +5652,25 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 // in an INFO-level artifact.
                 MGLOG_E("Program linking failed. State program ID: %u, backend program ID: %u, driver log: %s",
                         stateProgramObject->GetExternalIndex(), m_backendProgramId, log.data());
+                // The one link failure MobileGL can name a cause for that the driver's log never
+                // will: ESSL has no legal single declaration for a read+write image outside
+                // r32f/r32i/r32ui, so those are split into a coherent pair and the stage ends up
+                // declaring more image uniforms than the application did - against a
+                // GL_MAX_*_IMAGE_UNIFORMS that is still the driver's raw number, because lowering
+                // it would fail basic-api and NotSupported-out every case that only ever uses
+                // readonly/writeonly images. A shader declaring more than half a stage's budget in
+                // read+write images therefore links here and nowhere else, and without this line
+                // the next reader has only a generic driver message to go on.
+                for (const SplitImageUniformStage& split : splitImageUniformStages) {
+                    MGLOG_E("...and %u read+write image uniform(s) in stage %s were split into coherent "
+                            "read/write pairs, so that stage declares %u image uniform(s) more than the "
+                            "program did; GL_MAX_*_IMAGE_UNIFORMS for it is %d. If the driver log names "
+                            "image uniforms, that is the cause.",
+                            split.splitCount,
+                            MG_Util::ConvertGLEnumToString(
+                                MG_Util::ConvertShaderStageToGLEnum(split.stage)).c_str(),
+                            split.splitCount, AdvertisedStageImageUniformLimit(split.stage));
+                }
             } else {
                 MGLOG_D("Program linked successfully. ID: %u", m_backendProgramId);
             }

MobileGL/MG_Backend/DirectGLES/Utils.cpp

@@ -905,10 +905,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
             }
         } // namespace
 
-        String SplitReadWriteImageUniforms(const String& glslCode) {
+        String SplitReadWriteImageUniforms(const String& glslCode, Uint* outSplitCount) {
 #ifdef TRACY_ENABLE
             ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
 #endif
+            // Written before any early return, so the caller never reads a stale count.
+            if (outSplitCount != nullptr) *outSplitCount = 0;
             if (glslCode.find("image") == String::npos) {
                 return glslCode;
             }
@@ -1046,6 +1048,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     decl.writeName = MakeImageWriteAliasName(decl.name, glslCode, takenAliases);
                     takenAliases.push_back(decl.writeName);
                     decl.split = true;
+                    if (outSplitCount != nullptr) ++*outSplitCount;
                     // Both halves carry `coherent`; see BuildImageDeclaration. The
                     // single-declaration cases below stay as they were - nothing aliases them, so
                     // there is no visibility to restore and no reason to pay for the cache

MobileGL/MG_Backend/DirectGLES/Utils.h

@@ -228,7 +228,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
         // Runs on the transpiled ESSL, so it must see the bindings the frontend units were
         // already rewritten to and must run before those bindings are stripped - see the call
         // site in Managers.cpp.
-        String SplitReadWriteImageUniforms(const String& glslCode);
+        //
+        // `outSplitCount`, when given, receives the number of declarations that were actually
+        // doubled - i.e. exactly how many image uniforms this stage gained over what the
+        // application declared. Zero for every shader but a handful, and the only number the
+        // budget note above can be reported with.
+        String SplitReadWriteImageUniforms(const String& glslCode, Uint* outSplitCount = nullptr);
         // Prefix of the per-sampler float uniform that carries GL_TEXTURE_LOD_BIAS into
         // the shader (see EmulateTextureLodBias); the suffix is the sampler's own name.
         constexpr const char* LOD_BIAS_UNIFORM_PREFIX = "mg_lodBias_";

MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp

@@ -293,6 +293,41 @@ void main()
     EXPECT_EQ(CountOf(out, "memoryBarrierImage();"), 1u) << out;
 }
 
+// The split is the one thing that makes a stage declare MORE image uniforms than the application
+// did, and MobileGL keeps advertising GL_MAX_*_IMAGE_UNIFORMS unadjusted (lowering it would fail
+// basic-api and NotSupported-out every case that only uses readonly/writeonly images). So the
+// count has to be reportable, or a link failure caused by the doubling looks like a driver
+// mystery - which is what KHR-GL4x.shader_image_load_store.multiple-uniforms will hit the moment
+// the format work stops masking it.
+TEST(SplitReadWriteImageUniformsTest, TheSplitCountIsReportedToTheCaller) {
+    const String twoSplits = R"(#version 320 es
+layout(binding = 0, rgba8) uniform highp image2D goku;
+layout(binding = 1, rgba16f) uniform highp image2D gohan;
+layout(binding = 2, rgba8) uniform highp image2D storeOnly;
+void main()
+{
+    imageStore(goku, ivec2(0), imageLoad(goku, ivec2(0)));
+    imageStore(gohan, ivec2(0), imageLoad(gohan, ivec2(0)));
+    imageStore(storeOnly, ivec2(0), vec4(0.0));
+}
+)";
+    Uint splitCount = 99u;
+    SplitReadWriteImageUniforms(twoSplits, &splitCount);
+    EXPECT_EQ(splitCount, 2u) << "only the read+write pair counts; the store-only repair adds no uniform";
+
+    // Every early return has to write the count too, or a caller reads whatever was there before.
+    const String noImages = R"(#version 320 es
+layout(location = 0) out highp vec4 mg_FragColor;
+void main()
+{
+    mg_FragColor = vec4(1.0);
+}
+)";
+    splitCount = 99u;
+    SplitReadWriteImageUniforms(noImages, &splitCount);
+    EXPECT_EQ(splitCount, 0u);
+}
+
 // imageSize reads no texels and writes none, so it decides nothing; readonly is what keeps
 // such a declaration legal.
 TEST(SplitReadWriteImageUniformsTest, ImageSizeAloneDoesNotCountAsALoadOrAStore) {