CODEWITHSUNDEEP
c++borland-c++
Chang
Chang

Reputation: 37

Grayscale image converting Turbo C++

I am trying to convert an RGB image to a grayscale image. The RGB image is 160*120*4, while my grayscale is 160*120*1. However, it just give me plain black instead of what I want and takes a very long time.

This is what I wrote, please advise. Thanks

int i, j, sum;
Image = new unsigned char [ 160 * 120 * 1 ];
for( int j = 0; j < 120; j++ ) {

    for( int i = 0; i < 160; i++ ) {
    sum=0;
        sum += PaintBox1->Canvas->Pixels[ i ][ j ];
        sum += PaintBox1->Canvas->Pixels[ i ][ j ];
        sum += PaintBox1->Canvas->Pixels[ i ][ j ];
        sum += PaintBox1->Canvas->Pixels[ i ][ j ];
        *Image = sum/4;

        PaintBox2->Canvas->Pixels[ i ][ j ] = *Image;
        Image++;
    }
}

Upvotes: 0

Views: 722

Answers (2)

Spektre
Spektre

Reputation: 51903

The VCL canvas use TColor to hold color. However internal format is defined by the PixelFormat format of the Canvas ... The usual is pf32bit or pf24bit... The TColor is always 32bit however regardless of the image PixelFormat as a lot of conversions happens along the way (that is why it is so slow) but beware different PixelFormat settings can reverse the RGB order making also predefined colors like clYellow,... reversed too (rendering as clAqua,...) !!!

For example if you use VCLs 

Graphics::TBitmap *bmp=new Graphics::TBitmap;
bmp->PixelFormat=pf32bit;

then each pixel is a 32bit value. You can do this safely for bitmaps but I am not sure about Form/PaintBox Canvas as they are tied to window I suspect your PixelFormat is similar (most likely 24bit) so you need to change your code slightly:

int x,y,xs=160,ys=120;
DWORD rgb,bw;
Image = new unsigned char [ xs*ys*1 ];
for(y=0;y<ys;y++)
 for(x=0;x<xs;x++) 
    {
    rgb = PaintBox1->Canvas->Pixels[x][y];
    bw = ( rgb     &255) // sum r,g,b
        +((rgb>> 8)&255)
        +((rgb>>16)&255);
    bw/=3;               // normalize
    *Image = bw;         // write to your array
    Image++;
    bw*=0x00010101;      // convert intensity to grayscale RGB color (you can shift and or instead)
    PaintBox2->Canvas->Pixels[x][y]=TColor(bw); // write back
    } Image-=xs*ys;

The Pixels property is sloooow I would use VCL bitmap instead with use of its ScanLine property that is ~1000 and more times faster and also have load/save functions implemented so you can use bmp files easily. For more info see:

Also there are more ways to convert RGB to intensity see:

Upvotes: 0

Ken Wayne VanderLinde
Ken Wayne VanderLinde

Reputation: 19347

There's several problem in the code that I can see immediately. The most striking comes from within the innermost for loop:

sum=0;
sum += PaintBox1->Canvas->Pixels[ i ][ j ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ];
*Image = sum/4;

Here, you have simply added the same value to sum four times over, and then divided by four. This makes these six lines equivalent to

*Image = PaintBox1->Canvas->Pixels[ i ][ j ];

Clearly, you actually wanted to average each channel. If your RGB image were implemented as a three-dimensional array, this would probably look something like:

sum = 0;
sum += PaintBox1->Canvas->Pixels[ i ][ j ][ 0 ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ][ 1 ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ][ 2 ];
sum += PaintBox1->Canvas->Pixels[ i ][ j ][ 3 ];
*Image = sum/4;

However, from your code example, it looks like your RGB image is actually implemented as a two-dimensional array of (un)signed integers. In that case, the following code should suffice (provided integers are four bytes on your machine):

sum = 0;
unsigned int pixel = PaintBox1->Canvas->Pixels[ i ][ j ];
for(int k = 0; k < 4; ++k)
{
    sum += pixel & 0xFF;
    pixel >>= 1;
}
*Image = sum/4;

The other major problem I see is that you do not keep a pointer to the beginning of your grayscale array. You initialize it as

Image = new unsigned char[ 160 * 120 * 1 ];

which is good. But then each time through the loop, you've written

Image++;

Rather, you should keep a pointer to the beginning of the array, and have a temporary pointer which acts as an iterator:

// before the for loops
Image = new unsigned char[ 160 * 120 * 1 ];
unsigned char * temp = Image;

// at the end of the inner for loop:
temp++;

So you only move around the temp pointer, while Image stays fixed.

Upvotes: 1

Related Questions