Auto-fill(flooding) image help

[jjordan33]

Hi,

I have a function that is taking in an int for rows and cols, and a height limiter. It's supposed to be filling up the image from that marker up to the height limit. The 2d array is made up of red green and blue pixels like a ppm image. However, all it has managed to do so far is put a tiny speck of green on the image. Can someone take a look at my function and see if anything looks sketchy?

Thank you.

  void FloodMap::flood(int r, int c, int height){

    int tempVar = 0;

    if (r < 0 || r >= rows){
        return;
    }

    if (c < 0 || c >= cols){
        return;
    }

    if(map[r][c].g == 225){ // map is a class variable
        return;
    }

    if (height < map[r][c].r){
        return;
    }

    tempVar = map[r][c].r;
    
    map[r][c].r = 0; // red
    map[r][c].g = 225; // green. 225 is arbitrary. It's just any number less than 255
    map[r][c].b = 0; // blue

    flood(r+1, c, tempVar); // recursion
    flood(r-1, c, tempVar);
    flood(r, c+1, tempVar);
    flood(r, c-1, tempVar);
    flood(r+1, c+1, tempVar);
    flood(r-1, c+1, tempVar);
    flood(r+1, c-1, tempVar);
    flood(r-1, c-1, tempVar);

}

[jonnin]

this looks to do a 3x3 fill from the pixel it is called with.
if its out of bounds, return, ok.
if the current pixel is already green 225, don't fill the 3x3. this seems weird.
if the height is compared to the red component of the pixel, return and don't do anything (this seems really, really weird).

then do the same for the neighbors.
If the 2 weird logics are what you really intended, I can look deeper, but I suspect the one with the red component is making you get odd behavior.

[MikeStgt]

Is it purposeful to have for every recursive invocation of flood a new height? IMO this should be a dam/seawall/border where flooding ends similar to the edges of the picture. Do you actually want a "dynamic" dam?

[salem c]

What do you mean by 'height'?

To flood-fill an area within a perimeter, one would normally have something like

void FloodMap::flood(int r, int c, rgb_t perimeter) {
    if ( map[r][c] == perimeter ) return;
}

Also, naive flood-fills are extremely stack intensive due to the number of nested recursive calls.

[jjordan33]

>> Jonnin

I believe it is at least what I'm attempting to have happen, but my execution may be off. The image is a bird's eye view of a mountain range, from a satellite perspective I suppose. The image is completely greyscale and I'm trying to flood valleys and what-not with "green acid".
The 225 for green just looks like acid more to me than other values.

So, my logic was attempting to say, if the value is already "green acid" no further action required.


>> Salem

You lost me. What's the difference between mine and perimeter?


>> MikeStgt

If you're asking if I want the border to be different every time... it's not actually up to me haha. It's part of the assignment.


Thank you all.

[MikeStgt]

@salem c

Also, naive flood-fills are extremely stack intensive due to the number of nested recursive calls.

Some suggestions? I looked into GIMP source, found gimp_drawable_fill_boundary mentioned, alas not the procedure yet.
Where in your suggestion is the colorizing step, how is r and c moved up to the perimenter in all directions?
I would i) put for aesthetical reasons those calls

    flood(r+1, c, tempVar); // recursion
    flood(r-1, c, tempVar);
    flood(r, c+1, tempVar);
    flood(r, c-1, tempVar);
    flood(r+1, c+1, tempVar);
    flood(r-1, c+1, tempVar);
    flood(r+1, c-1, tempVar);
    flood(r-1, c-1, tempVar);

in two loops, and ii) set the perimeter (or boundary or hight) criterion to a fixed value valid for this fill.

BTW, the number of nested recursive calls is not "unendingly" high. The tendency to step in 8 directions is limited, after 2..3 gernerations most of the neighbouring pixels are already filled.

Edit: Added @salem c as meanwhile my append does not follow directly after the one I am referring. Just to make it clear who I ask my questions.

[salem c]

> >> Salem
> You lost me. What's the difference between mine and perimeter?
On a strictly syntactic level, none.

But height is such a poor name for the variable.
You're meant to be conveying meaning to your readers (including yourself). A well chosen variable name goes a long way to aiding code comprehension.

The compiler doesn't give a monkeys if you say
void FloodMap::flood(int r, int c, int height)
void FloodMap::flood(int r, int c, int a)
void FloodMap::flood(int r, int c, int banana)
void FloodMap::flood(int r, int c, int elephant)

It's not even constant from one invocation to the next.

    if (height < map[r][c].r){
        return;
    }
    tempVar = map[r][c].r;

Also, we can't test your code.
http://sscce.org/

[jjordan33]

Do I need to post more of my code or perhaps the file?

[ne555]

> But height is such a poor name for the variable.

The image is a bird's eye view of a mountain range

actually, it's a great name.
the problem are using `map.r' and `tempVar' to mean height.


> The 2d array is made up of red green and blue pixels like a ppm image.
>> The image is completely greyscale
¿which one?

> all it has managed to do so far is put a tiny speck of green on the image.
may be a problem with noise
try to give a little more tolerance to the comparison if (height+tolerance < map[r][c].height) return;

[MikeStgt]

@jjordan33

If you're asking if I want the border to be different every time... it's not actually up to me haha. It's part of the assignment.

Differen every time, yes, for the space within the boundary currently in question, but for sure not for every pixel of it.

Do I need to post more of my code or perhaps the file?

You promised to do so on Monday.

[jjordan33]

I meant do I need to post more of it so that my recursive function can be better solved.

I'll still post the whole thing on Monday.

[MikeStgt]

anything looks sketchy?

Yes, at least for me. In your OP lines 17..21 are:

    if (height < map[r][c].r){
        return;
    }

    tempVar = map[r][c].r;

Questions: You have a grey scale image consisting of RGB values? Black is RGB = {0, 0, 0} and white is RGB = {FF, FF, FF}? In that case it is OK to use R as substitute, you just stop at the red line ;)

Are the "valleys" you try to fill with green areas brighter/clearer than the surrounding "heights"? Then you should rethink why you refuse filling if (height < map[r][c].r) -- if the pixel in question is brighter than the darker border, do not stop, paint it green. Otherwise (the "sink" is dark, the "dam" is bright) the return is correct for the pixel in question.

Then -- I ask again -- you query the current pixel's red component, set tempVar with it,
tempVar = map[r][c].r;
and use just this tempVar as third argument for the recursive call treating the eight surrounding pixel. There this third argument is the new dam altitude. So if the current pixel is white (synonymical to R=FF) you set the border for the adjunct pixels to white. If you are filling a white plane, it is just the comparison < instead of <= in line 17 that by chance still could make it work. If the variable height is part of the assignment, this request is not very reasonable, or worded mistakable -- as always: "It ain't my fault."

In addition, if you deal with gray scale images you should consider the suggestion of ne555 (height+tolerance < map[r][c].r) return;

[jjordan33]

I have greenery!!

I'm in business now everyone. Thanks a bazillion.

[MikeStgt]

I have greenery!!

Congratulations!

Here at this side it is already Monday, but I am patient because I'd also like to know how the issue with the variable height part of the assignment turned out.

[MikeStgt]

I'll still post the whole thing on Monday.

Which Monday? ;)
... which year?