diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index d4b8a792..04ce7b76 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -12495,14 +12495,26 @@ namespace MobileGL::MG_Backend::DirectGLES { // probe exists to avoid. Nothing else in the loop is order-sensitive: the two // passthrough-tessellation sources it captures are a vertex and an evaluation stage, // and the three sets it accumulates are unions. + // + // The pre-rasterization stages are walked in PIPELINE order (the stage enum's), so a + // consumer's input-block redeclaration below can mirror the output block its producer + // has already emitted. Vector stageOrder; stageOrder.reserve(linkedStages.size()); for (SizeT index = 0; index < linkedStages.size(); ++index) { if (linkedStages[index] != ShaderStage::Fragment) stageOrder.push_back(index); } + std::stable_sort(stageOrder.begin(), stageOrder.end(), [&linkedStages](SizeT a, SizeT b) { + return static_cast(linkedStages[a]) < static_cast(linkedStages[b]); + }); for (SizeT index = 0; index < linkedStages.size(); ++index) { if (linkedStages[index] == ShaderStage::Fragment) stageOrder.push_back(index); } + // The output gl_PerVertex members the last pre-rasterization stage emitted, for the + // next stage's input redeclaration (RedeclareClipCullDistanceInputsInPerVertexBlock). + // A synthesized pass-through control stage copies its input block through, so the + // vertex stage's members are what an evaluation stage behind one sees. + String producerOutPerVertexMembers; // Set by whichever pre-rasterization stage's demoted mg_ViewportIndex global the text // pass turned into a varying; read by the fragment stage to decide whether to inject // the gate that consumes it. @@ -12739,6 +12751,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // before vertexStageEssl is captured, so a synthesized pass-through control // stage mirrors the block this produces. source = RedeclareClipCullDistancesInPerVertexBlock(std::move(source), glShaderType); + // ...and the consumer half: a stage reading gl_in[i].gl_ClipDistance/CullDistance + // redeclares its input block with the sizes the producer wrote. + source = RedeclareClipCullDistanceInputsInPerVertexBlock(std::move(source), glShaderType, + producerOutPerVertexMembers); + if (glShaderType != GL_FRAGMENT_SHADER) { + producerOutPerVertexMembers = + ExtractPerVertexBlockMembers(source, /*input=*/false).value_or(String()); + } // Position in the chain is arbitrary: this is the only header-level rewrite, it // edits #extension directives and never the body, and the replacement is the diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 880e265e..d378b961 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -1047,6 +1047,60 @@ namespace MobileGL::MG_Backend::DirectGLES { return result; } + String RedeclareClipCullDistanceInputsInPerVertexBlock(String glslCode, const GLenum shaderType, + const String& producerOutMembers) { +#ifdef TRACY_ENABLE + ZoneScopedC(TRACY_ZONECOLOR_BACKEND); +#endif + const Bool tessellation = shaderType == GL_TESS_CONTROL_SHADER || shaderType == GL_TESS_EVALUATION_SHADER; + if (!tessellation && shaderType != GL_GEOMETRY_SHADER) return glslCode; + if (producerOutMembers.find("gl_ClipDistance") == String::npos && + producerOutMembers.find("gl_CullDistance") == String::npos) { + return glslCode; + } + if (ExtractPerVertexBlockMembers(glslCode, /*input=*/true).has_value()) return glslCode; + + // Every gl_in member the stage reads has to be in the redeclared block, or the stage + // stops compiling; one the producer does not write cannot be added honestly. + static const std::regex memberRegex(R"(gl_in\s*\[[^\]]*\]\s*\.\s*(gl_[A-Za-z]+))"); + Bool readsDistances = false; + for (auto it = std::sregex_iterator(glslCode.begin(), glslCode.end(), memberRegex); + it != std::sregex_iterator(); ++it) { + const String member = (*it)[1].str(); + if (member == "gl_ClipDistance" || member == "gl_CullDistance") readsDistances = true; + if (producerOutMembers.find(member) == String::npos) return glslCode; + } + if (!readsDistances) return glslCode; + + // `invariant` qualifies outputs only. + String members = producerOutMembers; + for (SizeT pos = members.find("invariant "); pos != String::npos; pos = members.find("invariant ")) { + members.erase(pos, std::strlen("invariant ")); + } + while (!members.empty() && std::isspace(static_cast(members.back()))) members.pop_back(); + while (!members.empty() && (members.front() == '\n' || members.front() == '\r')) members.erase(0, 1); + String block = "in gl_PerVertex\n{\n" + members + "\n} gl_in["; + block += tessellation ? "gl_MaxPatchVertices" : ""; + block += "];\n"; + + // After the header (#directives, precision statements and the stage's layout-only + // in/out declarations, which a geometry stage's unsized gl_in[] depends on). + static const std::regex headerRegex( + R"(^\s*(#.*|precision\s.*|layout\s*\(.*\)\s*(in|out)\s*;\s*)?$)"); + SizeT insertAt = 0; + SizeT lineStart = 0; + while (lineStart < glslCode.size()) { + SizeT lineEnd = glslCode.find('\n', lineStart); + if (lineEnd == String::npos) lineEnd = glslCode.size(); + const String line = glslCode.substr(lineStart, lineEnd - lineStart); + if (!std::regex_match(line, headerRegex)) break; + insertAt = std::min(lineEnd + 1, glslCode.size()); + lineStart = lineEnd + 1; + } + glslCode.insert(insertAt, block); + return glslCode; + } + String RequestClipCullDistanceExtension(String glslCode) { static constexpr const char* kDirective = "#extension GL_EXT_clip_cull_distance : require\n"; if (glslCode.find("GL_EXT_clip_cull_distance") != String::npos || diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index 851a02c9..50887312 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -399,6 +399,18 @@ namespace MobileGL::MG_Backend::DirectGLES { // Fragment inputs and the control stage (whose block SPIRV-Cross already emits) are // left alone; a stage without the standalone declarations comes back unchanged. String RedeclareClipCullDistancesInPerVertexBlock(String glslCode, GLenum shaderType); + // The INPUT half of the same problem, for a tessellation control, tessellation evaluation + // or geometry stage that reads gl_in[i].gl_ClipDistance / gl_CullDistance. SPIRV-Cross + // never redeclares an ES input block, so the stage reads the implicitly sized built-in + // arrays - and a driver that packs cull distances after clip distances then places the + // cull array at the implicit clip size rather than at the size the previous stage wrote, + // reading the wrong component. Redeclares `in gl_PerVertex { ... } gl_in[]` with exactly + // the members of the previous stage's output block (`producerOutMembers`, from + // ExtractPerVertexBlockMembers), which is also the shape ES interface matching wants. + // Unchanged when the stage does not read either array, already declares its input block, + // reads a gl_in member the producer's block does not carry, or the producer has no block. + String RedeclareClipCullDistanceInputsInPerVertexBlock(String glslCode, GLenum shaderType, + const String& producerOutMembers); // Adds `#extension GL_EXT_clip_cull_distance : require` when `glslCode` names // gl_ClipDistance or gl_CullDistance and does not request it yet. For the synthesized // pass-through control stage, whose gl_in/gl_out member lists are mirrored from stages diff --git a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp index c0a53070..6fe0c24b 100644 --- a/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp +++ b/MobileGL/MG_Test/Backend/DirectGLES/EsslShaderPassTest.cpp @@ -34,6 +34,7 @@ using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_WRITE_ALIAS_PREFIX; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::IMAGE_WRITEONLY_ALIAS_PREFIX; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::ImageArrayUnitPlan; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RedeclareClipCullDistancesInPerVertexBlock; +using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RedeclareClipCullDistanceInputsInPerVertexBlock; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RemapImageArrayElementUnits; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::PointSizeExtensionName; using MobileGL::MG_Backend::DirectGLES::PrgramImpl::RemoveLayoutBinding; @@ -1675,3 +1676,90 @@ void main() { color = vec4(1.0); } EXPECT_TRUE(Contains(out, " highp float gl_ClipDistance[3];\n};")) << out; EXPECT_TRUE(Contains(out, "GL_EXT_clip_cull_distance")) << out; } + +// The consumer half: SPIRV-Cross never redeclares an ES input block, so a stage reading +// gl_in[i].gl_CullDistance gets the implicitly sized arrays, and a driver that packs cull +// distances after clip distances reads them from the wrong component +// (KHR-GL45.cull_distance.functional_test_item_{5,6}). The block mirrors the producer's. +TEST(RedeclareClipCullDistanceInputsTest, MirrorsTheProducerBlockIntoATessellationStage) { + const String producer = "\n highp vec4 gl_Position;\n highp float gl_ClipDistance[1];\n" + " highp float gl_CullDistance[7];\n"; + const String tes = R"(#version 320 es +#extension GL_EXT_clip_cull_distance : require +precision highp float; +precision highp int; +layout(triangles, ccw, equal_spacing) in; + +out gl_PerVertex +{ + highp vec4 gl_Position; + highp float gl_CullDistance[7]; +}; + +void main() +{ + gl_CullDistance[0] = dot(vec3(gl_in[0].gl_CullDistance[0], gl_in[1].gl_CullDistance[0], gl_in[2].gl_CullDistance[0]), gl_TessCoord); + gl_Position = gl_in[0].gl_Position; +} +)"; + const String out = RedeclareClipCullDistanceInputsInPerVertexBlock(tes, GL_TESS_EVALUATION_SHADER, producer); + EXPECT_TRUE(Contains(out, "layout(triangles, ccw, equal_spacing) in;\n\nin gl_PerVertex\n{\n" + " highp vec4 gl_Position;\n highp float gl_ClipDistance[1];\n" + " highp float gl_CullDistance[7];\n} gl_in[gl_MaxPatchVertices];\n")) + << out; + // Applied once only. + EXPECT_EQ(RedeclareClipCullDistanceInputsInPerVertexBlock(out, GL_TESS_EVALUATION_SHADER, producer), out); +} + +TEST(RedeclareClipCullDistanceInputsTest, GeometryStageGetsAnUnsizedBlockAfterItsLayouts) { + const String producer = "\n invariant highp vec4 gl_Position;\n highp float gl_CullDistance[8];\n"; + const String gs = R"(#version 320 es +#extension GL_EXT_clip_cull_distance : require +precision highp float; +layout(triangles) in; +layout(max_vertices = 3, triangle_strip) out; + +out gl_PerVertex +{ + highp vec4 gl_Position; + highp float gl_CullDistance[8]; +}; + +void main() +{ + gl_Position = gl_in[0].gl_Position; + gl_CullDistance[0] = gl_in[0].gl_CullDistance[0]; + EmitVertex(); +} +)"; + const String out = RedeclareClipCullDistanceInputsInPerVertexBlock(gs, GL_GEOMETRY_SHADER, producer); + EXPECT_TRUE(Contains(out, "layout(max_vertices = 3, triangle_strip) out;\n\nin gl_PerVertex\n{\n" + " highp vec4 gl_Position;\n highp float gl_CullDistance[8];\n} gl_in[];\n")) + << out; + EXPECT_FALSE(Contains(out, "invariant")) << out; +} + +TEST(RedeclareClipCullDistanceInputsTest, LeavesStagesItCannotRedeclareHonestlyAlone) { + const String producer = "\n highp vec4 gl_Position;\n highp float gl_ClipDistance[2];\n"; + // Reads gl_PointSize, which the producer's block does not carry. + const String readsPointSize = R"(#version 320 es +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_Position = gl_in[0].gl_Position * gl_in[0].gl_PointSize + vec4(gl_in[0].gl_ClipDistance[1]); EmitVertex(); } +)"; + EXPECT_EQ(RedeclareClipCullDistanceInputsInPerVertexBlock(readsPointSize, GL_GEOMETRY_SHADER, producer), + readsPointSize); + // Does not read the distances at all. + const String plain = R"(#version 320 es +layout(points) in; +layout(points, max_vertices = 1) out; +void main() { gl_Position = gl_in[0].gl_Position; EmitVertex(); } +)"; + EXPECT_EQ(RedeclareClipCullDistanceInputsInPerVertexBlock(plain, GL_GEOMETRY_SHADER, producer), plain); + // The producer wrote no distances, and a vertex stage has no gl_in at all. + EXPECT_EQ(RedeclareClipCullDistanceInputsInPerVertexBlock(readsPointSize, GL_GEOMETRY_SHADER, + "\n highp vec4 gl_Position;\n"), + readsPointSize); + EXPECT_EQ(RedeclareClipCullDistanceInputsInPerVertexBlock(readsPointSize, GL_VERTEX_SHADER, producer), + readsPointSize); +}