diff --git a/MobileGL/MG_Test/Backend/DirectGLES/BaseInstanceInjectionTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/BaseInstanceInjectionTest.cpp new file mode 100644 index 00000000..36cab57e --- /dev/null +++ b/MobileGL/MG_Test/Backend/DirectGLES/BaseInstanceInjectionTest.cpp @@ -0,0 +1,167 @@ +// MobileGL - MobileGL/MG_Test/Backend/DirectGLES/BaseInstanceInjectionTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// The gate on the gl_BaseInstance indirect lowering in +// MG_Backend/DirectGLES/Managers.cpp. That lowering declares a std430 storage block in the +// VERTEX stage, and a vertex-stage storage block is optional in both APIs: the minimum for +// GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS is 0 (GL 4.6 table 23.64, ES 3.2 table 21.44), and ARM's +// GLES driver takes that allowance - a Mali-G925-Immortalis reports 0 for it and for all three +// other graphics stages. +// +// Emitting the block on such a driver does not make it work. The driver refuses the program at +// link time ("The number of vertex shader storage blocks (1) is greater than the maximum number +// allowed (0)"), and because MobileGL's frontend GL_LINK_STATUS is glslang's rather than the +// driver's, the application is told the program linked and then every draw with it renders +// nothing. Dropping the indirect half instead keeps ordinary draws working and costs only the +// per-command baseInstance of an indirect draw. +// +// No GL context and no driver: the lowering is a pure String -> String pass over one capability. + +#include + +#include +#include + +using MobileGL::Bool; +using MobileGL::String; +using MobileGL::MG_Backend::DirectGLES::g_GLESCapabilities; +using MobileGL::MG_Backend::DirectGLES::PromoteDrawParameterGlobalsToUniforms; +using MobileGL::MG_Backend::DirectGLES::VertexStageStorageBlockUsable; + +namespace { + // The capability block is a process-global the backend fills in at init; restore whatever + // was there so ordering between this suite and any other that touches it cannot matter. + struct ScopedGLESCapabilitiesOverride { + ScopedGLESCapabilitiesOverride(): saved(g_GLESCapabilities) {} + ~ScopedGLESCapabilitiesOverride() { g_GLESCapabilities = saved; } + ScopedGLESCapabilitiesOverride(const ScopedGLESCapabilitiesOverride&) = delete; + ScopedGLESCapabilitiesOverride& operator=(const ScopedGLESCapabilitiesOverride&) = delete; + + MobileGL::MG_External::GLESCapabilities saved; + }; + + Bool Contains(const String& haystack, const String& needle) { + return haystack.find(needle) != String::npos; + } + + // What SPIRV-Cross hands the backend after LowerDrawParametersPass has demoted + // gl_BaseInstance to a Private global. + constexpr const char* kLoweredBaseInstanceVertexShader = R"(#version 310 es +highp int mg_BaseInstanceLowered; +void main() { + int instance = gl_InstanceID + mg_BaseInstanceLowered; + gl_Position = vec4(float(instance)); +} +)"; +} // namespace + +// One block is all the indirect view needs, so the predicate is a >= 1 test. +TEST(VertexStageStorageBlockUsableTest, RequiresAtLeastOneBlock) { + EXPECT_FALSE(VertexStageStorageBlockUsable(0)); + EXPECT_TRUE(VertexStageStorageBlockUsable(1)); + EXPECT_TRUE(VertexStageStorageBlockUsable(16)); +} + +// A driver that leaves the out-param untouched tells us nothing, and guessing "yes" is exactly +// what produces the unlinkable program. Unusable, not clamped up to one. +TEST(VertexStageStorageBlockUsableTest, ANegativeCountIsUnusableRatherThanClamped) { + EXPECT_FALSE(VertexStageStorageBlockUsable(-1)); + EXPECT_FALSE(VertexStageStorageBlockUsable(-2147483647 - 1)); +} + +TEST(BaseInstanceInjectionGate, DriverWithVertexStorageBlocksGetsTheIndirectView) { + const ScopedGLESCapabilitiesOverride capsGuard; + g_GLESCapabilities.IndirectDrawInstanceIdIncludesBaseInstance = false; + g_GLESCapabilities.MaxShaderStorageBufferBindings = 13; + g_GLESCapabilities.MaxVertexShaderStorageBlocks = 1; + + const String rewritten = + PromoteDrawParameterGlobalsToUniforms(kLoweredBaseInstanceVertexShader, GL_VERTEX_SHADER); + + EXPECT_TRUE(Contains(rewritten, "layout(std430, binding = 12) readonly buffer mg_IndirectParams")); + EXPECT_TRUE(Contains(rewritten, "uniform highp int mg_BaseInstanceWordIndex;")); + EXPECT_TRUE(Contains(rewritten, "#define mg_BaseInstanceLowered ((mg_BaseInstanceWordIndex > 0) ? " + "int(mg_indirectWords[uint(mg_BaseInstanceWordIndex - 1)]) : mg_BaseInstance)")) + << rewritten; +} + +// The bug this gate exists for. The block must not appear at all - not at a different binding, +// not behind a preprocessor guard: a declaration the driver counts is a declaration that makes +// the whole program unlinkable, and the frontend never surfaces that failure. +TEST(BaseInstanceInjectionGate, DriverWithoutVertexStorageBlocksDeclaresNoBlockAtAll) { + const ScopedGLESCapabilitiesOverride capsGuard; + g_GLESCapabilities.IndirectDrawInstanceIdIncludesBaseInstance = false; + g_GLESCapabilities.MaxShaderStorageBufferBindings = 13; + g_GLESCapabilities.MaxVertexShaderStorageBlocks = 0; + + const String rewritten = + PromoteDrawParameterGlobalsToUniforms(kLoweredBaseInstanceVertexShader, GL_VERTEX_SHADER); + + EXPECT_FALSE(Contains(rewritten, "mg_IndirectParams")) << rewritten; + EXPECT_FALSE(Contains(rewritten, "buffer")); + EXPECT_FALSE(Contains(rewritten, "mg_indirectWords")); + // Nothing reads the word index any more, so nothing may declare it either - its presence is + // what BackendProgramObjectImpl uses to decide whether to bind an indirect params buffer. + EXPECT_FALSE(Contains(rewritten, "mg_BaseInstanceWordIndex")); +} + +// Degraded, but still correct for every non-indirect draw: the plain mg_BaseInstance uniform is +// what the non-indirect draw entry points already write. +TEST(BaseInstanceInjectionGate, WithoutTheBlockBaseInstanceFallsBackToThePlainUniform) { + const ScopedGLESCapabilitiesOverride capsGuard; + g_GLESCapabilities.IndirectDrawInstanceIdIncludesBaseInstance = false; + g_GLESCapabilities.MaxShaderStorageBufferBindings = 13; + g_GLESCapabilities.MaxVertexShaderStorageBlocks = 0; + + const String rewritten = + PromoteDrawParameterGlobalsToUniforms(kLoweredBaseInstanceVertexShader, GL_VERTEX_SHADER); + + EXPECT_TRUE(Contains(rewritten, "uniform highp int mg_BaseInstance;")) << rewritten; + EXPECT_TRUE(Contains(rewritten, "#define mg_BaseInstanceLowered (mg_BaseInstance)")) << rewritten; + // The global declaration must be gone; leaving it would shadow the define. + EXPECT_FALSE(Contains(rewritten, "highp int mg_BaseInstanceLowered;\n")); +} + +// On a driver that both leaks baseInstance into gl_InstanceID and has no vertex storage block, +// the rebase has nothing to subtract. Subtracting the uniform instead would remove the base +// twice from every non-indirect draw, which is worse than not rebasing at all. +TEST(BaseInstanceInjectionGate, WithoutTheBlockInstanceIdRebaseCollapsesToIdentity) { + const ScopedGLESCapabilitiesOverride capsGuard; + g_GLESCapabilities.IndirectDrawInstanceIdIncludesBaseInstance = true; + g_GLESCapabilities.MaxShaderStorageBufferBindings = 13; + g_GLESCapabilities.MaxVertexShaderStorageBlocks = 0; + + const String rewritten = + PromoteDrawParameterGlobalsToUniforms(kLoweredBaseInstanceVertexShader, GL_VERTEX_SHADER); + + EXPECT_TRUE(Contains(rewritten, "#define mg_ZeroBasedInstanceID gl_InstanceID")) << rewritten; + EXPECT_FALSE(Contains(rewritten, "gl_InstanceID - (")); + EXPECT_FALSE(Contains(rewritten, "mg_indirectWords")); +} + +// The gate is scoped to the block, not to the whole pass: mg_DrawID and mg_BaseVertex are plain +// uniforms with no storage block behind them and must still be promoted on such a driver. +TEST(BaseInstanceInjectionGate, DrawIdAndBaseVertexArePromotedRegardless) { + const ScopedGLESCapabilitiesOverride capsGuard; + g_GLESCapabilities.IndirectDrawInstanceIdIncludesBaseInstance = false; + g_GLESCapabilities.MaxShaderStorageBufferBindings = 13; + g_GLESCapabilities.MaxVertexShaderStorageBlocks = 0; + + const String source = R"(#version 310 es +highp int mg_DrawID; +highp int mg_BaseVertex; +void main() { + gl_Position = vec4(float(mg_DrawID + mg_BaseVertex)); +} +)"; + + const String rewritten = PromoteDrawParameterGlobalsToUniforms(source, GL_VERTEX_SHADER); + + EXPECT_TRUE(Contains(rewritten, "uniform highp int mg_DrawID;")) << rewritten; + EXPECT_TRUE(Contains(rewritten, "uniform highp int mg_BaseVertex;")) << rewritten; +} diff --git a/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt b/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt index c88c7534..cab9b68f 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt +++ b/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt @@ -16,5 +16,22 @@ target_link_libraries( ${LINK_LIBRARIES} ) +add_executable( + BaseInstanceInjectionTest + BaseInstanceInjectionTest.cpp +) + +target_include_directories(BaseInstanceInjectionTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL +) + +target_link_libraries( + BaseInstanceInjectionTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + include(GoogleTest) gtest_discover_tests(EsslShaderPassTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) +gtest_discover_tests(BaseInstanceInjectionTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp b/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp index cbd801db..c69b8088 100644 --- a/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp +++ b/MobileGL/MG_Test/BackendLoader/BackendLoaderTest.cpp @@ -34,6 +34,17 @@ namespace { GLint maxFragmentImageUniforms = 4; GLint maxComputeImageUniforms = 5; bool maxGeometryImageUniformsQueried = false; + // Per-stage GL_MAX_*_SHADER_STORAGE_BLOCKS. The vertex and fragment pnames are ES 3.1, + // but the tessellation and geometry ones only exist from ES 3.2 on, so asking for them + // on an older context raises GL_INVALID_ENUM - the same shape as the buffer-texture and + // anisotropy probes. The "queried" flags are what pin that gating; the "raises error" + // knob is what pins the drain. + GLint maxTessControlSsboBlocks = 6; + GLint maxTessEvaluationSsboBlocks = 7; + GLint maxGeometrySsboBlocks = 8; + GLint maxFragmentSsboBlocks = 9; + bool tessAndGeometrySsboBlocksQueried = false; + bool perStageSsboBlockQueryRaisesError = false; GLfloat minFragmentInterpolationOffset = -0.75f; GLfloat maxFragmentInterpolationOffset = 0.625f; GLint fragmentInterpolationOffsetBits = 6; @@ -111,7 +122,30 @@ namespace { funcs.glGetIntegerv = [](GLenum pname, GLint* data) { switch (pname) { case GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS: - *data = g_fake.maxVertexSsboBlocks; + if (g_fake.perStageSsboBlockQueryRaisesError) { + g_fake.pendingError = GL_INVALID_ENUM; + } else { + *data = g_fake.maxVertexSsboBlocks; + } + break; + case GL_MAX_FRAGMENT_SHADER_STORAGE_BLOCKS: + if (g_fake.perStageSsboBlockQueryRaisesError) { + g_fake.pendingError = GL_INVALID_ENUM; + } else { + *data = g_fake.maxFragmentSsboBlocks; + } + break; + case GL_MAX_TESS_CONTROL_SHADER_STORAGE_BLOCKS: + g_fake.tessAndGeometrySsboBlocksQueried = true; + *data = g_fake.maxTessControlSsboBlocks; + break; + case GL_MAX_TESS_EVALUATION_SHADER_STORAGE_BLOCKS: + g_fake.tessAndGeometrySsboBlocksQueried = true; + *data = g_fake.maxTessEvaluationSsboBlocks; + break; + case GL_MAX_GEOMETRY_SHADER_STORAGE_BLOCKS: + g_fake.tessAndGeometrySsboBlocksQueried = true; + *data = g_fake.maxGeometrySsboBlocks; break; case GL_MAX_VERTEX_IMAGE_UNIFORMS: *data = g_fake.maxVertexImageUniforms; @@ -400,6 +434,10 @@ namespace { MobileGL::MG_External::GLESCapabilities MakeEs31Capabilities() { MobileGL::MG_External::GLESCapabilities caps; caps.GLESVersion = {3, 1, 0}; + // The probe reads its vertex storage-block gate from caps rather than re-querying the + // driver (FillInGLESCapabilities resolves the per-stage limits before calling it), so a + // caps struct handed to the probe directly has to carry what the fake reports. + caps.MaxVertexShaderStorageBlocks = g_fake.maxVertexSsboBlocks; return caps; } @@ -527,6 +565,83 @@ TEST(ImageUniformCapabilities, QueriesRealPerStageLimitsAndConservativelyGatesGe EXPECT_TRUE(g_fake.maxGeometryImageUniformsQueried); } +// The per-stage GL_MAX_*_SHADER_STORAGE_BLOCKS probes. These decide whether an application is +// told it may declare a storage block in a graphics stage, and on a driver that cannot serve one +// a wrong answer is not a cosmetic mis-report: the program is built, the driver refuses it at +// link time, the frontend reports LINK_STATUS true anyway, and every draw with it renders +// nothing. A Mali-G925-Immortalis reports 0 for vertex, both tessellation stages and geometry. +TEST(PerStageStorageBlockCapabilities, TakesTheDriverValuesAndGatesTessAndGeometryOnEs32) { + const auto funcs = MakeFakeGLESFunctions(); + + // ES 3.1: the tessellation and geometry pnames do not exist, so they must not be asked for + // and the stages must report the spec minimum of 0 rather than a hopeful driver number. + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 3; + MobileGL::MG_External::GLESCapabilities es31Caps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(es31Caps, funcs)); + EXPECT_EQ(es31Caps.MaxVertexShaderStorageBlocks, 3); + EXPECT_EQ(es31Caps.MaxFragmentShaderStorageBlocks, g_fake.maxFragmentSsboBlocks); + EXPECT_EQ(es31Caps.MaxTessControlShaderStorageBlocks, 0); + EXPECT_EQ(es31Caps.MaxTessEvaluationShaderStorageBlocks, 0); + EXPECT_EQ(es31Caps.MaxGeometryShaderStorageBlocks, 0); + EXPECT_FALSE(g_fake.tessAndGeometrySsboBlocksQueried); + + // ES 3.2: all five are real pnames and all five driver values must come through verbatim. + ResetFakeDriver(); + g_fake.maxVertexSsboBlocks = 3; + g_fake.glesMinorVersion = 2; + MobileGL::MG_External::GLESCapabilities es32Caps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(es32Caps, funcs)); + EXPECT_EQ(es32Caps.MaxVertexShaderStorageBlocks, 3); + EXPECT_EQ(es32Caps.MaxTessControlShaderStorageBlocks, g_fake.maxTessControlSsboBlocks); + EXPECT_EQ(es32Caps.MaxTessEvaluationShaderStorageBlocks, g_fake.maxTessEvaluationSsboBlocks); + EXPECT_EQ(es32Caps.MaxGeometryShaderStorageBlocks, g_fake.maxGeometrySsboBlocks); + EXPECT_EQ(es32Caps.MaxFragmentShaderStorageBlocks, g_fake.maxFragmentSsboBlocks); + EXPECT_TRUE(g_fake.tessAndGeometrySsboBlocksQueried); +} + +// Zero has to survive the round trip intact. It is the answer that matters most - it is what +// ARM's driver actually reports - so a probe that silently substituted a floor would put the +// bug straight back. +TEST(PerStageStorageBlockCapabilities, AZeroFromTheDriverIsReportedAsZero) { + const auto funcs = MakeFakeGLESFunctions(); + + ResetFakeDriver(); + g_fake.glesMinorVersion = 2; + g_fake.maxVertexSsboBlocks = 0; + g_fake.maxTessControlSsboBlocks = 0; + g_fake.maxTessEvaluationSsboBlocks = 0; + g_fake.maxGeometrySsboBlocks = 0; + g_fake.maxFragmentSsboBlocks = 16; + + MobileGL::MG_External::GLESCapabilities maliLikeCaps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(maliLikeCaps, funcs)); + + EXPECT_EQ(maliLikeCaps.MaxVertexShaderStorageBlocks, 0); + EXPECT_EQ(maliLikeCaps.MaxTessControlShaderStorageBlocks, 0); + EXPECT_EQ(maliLikeCaps.MaxTessEvaluationShaderStorageBlocks, 0); + EXPECT_EQ(maliLikeCaps.MaxGeometryShaderStorageBlocks, 0); + EXPECT_EQ(maliLikeCaps.MaxFragmentShaderStorageBlocks, 16); +} + +// A rejected query must leave no error behind for the application's first glGetError to find, +// and must fall back to the spec minimums rather than to whatever the untouched out-param held. +TEST(PerStageStorageBlockCapabilities, ARejectedQueryIsDrainedAndFallsBackToTheSpecMinimums) { + const auto funcs = MakeFakeGLESFunctions(); + + ResetFakeDriver(); + g_fake.perStageSsboBlockQueryRaisesError = true; + g_fake.maxVertexSsboBlocks = 12; + g_fake.maxFragmentSsboBlocks = 12; + + MobileGL::MG_External::GLESCapabilities caps; + ASSERT_TRUE(MobileGL::MG_Util::BackendLoader::FillInGLESCapabilities(caps, funcs)); + + EXPECT_EQ(caps.MaxVertexShaderStorageBlocks, 0); + EXPECT_EQ(caps.MaxFragmentShaderStorageBlocks, 4); + EXPECT_EQ(g_fake.pendingError, static_cast(GL_NO_ERROR)); +} + TEST(FragmentInterpolationCapabilities, QueriesOnlyWhenSupportedAndPreservesDriverLimits) { const auto funcs = MakeFakeGLESFunctions(); diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index a426f7fc..b42840ca 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -365,6 +365,13 @@ TEST(DirectGLESSanity, RebasesInstanceIdWhenIndirectDrawsLeakBaseInstance) { // MaxShaderStorageBufferBindings - 1 = 12, so a regression that stops reading the // probed cap and falls back to the struct default would surface as "binding = 7". caps.MaxShaderStorageBufferBindings = 13; + // The indirect lowering reads its baseInstance through a storage block declared in the + // VERTEX stage, which is optional in both APIs and which the GLESCapabilities default + // (0, the spec minimum) therefore denies. This suite is pinning the shape of that + // lowering, so it has to describe a driver that can actually have it - see + // VertexStageStorageBlockUsable and the BaseInstanceInjectionGate suite for the + // zero case. + caps.MaxVertexShaderStorageBlocks = 1; const MobileGL::String source = R"(#version 310 es highp int mg_BaseInstanceLowered; @@ -403,6 +410,9 @@ TEST(DirectGLESSanity, TheIndirectWordIndexIsOneBasedSoItsUnwrittenValueMeansNot auto& caps = MobileGL::MG_Backend::DirectGLES::g_GLESCapabilities; caps.IndirectDrawInstanceIdIncludesBaseInstance = false; caps.MaxShaderStorageBufferBindings = 13; + // See RebasesInstanceIdWhenIndirectDrawsLeakBaseInstance: without a vertex-stage + // storage block there is no word index to be one-based about. + caps.MaxVertexShaderStorageBlocks = 1; const MobileGL::String source = R"(#version 310 es highp int mg_BaseInstanceLowered; @@ -428,6 +438,10 @@ TEST(DirectGLESSanity, KeepsInstanceIdWhenIndirectDrawsAreConforming) { auto& caps = MobileGL::MG_Backend::DirectGLES::g_GLESCapabilities; caps.IndirectDrawInstanceIdIncludesBaseInstance = false; caps.MaxShaderStorageBufferBindings = 13; + // Set explicitly even though the assertions below would also hold on the degraded path: + // this case is about a CONFORMING driver leaving gl_InstanceID alone, and it would be a + // silent weakening for it to be exercising the no-storage-block fallback instead. + caps.MaxVertexShaderStorageBlocks = 1; const MobileGL::String source = R"(#version 310 es highp int mg_BaseInstanceLowered; @@ -442,6 +456,8 @@ void main() { EXPECT_EQ(rewritten.find("mg_ZeroBasedInstanceID"), MobileGL::String::npos); EXPECT_NE(rewritten.find("int instance = gl_InstanceID + mg_BaseInstanceLowered;"), MobileGL::String::npos); + // The indirect view is present on this driver, so the fallback must NOT have fired. + EXPECT_NE(rewritten.find("buffer mg_IndirectParams"), MobileGL::String::npos); } TEST(DirectGLESSanity, LeavesDrawParameterGlobalsAloneOutsideVertexShaders) {