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

*** Bug 210 has been marked as a duplicate of this bug. ***

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.

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.

(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!

I'd like to get this fixed for SDL 1.2.10 release, if possible.

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).

(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!