CS50 Filter - Blur

Question

Good day,

I have written the code for CS50 blur filter and I understand there's a better way to code it (with less if statements), but before rewriting I would like to understand why it doesn't work at the moment. It produces a very dark image, it seems the RGB values are too low. I am stuck and would gladly welcome some help. Thank you.

void blur(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; j++)
        {
            int counter = 1;
            
            temp[i][j].rgbtRed = image[i][j].rgbtRed;
            temp[i][j].rgbtGreen = image[i][j].rgbtGreen;
            temp[i][j].rgbtBlue = image[i][j].rgbtBlue;
            
            if (i != height - 1)
            {
                temp[i][j].rgbtRed += image[i+1][j].rgbtRed;
                temp[i][j].rgbtGreen += image[i+1][j].rgbtGreen;
                temp[i][j].rgbtBlue += image[i+1][j].rgbtBlue;
                counter++;
            }
            if (i != 0)
            {
                temp[i][j].rgbtRed += image[i-1][j].rgbtRed;
                temp[i][j].rgbtGreen += image[i-1][j].rgbtGreen;
                temp[i][j].rgbtBlue += image[i-1][j].rgbtBlue;
                counter++;
            }
            if (j != width - 1)
            {
                temp[i][j].rgbtRed += image[i][j+1].rgbtRed;
                temp[i][j].rgbtGreen += image[i][j+1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i][j+1].rgbtBlue;
                counter++;
            }
            if (j != 0)
            {
                temp[i][j].rgbtRed += image[i][j-1].rgbtRed;
                temp[i][j].rgbtGreen += image[i][j-1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i][j-1].rgbtBlue;
                counter++;
            }
            if (i != 0 && j != 0)
            {
                temp[i][j].rgbtRed += image[i-1][j-1].rgbtRed;
                temp[i][j].rgbtGreen += image[i-1][j-1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i-1][j-1].rgbtBlue;
                counter++;
            }
            if (i != height - 1 && j != width - 1)
            {
                temp[i][j].rgbtRed += image[i+1][j+1].rgbtRed;
                temp[i][j].rgbtGreen += image[i+1][j+1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i+1][j+1].rgbtBlue;
                counter++;
            }
            if (i != height - 1 && j != 0)
            {
                temp[i][j].rgbtRed += image[i+1][j-1].rgbtRed;
                temp[i][j].rgbtGreen += image[i+1][j-1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i+1][j-1].rgbtBlue;
                counter++;
            }
            if (i != 0 && j != width - 1)
            {
                temp[i][j].rgbtRed += image[i-1][j+1].rgbtRed;
                temp[i][j].rgbtGreen += image[i-1][j+1].rgbtGreen;
                temp[i][j].rgbtBlue += image[i-1][j+1].rgbtBlue;
                counter++;
            }
            image[i][j].rgbtRed = round(temp[i][j].rgbtRed / (counter * 1.0));
            image[i][j].rgbtGreen = round(temp[i][j].rgbtGreen / (counter * 1.0));
            image[i][j].rgbtBlue = round(temp[i][j].rgbtBlue / (counter * 1.0));
           
        }
    }
    
    
    return;
}

What have you done to verify a) what these values should be and b) what your code is doing to produce the wrong values? — Scott Hunter, Aug 13 '21 at 15:36
Try an image that is all one color, say [100,100,100] for simple math, and examine your intermediate results in a debugger or print them out to see if they match your expectations. — Retired Ninja, Aug 13 '21 at 15:40
With `temp[i][j].rgbtRed += image[i+1][j].rgbtRed;` etc you are overflowing the (probably) 8-bit integer. a) Copy the *whole* image to `temp[][]` first (or the other way afterwards), b) Use a separate `int` or `float` variable, and write the *average* into the image array. — Weather Vane, Aug 13 '21 at 15:59
There is no need the copy the whole 2D array. You don't need such a large array for temp, you only need a single element of that 2D array. — 12431234123412341234123, Aug 13 '21 at 18:17
Expanding on 124...'s comment, if you define a VLA for `temp`, if `width` and `height` are large enough, then `temp` could overflow the stack. You only need a _single_ pixel: `RGBTRIPLE temp;` Also, by refactoring, the loops can be simplified. See my cs50 blur answer: https://stackoverflow.com/questions/62330831/cs50x-filter-blur-receiving-a-runtime-error-on-first-nested-else-state-s/62331838#62331838 — Craig Estey, Aug 13 '21 at 18:37

chux - Reinstate Monica · Accepted Answer · 2021-08-14T12:03:00.333

why it doesn't work at the moment. It produces a very dark image

The sum formed via temp[i][j].rgbtRed += image[i+1][j].rgbtRed; and the like is overflowing temp[i][j].rgbtRed. @Weather Vane. The later division lowers the truncated sum toward black.

Use a wider type for the sum.

// RGBTRIPLE temp[height][width];
int r,g,b;  // If temp[i][j].rgbtRed width less than int, else use long, etc.

for (int i = 0; i < height; i++) {
    for (int j = 0; j < width; j++) {
        ...
        
            // temp[i][j].rgbtRed += image[i+1][j].rgbtRed;
            // temp[i][j].rgbtGreen += image[i+1][j].rgbtGreen;
            // temp[i][j].rgbtBlue += image[i+1][j].rgbtBlue;
            r += image[i+1][j].rgbtRed;
            g += image[i+1][j].rgbtGreen;
            b += image[i+1][j].rgbtBlue;

Untested candidate simplification:

void blur(int height, int width, RGBTRIPLE image[height][width]) {
  for (int i = 0; i < height; i++) {
    for (int j = 0; j < width; j++) {
      int r = 0;
      int g = 0;
      int b = 0;
      int counter = 0;
      for (int di = max(i - 1, 0); di < min(i + 2, height); di++) {
        for (int dj = max(j - 1, 0); dj < min(j + 2, width); dj++) {
          r += image[di][dj].rgbtRed;
          g += image[di][dj].rgbtGreen;
          b += image[di][dj].rgbtBlue;
          counter++;
        }
      }
      // simplified rounding
      image[i][j].rgbtRed = (r + counter / 2) / counter;
      image[i][j].rgbtGreen = (g + counter / 2) / counter;
      image[i][j].rgbtBlue = (b + counter / 2) / counter;
    }
  }
}

CS50 Filter - Blur

1 Answers1