Cs50's Problem Set 4 - Filter Less - Reflection Function

Question

I have written the function to reflect an images that were provided in the zip file as .bmps.

Upon some research, I've seen that many people who have solved this problem divided the width in the image by 2. However, I felt this wasn't applicable to my code.

The code does reflect the image as seen by eye but it does not meet any of the criteria provided by check50.

If width/2 is indeed necessary, even in my code, please do explain the logic behind implementing this in my instance of code:

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE coloursofaddress[width];
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            // Step 3
            coloursofaddress[width - 1 - j].rgbtRed = image[i][j].rgbtRed;
            coloursofaddress[width - 1 - j].rgbtGreen = image[i][j].rgbtGreen;
            coloursofaddress[width - 1 - j].rgbtBlue = image[i][j].rgbtBlue;
            
            // Step 4
            image[i][j].rgbtRed = coloursofaddress[j].rgbtRed;
            image[i][j].rgbtGreen = coloursofaddress[j].rgbtGreen;
            image[i][j].rgbtBlue = coloursofaddress[j].rgbtBlue;
        }
    }
    return;
}

The following are the error messages:

:( reflect correctly filters 1x2 image
    expected "0 0 255\n255 0...", not "5 0 0\n255 0 0..."
:( reflect correctly filters 1x3 image
    expected "0 0 255\n0 255...", not "5 0 0\n0 255 0..."
:( reflect correctly filters image that is its own mirror image
    expected "255 0 0\n255 0...", not "5 0 0\n255 0 0..."
:( reflect correctly filters 3x3 image
    expected "70 80 90\n40 5...", not "5 0 0\n40 50 6..."
:( reflect correctly filters 4x4 image
    expected "100 110 120\n7...", not "5 0 0\n0 0 0\n..." 

EDIT: Upon close further examination of the reflected image output by the above code, everything seems to be perfect except for a thin additional row(s?) of pixels with random colours - not sure why. It was visible by zooming in on the bottom part of the picture, but is not visible in the input .bmp.

EDIT 2: Upon even closer examination of the reflected image output, the line of "stray" pixels at the bottom seem to end smack in the middle. It seems to be the entire left half of the reflected image is 1 pixel unit higher than it's supposed to be, and the right half is just fine. A line showing this clear height division is faintly visible going down the center of the picture. This is starting to get funny.

Answer 1

You have two problems.

In step 4 half of the array has not been assigned a value yet. You use indetermined values for first row and old values for the other rows. This will look like shifting half of the image by 1 row.

You should use a standard swap mechanism to swap values. As mentioned in comments you don't need to bother with the separate fields but just swap the whole structure:

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE temp;
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            temp = image[i][j];
            image[i][width - 1 - j] = image[i][j];
            image[i][j] = temp;
        }
    }
    return;
}

Additionally, if you run the swapping the whole range of 0..width-1 you will end up swapping image[i][0] with image[i][width-1] and later image[i][width-1] with image[i][0] which swaps same pixels twice resulting in the original image.

Just run your loop with condition j < width/2:

// define a SWAP macro just for convenience during following explanation
#define SWAP(x,y)   \
  do {              \
    RGBTRIPLE temp = (x); \
    (x) = (y);      \
    (y) = temp;     \
  } while (0)


void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width/2; j++)
        {
            // Replace image[i] to make code shorter during explanation.
            RGBTRIPLE *row=image[i];
            SWAP(row[j], row[width - 1 - j]);
        }
    }
    return;
}

I did 2 replacements that are not really required but make the following text a lot shorter.

Why looping only until j < width/2 ? Let's look at the memory in each iteration if we want to revers an array of integers:

width==15
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     | 0| 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13|14|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

j=0; SWAP(row[0],row[14]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14| 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

...

j=5; SWAP(row[5],row[9]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14|13|12|11|10| 9| 6| 7| 8| 5| 4| 3| 2| 1| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

j=6; SWAP(row[6],row[8]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14|13|12|11|10| 9| 8| 7| 6| 5| 4| 3| 2| 1| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

j=7; SWAP(row[7],row[7]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14|13|12|11|10| 9| 8| 7| 6| 5| 4| 3| 2| 1| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

You can see that after swapping half of the elements, we are done. For odd number of elements, the last swap will just swap iteself having no effect.

If you now continue running up to width, you swap back again:

j=8; SWAP(row[8],row[6]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14|13|12|11|10| 9| 6| 7| 8| 5| 4| 3| 2| 1| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

...

j=13; SWAP(row[13],row[1]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     |14| 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13| 0|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

j=14; SWAP(row[14],row[0]);
row: +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
     | 0| 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13|14|
     +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

This is where we started.

It is obvious that we run SWAP(row[j],row[width-j-1]); and later the same with swapped index: SWAP(row[width-j-1],row[j]); which does the same.

Swapping twice restores the initial state and your image will not be changed.

Answer 2

I solved the issue with my (not-so-efficient) code! No need for width / 2. Used a temporary array and a for loop to swap the pixel colours of the left with the right! I don't really understand the width / 2 method as shown in other answers so this method works for me as I understand it.