MCShader: sort attributes according to VertexFormat

* MCShader: sort attributes according to VertexFormat This is necessary in order for shaders to work correctly on 1.17+. Previously, only simple shaders were tested which happened to match the order of replaceAttribute calls, so this issue went unnoticed. * MCShader: use uc_Position instead of uc_Vertex This is consistent with Minecraft's naming and allows us to derive the Minecraft name by removing the uc_ prefix. * MCShader: sort attributes in json based on VertexFormat While MC currently ignores the attributes json field, it's possible it will be used in the future, so we should make sure the order is correct. GitHub: #54
EssentialGG · Aug 29, 2023 · 7d0c9f4 · 7d0c9f4
1 parent f20ea7d
commit 7d0c9f4
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 19 deletions.
diff --git a/api/UniversalCraft.api b/api/UniversalCraft.api
@@ -241,6 +241,12 @@ public final class gg/essential/universal/UGraphics$CommonVertexFormats : java/l
 	public static final field POSITION_TEXTURE_COLOR_LIGHT Lgg/essential/universal/UGraphics$CommonVertexFormats;
 	public static final field POSITION_TEXTURE_COLOR_NORMAL Lgg/essential/universal/UGraphics$CommonVertexFormats;
 	public static final field POSITION_TEXTURE_LIGHT_COLOR Lgg/essential/universal/UGraphics$CommonVertexFormats;
+	@1.17.1-forge,1.18.1-forge,1.19.2-forge,1.19.3-forge,1.19.4-forge,1.20.1-forge
+	public final field mc Lcom/mojang/blaze3d/vertex/VertexFormat;
+	@1.16.2-fabric,1.17.1-fabric,1.18.1-fabric,1.19-fabric,1.19.1-fabric,1.19.2-fabric,1.19.3-fabric,1.19.4-fabric,1.20-fabric,1.20.1-fabric
+	public final field mc Lnet/minecraft/client/render/VertexFormat;
+	@1.12.2-forge,1.15.2-forge,1.16.2-forge,1.8.9-forge
+	public final field mc Lnet/minecraft/client/renderer/vertex/VertexFormat;
 	public static fun valueOf (Ljava/lang/String;)Lgg/essential/universal/UGraphics$CommonVertexFormats;
 	public static fun values ()[Lgg/essential/universal/UGraphics$CommonVertexFormats;
 }
@@ -1080,6 +1086,7 @@ public abstract interface class gg/essential/universal/shader/UShader {
 
 public final class gg/essential/universal/shader/UShader$Companion {
 	public final fun fromLegacyShader (Ljava/lang/String;Ljava/lang/String;Lgg/essential/universal/shader/BlendState;)Lgg/essential/universal/shader/UShader;
+	public final fun fromLegacyShader (Ljava/lang/String;Ljava/lang/String;Lgg/essential/universal/shader/BlendState;Lgg/essential/universal/UGraphics$CommonVertexFormats;)Lgg/essential/universal/shader/UShader;
 	@1.19.3-fabric,1.19.4-fabric,1.20-fabric,1.20.1-fabric
 	public final fun fromMcShader (Lnet/minecraft/client/gl/ShaderProgram;Lgg/essential/universal/shader/BlendState;)Lgg/essential/universal/shader/UShader;
 	@1.17.1-forge,1.18.1-forge,1.19.2-forge,1.19.3-forge,1.19.4-forge,1.20.1-forge

diff --git a/src/main/java/gg/essential/universal/UGraphics.java b/src/main/java/gg/essential/universal/UGraphics.java
@@ -749,7 +749,7 @@ public enum CommonVertexFormats {
         POSITION_TEXTURE_COLOR_NORMAL(DefaultVertexFormats.POSITION_TEX_COLOR_NORMAL),
         ;
 
-        private final VertexFormat mc;
+        public final VertexFormat mc;
 
         CommonVertexFormats(VertexFormat mc) {
             this.mc = mc;

diff --git a/src/main/kotlin/gg/essential/universal/shader/UShader.kt b/src/main/kotlin/gg/essential/universal/shader/UShader.kt
@@ -1,5 +1,7 @@
 package gg.essential.universal.shader
 
+import gg.essential.universal.UGraphics
+
 //#if MC>=11700
 //$$ import net.minecraft.client.render.Shader
 //#endif
@@ -27,9 +29,21 @@ interface UShader {
     fun getSamplerUniform(name: String): SamplerUniform = getSamplerUniformOrNull(name) ?: throw NoSuchElementException(name)
 
     companion object {
+        @Deprecated(
+            "Use the overload which takes a vertex format to ensure proper operation on all versions.",
+            replaceWith = ReplaceWith("UShader.fromLegacyShader(vertSource, fragSource, blendState, vertexFormat)")
+        )
         fun fromLegacyShader(vertSource: String, fragSource: String, blendState: BlendState): UShader {
             //#if MC>=11700
-            //$$ return MCShader.fromLegacyShader(vertSource, fragSource, blendState)
+            //$$ return MCShader.fromLegacyShader(vertSource, fragSource, blendState, null)
+            //#else
+            return GlShader(vertSource, fragSource, blendState)
+            //#endif
+        }
+
+        fun fromLegacyShader(vertSource: String, fragSource: String, blendState: BlendState, vertexFormat: UGraphics.CommonVertexFormats): UShader {
+            //#if MC>=11700
+            //$$ return MCShader.fromLegacyShader(vertSource, fragSource, blendState, vertexFormat)
             //#else
             return GlShader(vertSource, fragSource, blendState)
             //#endif

diff --git a/versions/1.17.1-fabric/src/main/kotlin/gg/essential/universal/shader/MCShader.kt b/versions/1.17.1-fabric/src/main/kotlin/gg/essential/universal/shader/MCShader.kt
@@ -3,6 +3,7 @@ package gg.essential.universal.shader
 
 import com.google.common.collect.ImmutableMap
 import gg.essential.universal.UGraphics
+import gg.essential.universal.UGraphics.CommonVertexFormats
 import net.minecraft.client.gl.GlUniform
 import net.minecraft.client.render.Shader
 import net.minecraft.client.render.VertexFormat
@@ -56,8 +57,8 @@ internal class MCShader(
     companion object {
         private val DEBUG_LEGACY = System.getProperty("universalcraft.shader.legacy.debug", "") == "true"
 
-        fun fromLegacyShader(vertSource: String, fragSource: String, blendState: BlendState): MCShader {
-            val transformer = ShaderTransformer()
+        fun fromLegacyShader(vertSource: String, fragSource: String, blendState: BlendState, vertexFormat: CommonVertexFormats?): MCShader {
+            val transformer = ShaderTransformer(vertexFormat)
 
             val transformedVertSource = transformer.transform(vertSource)
             val transformedFragSource = transformer.transform(fragSource)
@@ -107,11 +108,12 @@ internal class MCShader(
                 //#endif
             }
 
-            // The actual element doesn't matter here, Shader only cares about the names
-            val vertexFormat = VertexFormat(ImmutableMap.copyOf(transformer.attributes.associateWith { VertexFormats.POSITION_ELEMENT }))
+            val shaderVertexFormat = vertexFormat?.mc
+                // Legacy fallback: The actual element doesn't matter here, Shader only cares about the names
+                ?: VertexFormat(ImmutableMap.copyOf(transformer.attributes.associateWith { VertexFormats.POSITION_ELEMENT }))
 
             val name = DigestUtils.sha1Hex(json).lowercase()
-            return MCShader(Shader(factory, name, vertexFormat), blendState)
+            return MCShader(Shader(factory, name, shaderVertexFormat), blendState)
         }
     }
 }
@@ -141,8 +143,8 @@ internal class MCSamplerUniform(val mc: Shader, val name: String) : SamplerUnifo
     }
 }
 
-internal class ShaderTransformer {
-    val attributes = mutableSetOf<String>()
+internal class ShaderTransformer(private val vertexFormat: CommonVertexFormats?) {
+    val attributes = mutableListOf<String>()
     val samplers = mutableSetOf<String>()
     val uniforms = mutableMapOf<String, UniformType>()
 
@@ -173,21 +175,32 @@ internal class ShaderTransformer {
             replacements["gl_Color"] = "uc_FrontColor"
         }
 
-        fun replaceAttribute(needle: String, type: String, replacementName: String = "uc_" + needle.substringAfter("_"), replacement: String = replacementName) {
+        fun replaceAttribute(newAttributes: MutableList<Pair<String, String>>, needle: String, type: String, replacementName: String = "uc_" + needle.substringAfter("_"), replacement: String = replacementName) {
             if (needle in source) {
                 replacements[needle] = replacement
-                if (replacementName !in attributes) {
-                    attributes.add(replacementName)
-                    transformed.add("in $type $replacementName;")
-                }
+                newAttributes.add(replacementName to "in $type $replacementName;")
             }
         }
         if (vert) {
-            replaceAttribute("gl_Vertex", "vec3", replacement = "vec4(uc_Vertex, 1.0)")
-            replaceAttribute("gl_Color", "vec4")
-            replaceAttribute("gl_MultiTexCoord0.st", "vec2", "uc_UV0")
-            replaceAttribute("gl_MultiTexCoord1.st", "vec2", "uc_UV1")
-            replaceAttribute("gl_MultiTexCoord2.st", "vec2", "uc_UV2")
+            val newAttributes = mutableListOf<Pair<String, String>>()
+            replaceAttribute(newAttributes, "gl_Vertex", "vec3", "uc_Position", replacement = "vec4(uc_Position, 1.0)")
+            replaceAttribute(newAttributes, "gl_Color", "vec4")
+            replaceAttribute(newAttributes, "gl_MultiTexCoord0.st", "vec2", "uc_UV0")
+            replaceAttribute(newAttributes, "gl_MultiTexCoord1.st", "vec2", "uc_UV1")
+            replaceAttribute(newAttributes, "gl_MultiTexCoord2.st", "vec2", "uc_UV2")
+
+            if (vertexFormat != null) {
+                newAttributes.sortedBy { vertexFormat.mc.shaderAttributes.indexOf(it.first.removePrefix("uc_")) }
+                    .forEach {
+                        attributes.add(it.first)
+                        transformed.add(it.second)
+                    }
+            } else {
+                newAttributes.forEach {
+                    attributes.add(it.first)
+                    transformed.add(it.second)
+                }
+            }
         }
 
         fun replaceUniform(needle: String, type: UniformType, replacementName: String, replacement: String = replacementName) {