Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an actual way to randomly generate a point cloud of arbitary type #5102

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kunaltyagi
Copy link
Member

We'll find some way to use Boost to allow simple generators for the point types.

Eg: Use PointT itself to create the parameters and share the distribution across all (eg: no mixing of Uniform and Normal for the point)

That should make our tests and tutorials a bit easier to use

@kunaltyagi
Copy link
Member Author

Once done, we could deprecate the current x,y,z specific functions of CloudGenerator and then do using CloudGenerator = PointCloudGenerator;. Thoughts?

@kunaltyagi kunaltyagi added changelog: new feature Meta-information for changelog generation module: common kind: proposal Type of issue needs: feedback Specify why not closed/merged yet labels Dec 23, 2021
@larshg
Copy link
Contributor

larshg commented Feb 11, 2022

Is the intention to limit/simplify the cloudgenerator so you cannot have different distributions on each component (X/Y/Z) - what do we gain from this?

@kunaltyagi
Copy link
Member Author

Is the intention to limit/simplify the cloudgenerator so you cannot have different distributions on each component (X/Y/Z)

Definitely not. The PointGenerator can create different distributions as needed

what do we gain from this?

Currently, only PointXYZ can be generated. If I wish to generate PointNormal or PointXYZRGBA, I'm out of luck

@mvieth
Copy link
Member

mvieth commented Apr 5, 2022

Sorry for the late reply.
I agree, a proper way to generate random clouds of any type would be nice (related: issue #5097).
I think we should add a few point generators along with PointCloudGenerator. Otherwise, it probably won't be used much because users first have to define their own point generators. Also, we see whether PointCloudGenerator is nice to use in practice. Currently in PCL (tutorials, tests), random points are mostly created so that all fields have a uniform distribution. Ideally, we would have one point generator that works for all point types. That way, we don't need generators for each point types (PointXYZ, PointXYZRGB, PointNormal, PointXYZI, ..., lots of boilerplate code). I started something, but it isn't very elegant yet:

template<typename PointT>
class UniformPointGenerator1 {
  inline PointT
  run() {
    PointT point;
    auto fields=pcl::getFields<PointT>();
    for(const auto& field : fields) {
      switch (field.datatype) {
        case pcl::PCLPointField::UINT8:
        {
          std::uint8_t value = uint8_dist(rng, {reinterpret_cast<const char*> (&min) + field.offset,
                                                reinterpret_cast<const char*> (&max) + field.offset});
          memcpy (reinterpret_cast<const char*> (&point) + field.offset, &value, sizeof (std::uint8_t));
          break;
        }
        // TODO other cases
    }
    return point;
  }
  private:
    std::mt19937 rng;
    PointT min, max;
    std::uniform_real_distribution<float> float_dist;
    std::uniform_real_distribution<double> double_dist;
    std::uniform_int_distribution<std::uint8_t> uint8_dist;
    // TODO other distributions. Maybe possible to add only necessary distributions?
}

We could also consider following the std random (or std boost) interface more closely, which would e.g. mean to pass the random number generator (e.g. std::mt19937) when calling the fill or get functions.

There are also some drawbacks with std random we should consider, for example that it seems to be slower than boost random in msvc stl, that the implementations of rngs and/or distributions differ between stl implementations (gcc/clang/msvc), meaning that the results are not reproducible even with same seeds, and some other things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: new feature Meta-information for changelog generation kind: proposal Type of issue module: common needs: feedback Specify why not closed/merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants