C/C++ rotate an BMP image

Question

I'm trying to rotate an BMP image using C/C++ but it's not working.

I've made some functions for read, write and rotation. The read and write functions are working perfectly fine but not rotate from some reason.

EDIT(sin, cos and rotate function)

BMP struct:

struct BMP {
    int width;
    int height;
    unsigned char header[54];
    unsigned char *pixels;
    int size;
};

WRITE:

void writeBMP(string filename, BMP image) {
    string fileName = "Output Files/" + filename;
    FILE *out = fopen(fileName.c_str(), "wb");
    fwrite(image.header, sizeof(unsigned char), 54, out);
    int i;
    unsigned char tmp;
    for (i = 0; i < image.size; i += 3) {
        tmp = image.pixels[i];
        image.pixels[i] = image.pixels[i + 2];
        image.pixels[i + 2] = tmp;
    }
    fwrite(image.pixels, sizeof(unsigned char), image.size, out); // read the rest of the data at once
    fclose(out);
}

READ:

BMP readBMP(string filename) {
    BMP image;
    int i;
    string fileName = "Input Files/" + filename;
    FILE *f = fopen(fileName.c_str(), "rb");
    fread(image.header, sizeof(unsigned char), 54, f); // read the 54-byte header

    // extract image height and width from header
    image.width = *(int *) &image.header[18];
    image.height = *(int *) &image.header[22];

    image.size = 3 * image.width * image.height;
    image.pixels = new unsigned char[image.size]; // allocate 3 bytes per pixel
    fread(image.pixels, sizeof(unsigned char), image.size, f); // read the rest of the data at once
    fclose(f);

    for (i = 0; i < image.size; i += 3) {
        unsigned char tmp = image.pixels[i];
        image.pixels[i] = image.pixels[i + 2];
        image.pixels[i + 2] = tmp;
    }
    return image;
}

ROTATE:

BMP rotate(BMP image, double degree) {
    BMP newImage = image;
    unsigned char *pixels = new unsigned char[image.size];

    double radians = (degree * M_PI) / 180;
    int sinf = (int) sin(radians);
    int cosf = (int) cos(radians);

    double x0 = 0.5 * (image.width - 1);     // point to rotate about
    double y0 = 0.5 * (image.height - 1);     // center of image

    // rotation
    for (int x = 0; x < image.width; x++) {
        for (int y = 0; y < image.height; y++) {
            long double a = x - x0;
            long double b = y - y0;
            int xx = (int) (+a * cosf - b * sinf + x0);
            int yy = (int) (+a * sinf + b * cosf + y0);

            if (xx >= 0 && xx < image.width && yy >= 0 && yy < image.height) {
                pixels[(y * image.height + x) * 3 + 0] = image.pixels[(yy * image.height + xx) * 3 + 0];
                pixels[(y * image.height + x) * 3 + 1] = image.pixels[(yy * image.height + xx) * 3 + 1];
                pixels[(y * image.height + x) * 3 + 2] = image.pixels[(yy * image.height + xx) * 3 + 2];
            }
        }
    }
    newImage.pixels = pixels;
    return newImage;
}

MAIN:

int main() {
    BMP image = readBMP("InImage_2.bmp");
    image = rotate(image,180);
    writeBMP("Output-11.bmp", image);
    return 0;
}

That sin=0.8939966636(in radians) and cos=-0.44807361612(in radians) means this image should be rotated by 90 degree.

Here it's my original image:

And here it's moment my result in:

Could someone help me understand what I'm doing wrong here? I really need this function to work.

I cannot use any third party libraries for this code.

Answer 1

It must handle the rotation in the same pixel format the bmp uses. You are only transforming one byte for each pixel. The pixels look to be wider. This should be easy to fix now that the problem has been identified.

If you need more speed, notice that you have invariants (x and y) that are incremented for each iteration:

for (int y = 0; y < image.height; y++) {
        double a = x - x0;
        double b = y - y0;
        int xx = (int) (+a * cos - b * sin + x0);

Move the b outside of the loop and change the multiplication to addition:

double b = -y0;
for (int y = 0; y < image.height; ++y) {
    int xx = (int) (a * cos - b + x0);
    b += sin;

Notice that the a * cos is a constant for the whole y-loop? Fuse it to the b. Do the same for x0.

double b = a * cos - y0 + x0;
for (int y = 0; y < image.height; ++y) {
    int xx = (int) (- b);
    b += sin;

Notice that the -b costs as well? Negate b.

double b = -(a * cos - y0 + x0);
for (int y = 0; y < image.height; ++y) {
    int xx = (int) b;
    b -= sin;

See what we did there? Next: get rid of the doubles. Use fixed-point. Float-to-integer conversions can be costly. At best, they are useless here.

Last but not least, you are writing into memory vertically. This is very, very bad for the write combine and will dramatically reduce the performance. Consider changing the loop order so that x-loop is the innermost one.

Extra: use tiled memory layout for the image to improve memory locality when reading. Cache will work more efficiently. Not super important here as you only process the image once and tiling would be more expensive than the speed up. But if you want to animate the rotation then tiling should benefit you. Also, with tiling the performance does not fluctuate depending on the rotation angle so the animation will be more consistent (and faster).

EDIT: add explanation how to support more bytes per pixel:

pixels[(y * image.height + x) * 3 + 0] = image.pixels[(yy * image.height + xx) * 3 + 0];
pixels[(y * image.height + x) * 3 + 1] = image.pixels[(yy * image.height + xx) * 3 + 1];
pixels[(y * image.height + x) * 3 + 2] = image.pixels[(yy * image.height + xx) * 3 + 2];

This is starting to be a bit hard to read, but you do see what we are doing there?

Answer 2

BMP newImage = image;

This sets newImage to image. The BMP structure is:

     unsigned char *pixels;

So, both newImage and image have the same pixels pointer.

Unfortunately, the logic in your rotation code assumes that newImage and image are different, independent buffers.

You will need to do more work to create your newImage object. You will need to allocate its own image buffer, clear the memory to an empty image, and set its pixels to point to it.

To properly do this, your BMP class should be made Rule Of Three compliant.

Answer 3

You could copy your pixels (buffer) to a new, separate and same type/sized one, (as said before) then rotate it using something like this:

http://www.sourcetricks.com/2012/07/rotate-matrix-by-90-degrees.html

you could then recopy the new buffer back to your original pixel buffer, clean (free) it (edit:it means the copy ) and finally redraw all.

EDIT:

I was thinking in pseudo code: 
    rotate image 
    { 
    imagecopy = duplicate (image) 
    (do the rotation on imagecopy)  
    copypixels (imagecopy (to), image) 
    free imagecopy 
    } 

copy and dup are for loops but dups adds a malloc

and "newImage.pixels = pixels;" will not work, you must loop through both arrays and copy the values one by one like in "writeBMP"

oh and by

It must handle the rotation in the same pixel format the bmp uses.

i think he meant use INTEGERS, like int or long and not float or double that complicates your code for no benifit