[Solved] SDL Image Transparency Issues

Started by
3 comments, last by rip-off 14 years, 10 months ago
After scouring the GameDev forums (as well as other sources) for a few days, now, I've decided to post my issues to see if I can fix this odd thing. Nonetheless, my issue is that I am attempting to load an image (via the SDL_image library), remove the background (usually magenta or bright green), and render it out via OpenGL as a texture with transparency. It currently works without attempting transparency (so my SDL_Surface to OpenGL texture process works), but the issues begin when I try to have transparency. Using the following process, the texture returned is completely black, which I found very odd.

	SDL_Surface* Texture::enableTransparency(SDL_Surface *surface) {
		if(!surface)
			return NULL;

		Uint32 rmask, gmask, bmask, amask;

		// Setup the RGBA mask depending on the system's endian
		#if SDL_BYTEORDER == SDL_BIG_ENDIAN
			rmask = 0xff000000;
			gmask = 0x00ff0000;
			bmask = 0x0000ff00;
			amask = 0x000000ff;
		#else
			rmask = 0x000000ff;
			gmask = 0x0000ff00;
			bmask = 0x00ff0000;
			amask = 0xff000000;
		#endif

		// Obtain the color we want to be transparent
		Uint32 colorKey = getPixel(surface, 0, 0);

		// Set the color to be transparent
		if(SDL_SetColorKey(surface, SDL_SRCALPHA | SDL_RLEACCEL, colorKey) < 0)
			throw Exception(SDL_GetError());

		// Set the alpha transparency to be completely transparent
		if(SDL_SetAlpha(surface, SDL_SRCALPHA | SDL_RLEACCEL, SDL_ALPHA_TRANSPARENT) < 0)
			throw Exception(SDL_GetError());

		// Create a new empty surface that will hold the blitted image
		// Use 32-bit bits per pixel to store alpha values
		SDL_Surface *temp = SDL_CreateRGBSurface(SDL_SRCALPHA | SDL_RLEACCEL | surface->flags,
			surface->w, surface->h, 32,
			rmask, gmask, bmask, amask);

		// Blit the actual image over!
		if(SDL_BlitSurface(surface, NULL, temp, NULL) < 0)
			throw Exception(SDL_GetError());

		// We can free the original surface, since we no longer need it
		SDL_FreeSurface(surface);

		// Return the final surface that will hold the transparent image surface
		SDL_Surface* ret = SDL_DisplayFormatAlpha(temp);
		return ret;
	}

	Uint32 Texture::getPixel(SDL_Surface* get_surface, int x, int y) {
		if(!get_surface) {
			throw Exception("Surface is NULL");
			return 0;
		}
		if(x < 0 || x > get_surface->w || y < 0 || y > get_surface->h) {
			throw Exception("Invalid position");
			return 0;
		}
		if(SDL_LockSurface(get_surface) < 0)
			throw Exception(SDL_GetError());

		int bpp = get_surface->format->BytesPerPixel;
		Uint8 *p = (Uint8 *)get_surface->pixels + y * get_surface->pitch + x * bpp;

		Uint32 ret = 0;

		switch(bpp) {
			case 1:
				ret = *p;
				break;
			case 2:
				ret = *(Uint16 *)p;
				break;
			case 3:
				if(SDL_BYTEORDER == SDL_BIG_ENDIAN)
					ret = (p[0] << 16 | p[1] << 8 | p[2]);
				else
					ret = (p[0] | p[1] << 8 | p[2] << 16);
				break;
			case 4:
				ret = *(Uint32 *)p;
				break;
			default:
				ret = 0;
				break;
		}

		SDL_UnlockSurface(get_surface);
		return ret;
	}
I've also noted a few people discussing the various uses of GL_BLEND and the blending function to go along with it. I currently have GL_BLEND disabled (as I wasn't sure how to set it up). Should I have GL_BLEND enabled? (I am presuming yes) If so, what blending function should I use? It seems glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA) is the most popular. Thanks for any help! Any posts will be a great help. [Edited by - codemastermm on June 6, 2009 11:39:25 PM]
"Everything begins with Nu and everything ends with Nu. This is the truth! This is my belief... at least for now." - Mysteries of Life Volume 184 Chapter 26
Advertisement
Do you know what happens when you SDL_SetAlpha(surface, SDL_SRCALPHA | SDL_RLEACCEL, SDL_ALPHA_TRANSPARENT)? What happens is that you are setting the per-surface alpha value to transparent. SDL multiplies each pixel by the per-surface alpha during a blit operation. This is a possible reason why your surface is coming up black.

Your colour key setting appears to be incorrect too. It should be:
SDL_SetColorKey(surface, SDL_SRCCOLORKEY, colorKey)

I would ignore RLE acceleration, you are only doing a single blit during start up and the time spent RLE encoding the image would probably outweigh any benefit.

Using SDL_DisplayFormatAlpha() at the end of your function seems to be undoing all the work you had been doing. I assume the purpose of the function is to convert the surface to a known format suitable for OpenGL. How sure are you that the display format (which I am not sure has a lot of meaning when you are using an OpenGL display surface) matches the one you use when submitting the texture to OpenGL?

I would use SDL_SWSURFACE as the sole first argument to SDL_CreateRGBSurface().

On a different note, I am curious why these functions are members of the "texture" class. They are pure functions, they don't appear to modify any state. They could be free functions.

Finally freeing the input surface is a hidden side effect, not even hinted at by the name of the function. Using the surface after this function would cause problems.
Wow, thank you rip-off! I had no idea that RLE encoding would possibly be decremental. I also had no idea that SDL_DisplayFormatAlpha would undo the work that my function was performing.

As for the function itself, it is static (so it isn't modifying anything - it's just a helper function really :)) and the documentation stated that the input surface will be destroyed. However, with the changes you recommended, it seems that I won't have to destroy the surface - even better!

Here's what I have now for that function, however it still outputs a black area when rendering.

	SDL_Surface* Texture::enableTransparency(SDL_Surface *surface) {		if(!surface)			return NULL;		Uint32 rmask, gmask, bmask, amask;		// Setup the RGBA mask depending on the system's endian		#if SDL_BYTEORDER == SDL_BIG_ENDIAN			rmask = 0xff000000;			gmask = 0x00ff0000;			bmask = 0x0000ff00;			amask = 0x000000ff;		#else			rmask = 0x000000ff;			gmask = 0x0000ff00;			bmask = 0x00ff0000;			amask = 0xff000000;		#endif		// Obtain the color we want to be transparent		Uint32 colorKey = getPixel(surface, 0, 0);		// Set the color to be transparent		if(SDL_SetColorKey(surface, SDL_SRCCOLORKEY, colorKey) < 0)			throw Exception(SDL_GetError());		// Create a new empty surface that will hold the blitted image		// Use 32-bit bits per pixel to store alpha values		SDL_Surface *temp = SDL_CreateRGBSurface(SDL_SWSURFACE,			surface->w, surface->h, 32,			rmask, gmask, bmask, amask);		// Blit the actual image over!		if(SDL_BlitSurface(surface, NULL, temp, NULL) < 0)			throw Exception(SDL_GetError());		// We can free the original surface, since we no longer need it		SDL_FreeSurface(surface);		return temp;	}


[Edited by - codemastermm on June 6, 2009 10:50:58 PM]
"Everything begins with Nu and everything ends with Nu. This is the truth! This is my belief... at least for now." - Mysteries of Life Volume 184 Chapter 26
My apologies for double-posting, but I've managed to solve my issue! I did two things to fix my problem

(1) I converted my SDL_Surface to the display format before applying the color key, etc.

(2) I enabled GL_BLEND with the following:

glEnable(GL_BLEND);glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);


Here's my revised transparency function:
	SDL_Surface* Texture::enableTransparency(SDL_Surface *surface) {		if(!surface)			return NULL;		// Convert the surface to be formatted to our display format		SDL_Surface *formatted_surface = SDL_DisplayFormat(surface);		// We can remove the surface since it is no longer needed		SDL_FreeSurface(surface);		// Obtain the color we want to be transparent		Uint32 colorKey = getPixel(formatted_surface, 0, 0);		Uint32 rmask, gmask, bmask, amask;		// Setup the RGBA mask depending on the system's endian		#if SDL_BYTEORDER == SDL_BIG_ENDIAN			rmask = 0xff000000;			gmask = 0x00ff0000;			bmask = 0x0000ff00;			amask = 0x000000ff;		#else			rmask = 0x000000ff;			gmask = 0x0000ff00;			bmask = 0x00ff0000;			amask = 0xff000000;		#endif		// Set the color to be transparent		if(SDL_SetColorKey(formatted_surface, SDL_SRCCOLORKEY, colorKey) < 0)			throw Exception(SDL_GetError());		// Create a new empty surface that will hold the blitted image		// Use 32-bit bits per pixel to store alpha values		SDL_Surface *temp = SDL_CreateRGBSurface(SDL_SWSURFACE,			formatted_surface->w, formatted_surface->h, 32,			rmask, gmask, bmask, amask);		// Blit the actual image over!		if(SDL_BlitSurface(formatted_surface, NULL, temp, NULL) < 0)			throw Exception(SDL_GetError());		// We can free the original surface, since we no longer need it		SDL_FreeSurface(formatted_surface);		// Return the finalized surface		return; temp;	}


I also think I will be rewriting these static functions to simply set the provided SDL_Surface pointer to the new finalized surface instead of having to return it, but that's pretty easy to do :)
"Everything begins with Nu and everything ends with Nu. This is the truth! This is my belief... at least for now." - Mysteries of Life Volume 184 Chapter 26
Here your call to DisplayFormat is merely redundant, not detrimental. Again, the purpose of this function is to convert the input surface into a format suitable for OpenGL. With DisplayFormat(), you are doing two conversions, without it you are doing one.

You still need additional error checking.

Another note, you are using exceptions, but your function might not be exception safe. For example, in the last version you posted the input surface is freed in the first few lines. If an exception is thrown later on in the function, your input surface is freed. This might not be a great idea.

This topic is closed to new replies.

Advertisement