-
Notifications
You must be signed in to change notification settings - Fork 496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
64 bit big endian issues #426
Comments
This is interesting. People have gotten this working just fine on 32-bit powerpc platforms, so no one noticed this until now. You should probably also create an issue or PR in the original sm64-port repo as well. |
Perhaps related (at least, people following that issue will want to know about this): #334 |
It also appears the project this is forked from doesn't have issue tracking, only PRs. While I'd like to submit this PR directly with some justification, it may be nice to have a little bit of dialogue with other developers to see if this behavior has any real drawbacks (I'm pretty sure the struct allocation is going to be a 64 bit type anyway, so I don't think this is inflating memory usage). |
Fortunately that type is most likely only used in goddard (the mario head intro part), so it probably doesn't affect the rest of the game. The much more widely used |
The good news: I've isolated the byte swapped texture colors to be strictly a nouveau endianness bug (printed the RGBA values from little and big endian machines from host memory - they are the same). So this may be the only big endian bug present. That having been said - a lot of the code assumes little endian and manually swaps bytes from buffers in big endian order (and recombines them via bit shifts and logical &s). I do wonder how much the compiler is successfully ignoring this and how much of the code could benefit from a manual macro to not do this. Nouveau bug in question: |
Describe the bug
On 64 bit platforms (compiling with GCC), there seems to be an aliasing issue within a union for the displaylist struct. The union in question is DynUnion. 2/3 of the union members are 64 bit types on 64 bit platforms (all pointers), but the third is a 32 bit signed integer. Somewhere, the compiler's optimizer or perhaps the code itself is taking a liberty with this size, only copying the first half of the word into the address. On little endian architectures, this is not a big deal, on big endian it means that you get the wrong half of the integer and when processing the display list, instead of getting a parameter for the alpha value, you get an invalid enum (0). This is a show stopping bug, as the game won't start, and gd_printf fails at this point as the entire GL context fails to draw the framebuffer after this (you only get to the title screen before the goddard intro). I had to patch the gd_printf() function to tell me where the code was actually hanging up by doing an addition vprintf() with the va_list.
Fixing this bug was just a matter of promoting the type in that union to a signed 64 bit integer.
Additionally I had to patch the makefile to not sure -march, as for some reason powerpc GCC still varies for these flags (probably dates back to Apple's contributions, but that's not a difficult thing to address).
To Reproduce
Steps to reproduce the behavior:
1.) Compile on powerpc64 (big endian) linux
2.) Launch game
Expected behavior
The game to launch
Desktop (please complete the following information):
Additional context
There are still some endianness bugs hiding somewhere in this code. Some of this is due to endianness bugs directly in the nouveau stack (there's a byteswapping texture bug still there that I haven't found the root cause of yet). But, unlike with other games, GL_UNPACK_SWAP_BYTES=1 does not invert the issue, indicating that there's likely some other endianness bugs hiding in gfx_pc.c with regard to the type interpretation (somewhat funny, as the PNG textures are natively big endian, as were most of the assets originally in the game).
Here's the patch to make the game actually launch on powerpc64 linux:
The text was updated successfully, but these errors were encountered: