CS50 pset4 - filter (less comfortable), sepia function

Question

I wrote code for the sepia function of the filter pset. The code compiles without any errors but the output image is not entirely sepia. However, after I wrote the code in different way it works fine. Can someone explain why this happens?

First code:

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={0,0,0};
            RGBTRIPLE pix = image[i][j];

            pixel.rgbtBlue = round(0.272 * pix.rgbtRed + 0.534 * pix.rgbtGreen + 0.131 * pix.rgbtBlue);
            pixel.rgbtGreen = round(0.349 * pix.rgbtRed + 0.686 * pix.rgbtGreen + 0.168 * pix.rgbtBlue);
            pixel.rgbtRed = round(0.393 * pix.rgbtRed + 0.769 * pix.rgbtGreen + 0.189 * pix.rgbtBlue);

            if(pixel.rgbtBlue > 255)
            {
                pixel.rgbtBlue = 255;
            }
             if(pixel.rgbtGreen > 255)
            {
                pixel.rgbtGreen = 255;
            }

             if(pixel.rgbtRed > 255)
            {
                pixel.rgbtRed = 255;
            }

            image[i][j] = pixel;

        }
    }
    return;
}

first code output image:

first code output image

Second Code:

void sepia(int height, int width, RGBTRIPLE image[height][width])
{

    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            int red = image[i][j].rgbtRed;
            int green = image[i][j].rgbtGreen;
            int blue = image[i][j].rgbtBlue;

            //calculate sepia values
            int sepiaRed = round(.393 * red + .769 * green + .189 * blue);
            if(sepiaRed > 255)
            {
                sepiaRed = 255;
            }

            int sepiaGreen = round(.349 * red + .686 * green + .168 * blue);
            if(sepiaGreen > 255)
            {
                sepiaGreen = 255;
            }

            int sepiaBlue = round(.272 * red + .534 * green + .131 * blue);
            if(sepiaBlue > 255)
            {
                sepiaBlue = 255;
            }

            image[i][j].rgbtRed  = sepiaRed;
            image[i][j].rgbtGreen = sepiaGreen;
            image[i][j].rgbtBlue = sepiaBlue;


        }
    }
    return;
}

Second code output:

second code output image

score 3 · Accepted Answer · answered Sep 11 '21 at 15:44

3

The difference between the two versions is that the type in which you compute the new pixel value

In the first version the type is whatever the constituent members of RGBTRIPLE are - which I assume is an unsigned 8-bit integer.

 pixel.rgbtBlue = round(0.272 * pix.rgbtRed + 0.534 * pix.rgbtGreen + 0.131 * pix.rgbtBlue);

In this line (and the equivalents for green and red pixels), the value can exceeded 255 which then gets truncated by assignment to an 8-bit unsigned int on assignment to pixel.rgbtBlue.

The following clamp to saturation:

    if(pixel.rgbtBlue > 255)
    {
        pixel.rgbtBlue = 255;
    }

Will always excute false, as pixel.rgbtBlue cannot hold a value bigger than 255.

In the second version of the code, an int is used, which is larger, and in which truncation does not occur, allowing the clamp-to-255 to work correct.

answered Sep 11 '21 at 15:44

marko

9,029
4
30
46

You might want to think about enabling compiler warnings: It likely that the truncation would generate one. – marko Sep 11 '21 at 15:48
*"I assume is an unsigned 8-bit integer."* I believe this is correct - according to [this](https://cs50.stackexchange.com/a/16679), the structure contains 3x `BYTE`, aka, `uint8_t`. – Oka Sep 11 '21 at 15:50
2

Note that [*"truncation"*](https://en.wikipedia.org/wiki/Truncation) is not the correct term. This is [unsigned integer overflow](https://en.wikipedia.org/wiki/Integer_overflow#Definition_variations_and_ambiguity), resulting in the value *wrapping*. – Oka Sep 11 '21 at 15:59

score 0 · Answer 2 · answered Oct 06 '21 at 12:07

As people above already comment, the first one has a truncated value in the if conditions, as the avarage operation is resulting in a number bigger than 255... That's why the filter does not work properly. I would consider as well less "if/else" on the second one, using perhaps a "Ternary Operator", for a cleaner code:

        image[i][j].rgbtGreen = (sepiaGreen > 255) ? 255 : sepiaGreen;
        image[i][j].rgbtRed = (sepiaRed > 255) ? 255 : sepiaRed;
        image[i][j].rgbtBlue = (sepiaBlue > 255) ? 255 : sepiaBlue;

CS50 pset4 - filter (less comfortable), sepia function

2 Answers2