We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 208 - SDL_WM_SetIcon bug in the universal Mac OS X build of the SDL framework
Summary: SDL_WM_SetIcon bug in the universal Mac OS X build of the SDL framework
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 1.2.9
Hardware: PowerPC Mac OS X 10.4 (Intel)
: P2 blocker
Assignee: Christian Walther
QA Contact: Sam Lantinga
URL:
Keywords:
: 210 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-30 15:46 UTC by Ryan Clark
Modified: 2006-05-11 00:07 UTC (History)
2 users (show)

See Also:


Attachments
patch (4.83 KB, patch)
2006-05-10 18:48 UTC, Christian Walther
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Clark 2006-04-30 15:46:12 UTC
When calling SDL_WM_SetIcon on an Intel Mac using the universal binary build of the SDL framework, the resulting icon has colour issues.  My icon ends up displaying with a very red bias.  This is likely an endian problem with the SDL_WM_SetIcon function.

The same call to SDL_WM_SetIcon (identical calling code) works properly when using the non-universal SDL framework (ie. through Rosetta).
Comment 1 Sam Lantinga 2006-05-01 03:08:52 UTC
Date: Sun, 30 Apr 2006 13:40:41 +0200
From: Christian Walther <cwalther@gmx.ch>
To: sdl@libsdl.org
Subject: Re: [SDL] SDL_WM_SetIcon on Intel Mac (using the universal SDL f
-- Message: 1720   --  Next: 1721   -------------------------------------

Ryan Clark wrote:
> I'm using the universal build (Mac) of the SDL framework to create a
> universal version of my game. Everything has gone well, except one
> item: When I call SDL_WM_SetIcon to set the icon, the call works, but
> the result has colour problems. The icon ends up displaying with a
> very red bias.

I've never used SDL_WM_SetIcon, but upon a cursory glance at the source
(QZ_SetIcon() in SDL_QuartzWM.m), it seems that there are several things

wrong with it:

- It uses a big endian surface (even explicitly says so in a comment)
that should be little endian on little endian machines. Obviously this
was written before there were little endian Macs. That's probably the
bug you're seeing.

- It seems to make all pixels whose mask bits are set (which are all
that are not completely transparent if the mask was autogenerated from
an alpha channel) completely opaque, even if they should be
semi-transparent.

- It creates an intermediate surface, passes its pixels to
[NSBitmapImageRep initWith...], which does *not* copy the pixels, and
then frees the surface again. Depending on whether [NSApp
setApplicationIconImage: img] copies or retains the image, this could
lead to problems. This should probably be done by letting the
NSBitmapImageRep allocate the space and using SDL_CreateRGBSurfaceFrom()
on it.

- It passes non-premultiplied alpha data to [NSBitmapImageRep
initWith...], which wants premultiplied alpha.

I'll have a closer look and see if I can come up with a patch. I don't
have an Intel Mac to test it on, though. Can you file a bug in the
meantime (for your initial observation only, not for my unconfirmed
suspicions, to be on the safe side :) )? http://bugzilla.libsdl.org/

  -Christian
Comment 2 Ondra Hosek 2006-05-01 10:37:47 UTC
*** Bug 210 has been marked as a duplicate of this bug. ***
Comment 3 Ondra Hosek 2006-05-01 10:41:03 UTC
If anybody wants to see the effects of this bug, I have made a few screenshots. The image on the left is the Rosetta-emulated, correctly rendered icon, the image on the right is the wrong "interpretation" of the Intel build.

RGB stripes: http://ondrasplayground.on.funpic.de/mac-x86-sdl-dock/rgbstripes.png

Full spectra: http://ondrasplayground.on.funpic.de/mac-x86-sdl-dock/fullspectra.png

Hope this slightly helps with the diagnosis. By the by, I have an Intel Mac, so if you attach the patches here, I can try them out.
Comment 4 Christian Walther 2006-05-02 09:52:17 UTC
OK, the following is, experimentally verified and theoretically understood, what the current implementation does:

- It only works properly on big-endian systems. (But I have to say that Ondra's screenshots look different from what I'd have expected on a little-endian system. Seems I still don't completely understand it after all...)

- If SDL_SRCALPHA is not set for the icon surface you pass to it, the alpha channel (if present) is ignored. This is probably the intended result.

- If SDL_SRCALPHA is set, what ends up in the dock icon is the original icon alpha-composited onto a black background, with some pixels (but possibly the wrong ones) completely transparent and the rest completely opaque. The wrong pixels being transparent is caused by a bug in the mask treatment - it neglects that there can be unused bits that pad scanlines to whole bytes in the mask. I can't see any effect of the premultiplication issue in the fully transparent pixels (apparently the color values are automatically clamped to the alpha value), and there's no direct indication of the memory management issue (though that may not mean anything).

Unless someone else beats me to it, I will implement a version that does what I consider the proper thing, namely:

- If SDL_SRCALPHA is not set, the alpha channel will be ignored (as before).

- If SDL_SRCALPHA is set, the alpha channel will be used. For pixels whose mask bit is clear, the alpha value will be set to 0, for those whose mask bit is set it will be left alone. I'm not sure what to do with a per-surface alpha value (different from 255). Ignore it? Use it only for surfaces without an alpha channel? Use it for all surfaces (in addition to an alpha channel)? Any opinions?

(And of course, fix the endianness bug, avoid the memory management problem, and properly premultiply color values (with half-transparent pixels, this *is* visible).)

It may be a few days, though - my current schedule is a bit unpredictable.
Comment 5 Sam Lantinga 2006-05-02 11:25:41 UTC
(In reply to comment #4)

Sounds great.
BTW, the semantics are that SDL_SRCCOLORKEY can be combined with SDL_SRCALPHA,  and when SDL_SRCALPHA is set, the alpha channel takes precedence over per-surface alpha.

Thanks!
Comment 6 Sam Lantinga 2006-05-07 17:06:04 UTC
I'd like to get this fixed for SDL 1.2.10 release, if possible.
Comment 7 Christian Walther 2006-05-10 18:48:04 UTC
Created attachment 125 [details]
patch

So, here's a patch with a reimplementation of QZ_SetIcon() that does what I described above. I apologize for the delay, I've been quite busy in the last few days.

It appears to work here on 10.4.5 PPC in the limited testing that I've done; I'll try to test it on 10.3.9 and 10.2.8 as well, but that might take another week or so. Please test on i386.

Regarding alpha channels, per-surface alpha, and color keys, the same semantics as for regular blits to an RGB surface should apply (for the final icon composited onto the dock), unless I made a mistake - except in one pathological case: if the icon surface has an alpha channel, its SDL_SRCALPHA flag is not set (i.e. it has been explicitly cleared, since it's on by default for RGBA surfaces), and it has a color key, plus an explicit mask was specified (instead of the one autogenerated from the colorkey), then the color-keyed areas appear black instead of transparent. I found no elegant way of fixing this, was too lazy to implement the inelegant one, and decided that it isn't worth the effort (but if someone disagrees, I can do it).
Comment 8 Sam Lantinga 2006-05-11 00:07:22 UTC
(In reply to comment #7)
> Regarding alpha channels, per-surface alpha, and color keys, the same semantics
> as for regular blits to an RGB surface should apply (for the final icon
> composited onto the dock), unless I made a mistake - except in one pathological
> case: if the icon surface has an alpha channel, its SDL_SRCALPHA flag is not
> set (i.e. it has been explicitly cleared, since it's on by default for RGBA
> surfaces), and it has a color key, plus an explicit mask was specified (instead
> of the one autogenerated from the colorkey), then the color-keyed areas appear
> black instead of transparent. I found no elegant way of fixing this, was too
> lazy to implement the inelegant one, and decided that it isn't worth the effort
> (but if someone disagrees, I can do it).

This seems like a rare enough case that it should be fine.  We can revisit it if it comes up.
This patch is in subversion, thanks!