diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index efb45795..55ddadf6 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -4899,6 +4899,30 @@ namespace MobileGL::MG_Backend::DirectGLES { effectiveSpirv = &loweredSpirv; } + // ESSL cannot express gl_ViewportIndex either, but unlike the draw parameters + // there IS an extension that provides it - so this runs only when the driver does + // NOT advertise GL_OES_viewport_array. A driver that does keeps the builtin and + // gets the `#extension` request added to the decompiled source below instead. + // Demoting the builtin costs the multi-viewport routing (every invocation lands in + // viewport 0), which is the degradation ViewportArrayScenario already documents + // for this backend; NOT demoting it costs the whole program, because the stage + // fails to compile and every draw made with it silently renders nothing. + // Gated on the module actually declaring the output, so no other stage pays an + // optimizer round trip for it. + Vector loweredViewportSpirv; + if (!g_GLESCapabilities.SupportsViewportArray && + MG_Util::ShaderTranspiler::ShaderCompiler::DeclaresViewportIndexBuiltin(*effectiveSpirv) && + MG_Util::ShaderTranspiler::ShaderCompiler::LowerViewportIndexForEssl( + *effectiveSpirv, loweredViewportSpirv, enableSpirvValidation) && + !loweredViewportSpirv.empty()) { + effectiveSpirv = &loweredViewportSpirv; + MGLOG_D("Program %u stage %s writes gl_ViewportIndex, which this ES driver has " + "no GL_OES_viewport_array for. The builtin was demoted to a plain " + "global; every invocation renders into viewport 0.", + m_backendProgramId, + MG_Util::ConvertGLEnumToString(glShaderType).c_str()); + } + // GLSL ES has no ARRAY vertex inputs, and SPIRV-Cross refuses the whole module // rather than emulating them, so this has to happen before it sees the binary. Vector splitArrayInputSpirv; @@ -5080,6 +5104,17 @@ namespace MobileGL::MG_Backend::DirectGLES { source = RequestExtendedImageFormats(std::move(source), imageFormatBake.needsExtendedImageFormats && g_GLESCapabilities.SupportsExtendedImageFormats); + // The third header-level rewrite, for the builtin SPIRV-Cross prints bare: + // gl_ViewportIndex is in no version of ESSL core, so without this directive the + // stage does not compile and the whole program - not just its viewport routing - + // is lost. The token probe keeps the line off every other program and the + // capability gate keeps it off drivers that would hard-error on an unadvertised + // name; a driver without the extension took the LowerViewportIndexPass fallback + // above and its source no longer names the builtin at all, so the two are mutually + // exclusive by construction. Read `source` BEFORE it is moved from. + const Bool needsViewportArrayExtension = g_GLESCapabilities.SupportsViewportArray && + source.find("gl_ViewportIndex") != String::npos; + source = RequestViewportArrayExtension(std::move(source), needsViewportArrayExtension); source = RebindImageUniformsToFrontendUnits(std::move(source), stateProgramObject); // The completion half of the format bake, for the formats SPIRV-Cross throws on diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 92820c34..c74014ee 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -569,6 +569,43 @@ namespace MobileGL::MG_Backend::DirectGLES { return glslCode; } + String RequestViewportArrayExtension(String glslCode, Bool needed) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + // gl_ViewportIndex is desktop GL 4.1 core and is in ESSL only under + // GL_OES_viewport_array. SPIRV-Cross prints the identifier as-is and requests no + // extension for it - three lines away from the BuiltInLayer case, which DOES ask for + // one on ES - so an untouched decompile reaches the driver naming a builtin its core + // language has never heard of. The stage then fails to compile, the program is marked + // unusable and every draw made with it renders nothing while raising no GL error. + // + // Same `needed` contract as RequestExtendedImageFormats, and the same hard rule: + // `#extension` on a name the driver does not advertise is itself a compile error + // (ARM's compiler is strict about it), so this must never be emitted speculatively. + // A driver without the extension does not come through here at all - its module took + // the LowerViewportIndexPass fallback and the emitted source no longer names the + // builtin. + static constexpr const char* kDirective = "#extension GL_OES_viewport_array : require\n"; + static constexpr const char* kExtName = "GL_OES_viewport_array"; + if (!needed || glslCode.find(kExtName) != String::npos) { + return glslCode; + } + // Right after the #version line, for the reason spelled out above: it is the only + // position that must stay first, and ForceSupporterOutput's scan for the LAST + // #extension directive still finds whichever one that ends up being. + const SizeT versionPos = glslCode.find("#version"); + if (versionPos == String::npos) { + return kDirective + glslCode; + } + const SizeT lineEnd = glslCode.find('\n', versionPos); + if (lineEnd == String::npos) { + return glslCode + "\n" + kDirective; + } + glslCode.insert(lineEnd + 1, kDirective); + return glslCode; + } + String BakeImageFormatQualifiers(String glslCode, const UnorderedMap& esslFormatByUniformName) { #ifdef TRACY_ENABLE diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index a679a4aa..33f04bd5 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -154,6 +154,16 @@ namespace MobileGL::MG_Backend::DirectGLES { // extension - requesting an unadvertised extension is itself a compile error, so this is // never emitted speculatively. A no-op when not needed or already present. String RequestExtendedImageFormats(String glslCode, Bool needed); + // Adds `#extension GL_OES_viewport_array : require` when the emitted ESSL names + // gl_ViewportIndex. SPIRV-Cross prints that identifier and asks for nothing (unlike + // gl_Layer, which it backs with GL_NV_viewport_array2 on ES) and ESSL has no core + // spelling for it at any version, so the request has to be made here or the stage does + // not compile - which loses the whole program, not just the multi-viewport routing. + // `needed` is the caller's answer for the same reason as above: only it knows whether the + // driver advertises the extension, and requesting an unadvertised one is itself a compile + // error, so this is never emitted speculatively. A no-op when not needed or already + // present. + String RequestViewportArrayExtension(String glslCode, Bool needed); // Writes a format layout qualifier into the image declarations named in // `esslFormatByUniformName` that still have none. The completion half of the image-format // bake, and ONLY that: the SPIR-V pass (BakeImageFormatsPass) is what normally puts the diff --git a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp index 1e0bdb57..6ce2facd 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp +++ b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp @@ -21,6 +21,7 @@ using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ForceFlatIntegerVaryings; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_WRITE_ALIAS_PREFIX; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RemoveLayoutBinding; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RequestExtendedImageFormats; +using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RequestViewportArrayExtension; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::SplitReadWriteImageUniforms; namespace { @@ -550,3 +551,55 @@ void main() { imageStore(uni_image, ivec2(0), uvec4(1u)); } EXPECT_EQ(out, source); EXPECT_EQ(CountOf(out, "GL_NV_image_formats"), 1u) << out; } + +// --- GL_OES_viewport_array directive ------------------------------------------------------------- + +// SPIRV-Cross prints gl_ViewportIndex bare and requests nothing for it, and ESSL has no core +// spelling at any version - so without this directive the stage fails to compile, the program is +// marked unusable and every draw made with it silently renders nothing. +TEST(RequestViewportArrayExtensionTest, TheDirectiveGoesRightAfterTheVersionLine) { + const String source = R"(#version 320 es +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_ViewportIndex = gl_InvocationID; EmitVertex(); } +)"; + const String out = RequestViewportArrayExtension(source, true); + EXPECT_TRUE(Contains(out, "#version 320 es\n#extension GL_OES_viewport_array : require\n")) << out; +} + +// Never speculatively: ARM's compiler hard-errors on an `#extension` naming a string the driver +// does not advertise, so the caller's "not needed" answer has to be honoured exactly. A driver +// without the extension gets the LowerViewportIndexPass fallback instead. +TEST(RequestViewportArrayExtensionTest, NotNeededMeansNotEmitted) { + const String source = R"(#version 320 es +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_ViewportIndex = gl_InvocationID; EmitVertex(); } +)"; + EXPECT_EQ(RequestViewportArrayExtension(source, false), source); +} + +TEST(RequestViewportArrayExtensionTest, AnAlreadyPresentDirectiveIsNotDuplicated) { + const String source = R"(#version 320 es +#extension GL_OES_viewport_array : require +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_ViewportIndex = gl_InvocationID; EmitVertex(); } +)"; + const String out = RequestViewportArrayExtension(source, true); + EXPECT_EQ(out, source); + EXPECT_EQ(CountOf(out, "GL_OES_viewport_array"), 1u) << out; +} + +// The two image directives and this one share the insertion point, so a shader that needs both +// must end up with both - and with #version still first. +TEST(RequestViewportArrayExtensionTest, CoexistsWithTheImageFormatDirective) { + const String source = R"(#version 320 es +layout(r8ui, binding = 1) uniform writeonly highp uimage2D uni_image; +void main() { gl_ViewportIndex = 1; imageStore(uni_image, ivec2(0), uvec4(1u)); } +)"; + const String out = RequestViewportArrayExtension(RequestExtendedImageFormats(source, true), true); + EXPECT_EQ(out.find("#version 320 es"), 0u) << out; + EXPECT_TRUE(Contains(out, "#extension GL_NV_image_formats : require\n")) << out; + EXPECT_TRUE(Contains(out, "#extension GL_OES_viewport_array : require\n")) << out; +}