Creating opencv Mat to 2d grid

Question

I am building a search algorithm using OpenCV Mat where I convert the Mat to gray image and then check the pixel to sign it to walkable or not walkable with their coordinate. I use vector > grid. when I try to print the nodeID from the grid the program sudden shutdown (e.g grid.grid[10][10]->NodeID).

using namespace std;
int gridZise;
class location{
public:
    int x;
    int y;

};

class Node{
public:
    int gridX;
    int gridY;
    bool walkable;
    location worldPosition;
    int NodeID;


    int gCost;
    int hCost;
    Node *parent;

    Node(bool _walkable, int _gridX, int _gridY)
        {
            walkable = _walkable;
            gridX = _gridX;
            gridY = _gridY;
            NodeID = gridY * gridZise + gridX;
        }
    Node(int _gridX, int _gridY){
        gridX = _gridX;
        gridY = _gridY;
        NodeID = gridY * gridZise + gridX;
    }

    int fCost(){
        return gCost + hCost;
    }

};

class Grid{

public:
    cv::Mat map;
    vector<vector<Node*> > grid;
    int gridx;
    int gridy;


    Grid(cv::Mat _map){
        map = _map;
        gridx = map.cols;
        gridy = map.cols;
        gridZise = map.cols;
    }

    void CreateGrid(){
        // Set up sizes. (HEIGHT x WIDTH)
          grid.resize(gridy);
          for (int i = 0; i < gridy; ++i)
            grid[i].resize(gridx);

          // build up the grid
          for(int i=0; i <gridx;i++){
              for(int j=0; j < gridy;j++){
                  int pixel_val = map.at<int>(i,j);
                  bool _walkable = false;
                  if(pixel_val > 120){//if the value of the pixel is bigger than 120 is walkable
                       _walkable = true;
                  }
                  grid[i][j]->walkable = _walkable;
                  grid[i][j]->gridX = i;
                  grid[i][j]->gridY = j;
              }
          }
    }

    void PrintGrid(){
        for(int i=0; i <gridx;i++){
            for(int j=0; j < gridy;j++){
                cout << grid[i][j]->NodeID <<endl;
            }
        }
    }

    vector<Node> GetNeighbours(Node node)
        {
            vector<Node> neighbours;

            for (int x = -1; x <=1; x++)
            {
                for (int y = -1; y <= 1; y++)
                {
                    if (x == 0 && y == 0)
                        continue;

                    int checkX = node.gridX + x;
                    int checkY = node.gridY + y;

                    if(checkX >=0 && checkX < gridx && checkY >=0 && checkY < gridy)
                    {
                        Node neighbour(checkX,checkY);
                        neighbours.push_back(neighbour);
                    }
                }
            }
            return neighbours;
        }

    Node nodeFromLocation(location _node){
        Node currentNode = *grid[_node.x][_node.y];
        return currentNode;
    }

};


using namespace cv;
int main()
{
    cv::Mat img;
    img = imread("C:\\Users\\abdulla\\Pictures\\maze.jpg");

    if(img.empty()){
        cout<<"image not load "<<endl;
        return -1;
    }
    cvtColor(img,img,CV_BGR2GRAY);
    imshow("image",img);
    waitKey();
    Grid grid(img);

    grid.PrintGrid();

    return 0;
}

Thank you.

Answer 1

First, get rid of using namespace std;. While it may look convenient, you are setting yourself up for some nasty surprises (see this question).

The constructor of Grid uses the default constructor of Grid::grid, which creates an empty vector.

In PrintGrid you get undefined behaviour.

grid; // ok, grid is a member of Grid
grid[0]; // Returns a reference. No bounds checking is performed.
grid[0][0]; // undefined behaviour. grid[0] is outside of bounds.

That there is a method CreateGrid is not relevant here, because you never call it. But let's assume that you had called it. Then PrintGrid would work like this:

grid; // ok, grid is a member of Grid
grid[0]; // ok
grid[0][0]; // ok, return a pointer to a Node. Which you never initialized.
grid[0][0]->NodeID; // Undefined behaviour. You're trying to access a random memory location.

Do you really need to store the Nodes as pointers? You could as well use vector<vector<Node>>. That way, someone (std::vector<...) would be responsible for allocating and deleting the nodes. You can still use use pointers to point at the parents, that's fine as long as the pointers are used as a reference and not as ownership.

If you do need to store pointers, make use of smart pointers. That way, somebody would be responsible for deleting the nodes.

Finally, the class Grid is responsible for maintaining proper state. Always. So the constructor should already do what CreateGrid does. The problem is that you must not call CreateGrid from the constructor, because then you would be calling a method of an object who's lifetime has not yet started (someone correct me if I'm wrong on that one). If you want to keep it as a seperate function you could make CreateGrid static:

class Grid {
    Grid(cv::Mat _map):
           map(_map),
           gridx(_map.cols),
           gridy(_map.cols),
           gridZise(_map.cols),
    {
        GreateGrid(grid, map, gridx, gridy);
    }

//...
    static void CreateGrid(std::vector<std::vector<Node>> & grid, cv::Mat map, int gridx, int grid y) {
    //...
    }
//...
};