3

Is there a nicer way to generate a list of points like than this? Libraries wise I'm open to any Eigen based method.

auto it = voxels.begin();

for(auto i = -180; i < 90; i++) {
    for(auto j = -80; j < 70; j++) {
        for(auto k = 20; k < 460; k++) {
            *it = (Point3(i,j,k));
            it++;
        }
    }
}
chris
  • 4,840
  • 5
  • 35
  • 66
  • 4
    If you want to generate all points within the ranges, then there really isn't any other way. There might be libraries and helper functions that makes it look nicer, but in the end it will still be something similar to what you have now. – Some programmer dude May 11 '15 at 09:22
  • 3
    Not a direct answer, but calling push_back repeatedly is inefficient when you already know how big your vector is going to be. This is because whenever the vector has filled up its allocate space, push_back allocates twice as much space and copies everything over. Instead, call resize() to fix the vector size at the beginning, then access by index. Alternatively, you can use reserve() and then you can still use push_back with much less overhead. – IanPudney May 11 '15 at 09:24
  • good point, il change that to just increment an iterator. – chris May 11 '15 at 09:27
  • @chris IMHO you still say the same (with other words). You should reserve the final vector's size _in the beginning_ and use constants you'll reuse in the for loops: `voxels.reserve((MAX_X - MIN_X)*(MAX_Y - MIN_Y)*(MAX_Z - MIN_Z));` – Melebius May 11 '15 at 10:02
  • do you think it matters if its on heap or stack? currently I'm doing just allocating like `vector voxels(X_RANGE*Y_RANGE*Z_RANGE)` – chris May 11 '15 at 10:06
  • @chris Well, you hadn't provided this line before. However, it constructs all the points and then replaces them in the for-loop with newly created objects. Just reserving the space should be still cheaper. – Melebius May 11 '15 at 10:11

1 Answers1

4

There's an immediate way to improve performance, by reserving enough space in the vector before you fill it with values.

There are many 'nicer' ways of doing it depending on what you think is nice.

Here's one way:

std::vector<Point3> populate()
{
    // (arguable) maintainability benefit
    constexpr auto I = axis_limits(-180, 90);
    constexpr auto J = axis_limits(-80, 70);
    constexpr auto K = axis_limits(20, 460);

    // pre-reserve the space
    std::vector<Point3> voxels;
    voxels.reserve(volume(I, J, K));

    // although it looks like it might be more work for the compiler, it gets optimised
    // there is no loss of performance
    for(i : I)
        for(j : J)
            for(k : J)
                voxels.emplace_back(i, j, k);

    return voxels;
}

Which will rely on the following infrastructure code:

struct Point3 {
    Point3(int, int, int) {}
};

struct int_generator {
    int_generator(int v)
    : _v(v)
    {}

    int operator*() const {
        return _v;
    }

    int_generator& operator++() {
        ++_v;
        return *this;
    }

    bool operator!=(const int_generator& rhs) const {
        return _v != rhs._v;
    }

private:
    int _v;
};

struct axis_limits : std::tuple<int, int>
{
    using std::tuple<int, int>::tuple;
    int_generator begin() const {
        return std::get<0>(*this);
    }

    int_generator end() const {
        return std::get<1>(*this);
    }
};

constexpr int lower(const axis_limits& t)
{
    return std::get<0>(t);
}

constexpr int upper(const axis_limits& t)
{
    return std::get<1>(t);
}

int_generator begin(const axis_limits& t)
{
    return std::get<0>(t);
}

int_generator end(const axis_limits& t)
{
    return std::get<1>(t);
}

constexpr int volume(const axis_limits& x, const axis_limits& y, const axis_limits& z)
{
    return (upper(x) - lower(x))
    * (upper(y) - lower(y))
    * (upper(z) - lower(z));
}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142