Seeking code review for colorkey bug patch

Something told me to keep at it, and I eventually tracked it down. It was converting all the colorkey-related data just fine, but then it didn’t set the blend mode properly for the newly-created texture. I wrote this patch to fix that. Problem is, the only place I could find to make the change effective was inside of SDL_ConvertSurface(), and the fix I made might introduce unwanted side-effects. Can someone who knows what they’re doing take a look at this please?

Index: C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c===================================================================
— C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c (revision 4624)
+++ C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c (working copy)
@@ -843,8 +843,10 @@
&keyG, &keyB, &keyA);
SDL_SetColorKey(convert, 1,
SDL_MapRGBA(convert->format, keyR, keyG, keyB, keyA));
/* This is needed when converting for 3D texture upload */
SDL_ConvertColorkeyToAlpha(convert);

  •    /*This is needed in SDL_CreateTextureFromSurface*/
    
  •    SDL_SetSurfaceBlendMode(surface, SDL_BLENDMODE_BLEND);
    
    }
    SDL_SetClipRect(convert, &surface->clip_rect);

This might be a better fix, can you test it?

Index: SDL_surface.c===================================================================
— SDL_surface.c (revision 4627)
+++ SDL_surface.c (working copy)
@@ -851,7 +851,7 @@
/* Enable alpha blending by default if the new surface has an
* alpha channel or alpha modulation */
if ((surface->format->Amask && format->Amask) ||

  •    (copy_flags & SDL_COPY_MODULATE_ALPHA)) {
    
  •    (copy_flags & (SDL_COPY_COLORKEY|SDL_COPY_MODULATE_ALPHA))) {
       SDL_SetSurfaceBlendMode(convert, SDL_BLENDMODE_BLEND);
    
    }
    if ((copy_flags & SDL_COPY_RLE_DESIRED) || (flags & SDL_RLEACCEL)) {

On Sun, Jul 19, 2009 at 10:29 PM, Mason Wheeler wrote:

Something told me to keep at it, and I eventually tracked it down. It was
converting all the colorkey-related data just fine, but then it didn’t set
the blend mode properly for the newly-created texture. I wrote this patch
to fix that. Problem is, the only place I could find to make the change
effective was inside of SDL_ConvertSurface(), and the fix I made might
introduce unwanted side-effects. Can someone who knows what they’re doing
take a look at this please?

Index:
C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c


C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c
(revision 4624)
+++
C:/Users/Mason/Documents/SDL-1.3/SDL-1.3.0-4423/src/video/SDL_surface.c
(working copy)
@@ -843,8 +843,10 @@
&keyG, &keyB, &keyA);
SDL_SetColorKey(convert, 1,
SDL_MapRGBA(convert->format, keyR, keyG, keyB,
keyA));
/* This is needed when converting for 3D texture upload */
SDL_ConvertColorkeyToAlpha(convert);

  •    /*This is needed in SDL_CreateTextureFromSurface*/
    
  •    SDL_SetSurfaceBlendMode(surface, SDL_BLENDMODE_BLEND);
    
    }
    SDL_SetClipRect(convert, &surface->clip_rect);

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


-Sam Lantinga, Founder and President, Galaxy Gameworks LLC