[Patch] Textures

[Ian]

Could you try just stubbing out the texture upload code ?

Just stick a return after

void CReal3D::UploadTexture(uint32_t header, const uint16_t *texData)
{
return;
}

That will rule out any changes we made. I think the code is sound, but just good to rule it out.

[Spindizzi]

Erf, bad news
It doesn't crash when code is removed from uploadtexture

edit :
if I remove only case type==2, no crash
bad values of xpos and ypos ? (resulting of bad values Inside tables mipxbase ,mipybase, mipdivisor ?)

[Ian]

Can you get it to crash in debug mode and just get the debugger to see where the problem is?

[Bart]

Spindizzi:

If you're building with gcc, edit the Makefile and remove -O3 and also -s (if that exists) and add -g to the compiler and linker options.
"make clean" and then "make" again to recompile everything, then run it with gdb (which you should have in your path already if you can run gcc):

Code: Select all

gdb --args supermodel roms\daytona2.zip


When gdb starts, type 'r' and press enter to start running it. When it crashes, use the 'bt' command to print a stack trace.

You may already know all this but just in case...

[Ian]

One for harry .. since I don't have a great deal of understanding in exactly how this code works.

Code: Select all

            if (tileX == 1) {
              byte1 = texData[decode8x1[(yy^1) * tileX + ((xx + 0)^1)] / 2] >> 8;
              byte2 = texData[decode8x1[(yy^1) * tileX + ((xx + 0)^1)] / 2] & 0xFF;
            }

the exclusive or, (i guessed it was for endian swap?)

If you are reading say a 8x1 mipmap, and you read reading the last row, ie 7

[(yy^1) * tileX + ((xx + 0)^1)]
eg
The last part
if xx = 0
(0 + 0) ^ 1 = 1

7+1 = 8
That means we are array out of bounds? Unless I missed something here

I am guessing the code maybe should be something like this?

Code: Select all

            if (tileX == 1) {
              byte1 = texData[decode8x1[(yy^1)] / 2] >> 8;
              byte2 = texData[decode8x1[(yy)] / 2] & 0xFF;
            }

Or maybe swapped around. Not sure

[Spindizzi]

Bart, thanks for the advice, I do not know about that. However, I think it's a vc2017 compiler related trouble only, not gcc
Ian, you've found the right spot, it crashed here

[HarryTuttle]

One for harry .. since I don't have a great deal of understanding in exactly how this code works.

Thanks Ian, gonna double check again that case.

Yes, it's an endian swap. The XOR was used in the original code for both 8 and 16 bit textures. But when I swapped the numbers directly in the decode array (8x8) I removed the XOR for 16 bit textures.

However, for some reason, I had to keep it for 8 bit textures. So I tried to just replicate the same logic used for 16 bit ones while being cautious for out of bound array index. Evidently something slipped off (these "distraction" errors keep on haunting me)...

I am guessing the code maybe should be something like this?

Yeah that should be correct code wise. Or if you want to be verbose and maintain indentation:

Code: Select all

              byte1 = texData[decode8x1[(yy^1) * tileX + ((xx + 1)^1)] / 2] >> 8;
              byte2 = texData[decode8x1[(yy^1) * tileX + ((xx + 1)^1)] / 2] & 0xFF;


I'm going to check the visual results and keep you informed.

P.S.
Hey Ian, what's that AMD program you used to check the single texture mips?

[HarryTuttle]

I've to add that it's not the first time Supermodel compiled with gcc doesn't crash for out of bounds array indices. The last case was an off-by-one-error that should give, as I've read, unexpected results.

Someone here suggested to compile with these arguments: -fsanitize=address -ggdb, explanation here.

I've tried that but when linking it throws out an insane amount of errors relating to m68kmake.c, PPCDisasm.cpp, Z80Debug.cpp, etc... plus some other MSYS2 includes...

I don't want to even remotely mess with those things.

[Ian]

Probably

Code: Select all

              byte1 = texData[decode8x1[(yy^1)] / 2] >> 8;
              byte2 = texData[decode8x1[(yy^1)] / 2] & 0xFF;

would make more sense? Since the x component does nothing anyway.

I use AMD code xl. Setup something like this. Just hit the green button for run. Then hit pause, and it'll let you inspect the textures. You can also make it break on opengl errors and it'll show you directly in your source which line caused it. Pretty handy tool.

https://i.imgur.com/pejVqVY.png
https://i.imgur.com/WHOIjDz.png

[HarryTuttle]

Probably

byte1 = texData[decode8x1[(yy^1)] / 2] >> 8;
byte2 = texData[decode8x1[(yy^1)] / 2] & 0xFF;

would make more sense? Since the x component does nothing anyway.

Yeah, that one