0

Hello I am doing an assignment for Harvard CS50 please read the directions if you want to answer the question.

Here is my code for helpers.c

#include "helpers.h"
#include <math.h>

// Convert image to grayscale
void grayscale(int height, int width, RGBTRIPLE image[height][width])
{
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            RGBTRIPLE pixel = image[i][j];

            int newgray = round((pixel.rgbtBlue + pixel.rgbtGreen + pixel.rgbtRed)/ 3.00);
            image[i][j].rgbtBlue = newgray;
            image[i][j].rgbtGreen = newgray;
            image[i][j].rgbtRed = newgray;
        }
    }
    return;
}
// Convert image to sepia
void sepia(int height, int width, RGBTRIPLE image[height][width])
{
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            RGBTRIPLE pixel = image[i][j];

            //Reassign pixel colors based on formula
            image[i][j].rgbtRed = .393 * pixel.rgbtRed + .769 * pixel.rgbtGreen + .189 * pixel.rgbtBlue;
            image[i][j].rgbtGreen = .349 * pixel.rgbtRed + .686 * pixel.rgbtGreen + .168 * pixel.rgbtBlue;
            image[i][j].rgbtBlue = .272 * pixel.rgbtRed + .534 * pixel.rgbtGreen + .131 * pixel.rgbtBlue;
        }
    }
    return;
}

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

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    return;
}

My first question is why is it that when I replace image[i][j].rgbtBlue = newgray; in the grayscale function with pixel.rgbtBlue = newgray; it doesn't work. Isn't the pixel variable literally a copy of image[i][j].

My second question lies in the reflection function where I make RGBTRIPLE temp[height][width]; (RGBTRIPLE is a data structure using 1 byte for each RGB color in the images picture) and assign it to copy the pixels of the original image. I made it so that it would copy the first half of the picture and reflect it to the other side from the original image then from the copy (temp) I would copy the second half and paste it in the first half of the original image. Why does the first half (left half) come up as black?

Input: image, terminal command: ./filter -r tower.bmp outfile.bmp (tower.bmp is the input image and outfile is the output image)

Output: image

  • "Isn't the pixel variable literally a copy of image[i][j]". No, it is a copy of the value of image[i][j]. Modifying pixel won't modify image[i][j] – debanshu das May 12 '21 at 17:59
  • Does this answer your question? [What's the difference between passing by reference vs. passing by value?](https://stackoverflow.com/questions/373419/whats-the-difference-between-passing-by-reference-vs-passing-by-value) (Specifically the 2nd answer there) – Siguza May 12 '21 at 18:01
  • If the pixels come black, it is because they are getting set to 0 (atleast, that's the standard) – debanshu das May 12 '21 at 18:03
  • 1
    You dont need an entire array for temp. Just use one variable. – debanshu das May 12 '21 at 18:06
  • Are you sure that C has 2D arrays? Last time I read [n1570](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf) it did not have them (just arrays of arrays) – Basile Starynkevitch May 12 '21 at 18:12
  • `image[i][j].rgbtRed = .393 * pixel.rgbtRed + .769 * pixel.rgbtGreen + .189 * pixel.rgbtBlue;` is suspicious as `.393 * pixel.rgbtRed + .769 * pixel.rgbtGreen + .189 * pixel.rgbtBlue;` may exceed 255.0. – chux - Reinstate Monica May 12 '21 at 21:52

2 Answers2

1

There are a few issues with reflect().

  1. Only one element of temp is used at a time and never reused. Thus there is no need to create a potentially huge instance on stack.
  2. Index reflected_j = width - j for j equal to 0 will be equal width what is out of bounds for array of length width. This invokes UB. It should be reflected_j = width - j - 1
  3. It would be better to swap pixels while reflecting the image, not to play with half-copy.

Updated code:

void reflect(int height, int width, RGBTRIPLE image[height][width]) {
    for (int y = 0; y < height; y++) {
        for (int i = 0, j = width - 1; i < j; i++, j--) {
            RGBTRIPLE temp = image[y][i];
            image[y][i] = image[y][j];
            image[y][j] = temp;
        }
    }
}
tstanisl
  • 13,520
  • 2
  • 25
  • 40
0

As for your first question, it is because pixel is only a value copy of image[i][j]. It has a separate memory location of it's own, so modifying it wont change the image[i][j].
As for your second question it is because this line

image[i][reflected_j] = image[i][j];

should be

image[i][j]=image[i][reflected_j]

and this line

image[i][j] = temp[i][j];

should be

image[i][reflected_j]=temp[i][j]
debanshu das
  • 141
  • 2
  • 9