why is my CS50 filter edges code not working with check50?

Question

My cs50 filter edges function is not working, it compiles ok but when i run check50 the first test (edges correctly filters middle pixel) us correct while the others are incorrect just by the last value, like this:

:( edges correctly filters pixel on edge expected "213 228 255\n", not "213 228 140\n"

However, when I print the gx and gy for the red, green and blue alone, and the value of the squareroot, none of the values for the colors match.

now, this is my code for edges

void edges(int height, int width, RGBTRIPLE image[height][width])
{
    int sr = 0;
    int sb = 0;
    int sg = 0;
    int yr = 0;
    int yb = 0;
    int yg = 0;
    struct RGBTRIPle
    {
        int rgbtRed;
        int rgbtGreen;
        int rgbtBlue;
    };
    struct RGBTRIPle copia[height][width];
    struct RGBTRIPLe
    {
        int rgbtRed;
        int rgbtGreen;
        int rgbtBlue;
    };
    struct RGBTRIPLe copia2[height][width];
    //Implementing Gx
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            sr = 0;
            sb = 0;
            sg = 0;
            for (int m = i - 1; m <= i + 1; m++)
            {
                for (int c = j - 1; c <= j + 1; c++)
                {

                    if (m >= 0 && m < height && c >= 0 && c < width)
                    {

                        if (c == j - 1)
                        {
                            if (m == i - 1 || m == i + 1)
                            {
                                sr += -1 * image[m][c].rgbtRed;
                                sb += -1 * image[m][c].rgbtBlue;
                                sg += -1 * image[m][c].rgbtGreen;
                            }
                            else
                            {
                                sr += -2 * image[m][c].rgbtRed;
                                sb += -2 * image[m][c].rgbtBlue;
                                sg += -2 * image[m][c].rgbtGreen;
                            }

                        }
                        if (c == j + 1)
                        {
                            if (m == i - 1 || m == i + 1)
                            {
                                sr += image[m][c].rgbtRed;
                                sb += image[m][c].rgbtBlue;
                                sg += image[m][c].rgbtGreen;
                            }
                            else
                            {
                                sr += 2 * image[m][c].rgbtRed;
                                sb += 2 * image[m][c].rgbtBlue;
                                sg += 2 * image[m][c].rgbtGreen;
                            }

                        }
                        else //c = j
                        {
                            sr += 0 * image[m][c].rgbtRed;
                            sb += 0 * image[m][c].rgbtBlue;
                            sg += 0 * image[m][c].rgbtGreen;
                        }

                    }
                }
            }
            copia[i][j].rgbtRed = sr;
            copia[i][j].rgbtGreen = sg;
            copia[i][j].rgbtBlue = sb;
        }

    }
    //Implementing Gy
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            yr = 0;
            yb = 0;
            yg = 0;
            for (int m = i - 1; m <= i + 1; m++)
            {
                for (int c = j - 1; c <= j + 1; c++)
                {

                    if (m >= 0 && m < height && c >= 0 && c < width)
                    {

                        if (m == i - 1)
                        {
                            if (c == j - 1 || c == j + 1)
                            {
                                yr += -1 * image[m][c].rgbtRed;
                                yb += -1 * image[m][c].rgbtBlue;
                                yg += -1 * image[m][c].rgbtGreen;
                            }
                            else
                            {
                                yr += -2 * image[m][c].rgbtRed;
                                yb += -2 * image[m][c].rgbtBlue;
                                yg += -2 * image[m][c].rgbtGreen;
                            }

                        }
                        if (m == i + 1)
                        {
                            if (c == j + 1 || c == j - 1)
                            {
                                yr += image[m][c].rgbtRed;
                                yb += image[m][c].rgbtBlue;
                                yg += image[m][c].rgbtGreen;
                            }
                            else
                            {
                                yr += 2 * image[m][c].rgbtRed;
                                yb += 2 * image[m][c].rgbtBlue;
                                yg += 2 * image[m][c].rgbtGreen;
                            }

                        }
                        else //c = j
                        {
                            yr += 0 * image[m][c].rgbtRed;
                            yb += 0 * image[m][c].rgbtBlue;
                            yg += 0 * image[m][c].rgbtGreen;
                        }
                    }
                }
            }
            copia2[i][j].rgbtRed = yr;
            copia2[i][j].rgbtGreen = yg;
            copia2[i][j].rgbtBlue = yb;
        }
    }
    //Implementing math operation to calculate resulting color
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            int r = 0;
            int g = 0;
            int b = 0;
            image[i][j].rgbtRed = (int) round(sqrt((copia[i][j].rgbtRed * copia[i][j].rgbtRed) + (copia2[i][j].rgbtRed *
                                                   copia2[i][j].rgbtRed)));
            image[i][j].rgbtGreen = (int) round(sqrt((copia[i][j].rgbtGreen * copia[i][j].rgbtGreen) + (copia2[i][j].rgbtGreen *
                                                copia2[i][j].rgbtGreen)));
            image[i][j].rgbtBlue = (int) round(sqrt((copia[i][j].rgbtBlue * copia[i][j].rgbtBlue) + (copia2[i][j].rgbtBlue *
                                                    copia2[i][j].rgbtBlue)));
            r = image[i][j].rgbtRed;
            g = image[i][j].rgbtGreen;
            b = image[i][j].rgbtBlue;

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

Axolotl Axolotl · Accepted Answer · 2020-06-18T15:38:16

The problem you describe arises when you store round(sqrt((copia[i][j].rgbtRed * copia[i][j].rgbtRed) + (copia2[i][j].rgbtRed *copia2[i][j].rgbtRed))); into the variable image[i][j].rgbtRed(or any other variant thereof). This is because when calculating sqrt(gx^2 + gy^2) you are getting a number above 255. For example, you may get the integer value 395 after rounding. To store that value in to image[i][j].rgbtRed, C will store the value of 395 % 255, or 140, because the image cannot store values greater than 255, by definition.

This means that your if statements are useless, because the respective color values will never be greater than 255:

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

To solve this problem you have to cap the value before storing them into the image. A simple implementation of this would be by making a function called cap that returns 255 if an input is above 255.:

int cap(int rgb)
{
    if (rgb > 255)
    {
        return 255;
    }
    else
    {
        return rgb;
    }
}

You can then use this function in the following manner, which will solve your problem completely:

image[i][j].rgbtRed =  cap(round(sqrt((copia[i][j].rgbtRed * copia[i][j].rgbtRed) + (copia2[i][j].rgbtRed * copia2[i][j].rgbtRed))));
image[i][j].rgbtGreen = cap(round(sqrt((copia[i][j].rgbtGreen * copia[i][j].rgbtGreen) + (copia2[i][j].rgbtGreen * copia2[i][j].rgbtGreen))));
image[i][j].rgbtBlue = cap(round(sqrt((copia[i][j].rgbtBlue * copia[i][j].rgbtBlue) + (copia2[i][j].rgbtBlue * copia2[i][j].rgbtBlue))));

This will also shorten your code, make it look cleaner, and avoid unnecessary repetition.

why is my CS50 filter edges code not working with check50?

1 Answers