Saturn
Saturn

Reputation: 18149

Why is this flood-fill algorithm not working?

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.sstatic.net/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];
}

Upvotes: 0

Views: 1016

Answers (2)

walrii
walrii

Reputation: 3522

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.

Upvotes: 1

user1766873
user1766873

Reputation:

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

Upvotes: 0

Related Questions