[Fix, Test] (DirectGLES, MG_Test): request GL_OES_viewport_array in the emitted ESSL, or lower the builtin away

This commit is contained in:
2026-08-20 05:29:18 -04:00
parent f2f6430e34
commit bde14cae29
4 changed files with 135 additions and 0 deletions
@@ -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<unsigned int> 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<unsigned int> 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
+37
View File
@@ -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<String, String>& esslFormatByUniformName) {
#ifdef TRACY_ENABLE
+10
View File
@@ -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
@@ -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;
}