Fix shaders that redefine a GLSL built-in failing on SDL-GPU and GLES (#2245)

* Retry a rejected compile with the shader's own functions renamed

glsm/src/main/java/com/gtnewhorizons/angelica/glsm/shader/GlslVulkanPreprocess.java

@@ -1,9 +1,12 @@
 package com.gtnewhorizons.angelica.glsm.shader;
 
 import com.gtnewhorizons.angelica.glsm.GlslTransformUtils;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 import it.unimi.dsi.fastutil.objects.Object2ObjectLinkedOpenHashMap;
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.antlr.v4.runtime.Token;
+import org.antlr.v4.runtime.tree.TerminalNode;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.Nullable;
@@ -217,6 +220,40 @@ public final class GlslVulkanPreprocess {
         for (Token tok : index.occurrences(name)) edits.add(new Edit(tok.getStartIndex(), tok.getStopIndex(), replacement));
     }
 
+    static @Nullable String renameUserFunctions(String source) {
+        final GLSLParser.Translation_unitContext root;
+        try {
+            root = GlslTransformUtils.parseFullQuiet(source);
+        } catch (Exception e) {
+            return null;
+        }
+
+        Object2IntOpenHashMap<String> prototypes = null;
+        for (GLSLParser.External_declarationContext external : root.external_declaration()) {
+            final GLSLParser.Function_definitionContext function = external.function_definition();
+            final GLSLParser.DeclarationContext declaration = function == null ? external.declaration() : null;
+            final GLSLParser.Function_prototypeContext prototype = function != null ? function.function_prototype() : declaration != null ? declaration.function_prototype() : null;
+            final TerminalNode identifier = prototype == null ? null : prototype.IDENTIFIER();
+            if (identifier == null) continue;
+            final String name = identifier.getText();
+            if ("main".equals(name)) continue;
+            if (prototypes == null) prototypes = new Object2IntOpenHashMap<>();
+            prototypes.addTo(name, 1);
+        }
+        if (prototypes == null) return null;
+
+        final ShaderIndex index = new ShaderIndex(root, source);
+        final Set<String> inDirectives = index.directives().identifiers();
+        final List<Edit> edits = new ArrayList<>();
+        for (Object2IntMap.Entry<String> prototype : prototypes.object2IntEntrySet()) {
+            final String name = prototype.getKey();
+            if (inDirectives.contains(name)) continue;
+            if (index.occurrences(name).size() != prototype.getIntValue() + index.callsTo(name).size()) continue;
+            rename(index, name, RESERVED_RENAMED_PREFIX + name, edits);
+        }
+        return edits.isEmpty() ? null : applyEdits(source, edits);
+    }
+
     private static void rewriteReadOnlyImageCalls(List<GLSLParser.Postfix_expressionContext> calls, String replacement, Set<String> readOnlyImages, List<Edit> edits) {
         for (GLSLParser.Postfix_expressionContext call : calls) {
             if (call.RIGHT_PAREN() == null) continue;

glsm/src/main/java/com/gtnewhorizons/angelica/glsm/shader/SpirvCompiler.java

@@ -81,7 +81,16 @@ public final class SpirvCompiler {
             }
         }
 
-        final Result result = compileUncached(cleanSrc, shaderKind, debugName, opts);
+        Result result = compileUncached(cleanSrc, shaderKind, debugName, opts);
+        if (result.spirv() == null) {
+            final String renamed = GlslVulkanPreprocess.renameUserFunctions(cleanSrc);
+            final Result retry = renamed == null ? null : compileUncached(renamed, shaderKind, debugName, opts);
+            if (retry == null || retry.spirv() == null) {
+                return new Result(null, result.error(), writeFailureDump(debugName, shaderKind, result.error(), cleanSrc));
+            }
+            LOGGER.info("'{}' compiled after renaming its functions; shaderc rejected the original: {}", debugName, result.error());
+            result = retry;
+        }
         final ByteBuffer spirv = result.spirv();
         if (spirv != null) {
             final byte[] heap = ShaderCacheIO.toHeap(spirv);
@@ -121,8 +130,7 @@ public final class SpirvCompiler {
                 final int status = Shaderc.shaderc_result_get_compilation_status(result);
                 if (status != Shaderc.shaderc_compilation_status_success) {
                     final String msg = Shaderc.shaderc_result_get_error_message(result);
-                    final Path dumpPath = writeFailureDump(debugName, shaderKind, msg, cleanSrc);
-                    return new Result(null, msg == null ? "(no error message)" : msg, dumpPath);
+                    return new Result(null, msg == null ? "(no error message)" : msg, null);
                 }
                 final ByteBuffer spirv = Shaderc.shaderc_result_get_bytes(result);
                 if (spirv == null || spirv.remaining() == 0) {

glsm/src/main/java/com/gtnewhorizons/angelica/glsm/shader/SpirvShaderTranslator.java

@@ -59,8 +59,7 @@ public final class SpirvShaderTranslator {
         final SpirvCompiler.Result cr = SpirvCompiler.compile(pre.rewrittenSource(), shaderKind, debugName, SpirvCompiler.Options.vulkanRelaxed());
         if (cr.spirv() == null) {
             final String firstErr = cr.error() == null ? "(no error)" : cr.error().split("\\R", 2)[0];
-            LOGGER.error("shaderc failed for '{}' (kind=0x{}): {} (dumped to {})",
-                    debugName, Integer.toHexString(shaderKind), firstErr, cr.dumpPath() == null ? "<dump failed>" : cr.dumpPath());
+            LOGGER.error("shaderc failed for '{}' (kind=0x{}): {} (dumped to {})", debugName, Integer.toHexString(shaderKind), firstErr, cr.dumpPath() == null ? "<dump failed>" : cr.dumpPath());
             return null;
         }
         try {

glsm/src/test/java/com/gtnewhorizons/angelica/glsm/shader/SpirvCompilerDiskCacheTest.java

@@ -8,6 +8,7 @@ import org.junit.jupiter.api.io.TempDir;
 import org.lwjgl.system.MemoryUtil;
 import org.lwjgl.util.shaderc.Shaderc;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -15,12 +16,35 @@ import java.nio.file.Path;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @Tag("lwjgl3")
 class SpirvCompilerDiskCacheTest {
 
     private static final String SRC = "#version 460 core\nlayout(location = 0) in vec4 p;\nvoid main() { gl_Position = p; }\n";
 
+    static final String REDEFINES_CLAMP = """
+        #version 330 core
+        in vec4 uv;
+        layout(location = 0) out vec4 color;
+        float clamp(float min, float f, float max) {
+            if (f > max) f = max;
+            if (f < min) f = min;
+            return f;
+        }
+        void main() {
+            color = vec4(clamp(0.0, uv.x, 1.0), clamp(0.0, uv.y, 1.0), 0.0, 1.0);
+        }
+        """;
+
+    private static final String BROKEN_WITH_HELPER = """
+        #version 330 core
+        layout(location = 0) out vec4 color;
+        float helper(float t) { return t; }
+        void main() { color = vec4(helper(undeclared_var)); }
+        """;
+
     @TempDir
     Path dir;
 
@@ -58,4 +82,24 @@ class SpirvCompilerDiskCacheTest {
         MemoryUtil.memFree(r.spirv());
         assertFalse(Files.exists(dir.resolve("spirv")));
     }
+
+    @Test
+    void redefinedBuiltinCompilesAndIsStoredUnderTheOriginalSource() {
+        final SpirvCompiler.Options opts = SpirvCompiler.Options.vulkanForced460Core();
+        ShaderDiskCache.configure(dir, "t");
+        final SpirvCompiler.Result r = SpirvCompiler.compile(REDEFINES_CLAMP, Shaderc.shaderc_fragment_shader, "redefines-clamp", opts);
+        assertNotNull(r.spirv(), r.error());
+        MemoryUtil.memFree(r.spirv());
+        assertNull(r.dumpPath());
+        assertNotNull(ShaderDiskCache.get(SpirvCompiler.diskKey(REDEFINES_CLAMP, Shaderc.shaderc_fragment_shader, opts)));
+    }
+
+    @Test
+    void brokenShaderWithHelperReportsTheOriginalError() throws IOException {
+        final SpirvCompiler.Result r = SpirvCompiler.compile(BROKEN_WITH_HELPER, Shaderc.shaderc_fragment_shader, "broken-with-helper", SpirvCompiler.Options.vulkanForced460Core(), false);
+        assertNull(r.spirv());
+        assertTrue(r.error().contains("undeclared_var"), r.error());
+        assertNotNull(r.dumpPath());
+        assertFalse(Files.readString(r.dumpPath()).contains(GlslVulkanPreprocess.RESERVED_RENAMED_PREFIX));
+    }
 }

glsm/src/test/java/com/gtnewhorizons/angelica/glsm/shader/SpirvShaderTranslatorTest.java

@@ -131,6 +131,11 @@ class SpirvShaderTranslatorTest {
                 "emitted uniform must not be `uint` - `glUniform1i` only accepts int/bool uniforms");
     }
 
+    @Test
+    void redefinedBuiltin_translates() {
+        assertNotNull(SpirvShaderTranslator.glslToGlslEs(SpirvCompilerDiskCacheTest.REDEFINES_CLAMP, GL20.GL_FRAGMENT_SHADER, "redefines_clamp.frag"));
+    }
+
     @Test
     void uniformBoolWithInitializer_roundTripsAsBool() {
         final String src = """

glsm/src/test/java/com/gtnewhorizons/angelica/glsm/shader/UserFunctionRenameTest.java

@@ -0,0 +1,82 @@
+package com.gtnewhorizons.angelica.glsm.shader;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+class UserFunctionRenameTest {
+
+    @Test
+    void definitionForwardDeclarationAndCallsAreRenamed() {
+        final String out = GlslVulkanPreprocess.renameUserFunctions("""
+            #version 330 core
+            float fade(float t);
+            out vec4 color;
+            void main() { color = vec4(fade(0.5)); }
+            float fade(float t) { return t * t; }
+            """);
+        assertEquals("""
+            #version 330 core
+            float angelica_renamed_fade(float t);
+            out vec4 color;
+            void main() { color = vec4(angelica_renamed_fade(0.5)); }
+            float angelica_renamed_fade(float t) { return t * t; }
+            """, out);
+    }
+
+    @Test
+    void overloadsOfOneNameAreRenamedTogether() {
+        final String out = GlslVulkanPreprocess.renameUserFunctions("""
+            #version 330 core
+            out vec4 color;
+            float clamp(float min, float f, float max) { return f; }
+            vec2 clamp(vec2 f) { return vec2(clamp(0.0, f.x, 1.0), clamp(0.0, f.y, 1.0)); }
+            void main() { color = vec4(clamp(vec2(0.5)), 0.0, 1.0); }
+            """);
+        assertEquals("""
+            #version 330 core
+            out vec4 color;
+            float angelica_renamed_clamp(float min, float f, float max) { return f; }
+            vec2 angelica_renamed_clamp(vec2 f) { return vec2(angelica_renamed_clamp(0.0, f.x, 1.0), angelica_renamed_clamp(0.0, f.y, 1.0)); }
+            void main() { color = vec4(angelica_renamed_clamp(vec2(0.5)), 0.0, 1.0); }
+            """, out);
+    }
+
+    @Test
+    void nameUsedAsMacroArgumentIsLeftAlone() {
+        assertNull(GlslVulkanPreprocess.renameUserFunctions("""
+            #version 330 core
+            #define SAT(fn, x) fn(0.0, x, 1.0)
+            out vec4 color;
+            float clamp(float min, float f, float max) { return f; }
+            void main() { color = vec4(SAT(clamp, 0.5)); }
+            """));
+    }
+
+    @Test
+    void nameUsedInMacroBodyIsLeftAlone() {
+        assertNull(GlslVulkanPreprocess.renameUserFunctions("""
+            #version 330 core
+            #define FADE(x) fade(x)
+            out vec4 color;
+            float fade(float t) { return t * t; }
+            void main() { color = vec4(FADE(0.5)); }
+            """));
+    }
+
+    @Test
+    void nameSharedWithVariableIsLeftAlone() {
+        assertNull(GlslVulkanPreprocess.renameUserFunctions("""
+            #version 330 core
+            out vec4 color;
+            float fade(float t) { return t * t; }
+            void main() { float fade = 2.0; color = vec4(fade); }
+            """));
+    }
+
+    @Test
+    void shaderWithOnlyMainHasNothingToRename() {
+        assertNull(GlslVulkanPreprocess.renameUserFunctions("#version 330 core\nout vec4 color;\nvoid main() { color = vec4(1.0); }\n"));
+    }
+}

sdl-gpu/src/main/java/com/gtnewhorizons/angelica/sdlgpu/shader/ShaderManager.java

@@ -524,6 +524,10 @@ public final class ShaderManager {
         shObj.attachCount++;
     }
 
+    private static String stageLinkError(String stage, ShaderObject obj) {
+        return stage + (obj == null ? " shader missing" : " shader not compiled");
+    }
+
     public void linkProgram(int program) {
         final ProgramObject prog = programObjects.get(program);
         if (prog == null) return;
@@ -537,9 +541,11 @@ public final class ShaderManager {
         final ShaderObject fs = shaderObjects.get(prog.fragmentShader);
         final ShaderObject gs = prog.geometryShader != 0 ? shaderObjects.get(prog.geometryShader) : null;
 
-        if (vs == null || !vs.compiled || fs == null || !fs.compiled) {
+        final boolean vertexBad = vs == null || !vs.compiled;
+        final boolean fragmentBad = fs == null || !fs.compiled;
+        if (vertexBad || fragmentBad) {
             prog.linked = false;
-            prog.infoLog = "Shader not compiled";
+            prog.infoLog = !fragmentBad ? stageLinkError("Vertex", vs) : !vertexBad ? stageLinkError("Fragment", fs) : stageLinkError("Vertex", vs) + "; " + stageLinkError("Fragment", fs);
             return;
         }
 

sdl-gpu/src/test/java/com/gtnewhorizons/angelica/sdlgpu/shader/StageDiskCacheTest.java

@@ -53,6 +53,20 @@ class StageDiskCacheTest {
         void main() { undeclared_var = 1.0; }
         """;
 
+    private static final String REDEFINES_CLAMP = """
+        #version 330 core
+        in vec4 uv;
+        layout(location = 0) out vec4 color;
+        float clamp(float min, float f, float max) {
+            if (f > max) f = max;
+            if (f < min) f = min;
+            return f;
+        }
+        void main() {
+            color = vec4(clamp(0.0, uv.x, 1.0), clamp(0.0, uv.y, 1.0), 0.0, 1.0);
+        }
+        """;
+
     private static final byte[] POISON_SPIRV = {1, 2, 3, 4, 5, 6, 7, 8};
 
     private static final PerFrameUniformBlock BLOCK_A = new PerFrameUniformBlock(List.of(new Member("frameTimeCounter", UniformType.FLOAT)));
@@ -177,6 +191,37 @@ class StageDiskCacheTest {
         sm.deleteShader(shader);
     }
 
+    @Test
+    void redefinedBuiltinCompilesAndIsStored(@TempDir Path dir) {
+        ShaderDiskCache.configure(dir, "t");
+        ShaderManager.prewarmSpirv(REDEFINES_CLAMP, GL20.GL_FRAGMENT_SHADER);
+        assertNotNull(ShaderDiskCache.get(ShaderManager.prewarmKey(REDEFINES_CLAMP, GL20.GL_FRAGMENT_SHADER, null, null)));
+
+        ShaderManager.clearPrewarmCache();
+        final ShaderManager sm = new ShaderManager(null);
+        final int shader = sm.createShader(GL20.GL_FRAGMENT_SHADER);
+        sm.shaderSource(shader, REDEFINES_CLAMP);
+        sm.compileShader(shader);
+        assertTrue(object(sm, shader).compiled);
+        assertNotNull(ShaderDiskCache.get(ShaderManager.stageKey(sm.getShaderSource(shader), GL20.GL_FRAGMENT_SHADER)));
+        sm.deleteShader(shader);
+    }
+
+    @Test
+    void linkLogNamesTheMissingAndUncompiledStages() {
+        final ShaderManager sm = new ShaderManager(null);
+        final int program = sm.createProgram();
+        sm.linkProgram(program);
+        assertEquals("Vertex shader missing; Fragment shader missing", sm.getProgramInfoLog(program));
+
+        final int shader = sm.createShader(GL20.GL_VERTEX_SHADER);
+        sm.shaderSource(shader, BROKEN);
+        sm.compileShader(shader);
+        sm.attachShader(program, shader);
+        sm.linkProgram(program);
+        assertEquals("Vertex shader not compiled; Fragment shader missing", sm.getProgramInfoLog(program));
+    }
+
     @Test
     void corruptEntryFallsBackToCompilingAndIsReplaced(@TempDir Path dir) {
         ShaderDiskCache.configure(dir, "t");