#8916 closed patch
Enhance OSystem_SDL::setupIcon
| Reported by: | SF/canavan | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
OSystem_SDL::setupIcon imposes unneccesary size limitations on the icon to be installed. Furthermore, it computes a mask for the icon, although SDL_WM_SetIcon will do that automatically when mask==NULL and and the RGBSurface has an alpha channel.
Ticket imported from: #2072006. Ticket imported from: patches/1021.
Attachments (1)
Change History (12)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Hrm, I wasn't able to attach files to another tracker item, too, and so were several other people. See this support request I posted yesterday: <https://sourceforge.net/tracker/?func=detail&aid=2070259&group_id=1&atid=200001>. canavan, if you got the same error, you may want to chime up on that SR.
comment:3 by , 16 years ago
| Priority: | normal → high |
|---|
comment:4 by , 16 years ago
--- sdl.cpp.orig Fri Aug 22 21:43:29 CEST 2008 +++ sdl.cpp Sun Aug 24 17:22:20 CEST 2008 @@ -276,15 +276,21 @@ }
void OSystem_SDL::setupIcon() { - int w, h, ncols, nbytes, i; - unsigned int rgba[256], icon[32 * 32]; - unsigned char mask[32][4]; + int x, y, w, h, ncols, nbytes, i; + unsigned int rgba[256]; + unsigned int *icon;
sscanf(scummvm_icon[0], "%d %d %d %d", &w, &h, &ncols, &nbytes); - if ((w != 32) || (h != 32) || (ncols > 255) || (nbytes > 1)) { + if ((w > 512) || (h > 512) || (ncols > 255) || (nbytes > 1)) { warning("Could not load the icon (%d %d %d %d)", w, h, ncols, nbytes); return; } + icon=(unsigned int*)malloc(w*h*sizeof(unsigned int)); + if (icon==NULL) { + warning("Could not allocate the icon"); + return; + } + for (i = 0; i < ncols; i++) { unsigned char code; char color[32]; @@ -304,20 +310,20 @@
rgba[code] = col; } - memset(mask, 0, sizeof(mask)); - for (h = 0; h < 32; h++) { - const char *line = scummvm_icon[1 + ncols + h]; - for (w = 0; w < 32; w++) { - icon[w + 32 * h] = rgba[(int)line[w]]; - if (rgba[(int)line[w]] & 0xFF000000) { - mask[h][w >> 3] |= 1 << (7 - (w & 0x07)); - } + for (y = 0; y < h; y++) { + const char *line = scummvm_icon[1 + ncols + y]; + for (x = 0; x < w; x++) { + icon[x + w * y] = rgba[(int)line[x]]; } }
- SDL_Surface *sdl_surf = SDL_CreateRGBSurfaceFrom(icon, 32, 32, 32, 32 * 4, 0xFF0000, 0x00FF00, 0x0000FF, 0xFF000000); - SDL_WM_SetIcon(sdl_surf, (unsigned char *) mask); + SDL_Surface *sdl_surf = SDL_CreateRGBSurfaceFrom(icon, w, h, 32, w * 4, 0xFF0000, 0x00FF00, 0x0000FF, 0xFF000000); + if (NULL==(sdl_surf)) { + warning("SDL_CreateRGBSurfaceFrom(icon) failed"); + } + SDL_WM_SetIcon(sdl_surf, NULL); SDL_FreeSurface(sdl_surf); + free(icon); }
OSystem::MutexRef OSystem_SDL::createMutex(void) {
comment:5 by , 16 years ago
| Priority: | high → normal |
|---|
comment:6 by , 16 years ago
Since uploads don't currently work and cut-and-paste apparently mangles tabs, I've put a clean version of the patch on http://scummvm.canavan.de/SDLsetupIcon.patch
comment:7 by , 16 years ago
Attachment problems should be fixed now.
So, back to my questions: While in theory it's nice to support icons bigger than 32x32, in reality our built-in icon has a fixed size of 32x32, so why should we add a malloc to support bigger icons? Should we switch to a bigger icon, we could just increase the temp buffer in setupIcon to match, no?
Dropping the "mask" bit seems like a nice move, though.
comment:8 by , 16 years ago
While it should be trivial to update the constants scattered about setupIcon() in the unliekely event that the official scummvm icon should ever be changed to some other size, this artificial restriction makes it unneccesarily difficult to change to another icon format on platforms where 32x32 just doesn't look good enough.
I actually started inspecting/modifying setupIcon() due to a bug in SDL_WM_SetIcon(), but the added flexibility allows me to provide a suitable icon for IRIX's default window manager 4DWm, which has a constant icon size of 85x67. File Added: SDLsetupIcon.patch
comment:9 by , 16 years ago
OK, fair enough. I commited a slightly cleaned up version of this patch.
comment:10 by , 16 years ago
| Owner: | set to |
|---|---|
| Status: | new → closed |
| Summary: | simplify OSystem_SDL::setupIcon → Enhance OSystem_SDL::setupIcon |
comment:11 by , 6 years ago
| Component: | → --Other-- |
|---|
Was a patch supposed to be attached here?