cs50 week4 filter(less comfortable) blur and sepia function

Question

#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++)
        {
            image[i][j].rgbtGreen = round((image[i][j].rgbtGreen + image[i][j].rgbtBlue + image[i][j].rgbtRed) / 3.0);
            image[i][j].rgbtBlue = image[i][j].rgbtGreen;
            image[i][j].rgbtRed = image[i][j].rgbtGreen;
        }
    }
    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++)
        {
            //get values according to the given equasion.//

            int sepiaRed = round(.393 * image[i][j].rgbtRed + .769 * image[i][j].rgbtGreen + .189 * image[i][j].rgbtBlue);
            int sepiaGreen = round(.349 * image[i][j].rgbtRed + .686 * image[i][j].rgbtGreen + .168 * image[i][j].rgbtBlue);
            int sepiaBlue = round(.272 * image[i][j].rgbtRed + .534 * image[i][j].rgbtGreen + .131 * image[i][j].rgbtBlue);
            // update value for each color, the if checks if it exceeds 255 to cap the value to 255. //
            image[i][j].rgbtGreen = sepiaGreen;
            if (image[i][j].rgbtGreen > 255)
            {
                image[i][j].rgbtGreen = 255;
            }

            image[i][j].rgbtBlue = sepiaBlue;
            if (image[i][j].rgbtBlue > 255)
            {
                image[i][j].rgbtBlue = 255;
            }

            image[i][j].rgbtRed = sepiaRed;
            if (image[i][j].rgbtRed > 255)
            {
                image[i][j].rgbtRed = 255;
            }
        }
    }
    return;
}

// Reflect image horizontally
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++)
        {
            RGBTRIPLE temp = image[i][j];
            image[i][j] = image[i][width - j - 1];
            image[i][width - j - 1] = temp;
        }
    }
    return;
}

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE copy[height][width];
    int blue = 0;
    int red = 0;
    int green = 0;
    int counter = 0;
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            copy[i][j] = image[i][j];
        }
     }
     for (int i = 0; i < height; i++)
     {
        for (int j = 0; j < width; j++)
        {
            blue = 0;
            red = 0;
            green = 0;
            counter = 0;
            for (int s = i - 1; s < i + 2; s++)
            {
                for (int m = j - 1; m < j + 2; m++)
                {
                    //this condition will make sure if a grid is not in the loop it will not be included(handeling edgee case).//
                    if (s < 0 || m < 0 || s > height - 1 || m > width - 1)
                    {
                        continue;
                    }
                    else
                    {
                        blue += copy[s][m].rgbtBlue;
                        green += copy[s][m].rgbtGreen;
                        red += copy[s][m].rgbtRed;
                        counter++;
                    }
                }
            }
            image [i][j].rgbtBlue = round(blue / counter);
            image [i][j].rgbtGreen = round(green / counter);
            image [i][j].rgbtRed = round(red / counter);
        }
    }
    return;
}

I can't pass the test for function blur in the middle pixel. corners and edges I passed it. In the sepia function I cant seem to pass the more complex 33 image and 44 image (I passed the simple 3*3). What could be the problem? In one test the difference in values is as small as 1 but others it is like 100.

Questions asking for debugging help without any evidence of effort on the part of the programmer are liable to be closed rather quickly. Last time I looked, the CS50 development environment included an excellent debugger. Now would be a good time to learn how to use it. — r3mainer, Sep 28 '22 at 16:26
`round(blue / counter)` won't round it, because it was already truncated by the integer division. Try `round((double)blue / counter)` — Weather Vane, Sep 28 '22 at 16:38
`if (image[i][j].rgbtGreen > 255)` This won't work. The fields in that struct can only hold 8 bits, i.e. values up to 255. If you add up values to a larger value that will already be chopped when you assign it back to that field. You must use another variable with a larger range for doing the math and only then check for range. — Gerhardh, Sep 28 '22 at 17:20
If you have the test data of the failing test, your first step should be to feed that data into your function and step through each instruction in a debugger. Then you can inspect the variables' contents with their expected values. And of course you should also provide these test cases to use within your question. Don't just describe that "some test if only off by 1..." Show us: What test, what value, what expected value? — Gerhardh, Sep 28 '22 at 17:54
Please also describe which function produces values off-by-1 and which produces results way off. My guess: Wrong rounding is the reason for that off-by-one issue. Overrun of range can cause tha case with larger error. But we don't have magic chrystal balls and generally don't like wild guessing. — Gerhardh, Sep 28 '22 at 17:58
When I solved this problem, I forgot that in C you should divide not by e.g. 4, but 4.0 - it brought so much pain to find out what the hell was wrong with my code... — Black Beard, Feb 24 '23 at 13:18

chqrlie · Answer 1 · 2022-09-30T18:10:00.433

There are multiple problems:

In the sepia test, you check if the computed component exceeds 255 after storing it to the RGBTRIPLE member. This does not work because the members have type BYTE which cannot only hold values from 0 to 255 so any values beyond this range will have been reduced modulo 256 already.

Here is a modified version:

// 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++) {
            // get values according to the given equation. //
            int sepiaRed   = round(.393 * image[i][j].rgbtRed + .769 * image[i][j].rgbtGreen + .189 * image[i][j].rgbtBlue);
            int sepiaGreen = round(.349 * image[i][j].rgbtRed + .686 * image[i][j].rgbtGreen + .168 * image[i][j].rgbtBlue);
            int sepiaBlue  = round(.272 * image[i][j].rgbtRed + .534 * image[i][j].rgbtGreen + .131 * image[i][j].rgbtBlue);

            // cap the component values to 255. //
            if (sepiaRed   > 255)
                sepiaRed   = 255;
            if (sepiaGreen > 255)
                sepiaGreen = 255;
            if (sepiaBlue  > 255)
                sepiaBlue  = 255;

            // update the pixel values //
            image[i][j].rgbtRed   = sepiaRed;
            image[i][j].rgbtGreen = sepiaGreen;
            image[i][j].rgbtBlue  = sepiaBlue;
        }
    }
}

For the blur function, the way you average the component values is incorrect: you use integer arithmetics in red / counter so round does not actually do anything. You should either implement rounding using integer arithmetics or use floating point arithmetics in the computation for example by defining counter as a double.

Here is a modified version:

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width]) {
    RGBTRIPLE copy[height][width];
    for (int i = 0; i < height; i++) {
        for (int j = 0; j < width; j++) {
            copy[i][j] = image[i][j];
        }
     }
     for (int i = 0; i < height; i++) {
        for (int j = 0; j < width; j++) {
            int blue  = 0;
            int red   = 0;
            int green = 0;
            double counter = 0;
            for (int s = i - 1; s < i + 2; s++) {
                // check for top and bottom edge //
                if (s < 0 || s >= height)
                    continue;
                for (int m = j - 1; m < j + 2; m++) {
                    // check for left and right edge //
                    if (m < 0 || m >= width)
                        continue;
                    red   += copy[s][m].rgbtRed;
                    green += copy[s][m].rgbtGreen;
                    blue  += copy[s][m].rgbtBlue;
                    counter += 1;
                }
            }
            image[i][j].rgbtRed   = round(red   / counter);
            image[i][j].rgbtGreen = round(green / counter);
            image[i][j].rgbtBlue  = round(blue  / counter);
        }
    }
}

cs50 week4 filter(less comfortable) blur and sepia function

1 Answers1