Fix TriangleMesh::SamplePointsUniformly not sampling triangles unifor…

[ghost]

…mly (#6144)

Better uniform sampling of triangle meshes

Type

• Bug fix (non-breaking change which fixes an issue): Fixes TriangleMesh::SamplePointsUniformly doesn't sample triangles uniformly #6144
• New feature (non-breaking change which adds functionality). Resolves #
• Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

A problem was reported in the issue of uniform sampling not being really random nor uniform, as it iterated triangles deterministically and resulted in same triangle being picked many times in a row.

This change improves the situation by using std::discrete_distribution

Checklist:

• I have run python util/check_style.py --apply to apply Open3D code style
  to my code.
• This PR changes Open3D behavior or adds new functionality.
  • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
    updated accordingly.
    It seems that the random generators are not exposed to Python? so Python docs were not updated only in source C++ docs (hopefully I did not miss something)
  • I have added or updated C++ and / or Python unit tests OR included This also was not done but no tests exist for other random generators as well right?
• I will follow up and update the code if CI fails.
  In case I'm unavailable later anyone is welcome to modify the PR, since most of the heavy lifting is done inside std::discrete_distribution anyways, any edits will hopefully be easy, also hopefully CI won't fail the changes are very non intrusive
• For fork PRs, I have selected Allow edits from maintainers.

Description

Sample code based on original issue author

#include <iostream>
#include <open3d/Open3D.h>
int main() {
  auto sphere = open3d::geometry::TriangleMesh::CreateSphere();
  sphere->ComputeTriangleNormals();
  bool use_triangle_normals = true;
  for (int i = 0; i != 10; i++) {
    auto sampled_pts = sphere->SamplePointsUniformly(1, use_triangle_normals);
    for (int j = 0; j != sampled_pts->points_.size(); j++) {
      std::cout << "Pt: " << sampled_pts->points_[j].transpose()
                << " normal: " << sampled_pts->normals_[j].transpose()
                << std::endl;
    }
  }
  return 0;
}

Original result reported in issue:

Pt: 0.881371 -0.464317 0.035942 normal: 0.850028 -0.520898 0.0782187
Pt: 0.825666 -0.558827 0.0119147 normal: 0.850028 -0.520898 0.0782187
Pt: 0.836437 -0.534213 0.0587785 normal: 0.850028 -0.520898 0.0782187
Pt: 0.886223 -0.453663 0.0541698 normal: 0.850028 -0.520898 0.0782187
Pt: 0.887608 -0.454505 0.0335078 normal: 0.850028 -0.520898 0.0782187
Pt: 0.852025 -0.514956 0.0176215 normal: 0.850028 -0.520898 0.0782187
Pt: 0.873439 -0.475443 0.0480524 normal: 0.850028 -0.520898 0.0782187
Pt: 0.877829 -0.462695 0.0852355 normal: 0.850028 -0.520898 0.0782187
Pt: 0.86169 -0.487152 0.0977552 normal: 0.850028 -0.520898 0.0782187
Pt: 0.879574 -0.460373 0.0817351 normal: 0.850028 -0.520898 0.0782187

Example result after using std::discrete_distribution (note the result is random and will change every time we run, but you can see indeed it is much better now, much better than picking the same triangle every time, truly more random and uniform in terms of unit area on a mesh)

Pt: -0.643155   -0.5143    0.5607 normal: -0.648898 -0.554211  0.521326
Pt:  0.613348 -0.783479 0.0359056 normal:  0.647458 -0.758076 0.0782187
Pt:  0.785933  0.454903 -0.406661 normal:  0.788092  0.482943 -0.381676
Pt:   0.759766 -0.0684343  -0.639144 normal:   0.759048 -0.0597384  -0.648288
Pt: -0.706693  -0.20519  0.671707 normal: -0.740358 -0.177744  0.648288
Pt: 0.283229 0.133719 0.949072 normal: 0.354486 0.146833 0.923461
Pt: -0.00406476    0.816619    0.574857 normal: -0.0669537   0.850727   0.521326
Pt: -0.253326 -0.791642 -0.551146 normal: -0.199212 -0.829779 -0.521326
Pt:  0.536542 -0.374171   0.75161 normal:  0.554734 -0.339941  0.759415
Pt: -0.858788  0.159791  0.480014 normal: -0.829779  0.199212  0.521326

[update-docs]

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

[ghost]

I found a potential issue with using std::discrete_distribution it might affect other calls to other distributions (the uniform_real_distribution used to sampling points in a triangle)

Edit: I found this, so I guess maybe it is a separate issue to use different engines for different distributions?
https://stackoverflow.com/a/18081251/8094047

or maybe only this bug happens in my program, maybe open3d seeding avoid this

Edit2: from further investigation in my own program, it seems that it will likely be an issue for Open3D as well (random values from different distribution objects being correlated because they share the random engine), but it is a separate issue than this, so I guess I should not include it in this pr

[benjaminum]

I found a potential issue with using std::discrete_distribution it might affect other calls to other distributions (the uniform_real_distribution used to sampling points in a triangle)

Edit: I found this, so I guess maybe it is a separate issue to use different engines for different distributions? https://stackoverflow.com/a/18081251/8094047

or maybe only this bug happens in my program, maybe open3d seeding avoid this

Edit2: from further investigation in my own program, it seems that it will likely be an issue for Open3D as well (random values from different distribution objects being correlated because they share the random engine), but it is a separate issue than this, so I guess I should not include it in this pr

Yes I think investigating/fixing this is another PR

[benjaminum]

Running mesh.sample_points_uniformly(1) for 1000 times and collecting the sampled points yields the following.
Old behavior

New behavior