[Fix] (MG_Util/ShaderTranspiler): retry a legacy shader at 460 when it fails to parse as normalized 330 core, so sources using 420-era syntax without the matching #extension line keep compiling as they did on real drivers

MobileGL/MG_Test/Program/ProgramUtilTest.cpp

@@ -556,6 +556,81 @@ TEST_F(ProgramUtilTest, CompileSimpleVertexShader) {
     }
 }
 
+// Legacy desktop sources are normalized to "#version 330 core", which is stricter than the 460 they
+// used to be forced to. A shader declaring 330 while using 420-era syntax without the matching
+// #extension line is accepted by real drivers, so CompileShader retries it at 460 instead of failing.
+TEST_F(ProgramUtilTest, CompileShaderRetriesAt460WhenLegacyVersionRejects420Syntax) {
+    using namespace MG_Util::ShaderTranspiler;
+    String source = R"(#version 330
+layout(binding = 0) uniform sampler2D InSampler;
+in vec2 texCoord;
+out vec4 fragColor;
+void main() {
+    fragColor = texture(InSampler, texCoord);
+})";
+    PreprocessShaderSource(ShaderStage::Fragment, source);
+    // The normal path still emits 330 - the retry must not become the default.
+    ASSERT_EQ(source.find("#version 330 core"), 0u);
+
+    ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source};
+    auto res = ShaderCompiler::CompileShader(attrib);
+    if (!res) {
+        FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log;
+    }
+
+    // Same source compiled for the OpenGL environment must take the retry too.
+    ShaderAttrib glAttrib{
+        .shaderType = GL_FRAGMENT_SHADER, .sourceStr = source, .flags = ShaderCompileBits::CompileForOpenGL};
+    auto glRes = ShaderCompiler::CompileShader(glAttrib);
+    if (!glRes) {
+        FAIL() << "errc: " << glRes.error().errc << "\nlog: " << glRes.error().log;
+    }
+}
+
+TEST_F(ProgramUtilTest, CompileShaderStillFailsWithOriginalDiagnosticsWhenRetryCannotHelp) {
+    using namespace MG_Util::ShaderTranspiler;
+    String source = R"(#version 330
+in vec2 texCoord;
+out vec4 fragColor;
+void main() {
+    fragColor = thisFunctionDoesNotExist(texCoord);
+})";
+    PreprocessShaderSource(ShaderStage::Fragment, source);
+
+    ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source};
+    auto res = ShaderCompiler::CompileShader(attrib);
+    ASSERT_FALSE(res);
+    EXPECT_EQ(res.error().errc, -2);
+    EXPECT_NE(res.error().log.find("thisFunctionDoesNotExist"), String::npos) << res.error().log;
+}
+
+TEST_F(ProgramUtilTest, RetargetLegacyVersionDirectiveOnlyTouchesNormalizedDesktopCore) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    String normalized = "#version 330 core\nvoid main() {}\n";
+    EXPECT_TRUE(RetargetLegacyVersionDirectiveTo460(normalized));
+    EXPECT_EQ(normalized.find("#version 460 core"), 0u);
+
+    // Already modern: nothing to retarget.
+    String modern = "#version 460 core\nvoid main() {}\n";
+    EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(modern));
+    EXPECT_EQ(modern.find("#version 460 core"), 0u);
+
+    // ES and compatibility sources keep what they declared.
+    String es = "#version 300 es\nvoid main() {}\n";
+    EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(es));
+    EXPECT_EQ(es.find("#version 300 es"), 0u);
+
+    String compat = "#version 330 compatibility\nvoid main() {}\n";
+    EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(compat));
+    EXPECT_EQ(compat.find("#version 330 compatibility"), 0u);
+
+    // A commented-out directive is not the real one.
+    String commented = "// #version 330 core\nvoid main() {}\n";
+    EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(commented));
+    EXPECT_EQ(commented.find("#version 460"), String::npos);
+}
+
 const char* fs = R"(#version 150
 
 uniform sampler2D InSampler;

MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp

@@ -18,6 +18,7 @@
 #include "spirv-tools/libspirv.h"
 #include "spirv-tools/optimizer.hpp"
 
+#include "ShaderSourceProcessor.h"
 #include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
 #include <MG_Util/Converters/GLToGlslang/ProgramEnumConverter.h>
 
@@ -133,27 +134,23 @@ namespace MobileGL {
                 return Resources;
             }
 
-            Result<SharedPtr<glslang::TShader>> ShaderCompiler::CompileShader(const ShaderAttrib& attrib) {
-                auto shaderType = attrib.shaderType;
-                auto& sourceStr = attrib.sourceStr;
-
-                auto lang = MG_Util::ConvertGLEnumToEShLanguage(shaderType);
-                if (lang == EShLanguage::EShLangCount) {
-                    ResultInfo r;
-                    r.log += "Error: [Preprocess] Unsupported shader type: " + ConvertGLEnumToString(shaderType);
-                    r.errc = -1;
-                    return std::unexpected(r);
-                }
-
+            // One parse attempt. A glslang::TShader cannot be re-parsed, so a retry has to build a
+            // fresh one with byte-identical setup - hence a single factored body rather than two
+            // copies that could drift apart.
+            static Result<SharedPtr<glslang::TShader>> ParseShaderSource(EShLanguage lang, GLenum shaderType,
+                                                                         const String& source,
+                                                                         Flags<ShaderCompileBits> flags) {
                 SharedPtr<glslang::TShader> res;
                 auto& tshader = res;
                 tshader = MakeShared<glslang::TShader>(lang);
-                const char* src[] = {sourceStr.data()};
+                // setStrings gets no length array, so it relies on NUL termination: source must be an
+                // owning buffer that outlives parse(), never a StringView's substring.
+                const char* src[] = {source.c_str()};
                 tshader->setStrings(src, 1);
                 tshader->setNanMinMaxClamp(true);
                 tshader->setInvertY(true);
                 tshader->setPreamble("#undef VULKAN\n");
-                if (attrib.flags & ShaderCompileBits::CompileForOpenGL) {
+                if (flags & ShaderCompileBits::CompileForOpenGL) {
                     tshader->setEnvInput(glslang::EShSourceGlsl, lang, glslang::EShClientVulkan, 450);
                     tshader->setEnvClient(glslang::EShClientOpenGL, glslang::EShTargetOpenGL_450);
                     tshader->setEnvTarget(glslang::EShTargetSpv, glslang::EShTargetSpv_1_3);
@@ -182,6 +179,39 @@ namespace MobileGL {
                 return res;
             }
 
+            Result<SharedPtr<glslang::TShader>> ShaderCompiler::CompileShader(const ShaderAttrib& attrib) {
+                auto shaderType = attrib.shaderType;
+
+                auto lang = MG_Util::ConvertGLEnumToEShLanguage(shaderType);
+                if (lang == EShLanguage::EShLangCount) {
+                    ResultInfo r;
+                    r.log += "Error: [Preprocess] Unsupported shader type: " + ConvertGLEnumToString(shaderType);
+                    r.errc = -1;
+                    return std::unexpected(r);
+                }
+
+                const String source(attrib.sourceStr);
+                auto result = ParseShaderSource(lang, shaderType, source, attrib.flags);
+                if (result) return result;
+
+                // Legacy desktop sources are normalized to "#version 330 core", which parses under
+                // stricter rules than the 460 they used to be forced to: a shader declaring 330 while
+                // using e.g. layout(binding=...) without the matching #extension line compiles on real
+                // drivers but is rejected here. Retry once at 460 before reporting failure; a genuinely
+                // broken shader fails both attempts and keeps its original diagnostics.
+                String retrySource = source;
+                if (!MG_Util::ShaderTranspiler::RetargetLegacyVersionDirectiveTo460(retrySource)) {
+                    return result;
+                }
+
+                auto retryResult = ParseShaderSource(lang, shaderType, retrySource, attrib.flags);
+                if (!retryResult) return result;
+
+                MGLOG_D("CompileShader: %s only parsed after retargeting its legacy #version to 460",
+                        ConvertGLEnumToString(shaderType).c_str());
+                return retryResult;
+            }
+
             Result<SharedPtr<glslang::TProgram>> ShaderCompiler::LinkProgram(const ProgramAttrib& attrib) {
                 SharedPtr<glslang::TProgram> program = MakeShared<glslang::TProgram>();
                 for (auto& s : attrib.shaders) {

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp

@@ -633,6 +633,21 @@ namespace MobileGL {
                 InjectDepthRangeBuiltinShim(stage, source);
             }
 
+            Bool RetargetLegacyVersionDirectiveTo460(String& source) {
+                // Re-inspect rather than searching for the literal directive: it is not necessarily at
+                // offset 0 (a BOM or comments may precede it) and a commented-out "#version" elsewhere
+                // must not be mistaken for the real one.
+                const ShaderLanguageInfo info = InspectShaderLanguage(source);
+                if (!info.HasVersionDirective()) return false;
+                // Only the set NormalizeVersionDirective downgraded: desktop core below 400. ES and
+                // compatibility shaders keep whatever they declared.
+                if (info.profile != ShaderProfile::Core || info.version >= 400) return false;
+
+                source.replace(info.versionDirectiveStart, info.versionDirectiveEnd - info.versionDirectiveStart,
+                               "#version 460 core\n");
+                return true;
+            }
+
         } // namespace ShaderTranspiler
     } // namespace MG_Util
 } // namespace MobileGL

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h

@@ -20,6 +20,14 @@ namespace MobileGL {
     namespace MG_Util {
         namespace ShaderTranspiler {
             void PreprocessShaderSource(ShaderStage stage, String& source);
+
+            // Rewrites a "#version 330 core" directive that PreprocessShaderSource normalized down
+            // from a legacy desktop version back up to "#version 460 core". Returns false (leaving
+            // the source untouched) for anything else: ES, compatibility, or an already-modern
+            // declaration. Exists so a shader that only parses under the laxer 460 rules - e.g. it
+            // uses 420-era syntax without the matching #extension line, which real drivers tend to
+            // accept - can be retried instead of failing to compile.
+            Bool RetargetLegacyVersionDirectiveTo460(String& source);
         } // namespace ShaderTranspiler
     } // namespace MG_Util
 } // namespace MobileGL