0

I have a problem that is giving me trouble; my goal is to create an octree.

Actually, it's very cheap (but it's nearly enough for what I want to do with that octree).

My problem is that my std::vector<std::reference_wrapper<Point>> is filled with the same value. So my insertion creates an infinite loop. But here's the code, maybe it'll be more understandable. I put commentaries where my error occurs.

source.cpp

void main(){
    std::random_device rd;
    std::mt19937 rng(rd());
    std::uniform_real_distribution<double> uni(0, 2);
    auto random_integer = uni(rng);
    Point firstCenter = Point(1, 1, 1);
    Point firstHalfDimension = Point(1, 1, 1);
    Octree oct(firstCenter, firstHalfDimension);

    for (int i = 0; i < 3; ++i) {
        double x = uni(rng);
        double y = uni(rng);
        double z = uni(rng);
        Point ptmp = Point(x, y, z);
        std::cout << x << " " << y << " " << z << std::endl;
        auto po = std::ref(ptmp);
        oct.insert(po);
    }
}

Octree.hpp

class Octree {
    Node octree;
    Point firstCenter;
    Point firstHalfDimension;

public: 
    Octree() = default;
    Octree( Point& firstCenter, Point& firstHalfDimension) :
        firstCenter(firstCenter), firstHalfDimension(firstHalfDimension), octree(firstCenter, firstHalfDimension) {}

    void insert(std::reference_wrapper<Point> pt) {
        octree.insert(pt);
    }
};

Node.hpp (where the problem occurs)

#define MAXVAL 2
using Point = gmtl::Vec3d;

class Node {
    Point center;
    Point halfDimension;
    std::vector<std::reference_wrapper<Point>> datas;
    std::array<std::shared_ptr<Node>, 8> children;


    int getOctant(const std::reference_wrapper<Point> p) {
        int oct = 0;
        if (p.get()[0] >= center[0]) oct |= 4;
        if (p.get()[1] >= center[1]) oct |= 2;
        if (p.get()[2] >= center[2]) oct |= 1;
        return oct;
    }

    const bool isLeaf()  {
        return !children[0];
    }

public:

    Node(Point center, Point halfDimension) : center(center), halfDimension(halfDimension){
    }
    void insert(const std::reference_wrapper<Point> p) {
        if (isLeaf()){
            if (datas.size() == MAXVAL) { //Must subdivide
                std::cout << p.get()[0] << " " << p.get()[1] << " " << p.get()[2] << std::endl;
                for (int i = 0; i < datas.size(); ++i) {
                    std::cout << datas[i].get()[0] << " "
                        << datas[i].get()[1] << " "
                        << datas[i].get()[2] << std::endl;
 //The problem is here : the vector is filled with the same values, and it's the same value as p. let's say p = (0.4,0.7,0.8), then the for loop will show to the screen : 
 // 0.4 0.7 0.8
 // 0.4 0.7 0.8
 // 0.4 0.7 0.8
                }
                for (int i = 0; i < 8; ++i) {
                    Point newCenter;
                    newCenter[0] += halfDimension[0] * (i & 4 ? .5f : -.5f);
                    newCenter[1] += halfDimension[1] * (i & 2 ? .5f : -.5f);
                    newCenter[2] += halfDimension[2] * (i & 1 ? .5f : -.5f);
                    children[i] = std::make_shared<Node>(newCenter, halfDimension * .5);
                }
                int octant = getOctant(p);
                children[octant]->insert(p);
                for (int i = 0; i < datas.size(); ++i) {
                    int octant = getOctant(datas[i]);
                    children[octant]->insert(datas[i]);
                }


            }
            else { //Just add
                datas.push_back(p);
            }
        }
        else { //Non-leaf node
            children[getOctant(p)]->insert(p);
        }
    }
};

I really don't understand what I'm doing wrong.

TriskalJM
  • 2,393
  • 1
  • 19
  • 20
Raph Schim
  • 528
  • 7
  • 30
  • 5
    You are storing a reference to the `ptmp` variable, a variable which goes out of scope and is destructed each iteration of the loop. *Why* do you want a vector of references? What is the problem that is supposed to solve? Why can you use a vector of `Point` *objects* instead? – Some programmer dude Jul 24 '17 at 08:49
  • 1
    you'll get help faster if you: 1. reduce the code to the minimum number of lines required to demonstrate the problem 2. post actual error messages – Richard Hodges Jul 24 '17 at 08:50
  • RichardHodges I'll edit my post, so it is the minimum. There is no error message... It's an execution error. @Someprogrammerdude In fact, I'll have billions of points, and I though it was lighter to store only references... – Raph Schim Jul 24 '17 at 09:01
  • I believe gmtl::Vec3d only contains a size and a vector of values. – Raph Schim Jul 24 '17 at 09:09
  • 3
    Even if you make the `ptmp` object have a longer lifetime, all references would reference the *one single* `ptmp` object anyway. *And* remember that the `std::reference_wrapper` object takes up some space itself, no object can have zero size. Before you attempt to optimize, write as simple and straight-forward code as possible. Then ***if*** (and only if) the "performance" is not good enough (which often *is* good enough) then you measure and see what you can do to optimize. – Some programmer dude Jul 24 '17 at 09:09
  • 1
    Yeah! I understand... I removed reference_wrapper here, and tried with Points only. Now I know that I have the same issue (infinite recursion) but it's not due to reference_wrapper. – Raph Schim Jul 24 '17 at 09:25
  • 1
    A reference has to reference something. That something still takes memory. In order to reference billions of points, you still MUST HAVE billions of points somewhere, even if they are not actually in this vector. – Kenny Ostrom Jul 24 '17 at 16:16

1 Answers1

2
std::vector<std::reference_wrapper<Point>> datas;

Something has to own the objects that this vector contains references to.

You filled your vector with references to objects that no longer exist. It's not clear why you think you need a vector of references, but you shouldn't do something unusual unless you have a very good explanation for why the normal way (a vector of values) won't work for you.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • You're right. Actually I believe I don't need this vector. I Thought it would save space, but no. I was misinterpreting what is really a reference wrapper. I just have to fill my vectors with values, and when it comes to create children, I full my children and empty the vector of the mother so it don't take place uselessly. – Raph Schim Jul 25 '17 at 06:22