Thread: How to make a GrayScale filter?

  1. #1
    Registered User
    Join Date
    Nov 2005
    Posts
    673

    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.

  2. #2
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    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
    Last edited by mike_g; 02-22-2008 at 05:05 PM.

  3. #3
    Registered User
    Join Date
    Nov 2005
    Posts
    673
    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.

  4. #4
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    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 )
    Should instead be:
    Code:
    for (int y = 0; y < SDL_GetVideoSurface()->h; y++ )

  5. #5
    Registered User
    Join Date
    Nov 2005
    Posts
    673
    The putpixel and getpixel are actually my functions that i adapted from the ones SDL provides. Also thanks for the assistance.

  6. #6
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    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.

    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.

  7. #7
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    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.

    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;
    }

  8. #8
    Registered User
    Join Date
    Nov 2005
    Posts
    673
    Just to let you know, your algorithm for greyscale doesn't work. it goes extremely dark.

  9. #9
    Registered User
    Join Date
    Nov 2005
    Posts
    673
    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.

  10. #10
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    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++;
        }

    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;
    Last edited by mike_g; 02-24-2008 at 06:48 AM.

  11. #11
    Registered User
    Join Date
    Nov 2005
    Posts
    673
    I hate to argue, but if i use your algorithm it makes my picture almost black. and the one i showed above doesnt.

  12. #12
    Dr Dipshi++ mike_g's Avatar
    Join Date
    Oct 2006
    Location
    On me hyperplane
    Posts
    1,218
    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.
    Last edited by mike_g; 02-25-2008 at 06:05 AM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. How to make a Packet sniffer/filter?
    By shown in forum C++ Programming
    Replies: 2
    Last Post: 02-22-2009, 09:51 PM
  2. HELP!wanting to make full screen game windowed
    By rented in forum Game Programming
    Replies: 3
    Last Post: 06-11-2004, 04:19 AM
  3. make all rule
    By duffy in forum C Programming
    Replies: 9
    Last Post: 09-11-2003, 01:05 PM
  4. Question about atheists
    By gcn_zelda in forum A Brief History of Cprogramming.com
    Replies: 160
    Last Post: 08-11-2003, 11:50 AM
  5. Replies: 6
    Last Post: 04-20-2002, 06:35 PM