2

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.

Gerhardh
  • 11,688
  • 4
  • 17
  • 39
thefailagent
  • 73
  • 7
  • 2
    Assume you enter the inner loop for the first time, j=0. You set a value of `coloursofaddress[width - 1]`and then immediately use `coloursofaddress[0]`, which wasn't set to anything in particular. – n. m. could be an AI Aug 18 '20 at 04:09
  • 2
    Note: only 1 line needed, not 3: `coloursofaddress[width - 1 - j] = image[i][j];` – chux - Reinstate Monica Aug 18 '20 at 04:20
  • If I use the one line instead of 3, will all the colours will be set by themselves? This is me having to get at the individual red/green/blue colours of the structure RGBTRIPLE set by the problem set. – thefailagent Aug 18 '20 at 04:43
  • @n.'pronouns'm. - Oh I see what you are saying. But the pixel colour should eventually "correct" itself as it iterates over the whole row. So when j = 0; lets say width is 10, causing the first ever pixel to be the tenth element of the array. But the 1st element of the array is not set yet, and no colour change is assigned to first pixel. So eventually the last pixel of that row will set itself to the first element of the array, and then coloursofaddress[0] will have a value of the (last pixel). Doesn't that have the same effect? – thefailagent Aug 18 '20 at 04:59
  • Your edit states the result is close to correct. Is it really mirrored at all? If you run the whole range from left to right you should see same image as before because you swap twice. Or do something like swapping values twice. – Gerhardh Aug 18 '20 at 05:19
  • 1
    Your idea is doable (but not optimally efficient), and indeed you need to iterate the whole line, not only half: You fill a buffer line of pixels with the pixels in a line of the image, in reverse order, line by line, and write it back. Your only mistake is that you should **fill the entire buffer line first** and copy it back only then. This will be achieved by lifting the step 4 out of the inner loop into its own loop. As it stands, you "almost reverse" the image, shifted by half a line, only by accident, as explained by Gerhardt. – Peter - Reinstate Monica Aug 18 '20 at 05:41
  • Hi Gerhardh. Yes, when I run my code through ./filter, I see the reflected image. I use the whole range of width because I'm technically only swapping once, twice (if that makes sense) - 1st pixel is indexed to 10th element in a 10-element width array and as the loop goes to the last pixel, it is indexed to the 1st element. It's hard to explain but it works, save for the extra row of pixels in the edit – thefailagent Aug 18 '20 at 05:43
  • Another remark is that you can define a pointer to each row in the outer loop which you can iterate with just one index; depending on your compiler and optimization levels that may be more efficient. – Peter - Reinstate Monica Aug 18 '20 at 05:47
  • Hi Peter, thank you for your comment. I think I get what you mean. I will try this out and get back to you soon. – thefailagent Aug 18 '20 at 05:47
  • 1
    "But the pixel colour should eventually "correct" itself" No. You only touch each pixel once, so once a pixel is set to a wrong value, it is stuck there forever. – n. m. could be an AI Aug 18 '20 at 06:13
  • 1
    Your resulting image is very similar to the desired reflected image, but not identical, because half of it is shifted one scanline down. In order to understand what's going on, I recommend you try to run a small (say 10x10) image through your program, and *print* both source and target pixels (as numbers) so you could easily compare the two. – n. m. could be an AI Aug 18 '20 at 06:20
  • Rolled back previous version. Vandalizing the question by removing all the code is not considered acceptable. If your problem is solved by an provided answer, you can accept that answer. If you solved it differently, you can post your own answer. – Gerhardh Aug 18 '20 at 08:11

2 Answers2

2

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.

Gerhardh
  • 11,688
  • 4
  • 17
  • 39
0

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.

thefailagent
  • 73
  • 7
  • Using a temp copy of the whole array is a terrible idea if there is no good reason to do so. I assume, my solution works for you but you didn't take enough time to understand it. Correct? – Gerhardh Aug 20 '20 at 06:46
  • Thank you for taking the time to explain this to me! Yes, it makes sense to me now and is much more efficient than the weird code I had made. – thefailagent Aug 26 '20 at 08:25