# How to make a GrayScale filter?

• 02-22-2008
Raigne
How to make a GrayScale filter?
I am using SDL for simplicities sake, but I have found myself not being able to make a grayscale filter. I am curious if anyone here has a website or even code that would help me solve this problem. I am currently working on some test code to get it working. If I solve it I will post ASAP, but it isn't likely to happen. Thanks in advance.
• 02-22-2008
mike_g
Theres different ways you can calculate greyscale, which give different results.

One method would be to add the rgb components together then divide:
Code:

`c =  (r + g + b)/3;`
Or you could measure it by the brightest element:
Code:

`c = r; if(g > c) c = g; if(b > c) c = b;`
Or you coudl give bias to certain elements. This is from an example I found, thats meant to give a good grey:
Code:

`c =  ((r * 61) + (g * 174) + (b * 21)) / 256;`
Edit: oh, and you might find this tut helpful: http://student.kuleuven.be/~m0216922/CG/color.html
• 02-22-2008
Raigne
Does this look good?

Code:

```SDL_LockSurface(SDL_GetVideoSurface());                 for ( int x = 0; x != SDL_GetVideoSurface()->w; ++x )                 {                         for ( int y = 0; y != SDL_GetVideoSurface()->h; ++y )                         {                                 Uint32 pixel = getpixel(SDL_GetVideoSurface(),x,y);                                 Uint8 r = 0;                                 Uint8 g = 0;                                 Uint8 b = 0;                                 SDL_GetRGB(pixel, SDL_GetVideoSurface()->format, &r, &g, &b);                                 r = g = b = (( r+g+b )/3);                                 putpixel(SDL_GetVideoSurface(),x,y, SDL_MapRGB(SDL_GetVideoSurface()->format, r,g,b));                         }                 }                 SDL_UnlockSurface(SDL_GetVideoSurface());```
EDIT:
I wont always be doing the entire screen, but more asking about my method of doing it.
• 02-22-2008
mike_g
Well I made my own read/write pixel functions, so I'm not sure exactly how the built in ones work, but it looks like you have more or less the right idea. A few pointers though:

You don't want to be declaring your variables inside a loop.

Theres no need to to initalize r, g, and b

You could save some cpu time by doing something like:
Code:

```b = (( r+g+b )/3); putpixel(SDL_GetVideoSurface(),x,y, SDL_MapRGB(SDL_GetVideoSurface()->format, b,b,b));```
Also i think theres something wrong with your loops here:
Code:

`for ( int y = 0; y != SDL_GetVideoSurface()->h; ++y )`
Code:

`for (int y = 0; y < SDL_GetVideoSurface()->h; y++ )`
• 02-22-2008
Raigne
The putpixel and getpixel are actually my functions that i adapted from the ones SDL provides. Also thanks for the assistance.
• 02-23-2008
VirtualAce
Quote:

You don't want to be declaring your variables inside a loop.
The compiler will most certainly optimize this and move the redundant init outside of the loop. Since this is really just a sub esp,1 (BYTE) the compiler will do this outside of the loop.

Quote:

Theres no need to to initalize r, g, and b
It is a good practice to initialize your variables prior to usage. If SDL_GetRGB () failed for some reason you could guarantee that your variables would at least be 0 thus causing a black output. Without this you could end up with any mixture of r,g,b on an error condition since you would be using un-init vars.
• 02-23-2008
mike_g
Quote:

The compiler will most certainly optimize this and move the redundant init outside of the loop. Since this is really just a sub esp,1 (BYTE) the compiler will do this outside of the loop.
Good point. I had not thought of that.

Quote:

It is a good practice to initialize your variables prior to usage. If SDL_GetRGB () failed for some reason you could guarantee that your variables would at least be 0 thus causing a black output. Without this you could end up with any mixture of r,g,b on an error condition since you would be using un-init vars.
Yes, but initializing 3 variables say maybe 500,000 times each loop is going to create a reasonable amount of overhead. But then I guess converting to greyscale is not something thats likely to be performed very often. Then again I doubt SDL_GetRBG can fail as long as you are reading inside the surface boundaries.

Also heres a greyscale filter I found that I coded a while back. Incase anyone ever needs one :) It assumes 32 bit colour tho.
Code:

```int Greyscale(SDL_Surface *image) {     char* pixels = NULL;     unsigned bytes = image->w * image->h * 4;     if(! (pixels = (char*)malloc( bytes ))) return 1;     // copy image data to ram     char *p1 = pixels;     char *p2 = (char*)image->pixels;     for(int i=0; i<bytes; i++, p1++, p2++) *p1 = *p2;         // convert to greyscale     p1 = pixels;     p2 = pixels;     unsigned col, a, b, size=bytes/4;     for(int a=0; a<size; a++)     {         col = 0;         for(b=0; b<3; b++, p1++) col = *p1; //read colours         p1++; //skip alpha byte         col /= 3;         for(b=0; b<3; b++, p2++) *p2 = col;  //write greyscale         p2++;     }         // copy greyscale to image     p1 = pixels;     p2 = (char*)image->pixels;     for(int i=0; i<bytes; i++, p1++, p2++) *p2 = *p1;        free(pixels);     return 0; }```
• 02-23-2008
Raigne
Just to let you know, your algorithm for greyscale doesn't work. it goes extremely dark.
• 02-23-2008
Raigne
Also, another question. Would it be faster to reload the image if your through with greyscale, create a copy surface, or just run greyscale each frame. The latter seems the most inefficient, but the other two seem to be about the same.
• 02-24-2008
mike_g
Quote:

Just to let you know, your algorithm for greyscale doesn't work. it goes extremely dark.
Youre wrong, it does work :P Thats the effect you get by dividing the colour by 3. If you want something brighter you could change the greyscale conversion to:
Code:

```    for(int a=0; a<size; a++)     {         col = 0;         for(b=0; b<3; b++, p1++) if(*p1 > col) col = *p1; //read colours         p1++; //skip alpha byte         for(b=0; b<3; b++, p2++) *p2 = col;  //write greyscale         p2++;     }```
;)
Quote:

Also, another question. Would it be faster to reload the image if your through with greyscale, create a copy surface, or just run greyscale each frame. The latter seems the most inefficient, but the other two seem to be about the same.
Replace the image or create a greyscale copy of it. Converting every loop is going to hammer the cpu.

Edit: Alternatively if you just want to brighten the image but keep the same gradient add this:
Code:

```float brighten = 100.0 / col * inc_percent; if( (col+=brighten) >255 ) col = 255;```
• 02-24-2008
Raigne
I hate to argue, but if i use your algorithm it makes my picture almost black. and the one i showed above doesnt.
• 02-25-2008
mike_g
Yeah actually i see what you mean now. My version originally picked the brightest element instead of dividing by 3, which worked fine. Tbh I can't see whats wrong with the changes I made and I cant be bothered to find out.

Edit: its probably because I never declared the chars in the function as unsigned.
Edit2: yep it is.