mupen64plus-video-glide64mk2 icon indicating copy to clipboard operation
mupen64plus-video-glide64mk2 copied to clipboard

Bug in TxImage.cpp (TxImage::readPNG(...))

Open JayTee42 opened this issue 10 years ago • 4 comments

Hi guys, thanks for your great work - keep it up.

I'd like to point out a little bug: When trying to run Super Smash Bros. N64 with an additional texture pack, glide64mk2 failed to load the provided PNG files. I did some debugging research: The problem is located inside GlideHQ/TxImage.cpp (uint8* TxImage::readPNG(FILE* fp, int* width, int* height, uint16* format)): The vars o_width and o_height should be png_uint_32 instead of int (allowing you to omit the cast). After recompiling everything worked well for me.

Merry Christmas! J.T.

JayTee42 avatar Dec 24 '14 12:12 JayTee42

I don't see how changing these variables from int to uint could fix anything. Perhaps you were originally using an older build which had a bug, and by downloading the latest code and rebuilding from source, you built a newer version without the bug?

richard42 avatar Jan 01 '15 17:01 richard42

I downloaded the latest bins and tried to use them for loading the textures, but they wouldn't load (more precisely, the BMP files did actually load, but the PNGs didn't), so I got the source and started debugging.

For all PNGs that were loaded, I realized png_ptr to become null after calling png_get_IHDR (line 102), so I checked the API (http://refspecs.linuxbase.org/LSB_4.0.0/LSB-Desktop-generic/LSB-Desktop-generic/libpng12.png.get.ihdr.1.html) just to see they are using png_uint_32.

After changing, png_get_IHDR works like a charm (just tried to reproduce it, still behaving that way). It might be related to sizeof(int) on different systems ... ?

JayTee42 avatar Jan 01 '15 19:01 JayTee42

Just checked it out, sizeof(int) is 4 on my system, but png_uint_32 is typedefed as

typedef unsigned long int png_uint_32; //pngconf.h

and sizeof(unsigned long int) prints 8. So you are passing pointers to 4-byte-variables that are assumed to provide 8 bytes of space. Hope this makes it clear :-)

JayTee42 avatar Jan 01 '15 20:01 JayTee42

Thanks very much for tracking this down; I'll make a patch to fix it. I never really liked every project having it's own typedefs, and this shows why. It's pretty surprising that a png_uint_32 would be 64 bits long.

richard42 avatar Jan 02 '15 00:01 richard42