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 627

Summary: blitting alpha value 255 does not fully overwrite image underneath
Product: SDL Reporter: Czirkos Zoltan <cirix>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: thorbrian
Version: 1.2.13Keywords: target-1.2.14
Hardware: x86   
OS: Linux   

Description Czirkos Zoltan 2008-09-16 13:10:01 UTC
hi

i try to overlay an image with another one that has an alpha channel. but if the alpha channel is 255, it does not overwrite it fully.

small test case included; check the saved bmp, the red area has slight white striped, but it should not.

i know this is not that important for the human eye; but after doing this, i use a scale2x effect, and pixels are compared in that for equality. and those almost unvisible stripes come out non-equal.

test:
#include <SDL.h>

int main()
{
  SDL_Surface *tile, *hole;
  SDL_Rect rect;
  int y;

  tile=SDL_CreateRGBSurface(0, 24, 24, 32, 0, 0, 0, 0);
  for (y=0; y<tile->h; y++) {
    int r[]={ 0xad, 0x00, 0x00, 0xff };
	int g[]={ 0xb5, 0x42, 0x00, 0xff };
	int b[]={ 0xf9, 0xf5, 0x00, 0xff };

	rect.x=0;
	rect.y=y;
	rect.w=tile->w;
	rect.h=1;
	
	SDL_FillRect(tile, &rect, SDL_MapRGB(tile->format, r[y%4], g[y%4], b[y%4]));
  }
  
  hole=SDL_CreateRGBSurface(0, tile->w, tile->h, 32, 0xff000000, 0x00ff0000, 0x0000ff00, 0x000000ff);
  SDL_FillRect(hole, NULL, SDL_MapRGBA(hole->format, 0xe0, 0x20, 0x20, 0xff));
  rect.x=hole->w/3;
  rect.y=0;
  rect.w=hole->w/3;
  rect.h=hole->h;
  SDL_FillRect(hole, &rect, SDL_MapRGBA(hole->format, 0, 0, 0, 0x00));

  SDL_BlitSurface(hole, NULL, tile, NULL);
  SDL_SaveBMP(tile, "overlay.bmp");
  
  SDL_FreeSurface(tile);
  SDL_FreeSurface(hole);
  
  return 0;
}
Comment 1 Ryan C. Gordon 2009-09-13 16:33:22 UTC
Tagging this bug with "target-1.2.14" so we can try to resolve it for SDL 1.2.14.

Please note that we may choose to resolve it as WONTFIX. This tag is largely so we have a comprehensive wishlist of bugs to examine for 1.2.14 (and so we can close bugs that we'll never fix, rather than have them live forever in Bugzilla).

--ryan.
Comment 2 Sam Lantinga 2009-09-27 17:03:26 UTC
I just tried this with the latest SDL snapshot and didn't have any problems:
http://www.libsdl.org/tmp/SDL-1.2.zip

Has this been fixed?
Comment 3 Brian Fisher 2009-10-08 11:06:53 UTC
I'm not the original bug reporter, but another sufferer from it, who's patched it for myself in the past, so...

I don't know about that snapshot, but 1.2.13 was definitely broken, and the SVN version still looks broken - the problem is this macro here:
-----------------
#define ALPHA_BLEND(sR, sG, sB, A, dR, dG, dB)	\
do {						\
	dR = (((sR-dR)*(A))>>8)+dR;		\
	dG = (((sG-dG)*(A))>>8)+dG;		\
	dB = (((sB-dB)*(A))>>8)+dB;		\
} while(0)
-----------------
The problem is the >> 8 rounds down, lowering the final value by 1 in many cases. For instance, when you blend a fully opaque full brightness color channel (color 255, alpha 255) over a black channel (color 0), the math comes out like this:

dest = (((255-0)*255)>>8) + 0
dest = (255*255)>>8
dest = 65025 >> 8 
dest = 254 (rounded down from a little over 254)

-----
A more correct math, with minimal extra cost would be this:

dest = (((src - dest)*alpha + 255) >> 8) + dest

----

running the same failing case from above with the new math:

dest = (((255 - 0)*255 + 255) >> 8) + 0
dest = (255*255 + 255) >> 8
dest = 255*256 >> 8
dest = 255

and it still works for blending with an alpha of zero, cause you get:

dest = (255 >> 8) + dest
dest = 0 + dest
dest = dest

----

and the new math also works better when you want to either blend with an alpha of 1, going from 0 to 255, or blend with an alpha of 255, going from 0 to 1.

the old math gives:

dest = ((255*1) >> 8) + 0
dest = 255 >> 8
dest = 0

where the new gives:

dest = ((255*1 + 255) >> 8) + 0
dest = 510 >> 8
dest = 1

you can see that adding the 255 to compensate for rounding down fixes it so the dest is actually changed.
Comment 4 Sam Lantinga 2009-10-10 00:49:13 UTC
Thanks!  This is fixed for SDL 1.2 and 1.3.