Bitmap loader C++

Question

I know I am missing something but I don't know what. This piece of code helps me for saving images into BMP files. But when I use it, I get a row of black pixels at the top of the image, and the image is shifted to the right. Any ideas ? Thanks !

struct CVImporter::BITMAPFILEHEADER
{
    ushort bfType;
    uint   bfSize;
    uint   bfReserved;
    uint   bfOffBits;
};

struct CVImporter::BITMAPINFOHEADER
{
    uint  biSize;
    int   biWidth;
    int   biHeight;
    short biPlanes;
    short biBitCount;
    uint  biCompression;
    uint  biSizeImage;
    int   biXPelsPerMeter;
    int   biYPelsPerMeter;
    uint  biClrUsed;
    uint  biClrImportant;
};

struct CVImporter::RGBQUAD
{
    uchar rgbBlue;
    uchar rgbGreen;
    uchar rgbRed;
    uchar rgbReserved;
};

struct CVImporter::BITMAPINFO
{
    TBITMAPINFOHEADER bmiHeader;
    TRGBQUAD          bmiColors[256];
};

//-----------------------------------------------------------------------------
//
void
CVImporter::WriteBitmapFileHeader( std::ofstream& stream,
                                   const BITMAPFILEHEADER& header )
{
    WriteToStream( stream, header.bfType );
    WriteToStream( stream, header.bfSize );
    WriteToStream( stream, header.bfReserved );
    WriteToStream( stream, header.bfOffBits );
}

//-----------------------------------------------------------------------------
//
void
CVImporter::WriteBitmapInfoHeader( std::ofstream& stream,
                                   const BITMAPINFOHEADER& infoHeader )
{
    WriteToStream( stream, infoHeader.biSize );
    WriteToStream( stream, infoHeader.biWidth );
    WriteToStream( stream, infoHeader.biHeight );
    WriteToStream( stream, infoHeader.biPlanes );
    WriteToStream( stream, infoHeader.biBitCount );
    WriteToStream( stream, infoHeader.biCompression );
    WriteToStream( stream, infoHeader.biSizeImage );
    WriteToStream( stream, infoHeader.biXPelsPerMeter );
    WriteToStream( stream, infoHeader.biYPelsPerMeter );
    WriteToStream( stream, infoHeader.biClrUsed );
    WriteToStream( stream, infoHeader.biClrImportant );
}

//-----------------------------------------------------------------------------
//
void
CVImporter::WriteBitmapRGBQuad( std::ofstream& stream, const RGBQUAD& quad )
{
    WriteToStream( stream, quad.rgbBlue );
    WriteToStream( stream, quad.rgbGreen );
    WriteToStream( stream, quad.rgbRed );
    WriteToStream( stream, quad.rgbReserved );
}


//-----------------------------------------------------------------------------
//
void
CVImporter::WriteBitmapInfo( std::ofstream& stream,
                             const BITMAPINFO& info )
{
    WriteBitmapInfoHeader( stream, info.bmiHeader );
    for( uint i = 0; i < 256; ++i )
        WriteBitmapRGBQuad( stream, info.bmiColors[i] );
}

//-----------------------------------------------------------------------------
//
void
CVImporter::LoadBitmapFile( const CLString& fileName,
                            CVBitmap::Ptr bm ) throw(IOException)
{
    if( bm.IsNull() ) throw( IOException("Pointer should not be null", CL_ORIGIN) );

    // Verify the extension of the file
    CLString ext("");
    CLFileSystem::GetExtension( fileName, ext );
    if( ext != "bmp" )
        throw( IOException("Bad file extension (should be bmp)", CL_ORIGIN) );

    uint bytesPerPixel {3};
    uint imgDataSize = (bm->GetWidth()*bytesPerPixel)*bm->GetHeight();

    BITMAPFILEHEADER bmFile;
    BITMAPINFO bmInfo;

    bmFile.bfType = (ushort)0x4D42;
    bmFile.bfSize = imgDataSize + sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFO);
    bmFile.bfReserved = 0;
    bmFile.bfOffBits = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFO);

    bmInfo.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
    bmInfo.bmiHeader.biWidth = (ulong)bm->GetWidth();
    bmInfo.bmiHeader.biHeight = (ulong)bm->GetHeight();
    bmInfo.bmiHeader.biPlanes = 1;
    bmInfo.bmiHeader.biBitCount = (ushort) 8*bytesPerPixel;
    bmInfo.bmiHeader.biCompression = 0; //BI_RGB;
    bmInfo.bmiHeader.biSizeImage = imgDataSize;
    bmInfo.bmiHeader.biXPelsPerMeter = 0;
    bmInfo.bmiHeader.biYPelsPerMeter = 0;
    bmInfo.bmiHeader.biClrUsed = 256;
    bmInfo.bmiHeader.biClrImportant = 256;

    for( uint i = 0; i < 256; ++i )
    {
        bmInfo.bmiColors[i].rgbBlue = i;
        bmInfo.bmiColors[i].rgbGreen = i;
        bmInfo.bmiColors[i].rgbRed = i;
        bmInfo.bmiColors[i].rgbReserved = 0;
    }

    std::ofstream stream( fileName.c_str(), std::ios::binary );
    WriteBitmapFileHeader( stream, bmFile );
    WriteBitmapInfo( stream, bmInfo );

    uint padding = (4 - ((3 * bm->GetWidth()) % 4)) % 4;
    char padding_data[4] {0, 0, 0, 0};

    for( uint i = 0; i < bm->GetHeight(); ++i )
    {
        uchar* data_ptr = bm->GetBits() + ((bm->GetHeight()-i)*bm->GetWidthStep());
        stream.write( reinterpret_cast<char*>(data_ptr), sizeof(char) *
                      bm->GetByteSize() * bm->GetWidth() );

        stream.write( padding_data, padding );
    }

    stream.close();
}


//-----------------------------------------------------------------------------
//
template<typename T>
inline void
CVImporter::WriteToStream( std::ofstream& stream, const T& t )
{
    stream.write( reinterpret_cast<const char*>(&t), sizeof(T) );
}

Answer 1

There are two immediate problems with your code.

First off, structure members are usually aligned on 4-byte memory addresses (SO: structure padding and structure packing). That means that all of a char, short, and int will occupy 4 bytes, and for the first two there are simply a few unused bytes in memory. This is usually a good thing, because memory access -- read and write -- is usually faster when the processor can read from aligned memory. However, if your structure consists of different-sized members, you have to be careful when reading or writing a file. On reading, some of your data may disappear into the 'unused' bytes, and on writing, you will save these unused bytes into your file.

You say you already tried the reported size values instead of sizeof, but that only partially resolves it. The correct size will be written to your file, but it will still be the wrong data -- because you are still writing the padding bytes.

The solution is to tell your compiler you do not want to automatically add padding between your structure members. There are different ways for different compilers, and you don't mention yours, but the SO Question I pointed to above contains a few examples. If all else fails, look it up in the manual for your compiler.

Another strategy is to manually read and write each structure member instead of the structure in its entirety, but that has hardly any advantages in your situation.

This should solve the problem that a few columns of the right are wrapped over to the left. Your BMP reader seems to be quite forgiving as lots of values end up 'wrong', but in the end it still starts displaying the image from the wrong starting position.

---

Problem #2 is, fortunately, easier. From your code:

for( uint i = 0; i < bm->GetHeight(); ++i )
{
    uchar* data_ptr = bm->GetBits() + ((bm->GetHeight()-i)*bm->GetWidthStep());
    ...

(You forgot to include the function GetWidthStep but I'm guessing it returns the length of a single bitmap line.) You mean to grab a pointer to the start of the last line first, then the line above it, and so on, until line #0. However, you are off by one line!

With i=0, you calculate the pointer as start + (height-i) * width, so it's start + height * width. That points to the 'line' immediately after your image, at height * width! You can see that if you mentally fill in real values for 'height'.

So you are grabbing pointers to lines 1 to height, while you should have used 0 to height-1. Use this instead:

uchar* data_ptr = bm->GetBits() + ((bm->GetHeight()-1-i)*bm->GetWidthStep());

-- note the -1 after GetHeight() -- to get it to work.