Ignore unknown opcode for 0x3f

[Pokechu22]

Fixes the unknown opcode with Prince of Persia: Rival Swords (https://bugs.dolphin-emu.org/issues/9203).

This bug occurs because the game has some sort of caching layer to reduce the number of calls to GX functions. For instance, calls to their function to configure vertex descriptor (at 8010d558) compare with a cached value before calling GXSetVtxDesc (at 80499164). I'm not sure whether this actually has any tangible benefit, as those functions don't directly send graphics commands but instead update other internal state, but still, it's something they do.

They have a function that draws a black rectangle over the entire screen (at 80111b40 via 800ae644), which among other things configures the vertex descriptor to have a position and a color, and then draws the rectangle:

80 0004 // Primitive GX_DRAW_QUADS (0) VAT 0, 4 vertices 16 bytes/vertex 64 total bytes
x            y            z             color
00000000 (0) 00000000 (0) 00000000 (0)  00000000 // vertex 1
00000000 (0) 3f800000 (1) 00000000 (0)  00000000 // vertex 2
3f800000 (1) 3f800000 (1) 00000000 (0)  00000000 // vertex 3
3f800000 (1) 00000000 (0) 00000000 (0)  00000000 // vertex 4

However, when using Bink Video, Bink's drawing function (at 801e088c) does not use their caching system and instead directly calls GXSetVtxDesc. Thus, their cache doesn't match reality, so when it comes time to draw the black rectangle, the cache function thinks that the vertex descriptor is already configured correctly and thus never calls GXSetVtxDesc. The vertex descriptor that ends up being used has a position only, without any color data; the same data is thus interpreted this way instead:

80 0004 // Primitive GX_DRAW_QUADS (0) VAT 0, 4 vertices 12 bytes/vertex 48 total bytes
x            y            z
00000000 (0) 00000000 (0) 00000000 (0) // vertex 1
00000000 (0) 00000000 (0) 3f800000 (1) // vertex 2
00000000 (0) 00000000 (0) 3f800000 (1) // vertex 3
3f800000 (1) 00000000 (0) 00000000 (0) // vertex 4
3f // Unknown opcode
80 0000 // Primitive GX_DRAW_QUADS (0) Vat 0, 0 vertices 12 bytes/vertex 0 total bytes
00000000 00000000 00000000 // NOP (12x)

Things work out on later frames as the cache state and the actual GX state ends up in sync (though I'm not entirely sure what ends up making them consistent).

Using the hardware fifoplayer, I was able to confirm that the 3f was being read but ignored, by replacing it with 0x18 and observing that that produced a hang.

---

It does seem like the developers were aware that this kind of issue could happen, because the Bink drawing code is modified (the Bink changelog mentions, more or less, that the relevant code is in wiitextures.c which is distributed in source form to developers, and I've seen other games that modify it too). But rather than changing the Bink code to use their caching functions, they instead did a weird halfhearted workaround:

u32 old_pos, old_color_0, old_tex_0, old_tex_1;
// 801e09b8
GXClearVtxDesc();
// -snip-
// 801e0a18
GXGetVtxDesc(9 /* GX_VA_POS */, &old_pos);
GXGetVtxDesc(0xb /* GX_VA_CLR0 */, &old_color_0);
GXGetVtxDesc(0xd /* GX_VA_TEX0 */, &old_tex_0);
GXGetVtxDesc(0xe /* GX_VA_TEX1 */, &old_tex_1);
GXSetVtxDesc(9 /* GX_VA_POS */, 1 /* GX_DIRECT */);
GXSetVtxDesc(0xb /* GX_VA_CLR0 */, 1 /* GX_DIRECT */);
GXSetVtxDesc(0xd /* GX_VA_TEX0 */, 1 /* GX_DIRECT */);
GXSetVtxDesc(0xe /* GX_VA_TEX1 */, 1 /* GX_DIRECT */);
// -snip-, skipping actual drawing code and going to the end of the function
// 801e1368                        
GXSetVtxDesc(9 /* GX_VA_POS */, old_pos);
GXSetVtxDesc(0xb /* GX_VA_CLR0 */, old_color_0);
GXSetVtxDesc(0xd /* GX_VA_TEX0 */, old_tex_0);
GXSetVtxDesc(0xe /* GX_VA_TEX1 */, old_tex_1);

But, this doesn't work, as GXClearVtxDesc clears the vertex descriptor, so GXGetVtxDesc just returns the default values, and thus the ending state is different from the starting state.

I attempted to make a game patch that moves the GXClearVtxDesc call after the GXGetVtxDesc calls. This patch does solve the unknown opcode issue (at least in the situation I tested), but there are a lot of other issues because other things are also out of sync (later versions of Bink reset the state afterwards, but this version doesn't completely do it, resulting in a red flash on the frame where the unknown opcode would previously appear; here's a fifolog of the patched version). This patch isn't included in the PR.

$HackyFix
0x801E09B8:dword:0x60000000
0x801E0A48:dword:0x4BE23944
0x8000438C:dword:0x48495669
0x80004390:dword:0x38600009
0x80004394:dword:0x481DC6B8

For comparison purposes, I also looked at other games: Raving Rabbids (at 803783c8), the Activision Demo Pack (80104d80 for DemoLauncher.elf), and Shrek the Third (8021e1e8); none of them have this kind of call to GXSetVtxDesc, though the Activision Demo Pack does have the Bink code changed to use their own state-tracking cache code.

I think the home-menu code also doesn't use their wrapper, so even changing Bink code for Rival Swords wouldn't fix the issue entirely, though I don't know for sure what would be broken.

---

I also did some refactoring to allow changing GATHER_PIPE_SIZE, in an attempt at making unknown opcode errors easier to debug; it's not perfect, but it helps. Those changes are only refactoring, and should not produce any different behaviors.

[JMC47]

I did a quick glance at the code, and it seems more of the differences are related to removing the magic number. Everything seems to work and it's got an approval.