Why is this flood-fill algorithm not working?

Question

For this question: Is there a proper algorithm for detecting the background color of a figure?, I will need to create a flood-fill algorithm to be able to separate all my pixels in groups of the same color.

I did this recursively, but it gives me a stack-overflow error. So I had to choose the iterative, queue based algorithm found here: http://en.wikipedia.org/wiki/Flood_fill#Alternative_implementations

First of all, I begin a search over all the matrix elements (elements being instances of the Pixel class).

private PixelGroup[] generatePixelGroupsFromMatrix(Pixel[][] matrix) {
    PixelGroup[] tempGroups = new PixelGroups[9999999]; // Nevermind the 9999999....
    int groupsFound = 0;
    Pixel pixel;

    for (int y = 0; y < matrix.length; ++y) {
        for (int x = 0; x < matrix[0].length; ++x) {
            pixel = matrix[y][x];
            if (!pixel.evaluated) {
                // This pixel has never been evaluated
                // Therefore, it belongs to a new group
                // First, we make a new group
                PixelGroup newGroup = new PixelGroup();
                // Begin search for connected pixels with the same color. All pixels found will belong to this new group.
                findPixelsConnectedWith(pixel,newGroup);
                tempGroups[groupsFound] = newGroup;
                ++groupsFound;
            }
        }
    }

    PixelGroup[] result = new PixelGroup[groupsFound];
    for (int i = 0; i < groupsFound; ++i) {
        result[i] = tempGroups[i];
    }

    return result;
}

So, Pixel has the following values: x, y, evaluated (boolean) and color (integer). Then, PixelGroup is simply a class capable of holding pixels (it works).

And this is the method that is giving me trouble:

private void findPixelsConnectedWith(Pixel pixel, GroupOfPixels group) {
    QueueOfPixels queue = new QueueOfPixels();
    queue.add(pixel);

    Pixel currentPixel;
    int x,y;
    Pixel neighbor;
    while((currentPixel = queue.nextPixel()) != null) {
        if (currentPixel.color == pixel.color && !currentPixel.evaluated) {
            // This pixel has the required color, and has not been evaluated. It meets our needs.
            // Add to group.
            group.addPixel(currentPixel);
            // Flag it as evaluated. So in the future, it will be ignored.
            currentPixel.evaluated = true;
            // Evaluate all 8 possible directions to find neighbor pixels
            int[] xDirections = {0,1,1,1,0,-1,-1,-1};
            int[] yDirections = {-1,-1,0,1,1,1,0,-1};
            for (int i = 0; i < 8; ++i) {
                x = xDirections[i];
                y = yDirections[i];
                if (pixelExists(currentPixel.y + y,currentPixel.x + x)) {
                    // There exists a pixel in this direction!
                    neighbor = getPixel(currentPixel.y + y,currentPixel.x + x);
                    queue.add(neighbor);
                }
            }
        }
    }

}

If you're curious, here is my QueueOfPixels class. I had to make my own with only vectors (school assignment requirement): https://codereview.stackexchange.com/questions/17823/vector-based-flood-fill-algorithm-queue-class (as far as I can tell, it simply works).

WHAT IS THE PROBLEM?

Alright, I have tested this with this image, which is 5x2 pixels (you'll need to zoom in a lot to see it): https://i.stack.imgur.com/xV0Lf.gif - the first row only has black pixels, and the second they're white. The program tells me it has found 6 pixel groups (when it should have been only 2!)

WHAT HAVE I TRIED TO DEBUG THE PROBLEM?

Well, first, before calling findPixelsConnectedWith, I placed this line:

System.out.println("The pixel (" + x + "," + y + ") has not been evaluated. Evaluating now.");

And this was the result:

The pixel (0,0) has not been evaluated. Evaluating now.
The pixel (1,0) has not been evaluated. Evaluating now.
The pixel (2,0) has not been evaluated. Evaluating now.
The pixel (3,0) has not been evaluated. Evaluating now.
The pixel (4,0) has not been evaluated. Evaluating now.
The pixel (0,1) has not been evaluated. Evaluating now.

So, as you can see, it seems like the code is unable to work with the first row (black pixels) since it thinks that every pixel in that row has not been evaluated (I expected it to say that (0,0) was not evaluated and done). But when it starts working with the second row, it does seem to work as expected (find (0,1) and then it is over).

But I still am unable to find out what is going on. Any ideas?

Edit: My getPixel and pixelExists functions:

private boolean pixelExists(int y, int x) {
    return (y > 0 && y < pixelMatrix.length) && (x > 0 && x < pixelMatrix[0].length);
}

private Pixel getPixel(int y, int x) {
    return pixelMatrix[y][x];
}

You should use java.util.Vector instead of PixelGroup[]. You've said "vector" in several places where I think you meant "array". PixelGroup[] is an array, not a vector. — walrii, Oct 23 '12 at 00:48
It's odd that your pixelExists() and getPixel() methods are called using y,x not x,y. You've not shown us the code. Is y,x intentional? and does getPixel refer to the same matrix[][] that was passed to generatePixelGroupsFromMatrix()? Put in a print every time you add to your queue. — walrii, Oct 23 '12 at 00:49
@walrii: Ah, sorry. Well, in that case, I meant to say array instead of vector. — Saturn, Oct 23 '12 at 01:22
@walrii: Yeah, it was intentional. I think I am using them properly anyway - there, I edited the post. — Saturn, Oct 23 '12 at 01:23
To "figure out what's going on" you should debug your code - it's an incredibly valuable skill to have and you should develop it. Use your IDE to "debug" your application and set a break point at the start of the code that's need examination, then step through the code looking at all the variable values. You should be able to spot the problem (that's what I would do). — Bohemian, Oct 23 '12 at 01:29
Perhaps this is a translation error as you wrote the question, but PixelGroup[] result = new PixelGroup[gruposEncontrados]; should probably use groupsFound instead of gruposEncontrados. — walrii, Oct 23 '12 at 01:33
@walrii: Yes, sorry, I write my code in spanish but for the sake of the question I translate it. Thank you :) — Saturn, Oct 23 '12 at 02:07

score 1 · Accepted Answer · answered Oct 23 '12 at 01:31

Your pixelExists method should use y >= 0 and x >= 0 instead of y > 0 and x > 0.

private boolean pixelExists(int y, int x) {
    return (y >= 0 && y < pixelMatrix.length) && (x >= 0 && x < pixelMatrix[0].length);
}

This may not be the only problem, but it will certainly prevent you from getting the correct answers.

score 0 · Answer 2 · answered Oct 23 '12 at 00:46

0

Maybe pixelExists method has "y > 0" part while it should has "y>=0"?

answered Oct 23 '12 at 00:46

Why is this flood-fill algorithm not working?

2 Answers2