pset4 blur; artifacts on a white parts of an image

Question

I believe I've done this part of filter-more task pretty well, but, unfortunately, an output image has some artifacts - blur works badly with white parts of an image.

void blur(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE *grid[3][3];
    long averageB, averageG, averageR;

    // Checks If grid's pixels exist
    for(int y = 0; y < height; y++)
    {
      for(int x = 0; x < width; x++)
      {
        int n = 1;

        if(y - 1 >= 0 && x - 1 >= 0)
        {
            n++;
            grid[0][0] = &image[y - 1][x - 1];
        }
        else
            grid[0][0] = NULL;

        if(y - 1 >= 0)
        {
            n++;
            grid[0][1] = &image[y - 1][x];
        }
        else
            grid[0][1] = NULL;

        if(y - 1 >= 0 && x + 1 <= width - 1)
        {
            n++;
            grid[0][2] = &image[y - 1][x + 1];
        }
        else
            grid[0][2] = NULL;

        if(x - 1 >= 0)
        {
            n++;
            grid[1][0] = &image[y][x-1];
        }
        else
            grid[1][0] = NULL;

        grid[1][1] = &image[y][x];

        if(x + 1 <= width - 1)
        {
            n++;
            grid[1][2] = &image[y][x + 1];
        }
        else
            grid[1][2] = NULL;

        if(y + 1 <= height - 1 && x - 1 >= 0)
        {
            n++;
            grid[2][0] = &image[y + 1][x  - 1];
        }
        else
            grid[2][0] = NULL;

        if(y + 1 <= height - 1)
        {
            n++;
            grid[2][1] = &image[y + 1][x];
        }
        else
            grid[2][1] = NULL;

        if(y + 1 <= height - 1 && x + 1 <= width - 1)
        {
            n++;
            grid[2][2] = &image[y + 1][x + 1];
        }
        else
            grid[2][2] = NULL;


        // Cacluclate an average for every colour
        for(int a = 0; a < 3; a++)
        {
            for(int b = 0; b < 3; b++)
            {
                if(grid[a][b] != NULL)
                {
                     averageB += (*grid[a][b]).rgbtBlue;
                     averageG += (*grid[a][b]).rgbtGreen;
                     averageR += (*grid[a][b]).rgbtRed;
                }
            }
        }

        averageB = averageB / n;
        averageG = averageG / n;
        averageR = averageR / n;


        grid[1][1]->rgbtBlue = averageB;
        grid[1][1]->rgbtGreen = averageG;
        grid[1][1]->rgbtRed = averageR;

      }
    }

    return;
}

d

Answer 1

You are not capping the value of the average to 255 as you are supposed to. You need to take the final value to be the minimum of the average for the color and of 255. otherwise you are going to get overbright artifacts.

I also would not set values of grid to NULL but to a single element that represents black. this will better serve you in the future

not a major problem or incorrectness i would just build up your values for the average from the start and avoid creating grid entirely. also I would just use two nested for loops with proper boundary conditions using min and max instead of the half a dozen corner cases you code for in the if's.

The other major problem is that you are not making a copy of the unfiltered source image to source from as you do the aggregation/filter work. The result is that as you modify the image by applying the blur filter other cells values are then calculated not on the original image but on the partially updated blurred image. Put another way, the order in which you are processing the cells ends up mattering but it should not.. If you started from the bottem right instead of top left the values would be different but this should be such that it could all be done in parallel: no pixel's updated value depends on the updated value of any other pixel they should all be based on the initial state.

If you dont understand what I'm getting at, consider just two pixels as if you had a height of 1 and rows of 2. you calculate the value of image[0][0] before image[0,1] and when you calculate image[0,1]'s value you are not using the original value of image[0][0] in the computation you are using the updated value. That's incorrect.

Each pixel of the blurred image should be computed from the initial values of its neighbor pixels. This kind of shoots a major hole in your scheme of finalizing a pixel update through assignment via the pointer you put into grid at the end. You should be assigning into a new image to be copied back over the input image upon completion OR by copying the image to a temp array first and basing your grid pointers off of the temp copy and having your assignment at the end of your loop pass assign into the target pixel of the image.

Let me demonstrate briefly:

//copy to work from
RGBTriple temp[height][width];

for (int i = 0; i < height; i++)
{
    for (int j = 0; j < column; j++)
    {
        temp[i][j] = image[i][j]; // copy over each pixel we could 
        // note we could copy the row all at once and remove this loop by using memcpy 
        // if we wanted to. but i wont get into that.
    }

    // Now use your code as you had
    // but assign grid[a][b] from &temp[a][b] instead of &image[a][b].

    // your logic for the bulk of the algorithm

    // finally your assignment
    // instead of grid[1][1] use image[y][x]
    image[y][x]->rgbtBlue = averageB;
    image[y][x]->rgbtGreen = averageG;
    image[y][x]->rgbtRed = averageR;
    // now image is modified but the filtration algorithm passes wont see the new values as they should all be independent of one another.
}

Answer 2

I wrote my algorithm as @UpAndAdam said. It's much easier to read, looks cleaner and now computes new pixels' values from an input image, not from modified image.

void blur(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE tempimage[height][width];
    for(int y = 0; y < height; y++)
    {
        for(int x = 0; x < width; x++)
        {
            int n = 0, sumB = 0, sumG = 0, sumR = 0;

            for(int a = -1; a <= 1; a++)
            {
                for(int b = -1; b <= 1; b++)
                {
                  if((y + a >= 0 &&  y + a <= height - 1) && (x + b >= 0 && x + b <= width - 1))
                  {
                      n++;
                      sumB += image[y + a][x + b].rgbtBlue;
                      sumG += image[y + a][x + b].rgbtGreen;
                      sumR += image[y + a][x + b].rgbtRed;
                  }

                }
            }
            tempimage[y][x].rgbtBlue = fmin(255, round(sumB / n));
            tempimage[y][x].rgbtGreen = fmin(255, round(sumG / n));
            tempimage[y][x].rgbtRed = fmin(255, round(sumR / n));
        }
    }

    for(int z = 0; z < height; z++)
    {
         memcpy(image[z], tempimage[z], sizeof(RGBTRIPLE) * width);
    }

    return;
}