I have been struggling with implementing the blur function these past days and am on the verge of a meltdown.
I think the issue might lie in the usage of the pointer and address, but I am unsure. For some reason, whenever I try to get the pixel that the loop had changed in the previous loop, I end up with something completely different.
Output from CS50 checker:
first row: (10, 20, 30), (40, 50, 60), (70, 80, 90)
second row: (110, 130, 140), (120, 140, 150), (130, 150, 160)
third row: (200, 210, 220), (220, 230, 240), (240, 250, 255)
Block 00
R: 10 G: 20 B: 30 (check middle main block)
R: 110 G: 130 B: 140 (check south block)
R: 120 G: 140 B: 150 (check southeast block)
R: 40 G: 50 B: 60 (check east block)
Block 01
R: 40 G: 50 B: 60 (check middle main block)
R: 120 G: 140 B: 150 (check south block)
R: 110 G: 130 B: 140 (check southwest block)
R: 130 G: 150 B: 160 (check southeast block)
R: 49 G: 247 B: 0 (suppose to be west block (10, 20, 30) - not sure what happened here)
R: 70 G: 80 B: 90 (check east block)
Block 0 2
R: 70G: 80B: 90
R: 130G: 150B: 160
R: 120G: 140B: 150
R: 0G: 0B:...
I have enclosed my code below:
// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
RGBTRIPLE temp[height][width];
for (int i = 0; i < height; i )
{
for (int j = 0; j < width; j )
{
temp[i][j] = image[i][j];
}
}
for (int h = 0; h < height; h )
{
for (int w = 0; w < width; w )
{
printf("Block %i %i", h, w);
int totalRed = 0;
int totalGreen = 0;
int totalBlue = 0;
float cellCounter = 0.0;
//adds rgb for designated cell
rgbAdd(&temp[h][w], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
//north row has to be at least 0
if ((h - 1) >= 0)
{
//tally up north cell
rgbAdd(&temp[h - 1][w], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
//east col has to be at least 0 for northeast corner to exist
if ((w - 1) >= 0)
{
//tally up northeast corner
rgbAdd(&temp[h - 1][w - 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
//west col has to be (width - 1) or less for northwest corner to exist
if ((w 1) <= (width - 1))
{
//tally up northwest corner
rgbAdd(&temp[h - 1][w 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
}
//south row has to be (height -1) or less
if ((h 1) <= height - 1)
{
//tally up south cell
rgbAdd(&temp[h 1][w], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
//east col has to be at least 0 for southeast corner to exist
if ((w - 1) >= 0)
{
//tally up northeast corner
rgbAdd(&temp[h 1][w - 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
//west col has to be less than (width - 1) for southwest corner to exist
if ((w 1) <= (width - 1))
{
//tally up northwest corner
rgbAdd(&temp[h 1][w 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
}
//west col has to be at least 0
if ((w - 1) >= 0)
{
//tally up west point
rgbAdd(&temp[h - 1][w - 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
//east col has to be equal to or less than (width - 1)
if ((w 1) <= (width - 1))
{
//tally up east point
rgbAdd(&temp[h][w 1], &totalRed, &totalGreen, &totalBlue);
cellCounter = 1.0;
}
image[h][w].rgbtRed = round(totalRed / cellCounter);
image[h][w].rgbtGreen = round(totalGreen / cellCounter);
image[h][w].rgbtBlue = round(totalBlue / cellCounter);
}
}
return;
}
//total the red, green, blue values of the original and surrounding cells
void rgbAdd(RGBTRIPLE *p, int *totalRed, int *totalGreen, int *totalBlue)
{
RGBTRIPLE pixel = *p;
BYTE pRed = pixel.rgbtRed;
printf("R: %i", pRed);
BYTE pGreen = pixel.rgbtGreen;
printf("G: %i", pGreen);
BYTE pBlue = pixel.rgbtBlue;
printf("B: %i", pBlue);
*totalRed = pRed;
*totalGreen = pGreen;
*totalBlue = pBlue;
}
CodePudding user response：
There is an error in the coordinates of the west point: you wrote rgbAdd(&temp[h - 1][w - 1], &totalRed, &totalGreen, &totalBlue); instead of:
rgbAdd(&temp[h][w - 1], &totalRed, &totalGreen, &totalBlue);
It is very frustrating to stare at the obvious and not see it, but it happens to all programmers...
To make it harder for bugs to hide in plain sight, I recommend simplifying the code along these ideas:
- use a structure for the averaging state
- update the colors and pixel count in
rgbAvgAdd()for pixels in a consistent order top to botton, left to right - store the final value in
rgbAvgEnd() - the north, south, east and west metaphors are not used consistently in your code, I would just remove these comments for clarity.
Here is the modified code:
typedef struct AvgState {
int red, green, blue;
int count;
} AvgState;
// update the red, green, blue components and number of pixels
void rgbAvgAdd(AvgState *s, RGBTRIPLE pixel) {
s->red = pixel.rgbtRed;
s->green = pixel.rgbtGreen;
s->blue = pixel.rgbtBlue;
s->count = 1;
}
// update the red, green, blue components and number of pixels
void rgbAvgEnd(AvgState *s, RGBTRIPLE *p) {
double num = s->count;
p->rgbtRed = round(s->red / num);
p->rgbtGreen = round(s->green / num);
p->rgbtRed = round(s->blue / num);
}
// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width]) {
// use a local copy of the image (potential UB for large images)
RGBTRIPLE temp[height][width];
for (int i = 0; i < height; i ) {
for (int j = 0; j < width; j ) {
temp[i][j] = image[i][j];
}
}
for (int h = 0; h < height; h ) {
for (int w = 0; w < width; w ) {
AvgState state = { 0, 0, 0, 0 };
if (h > 0) {
// handle pixels from the row above
if (w > 0) {
rgbAvgAdd(&state, temp[h - 1][w - 1]);
}
if ((1)) {
rgbAvgAdd(&state, temp[h - 1][w]);
}
if (w 1 < width) {
rgbAvgAdd(&state, temp[h - 1][w 1]);
}
}
if ((1)) {
// handle pixels from the current row
if (w > 0) {
rgbAvgAdd(&state, temp[h][w - 1]);
}
if ((1)) {
rgbAvgAdd(&state, temp[h][w]);
}
if (w 1 < width) {
rgbAvgAdd(&state, temp[h][w 1]);
}
}
if (h 1 < height) {
// handle pixels from the row below
if (w > 0) {
rgbAvgAdd(&state, temp[h 1][w - 1]);
}
if ((1)) {
rgbAvgAdd(&state, temp[h 1][w]);
}
if (w 1 < width) {
rgbAvgAdd(&state, temp[h 1][w 1]);
}
}
rgbAvgEnd(&state, &image[h][w]);
}
}
}