Flipping a union of pixels

[triumphost]

I have a union defined as so:

typedef union RGB
{
    unsigned Color;
    struct
    {
        unsigned char B, G, R, A;
    };
} *PRGB;

I'm reading 32 bit and 24 bit bitmaps. I'm trying to Read Pixels from my RGB union and set pixels as well. I know bitmaps are sometimes stored upside down and sometimes up-right. How can I set the pixels at a specific position in the bitmap to a color?

I tried:

RGB BMPS::GET(int x, int y)
{
    return Pixels[y * width + x];
}

void BMPS::SET(int x,int y, RGB color)
{
    Pixels[y * width + x] = color;
}

But sometimes the changed pixels never show and sometimes when they do, they are on the opposite end of the bmp. I'm told I need to flip the bitmap which is contained in my union but I don't know how to flip the union upside down. I'm also told I need to pad the 24 bit bmp but not the 32 bit bmp. I have no idea how to do that using scanlines even though I read a ton of tutorials on this so I used CreateDIBitmap instead as shown below:

HeaderFile:

#ifndef BITMAPS_HPP_INCLUDED
#define BITMAPS_HPP_INCLUDED

#include "GDIPlus.h"

class Bitmap
{
    private:
        PRGB Pixels;
        BITMAPINFO Info;
        int width, height, size;
        BITMAPFILEHEADER bFileHeader;

    protected:
        HDC DC;
        HBITMAP Image;

    public:
        ~Bitmap();

        Bitmap(){}
        Bitmap(const char* FilePath);

        bool Save(const char* FilePath);

        HDC ReturnDC(){return DC;}

        int Width(){return (width < 0 ? -width : width);}

        int Height(){return (height < 0 ? -height : height);}

        void Size(int& Width, int& Height){Width = width; Height = height;}

        int Size(){return size;}

        PRGB GetPixels(){return Pixels;}
};

#endif // BITMAPS_HPP_INCLUDED

Implementation File:

#include "Bitmaps.hpp"

Bitmap::~Bitmap()
{
    if (Pixels)
        delete[] Pixels;
    DeleteDC(DC);
    DeleteObject(Image);
}

Bitmap::Bitmap(const char* FilePath)
{
    HANDLE hFile = CreateFile(FilePath, GENERIC_READ, FILE_SHARE_READ, 0, OPEN_EXISTING, 0, 0);

    if (hFile == 0){ throw "Error Bitmap Doesn't Exist"; return; }

    DWORD Read;
    memset(&Info, 0, sizeof(BITMAPINFO));

    ReadFile(hFile, &bFileHeader, sizeof(bFileHeader), &Read, 0);
    ReadFile(hFile, &Info.bmiHeader, sizeof(Info.bmiHeader), &Read, 0);

    if ((Info.bmiHeader.biBitCount != 24) && (Info.bmiHeader.biBitCount != 32))
    {
        CloseHandle(hFile);
        throw "The Image Loaded is neither 24 or 32bits.";
        return;
    }

    if (bFileHeader.bfType != 0x4D42)
    {
        CloseHandle(hFile);
        throw "The File Is Not A Bitmap.";
        return;
    }

    if (Info.bmiHeader.biCompression != BI_RGB)
    {
        CloseHandle(hFile);
        return;
    }

    width = Info.bmiHeader.biWidth;
    height = Info.bmiHeader.biHeight;
    size = Info.bmiHeader.biSizeImage; //((bmp.bmWidth * BitsPerPixel + 31) / 32) * 4 * bmp.bmHeight;

    Pixels = new RGB[size];
    SetFilePointer(hFile, bFileHeader.bfOffBits, 0, FILE_BEGIN);
    ReadFile(hFile, Pixels, size, &Read, 0);

    DC = GetDC(0);
    Image = CreateDIBitmap(DC, &Info.bmiHeader, CBM_INIT, Pixels, &Info, DIB_RGB_COLORS);
    CloseHandle(hFile);
}

bool Bitmap::Save(const char* FilePath)
{
    DWORD Written;

    HANDLE hFile = CreateFile(FilePath, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
    if (hFile == 0)
    {
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, &bFileHeader, sizeof(BITMAPFILEHEADER), &Written, 0))
    {
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, &Info.bmiHeader, sizeof (BITMAPINFOHEADER), &Written, 0))
    {
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, Pixels, Info.bmiHeader.biSizeImage, &Written, 0))
    {
        CloseHandle(hFile);
        return false;
    }
    return true;
}

[mike_2000_17]

You've been busy! Pretty nice code you have there.

There is one important mistake for sure, you allocate your pixel array as Pixels = new RGB[size]; where size is the size in bytes of the image. So, you need to allocate the pixels as Pixels = new RGB[width * height];.

The second error is to make the assumption that (sizeof(unsigned) == 4) that may not be the case on some computers. If you want a fixed and known size, you need to use the header <stdint.h> (or <cstdint> if you have a recent-enough compiler), and use the type uint32_t in your union:

typedef union RGB
{
    uint32_t Color;
    struct
    {
        unsigned char B, G, R, A;
    };
} *PRGB;

Another bone I have to pick is with the throwing of a string literal. You shouldn't do that. Prefer to throw an exception object. The standard provides a few useful ones, or you can make your own classes derived from the std::exception class.

Now, to the point of your questions.

The bitmap format (uncompressed, at 24 or 32 bpp) stores the colors as Blue-Green-Red(-Alpha) in little-endian byte-order (i.e. the Blue byte is the least-significant and stored first). So, that is exactly how you should read the pixels from the file. And that is not what you are doing at all. And, you do have to consider the padding at the end of the rows (to make it an even multiple of 4 bytes). You cannot read the values from the file into the Pixels array directly, you need an intermediary array of unsigned chars. Here is a basic reading method:

unsigned char* pixBuffer = new unsigned char[size];
SetFilePointer(hFile, bFileHeader.bfOffBits, 0, FILE_BEGIN);
ReadFile(hFile, pixBuffer, size, &Read, 0);

int bytes_per_pixel = Info.bmiHeader.biBitCount / 8;
Pixels = new RGB[width * height];
unsigned char* buf_position = pixBuffer;
for(int i = 0; i < height; ++i) {
  for(int j = 0; j < width; ++j) {
    Pixels[i * width + j].B = *(buf_position++);
    Pixels[i * width + j].G = *(buf_position++);
    Pixels[i * width + j].R = *(buf_position++);
    Pixels[i * width + j].A = (Info.bmiHeader.biBitCount > 24 ? *(buf_position++) : 0);
  };
  // step over the padding at the end of the rows:
  if(Info.bmiHeader.biBitCount == 24)
    buf_position += width % 4;
};
delete[] pixBuffer;

With the above, it doesn't matter what endianness your platform has, or how the RGBA values are laid out in your union (so, you lay them out the way you need them for using your bitmap object). If you want to save the bitmap, you'll have to do the reverse operation.

As for the flipping, the defacto standard with images is that the pixel (0,0) represents the upper-left corner (raster scan order), but the bitmaps are special because the pixel (0,0) is the lower-left corner. Which explains why it gets drawn upside down. If you need the image such that pixel (0,0) is the upper-left corner, you'll have to swap all the rows to flip the image.

[triumphost]

Ahhhh thank you lots!!! I almost had that. I was using BYTE* instead of unsigned char and I wasn't sure how to get the bytes into the Pixels of type RGB so i deleted it all and used what I posted. I also used the Dib because I was told it'd make drawing on an HDC easier which I have accomplished via the Dib.

I will try exactly what you told me and I'll post back here if unsuccessful. THANK YOU SO MUCH! I spent hours on that lol. Was indeed quite busy learning it all :). ++REP++

[triumphost]

Hey I followed your advice and made the changes. I modified it for flipping the rows and stuff but I cannot get it to save. I also saw that you didn't use the BytesPerPixel thing so I implemented that. I also had to do absolute value of height for loading 32 bit bmps since it was a bad alloc when it was negative.

Loading with this works flawless:

    width = Info.bmiHeader.biWidth;
    height = Info.bmiHeader.biHeight;
    size = Info.bmiHeader.biSizeImage;

    unsigned char* TEMP = new unsigned char[size];             //SIZE As BYTES.
    SetFilePointer(hFile, bFileHeader.bfOffBits, 0, FILE_BEGIN);
    ReadFile(hFile, TEMP, size, &Read, 0);
    CloseHandle(hFile);

    unsigned char* BuffPos = TEMP;
    height = (height < 0 ? -height : height);
    int BytesPerPixel = Info.bmiHeader.biBitCount / 8;
    Pixels = new RGB[width * height * BytesPerPixel];       //WIDTH * HEIGHT * BYTES-PP

    for (int I = 0; I < height; I++)
    {
        for (int J = 0; J < width; J++)
        {                                                                 //Flips The ScanLines/Rows.
            Pixels[(height - 1 - I) * width + J].B = *(BuffPos++);
            Pixels[(height - 1 - I) * width + J].G = *(BuffPos++);
            Pixels[(height - 1 - I) * width + J].R = *(BuffPos++);
            Pixels[(height - 1 - I) * width + J].A = (Info.bmiHeader.biBitCount > 24 ? *(BuffPos++) : 0);
        }
        if(Info.bmiHeader.biBitCount == 24)
            BuffPos += width % 4;
    }

    delete[] TEMP;
}

Now I tried to apply the reverse to the Saving but it crashes when it reaches: cout<<"This far";

The code is as follows:

bool BMPS::Save(const char* FilePath)
{
    DWORD Written;
    int BytesPerPixel = Info.bmiHeader.biBitCount / 8;
    unsigned char* TEMP = new unsigned char[width * height * BytesPerPixel];        //Not sure if to use W*H * Bytes-PP
    unsigned char* BuffPos = TEMP;
    memset(TEMP, 0, width * height * BytesPerPixel);                       //So I don't have to set the alpha to 0's for 24-bit ?

    for(int I = 0; I < height; ++I)
    {
        for(int J = 0; J < width; ++J)
        {                                                                   //Flip The ScanLines/Rows back to normal.
            *(BuffPos++) = Pixels[(height - 1 - I) * width + J].B;
            *(BuffPos++) = Pixels[(height - 1 - I) * width + J].G;
            *(BuffPos++) = Pixels[(height - 1 - I) * width + J].R;

            if (Info.bmiHeader.biBitCount > 24)
                *(BuffPos++) = Pixels[(height - 1 - I) * width + J].A;
        }
        if(Info.bmiHeader.biBitCount == 24)
            BuffPos += width % 4;
    }

    HANDLE hFile = CreateFile(FilePath, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
    if (hFile == 0)
    {
        delete[] TEMP;
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, &bFileHeader, sizeof(BITMAPFILEHEADER), &Written, 0))
    {
        delete[] TEMP;
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, &Info.bmiHeader, sizeof (BITMAPINFOHEADER), &Written, 0))
    {
        delete[] TEMP;
        CloseHandle(hFile);
        return false;
    }

    if (!WriteFile(hFile, TEMP, size, &Written, 0))
    {
        delete[] TEMP;
        CloseHandle(hFile);
        return false;
    }
    cout<<"This far"<<endl;
    delete[] TEMP;          //Crashes the program. W

    return true;
}

[mike_2000_17]

The number of pixels to allocate is width * height. You shouldn't have the BytesPerPixel in there. It should be:

Pixels = new RGB[width * height];

As for the crash, it might have something to do with the fact that your array, that TEMP points to, is too small for the image size in bytes. Because of the padding at the end of the rows, the total size in bytes of the image is not equal to width * height * BytesPerPixel, it is slightly bigger than that. You should use the value stored in Info.bmiHeader.biSizeImage as the number of unsigned char to allocate. As in:

unsigned char* TEMP = new unsigned char[Info.bmiHeader.biSizeImage];

And use the same size value in the memset().

Finally, don't forget to close the file handle at the end of the function (you do it in all the if-statements, but you forgot to do it at the "normal" end).

Other than that, I'm not sure what could be causing the crash.

[triumphost]

Ahh Solved! You're correct lol. When I don't close the file handle, it crashes so bad. I close it and it works perfectly fine; with the changes you suggested.