Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#69 closed defect (fixed)

Loading .ICO wrong image on Big endian Mac

Reported by: Thomas Bernard Owned by: Thomas Bernard
Priority: minor Milestone: 2.5
Component: GrafX2 Version: 2.5
Keywords: ico Cc:

Description

Everything works well on My PC, but the colors are wrong if I load a .ICO file on my PowerPC Mac.
I suspect a bug in loading little endian stuff on a big endian machine

Attachments (2)

grafX2_load_ico_Bigendian.png (12.0 KB ) - added by Thomas Bernard 6 years ago.
gfx2.ico loaded on a big endian machine
IMG_Load_bug_PPC.png (3.8 KB ) - added by Thomas Bernard 6 years ago.
sdl_image_test result on a PowerPC Mac OS X 10.4.11

Download all attachments as: .zip

Change History (12)

by Thomas Bernard, 6 years ago

gfx2.ico loaded on a big endian machine

comment:1 by Thomas Bernard, 6 years ago

Owner: changed from pulkomandy to Thomas Bernard
Status: newaccepted
Version: 2.42.5WIP

comment:2 by Thomas Bernard, 6 years ago

in fact it is loaded via Load_SDL_Image() so SDL IMG_Load()

comment:3 by PulkoMandy, 6 years ago

When I ported GrafX2 from DOS I was using a PowerPC machine, so back then I found and fixed a lot of these problems. But I have not tested this in a while and I currently don't have a machine where I can easily test it (maybe an Amiga 4000 or an old PPC based X terminal which can run NetBSD...)

Anyways, it seems that Get_SDL_pixel_hicolor is doing strange things. It uses an hardcoded SDL_LIL_ENDIAN test for 24bit data and nothing for 32bit. Isn't there a better way to do this depending on the SDL_Surface format? Especially as we are shifting things again after using that function.

Either Get_SDL_pixel_hicolor should return a T_Components, or it should return an uint32_t without any bitswapping, and we should use the SDL_Surface format to decode that. But not both.

comment:4 by Thomas Bernard, 6 years ago

well it looks like a bug in SDL code to load ICO because loading another format with SDL works (color TIFF file)

comment:5 by Thomas Bernard, 6 years ago

Summary: Loading .ICO wrong image on Big endian machineLoading .ICO wrong image on Big endian Mac

I think the problem lies in SDL_Image,
ICO files are read with system loader, but probably wrongly converted...
http://hg.libsdl.org/SDL_image/file/0c88c81b59c9/IMG_UIImage.m

comment:6 by PulkoMandy, 6 years ago

Get_SDL_pixel_hicolor does different things for 24-bit and 32-bit images and ignores the pixel format given by SDL_Image (in the SDL_Surface "format" field). We should check this code very carefully to make sure there isn't a problem (the fact that Get_SDL_pixel_hicolor does endian-swap for 24 but not 32bit images makes no sense, so we very likely have a problem here). When our code is perfectly clean we can go up to SDL_Image.

comment:7 by Thomas Bernard, 6 years ago

Well I made a minimal test tool for IMG_Load()
https://gitlab.com/miniupnp/grafX2/tree/sdl_image_test
and it wrongly shows the .ICO file, so I'm sure there is a bug in SDL Image on Mac

by Thomas Bernard, 6 years ago

Attachment: IMG_Load_bug_PPC.png added

sdl_image_test result on a PowerPC Mac OS X 10.4.11

comment:8 by Thomas Bernard, 6 years ago

I reorganized a bit the Load_BMP() and Load_PNG() code and added ICO image loading :
https://gitlab.com/miniupnp/grafX2/commits/bmp_ico

comment:9 by Thomas Bernard, 6 years ago

Resolution: fixed
Status: acceptedclosed

comment:10 by Thomas Bernard, 6 years ago

Fixed by :

  • implementing ICO loading instead of using SDL bugged (on OS X) code.
  • fixing the BMP code on big endian machines
Note: See TracTickets for help on using tickets.