0

folks!

I'm working on the edge detection algorithm, but for some reason, the image is turning out too yellow. I wonder if someone can give me some sort of hint at what I'm doing wrong, since I haven't been able to figure it out for about a week. Here's the code for reference:

// Detect edges
void edges(int height, int width, RGBTRIPLE image[height][width])
{
    // Create a copy of the image
    RGBTRIPLE(*copy)[width] = calloc(height, width * sizeof(RGBTRIPLE));
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            // Copy each pixel from image to the copy
            copy[i][j] = image[i][j];
        }
    }

    // Adjacent coordinates, and # of adjacent pixels
    static int adjacent_positions[9][2] = {{0, 0}, {-1,0}, {-1, -1}, {0, -1}, {1, -1}, {1, 0}, {1, 1}, {0, 1}, {-1, 1}};
    size_t limit = sizeof(adjacent_positions) / sizeof (adjacent_positions[0]);

    for (int i = 0; i < height; i++)
    {
        // And each pixel
        for (int j = 0; j < width; j++)
        {
            // We store an array of the adjacent pixels values. Each pixel has its own.
            // The array can be read as: [R, G, B] values.
            int results[] = {0, 0, 0};

            // Store GxGy for RGB for this pixel's adjacents
            int gx_r = 0, gx_g = 0, gx_b = 0;
            int gy_r = 0, gy_g = 0, gy_b = 0;

            // We loop through all 9 adjacent items for that pixel, including itself.
            for (int k = 0; k < limit; k++)
            {
                // Adjacent coordinates
                int adj_i = i + adjacent_positions[k][0];
                int adj_j = j + adjacent_positions[k][1];

                // Declare bounds
                bool i_out_of_bounds = adj_i < 0 || adj_i >= height;
                bool j_out_of_bounds = adj_j < 0 || adj_j >= width;

                // If out of bounds, we skip
                if (i_out_of_bounds || j_out_of_bounds) {
                    continue;
                }

                // Take Gx kernel values (Left/Right)
                if (adj_j < 0 && adj_i != 0)
                {
                    // N * -1
                    gx_r += image[adj_i][adj_j].rgbtRed * -1;
                    gx_g += image[adj_i][adj_j].rgbtGreen * -1;
                    gx_b += image[adj_i][adj_j].rgbtBlue * -1;
                }
                else if (adj_j < 0 && adj_i == 0)
                {
                    // N * -2
                    gx_r += image[adj_i][adj_j].rgbtRed * -2;
                    gx_g += image[adj_i][adj_j].rgbtGreen * -2;
                    gx_b += image[adj_i][adj_j].rgbtBlue * -2;
                }
                else if (adj_j > 0 && adj_i != 0)
                {
                    // N * 1
                    gx_r += image[adj_i][adj_j].rgbtRed * 1;
                    gx_g += image[adj_i][adj_j].rgbtGreen * 1;
                    gx_b += image[adj_i][adj_j].rgbtBlue * 1;
                }
                else if (adj_j > 0 && adj_i == 0)
                {
                    // N * 2
                    gx_r += image[adj_i][adj_j].rgbtRed * 2;
                    gx_g += image[adj_i][adj_j].rgbtGreen * 2;
                    gx_b += image[adj_i][adj_j].rgbtBlue * 2;
                }
                else if (adj_j == 0)
                {
                    // N * 0
                    gx_r += image[adj_i][adj_j].rgbtRed * 0;
                    gx_g += image[adj_i][adj_j].rgbtGreen * 0;
                    gx_b += image[adj_i][adj_j].rgbtBlue * 0;
                }

                // Take Gy kernel values (Up/Down)
                if (adj_i < 0 && adj_j != 0)
                {
                    // N * -1
                    gy_r += image[adj_i][adj_j].rgbtRed * -1;
                    gy_g += image[adj_i][adj_j].rgbtGreen * -1;
                    gy_b += image[adj_i][adj_j].rgbtBlue * -1;
                }
                else if (adj_i < 0 && adj_j == 0)
                {
                    // N * -2
                    gy_r += image[adj_i][adj_j].rgbtRed * -2;
                    gy_g += image[adj_i][adj_j].rgbtGreen * -2;
                    gy_b += image[adj_i][adj_j].rgbtBlue * -2;
                }
                else if (adj_i > 0 && adj_j != 0)
                {
                    // N * 1
                    gy_r += image[adj_i][adj_j].rgbtRed * 1;
                    gy_g += image[adj_i][adj_j].rgbtGreen * 1;
                    gy_b += image[adj_i][adj_j].rgbtBlue * 1;
                }
                else if (adj_i > 0 && adj_j == 0)
                {
                    // N * 2
                    gy_r += image[adj_i][adj_j].rgbtRed * 2;
                    gy_g += image[adj_i][adj_j].rgbtGreen * 2;
                    gy_b += image[adj_i][adj_j].rgbtBlue * 2;
                }
                else if (adj_i == 0)
                {
                    // N * 0
                    gy_r += image[adj_i][adj_j].rgbtRed * 0;
                    gy_g += image[adj_i][adj_j].rgbtGreen * 0;
                    gy_b += image[adj_i][adj_j].rgbtBlue * 0;
                }
            }

            // Combine GxGy = square root of => Gx^2 + Gy^2
            int r = round(sqrt((gx_r * gx_r) + (gy_r * gy_r)));
            int g = round(sqrt((gx_g * gx_g) + (gy_g * gy_g)));
            int b = round(sqrt((gx_b * gx_b) + (gy_b * gy_b)));

            // Ensure black or white pixels, to avoid "yellowness"
            const int final_red   = (r > 255) ? 255 : 0;
            const int final_green = (g > 255) ? 255 : 0;
            const int final_blue  = (b > 255) ? 255 : 0;

            copy[i][j].rgbtRed = final_red;
            // printf("Copy Red: %i\n", final_red);

            copy[i][j].rgbtGreen = final_green;
            // printf("Copy Green: %i\n", final_green);

            copy[i][j].rgbtBlue = final_blue;
            // printf("Copy Blue: %i\n\n", final_blue);

        }
    }

    // Update original image
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            image[i][j] = copy[i][j];
        }
    }

    // Free up copied memory
    free(copy);
    return;
}

My initial thought was that I was not setting the variables at the end to be either 255, or 0. But this doesn't seem to be the case. The closest I got from getting the edges, is changing the current logic to get the final_red, final_blue and final_green to:

// Ensure black or white pixels, to avoid "yellowness"
            copy[i][j].rgbtRed = r; // Instead of tweaking to 255 or 0, we keep to the round of the GxGy squares
            copy[i][j].rgbtGreen = g;
            copy[i][j].rgbtBlue = b;

This results in an image that actually captures the edges, but the image gets very psychedelic, and very colorful (even though the edges are there).

Can someone give me a shout about something I'm doing wrong? I've been away from this code for a couple of days, so I might be forgetting something. Also, tips for refactoring are GREATLY appreciated, as I'm hating to see those gigantic if/elses.

1
  • Hi Marco. Don't know if there are more issues, but here: '(r > 255) ? 255 : 0;' there is something off. Read again the especifications and think what will be the result in case 'r' is less than 255. If that won't fix the code let me know, and I'll try to take a closer look if I have some spare time.
    – Tritum
    May 18 '20 at 10:16
1

When checking the values of adj_i and adj_j in your 'if' conditions, you are not checking against your adjacent_positions arrays, since you added the values of i an j to them here: int adj_i = i + adjacent_positions[k][0] and here: int adj_j = j + adjacent_positions[k][1]. Fixing those, by substracting 'i' and 'j' in the conditions, for example: if (adj_j -j < 0 && adj_i -i != 0) or some similar solution, you'll be good to go. Well, that is, if you fixed the issue from my previous comment.

4
  • Hey @Tritum, I saw you comment but I couldn't slot some time to work on the PSET. I'll take some time to work on it this week and let you know how it goes. Thanks a lot! May 21 '20 at 14:05
  • That's great! Indeed, I can see the edges now. But I still can't wrap my head around why I should subtract j and i. May 21 '20 at 14:40
  • You're using the variable adj_x, which is the result of the addition of i/j to the coordinates from your array 'adjacent_positions', to get the 3x3 matrix positions around the current pixel. On those coordinates you get the real index of the current pixel, for example if i = 200 and k = 1 (-1 in your matrix), you get the position 199 for 'i', and you use it for 3 things: 1- To skip the pixel if you're reaching out of limits. 2- To retrieve its value to do your math. The third thing you use it is to check the position to apply the factor for every pixel depending of its position.
    – Tritum
    May 21 '20 at 15:45
  • The problem with that is that in this case you don't need the real pixel position (i + adjacent_positions[x], but the actual value of your array. In the previous example if you compare the 'adj_x' value 199, with 0 like here: 'adj_j < 0 && adj_i == 0', it will never check out, and most of the cases the if condition won't do the job. Do you see it now? You can check it, by putting a printf statement inside your if conditions and you'll see that most of the cases some parts of the code don't execute.
    – Tritum
    May 21 '20 at 16:02

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .