icculus / mojoshader

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

PowerPC support seems broken #73

Closed Fancy2209 closed 1 week ago

Fancy2209 commented 2 weeks ago

I originally noticed this by trying to use FNA on ArchPOWER Wii-Linux. It seems for a very long time now the PowerPC support for MojoShader has been broken (I tested the last commit made in 2016 and it was already broken by then). It seems MojoShader fails to parse Effects on Big Endian Platforms, throws ERROR: Unsupported shader type or not a shader at all on a valid shader. I tested with SpriteEffect.xfb and SpriteEffect.xfb from FNA with the testparse util. I was trying to fix this issue but no longer have any lead on why it's failing, current issue is that it's trying to parse the magic as the version token for some reason. Here's a patch file with what I could fix, (gcc only defines powerpc lowercase and testparse doesn't check the right magic on powerpc).

diff --git a/mojoshader_internal.h b/mojoshader_internal.h
index 711080b..070fd63 100644
--- a/mojoshader_internal.h
+++ b/mojoshader_internal.h
@@ -296,7 +296,7 @@ typedef uint64_t uint64;

 // Byteswap magic...

-#if ((defined __GNUC__) && (defined __POWERPC__))
+#if ((defined __GNUC__) && (defined(__POWERPC__) || defined(__powerpc__)))
     static inline uint32 SWAP32(uint32 x)
     {
         __asm__ __volatile__("lwbrx %0,0,%1" : "=r" (x) : "r" (&x));
@@ -307,7 +307,7 @@ typedef uint64_t uint64;
         __asm__ __volatile__("lhbrx %0,0,%1" : "=r" (x) : "r" (&x));
         return x;
     } // SWAP16
-#elif defined(__POWERPC__)
+#elif (defined(__POWERPC__) || defined(__powerpc__))
     static inline uint32 SWAP32(uint32 x)
     {
         return ( (((x) >> 24) & 0x000000FF) | (((x) >>  8) & 0x0000FF00) |
diff --git a/utils/testparse.c b/utils/testparse.c
index c739aa9..a4ae8d8 100644
--- a/utils/testparse.c
+++ b/utils/testparse.c
@@ -769,10 +769,17 @@ static int do_parse(const char *fname, const unsigned char *buf,
     int retval = 0;

     // magic for an effects file (!!! FIXME: I _think_).
+    #if (defined(__POWERPC__) || defined(__powerpc__))
+    if ( ((buf[0] == 0x10) && (buf[1] == 0x90) &&
+          (buf[2] == 0xFF) && (buf[3] == 0xEF)) ||
+         ((buf[0] == 0xFC) && (buf[1] == 0xB0) &&
+          (buf[2] == 0x0F) && (buf[3] == 0xCB)) )
+    #else
     if ( ((buf[0] == 0x01) && (buf[1] == 0x09) &&
           (buf[2] == 0xFF) && (buf[3] == 0xFE)) ||
          ((buf[0] == 0xCF) && (buf[1] == 0x0B) &&
           (buf[2] == 0xF0) && (buf[3] == 0xBC)) )
+    #endif
     {
 #ifdef MOJOSHADER_EFFECT_SUPPORT
         const MOJOSHADER_effect *effect;
flibitijibibo commented 1 week ago

Took a quick look and it actually looks like all the reading is done with read_ui32 which does the swap... so I'm not sure what's up.

A quick test (we could maybe add this to MojoShader CI?):

git clone https://github.com/icculus/mojoshader
mkdir mojoshader/flibitBuild
cd mojoshader/flibitBuild
cmake ..
make
curl -LO https://github.com/FNA-XNA/FNA/raw/refs/heads/master/src/Graphics/Effect/StockEffects/FXB/SpriteEffect.fxb
./testparse glsl120 SpriteEffect.fxb
flibitijibibo commented 1 week ago

Pushed a couple fixes to the header and string readers. My guess is the magic number checks in effects.c still need swapping but otherwise reading should work better now (maybe even entirely?).

Fancy2209 commented 1 week ago

Fixed by #76