1

I have an openvdb grid that I would like to iterate using a functor and openvdb::tools::foreach.

//the grid I am iterating on
Grid G;

//the operator used to update each single voxel of G
struct Functor{
  inline void operator()(const Grid::ValueOnCIter& iter) const {
  }
};

If the operation involved only G I could have simply called

  Functor op;
  openvdb::tools::foreach(visibleGrid->cbeginValueOn(), op, true, true);

At each voxel (iteration) though I need to access and modify additional grid(s) based on the computed value of the iteration step.

My inital solution involved providing to the functor the accessor of the additional grid(s):

struct Functor{
  Grid2::Accessor grid2_accessor;

  Functor( Grid2::Accessor& a) : grid2_accessor(a){}

  inline void operator()(const Grid::ValueOnCIter& iter) const {
      //use grid2_accessor based on iter.getCoord()
  }
};

the accessor is provided to Functor at construction time and moreover each thread of the parallel for get a copy of the functor:

  Functor op(G2->getAccessor() );
  openvdb::tools::foreach(G1->cbeginValueOn(), op, true, **false**);

Unfortunately this solution does not work since:

  • the accessor must not be const to be accessed
  • but Functor::operator() must be a const method to be used by tools::foreach

A second dirty solution was to declare the Functor accessor copy as mutable. This solution does not work in Debug due to an openvdb assertion failing (most probably a memory leak).

Is there a solution to the problem? E.g. a tools::foreach that does not require operator() to be const.

Pierluigi
  • 2,212
  • 1
  • 25
  • 39

1 Answers1

1

It is not safe to use the same ValueAccessor in different threads. Instead, you want to have unique ValueAccessors for each thread but share the underlying tree.

Define your Functor like this instead:

struct Functor {
    Grid2& mGrid2;
    Functor(Grid2& grid2) : mGrid2(grid2) {}

    void operator()(const Grid::ValueOnCIter& iter) const {
        Grid2::Accessor grid2Acc(grid2.getAccessor()); // This is allowed because Grid2 is a reference
        // Do what you want
    } 
}

The reason you can't find a non-const version of the operator is because the underlying implementation relies on tbb. The motivation they give in the tbb documentation is:

Because the body object might be copied, its operator() should not modify the body. Otherwise the modification might or might not become visible to the thread that invoked parallel_for, depending upon whether operator() is acting on the original or a copy. As a reminder of this nuance, parallel_for requires that the body object's operator() be declared const.

Because of this you should not expect a non-const version any time soon.

EDIT: As noted in comments it is possible to reuse the cache in the ValueAccessor. However, as it is now a member of the class you will have problem to modify the tree using it in the operator (as setValue is non-const). If you know no one else is writing to the same memory place you can do a small hack:

struct Functor {
    Grid2::ValueAccessor mGrid2Acc;
    Functor(Grid2::ValueAccessor grid2Acc) : mGrid2Acc(grid2Acc) {}

    void operator()(const Grid::ValueOnCIter& iter) const {
        const Grid2::ValueType& v = mGrid2Acc.getValue(iter.getCoord());
        Grid2::ValueType& non_const_v = const_cast<Grid2::ValueType&>(v);
        // modify the value as you please, however a race condition will occur
        // if more than 1 thread write to the same location
    } 
}

I would still prefer the first solution. You can cache a certain leaf node by calling probeLeaf(openvdb::Coord& ijk) on the accessor.

pingul
  • 3,351
  • 3
  • 25
  • 43
  • Good point! By instantiating the accessor every time though I am losing the last cached tree path accessed. This gives me a performance penality – Pierluigi Aug 04 '16 at 13:48
  • @Pierluigi That is true. What I should have added as well is that you could just pass the accessor by value instead of by reference, that way you keep the cache intact. – pingul Aug 04 '16 at 16:34