From bea3086b41662d0c21c6011987961149bb6f12e0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 22:53:25 -0400 Subject: [PATCH] [Diagnostic, Test] (DirectGLES): name the image-uniform split as a cause when the backend link fails --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 53 ++++++++++++++++++- MobileGL/MG_Backend/DirectGLES/Utils.cpp | 5 +- MobileGL/MG_Backend/DirectGLES/Utils.h | 7 ++- .../Backend/DirectGLES/EsslShaderPassTest.cpp | 35 ++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 34153f10..a4fe2568 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4754,6 +4754,22 @@ namespace MobileGL::MG_Backend::DirectGLES { return reflectionName; } + // GL_MAX__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 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 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); } diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 16e2dbc8..a6a6bd8d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/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 diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index 9520d243..e4712ad1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/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_"; diff --git a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp index c8e6702b..91ead160 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp +++ b/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) {