diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ViewportArrayScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ViewportArrayScenario.cpp index 44a75679..18f0077b 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ViewportArrayScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ViewportArrayScenario.cpp @@ -30,15 +30,23 @@ // applies the flip to viewport 0 and forgets the other fifteen renders a correct-looking FBO and // an upside-down window - the classic multi-viewport bug, and invisible to every FBO-only case. // -// HONEST LIMIT OF THIS FILE. DirectGLES SKIPS every case: GLES has one viewport, one scissor -// rectangle and no gl_ViewportIndex, so routing to index > 0 is an emulation feature that has -// not been built (the Espryt half of KHR-GL43.viewport_array's rendering group is deliberately -// still red). The skip is explicit rather than silent so a future emulation lands here as a -// failing test and not as a test that was quietly never running. DirectVulkan additionally -// skips when the device lacks the multiViewport feature - Vulkan then forbids a pipeline from -// declaring more than one viewport at all, which is a device limit and not a MobileGL bug; -// lavapipe (every CI lane) and both Mali/Adreno devices support it, so the cases do run where -// it matters. +// BOTH BACKENDS RUN EVERY CASE, by two completely different routes, which is the point of +// keeping them in one file. DirectVulkan declares sixteen viewports on the pipeline and lets the +// hardware route. DirectGLES has one viewport, one scissor rectangle and one depth range and no +// gl_ViewportIndex at all, so it EMULATES: the builtin becomes a flat varying, the fragment stage +// gets a gate, and the draw is replayed once per distinct viewport state (Managers.h, +// ForEachViewportRoutingPass). Every assertion below is about pixels, so it cannot tell the two +// apart - which is exactly what has to be true. +// +// DirectVulkan skips when the device lacks the multiViewport feature - Vulkan then forbids a +// pipeline from declaring more than one viewport at all, which is a device limit and not a +// MobileGL bug; lavapipe (every CI lane) and both Mali/Adreno devices support it, so the cases do +// run where it matters. +// +// The last case is the negative control for the emulation and runs on DirectGLES only: it builds +// the SAME program with the emulation switched off and requires the routing to collapse onto +// viewport 0. Without it every assertion above could be satisfied by a backend that happened to +// be right for some other reason, and the emulation's own switch would be untested. #include #include @@ -47,6 +55,10 @@ #include "../Harness/HeadlessGL.h" #include "../Harness/ScenarioFixture.h" +// For the emulation switch the negative-control case below flips. Nothing else in this file needs +// to know which backend it is running on. +#include + #ifdef GLAPI #undef GLAPI #endif @@ -142,13 +154,6 @@ void main() { fragColor = gl_FragCoord.z; } ScenarioTest::SetUp(); if (!Ready()) return; - if (Gl().BackendName() == "DirectGLES") { - GTEST_SKIP() << "gl_ViewportIndex routing is not emulated on DirectGLES: GLES has one viewport " - "and one scissor rectangle, so every index rasterizes as index 0. The indexed " - "STATE is still asserted (MG_Test RenderStateTest); this is the deferred " - "rendering half of KHR-GL43.viewport_array."; - } - GLint maxViewports = 0; glGetIntegerv(GL_MAX_VIEWPORTS, &maxViewports); ASSERT_GE(maxViewports, kViewportCount) << "GL 4.3 core requires GL_MAX_VIEWPORTS >= 16"; @@ -520,11 +525,78 @@ void main() { fragColor = vec4(float(gsIndex) * 16.0 / 255.0, 0.0, 0.0, 1.0); } DestroyIntTarget(target); } - // --- 4. an explicitly EMPTY scissor box clips, it does not mean "never written" -------- + // --- 4. the negative control for the DirectGLES emulation ----------------------------- // - // Deliberately NOT a ViewportArrayScenario case, because it must run on DirectGLES - the - // backend that got it wrong - and that fixture skips there. It needs none of the routing: - // one viewport, one scissor rectangle, no geometry stage. + // Everything above is a claim about pixels, and a claim about pixels cannot tell an + // emulation that works from a backend that was going to be right anyway. This case builds + // the SAME program with MOBILEGL_FORCE_VIEWPORT_ARRAY_EMULATION off and requires case 1's + // result to COLLAPSE: with no routing, every geometry invocation rasterizes against + // viewport 0's rectangle, so the last invocation paints the whole surface and every cell + // reads 15 instead of its own index. That is the pre-emulation behaviour this backend had + // (and the failure signature KHR-GL43.viewport_array reported on it), pinned here so that + // (a) the three cases above are known to be testing the emulation and not the weather, + // and (b) the switch itself has a test. + // + // DirectGLES only: the flag steers nothing on DirectVulkan, which routes natively. + TEST_F(ViewportArrayScenario, WithoutTheEmulationEveryIndexCollapsesOntoViewportZero) { + if (Gl().BackendName() != "DirectGLES") { + GTEST_SKIP() << "the emulation switch is a DirectGLES concern; DirectVulkan routes " + "gl_ViewportIndex natively and ignores it"; + } + + // The feature table is a process-global and this fixture shares its context with every + // other scenario in the process, so the restore is not optional. + struct ScopedEmulationOff { + ScopedEmulationOff(): saved(MobileGL::MG_Config::Features.ViewportArrayEmulation) { + MobileGL::MG_Config::Features.ViewportArrayEmulation = + MobileGL::MG_Config::QuirkOverride::ForceOff; + } + ~ScopedEmulationOff() { MobileGL::MG_Config::Features.ViewportArrayEmulation = saved; } + MobileGL::MG_Config::QuirkOverride saved; + }; + + IntTarget target = MakeIntTarget(kSurfaceSide, kSurfaceSide); + SetupGridViewports(kCellSize, kCellSize); + + GLuint unroutedProgram = 0; + { + const ScopedEmulationOff scopedEmulationOff; + // A FRESH program: the emitted ESSL is decided at link time and memoized on a key + // that carries this flag, so reusing m_program would just replay the routed build. + unroutedProgram = BuildProgram(kGridGeometrySource, kIntFragmentSource); + ASSERT_NE(unroutedProgram, 0u) << "unrouted program failed to build: " << m_buildLog; + glUseProgram(unroutedProgram); + glBindVertexArray(m_vao); + glDrawArrays(GL_POINTS, 0, 1); + ASSERT_EQ(glGetError(), GL_NO_ERROR); + } + + const std::vector pixels = ReadInts(kSurfaceSide, kSurfaceSide); + // Cell (0, 0) IS viewport 0's rectangle, so it is the one cell an unrouted draw paints + // with something. Everything it holds comes from the last geometry invocation. + EXPECT_EQ(CellCentre(pixels, kSurfaceSide, 0, 0), kViewportCount - 1) + << "with the emulation off, viewport 0's rectangle must hold the LAST invocation's " + "index - if it holds 0 the routing is still happening and this control proves " + "nothing"; + for (int y = 0; y < kGridSide; ++y) { + for (int x = 0; x < kGridSide; ++x) { + if (x == 0 && y == 0) continue; + EXPECT_EQ(CellCentre(pixels, kSurfaceSide, x, y), kUnwritten) + << "cell (" << x << ", " << y << ") is outside viewport 0's rectangle and an " + << "unrouted draw cannot reach it"; + } + } + + glUseProgram(0); + glDeleteProgram(unroutedProgram); + DestroyIntTarget(target); + } + + // --- 5. an explicitly EMPTY scissor box clips, it does not mean "never written" -------- + // + // Deliberately NOT a ViewportArrayScenario case, because that fixture's geometry stage + // routes and this claim needs none of it: one viewport, one scissor rectangle, no + // geometry stage - and it has to hold identically whether or not anything routes. // // glScissor(0, 0, 0, 0) is legal GL meaning "the scissor test rejects every fragment", // but it is byte-identical to the all-zero rectangle a context starts with, whose meaning diff --git a/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt b/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt index cab9b68f..8393a411 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt +++ b/MobileGL/MG_Test/Backend/DirectGLES/CMakeLists.txt @@ -32,6 +32,23 @@ target_link_libraries( ${LINK_LIBRARIES} ) +add_executable( + ViewportIndexRoutingTest + ViewportIndexRoutingTest.cpp +) + +target_include_directories(ViewportIndexRoutingTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL +) + +target_link_libraries( + ViewportIndexRoutingTest 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) +gtest_discover_tests(ViewportIndexRoutingTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/Backend/DirectGLES/ViewportIndexRoutingTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/ViewportIndexRoutingTest.cpp new file mode 100644 index 00000000..5959d4ba --- /dev/null +++ b/MobileGL/MG_Test/Backend/DirectGLES/ViewportIndexRoutingTest.cpp @@ -0,0 +1,179 @@ +// MobileGL - MobileGL/MG_Test/Backend/DirectGLES/ViewportIndexRoutingTest.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 ESSL half of the gl_ViewportIndex routing emulation (MG_Backend/DirectGLES/Managers.cpp). +// GLES has one viewport, one scissor rectangle and one depth range where GL 4.1 has sixteen of +// each selected per primitive, and the target device has no GL_OES_viewport_array to borrow, so +// DirectGLES turns the builtin into an ordinary flat varying and gives the fragment stage a gate +// the draw path replays against. +// +// Both passes are pure String -> String over what SPIRV-Cross emits once LowerViewportIndexPass +// has demoted the builtin, so no GL context and no driver: the shapes they have to survive - and +// the ones they must refuse - can be pinned here rather than only on a device. What they cannot +// pin is that the routing produces the right pixels; that is +// MG_IntegrationTest/Scenarios/ViewportArrayScenario.cpp, which runs the same claim through both +// backends. + +#include + +#include + +using MobileGL::Bool; +using MobileGL::String; +using MobileGL::MG_Backend::DirectGLES::InjectViewportIndexPassGate; +using MobileGL::MG_Backend::DirectGLES::PromoteViewportIndexGlobalToVarying; + +namespace { + Bool Contains(const String& haystack, const String& needle) { + return haystack.find(needle) != String::npos; + } + + // What SPIRV-Cross hands the backend for a geometry stage after LowerViewportIndexPass has + // demoted gl_ViewportIndex: a plain file-scope global the shader still writes and which, until + // this pass runs, nothing anywhere reads. + constexpr const char* kLoweredGeometryShader = R"(#version 320 es +layout(invocations = 16, points) in; +layout(max_vertices = 4, triangle_strip) out; + +layout(location = 0) flat out int gsIndex; +int mg_ViewportIndex; + +void main() +{ + gsIndex = gl_InvocationID; + mg_ViewportIndex = gl_InvocationID; + gl_Position = vec4(-1.0, -1.0, 0.0, 1.0); + EmitVertex(); + EndPrimitive(); +} +)"; + + constexpr const char* kFragmentShader = R"(#version 320 es +precision mediump float; +precision highp int; + +layout(location = 0) flat in int gsIndex; +layout(location = 0) out highp vec4 fragColor; + +void main() +{ + fragColor = vec4(float(gsIndex)); +} +)"; +} // namespace + +// The promotion itself. The declaration becomes an interface variable and the STORE is left +// exactly where it was - the pass must not touch the body, because the body is the application's. +TEST(ViewportIndexRoutingTest, TheDemotedGlobalBecomesAFlatVarying) { + String source = kLoweredGeometryShader; + ASSERT_TRUE(PromoteViewportIndexGlobalToVarying(source)) << source; + + EXPECT_TRUE(Contains(source, "flat out highp int mg_ViewportIndex;")) << source; + EXPECT_FALSE(Contains(source, "\nint mg_ViewportIndex;")) << source; + EXPECT_TRUE(Contains(source, " mg_ViewportIndex = gl_InvocationID;")) << source; +} + +// FLAT is the semantics and not a hint: GL takes a primitive's viewport index from its provoking +// vertex, and flat interpolation is what delivers that. An interpolated integer would not even +// compile in ESSL, so losing the qualifier fails loudly - but silently losing it to a `smooth` +// rewrite somewhere downstream would route by whichever vertex the rasterizer felt like. +TEST(ViewportIndexRoutingTest, ThePromotedVaryingIsFlatAndCarriesNoExplicitLocation) { + String source = kLoweredGeometryShader; + ASSERT_TRUE(PromoteViewportIndexGlobalToVarying(source)); + + const size_t declPos = source.find("flat out highp int mg_ViewportIndex;"); + ASSERT_NE(declPos, String::npos) << source; + // No layout(location = N): the two stages are transpiled independently and cannot agree on a + // number, so the varying is matched by NAME. A location that appeared here would have to + // appear identically in the fragment stage, which nothing can guarantee. + const size_t lineStart = source.rfind('\n', declPos); + const String declLine = source.substr(lineStart + 1, declPos - lineStart - 1); + EXPECT_EQ(declLine, "") << "the declaration must start its own line, with no layout qualifier"; +} + +// A precision-qualified declaration is the same declaration. SPIRV-Cross prints one or the other +// depending on what the module carried, and a pass that only matched the bare form would leave +// half the drivers unrouted while reporting success. +TEST(ViewportIndexRoutingTest, APrecisionQualifiedDeclarationIsPromotedToo) { + String source = "#version 320 es\nhighp int mg_ViewportIndex;\nvoid main() { mg_ViewportIndex = 3; }\n"; + ASSERT_TRUE(PromoteViewportIndexGlobalToVarying(source)) << source; + EXPECT_TRUE(Contains(source, "flat out highp int mg_ViewportIndex;")) << source; +} + +// A stage that never routed must come out byte-identical, because every stage of every program on +// this backend goes through the pass. +TEST(ViewportIndexRoutingTest, AStageWithoutTheGlobalIsUntouched) { + const String before = kFragmentShader; + String source = before; + EXPECT_FALSE(PromoteViewportIndexGlobalToVarying(source)); + EXPECT_EQ(source, before); +} + +// The one shape that would silently break a shader: a name that ends in mg_ViewportIndex but is +// not the declaration. Only a declaration starting its own line may be rewritten. +TEST(ViewportIndexRoutingTest, ADeclarationThatIsNotAtLineStartIsRefused) { + const String before = "#version 320 es\nuniform highp int mg_ViewportIndex;\nvoid main() {}\n"; + String source = before; + EXPECT_FALSE(PromoteViewportIndexGlobalToVarying(source)); + EXPECT_EQ(source, before); +} + +// The fragment gate. Three things have to be true at once: the varying and the uniform are +// declared, the application's entry point survives under a new name, and the new entry point +// discards on a mask miss and calls the old one otherwise. +TEST(ViewportIndexRoutingTest, TheFragmentGateWrapsTheEntryPoint) { + String source = kFragmentShader; + ASSERT_TRUE(InjectViewportIndexPassGate(source)) << source; + + EXPECT_TRUE(Contains(source, "flat in highp int mg_ViewportIndex;")) << source; + EXPECT_TRUE(Contains(source, "uniform highp int mg_ViewportPassMask;")) << source; + EXPECT_TRUE(Contains(source, "void mg_ViewportGatedMain()")) << source; + EXPECT_TRUE(Contains(source, "discard;")) << source; + EXPECT_TRUE(Contains(source, "mg_ViewportGatedMain();")) << source; + // The application's body is not edited, only renamed. + EXPECT_TRUE(Contains(source, " fragColor = vec4(float(gsIndex));")) << source; + // Exactly one entry point remains, and it is the wrapper. + EXPECT_EQ(source.find("void main()"), source.rfind("void main()")) << source; +} + +// The shift operand has to be clamped. GL leaves a gl_ViewportIndex outside [0, MAX_VIEWPORTS) +// undefined and the emulation is free to pick anything, but an ESSL shift by >= 32 is undefined +// in a way that can take the whole draw with it - so the gate must not be able to reach one. +TEST(ViewportIndexRoutingTest, TheGateClampsTheShiftIntoRange) { + String source = kFragmentShader; + ASSERT_TRUE(InjectViewportIndexPassGate(source)); + EXPECT_TRUE(Contains(source, "mg_ViewportPassMask >> (mg_ViewportIndex & 15)")) << source; +} + +// A fragment stage that READS gl_ViewportIndex has no ESSL spelling for it either, and the +// routing varying is exactly the value it wanted. This is the only place the read can be repaired +// - LowerViewportIndexPass deliberately demotes outputs only, because a demoted input would +// answer from an undefined global. +TEST(ViewportIndexRoutingTest, AFragmentStageReadOfTheBuiltinIsRedirectedOntoTheVarying) { + String source = R"(#version 320 es +precision highp int; +layout(location = 0) out highp vec4 fragColor; +void main() +{ + fragColor = vec4(float(gl_ViewportIndex)); +} +)"; + ASSERT_TRUE(InjectViewportIndexPassGate(source)) << source; + EXPECT_FALSE(Contains(source, "gl_ViewportIndex")) << source; + EXPECT_TRUE(Contains(source, "fragColor = vec4(float(mg_ViewportIndex));")) << source; +} + +// A stage the pass declines must reach the driver exactly as it arrived, not half-rewritten. +// The caller logs the decline and the program still renders - unrouted, which is the old +// behaviour - so a partially edited source here would turn a degradation into a broken shader. +TEST(ViewportIndexRoutingTest, AStageWithNoEntryPointIsDeclinedWithoutBeingEdited) { + const String before = "#version 320 es\nprecision highp int;\nhighp int f() { return gl_ViewportIndex; }\n"; + String source = before; + EXPECT_FALSE(InjectViewportIndexPassGate(source)); + EXPECT_EQ(source, before); +} diff --git a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp index f4851cdf..9c3ced4d 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp @@ -135,7 +135,7 @@ void main() { EsslTranslationKeyInputs inputs; inputs.spirv = &spirv; inputs.shaderType = GL_FRAGMENT_SHADER; - inputs.supportsViewportArray = false; + inputs.viewportIndexLoweringArmed = false; inputs.supportsNoperspectiveInterpolation = false; inputs.maxColorTextureSamples = 4; inputs.maxIntegerSamples = 1; @@ -855,8 +855,8 @@ TEST_F(TranslationCacheTest, L2KeyMovesWithEveryGateThatSteersTheEsslChain) { } { // arms LowerViewportIndexForEssl EsslTranslationKeyInputs v = base; - v.supportsViewportArray = true; - variants.emplace_back("supportsViewportArray", BuildEsslTranslationKey(v)); + v.viewportIndexLoweringArmed = true; + variants.emplace_back("viewportIndexLoweringArmed", BuildEsslTranslationKey(v)); } { // arms EmulateNoPerspectiveForEssl EsslTranslationKeyInputs v = base; @@ -1008,7 +1008,7 @@ TEST_F(TranslationCacheTest, L2RunsTheEmitterOncePerDistinctKey) { // ... and a gate that only steers the SPIR-V pass chain still moves the key, so the // emitter runs again even though this stand-in ignores the bit. inputs = BaselineEsslInputs(spirv); - inputs.supportsViewportArray = true; + inputs.viewportIndexLoweringArmed = true; (void)translate(inputs); EXPECT_EQ(emitCount, 3); }