icculus / mojoshader

Use Direct3D shaders with other 3D rendering APIs.
https://icculus.org/mojoshader/
zlib License
122 stars 36 forks source link

Fix secondary outputs in MRT shaders being discarded #36

Closed kg closed 3 years ago

kg commented 3 years ago

For some reason mojoshader was setting the location of all attributes other than COLOR0 to garbage, so the shader's other outputs were being discarded and the secondary RTs were always black.

flibitijibibo commented 3 years ago

It's set to this because the linker is meant to handle this instead:

https://github.com/FNA-XNA/MojoShader/blob/upstream/mojoshader_common.c#L1061

The issue is that the vertex shader needs to be checked for the COLOR1-3 attribs, not just the pixel shader.

flibitijibibo commented 3 years ago

Try this diff instead:

diff --git a/mojoshader_common.c b/mojoshader_common.c
index 1f2566f..f5dd24d 100644
--- a/mojoshader_common.c
+++ b/mojoshader_common.c
@@ -1071,16 +1071,16 @@ void MOJOSHADER_spirv_link_attributes(const MOJOSHADER_parseData *vertex,
     const uint32 texcoord0Loc = pTable->attrib_offsets[MOJOSHADER_USAGE_TEXCOORD][0];

     // We need locations for color outputs first!
-    for (i = 0; i < pixel->output_count; i ++)
+    for (i = 1; i < 4; i ++)
     {
-        const MOJOSHADER_attribute *pAttr = &pixel->outputs[i];
-        assert(pAttr->usage == MOJOSHADER_USAGE_COLOR);
-        if (pAttr->index == 0)
-            continue;
-
-        pOffset = pTable->attrib_offsets[pAttr->usage][pAttr->index];
-        ((uint32 *) pixel->output)[pOffset] = attr_loc;
-        attr_loc++;
+        pOffset = pTable->attrib_offsets[MOJOSHADER_USAGE_COLOR][i];
+        vOffset = vTable->attrib_offsets[MOJOSHADER_USAGE_COLOR][i];
+        if (pOffset)
+            ((uint32 *) pixel->output)[pOffset] = attr_loc;
+        if (vOffset)
+            ((uint32 *) vertex->output)[vOffset] = attr_loc;
+        if (pOffset || vOffset)
+            attr_loc++;
     } // for

     // Okay, now we can start linking pixel/vertex attributes
@@ -1089,7 +1089,7 @@ void MOJOSHADER_spirv_link_attributes(const MOJOSHADER_parseData *vertex,
         const MOJOSHADER_attribute *pAttr = &pixel->attributes[i];
         if (pAttr->usage == MOJOSHADER_USAGE_UNKNOWN)
             continue; // Probably something like VPOS, ignore!
-        if (pAttr->usage == MOJOSHADER_USAGE_COLOR && pAttr->index == 0)
+        if (pAttr->usage == MOJOSHADER_USAGE_COLOR && pAttr->index < 4)
             continue;

         // The input may not exist in the output list!
@@ -1110,7 +1110,7 @@ void MOJOSHADER_spirv_link_attributes(const MOJOSHADER_parseData *vertex,
             continue;
         if (vAttr->usage == MOJOSHADER_USAGE_POINTSIZE && vAttr->index == 0)
             continue;
-        if (vAttr->usage == MOJOSHADER_USAGE_COLOR && vAttr->index == 0)
+        if (vAttr->usage == MOJOSHADER_USAGE_COLOR && vAttr->index < 4)
             continue;

         if (!pTable->attrib_offsets[vAttr->usage][vAttr->index])
flibitijibibo commented 3 years ago

Reworked the linker to correctly prioritize pixel shader output locations, while also separating them from inputs in a way that doesn't cause clashes between non-output COLORn attributes and also allows for a variable max for color outputs:

https://github.com/FNA-XNA/MojoShader/commit/a11f5a477206c2815f5891f97527dd9bf70cbcfd

This should fix MRT without breaking the linker.