cartographer-project / point_cloud_viewer

View billions of points in your browser.
Apache License 2.0
337 stars 98 forks source link

Remove OctreeMeta's Default instance #431

Closed nnmm closed 4 years ago

nnmm commented 4 years ago

This is to make https://github.com/googlecartographer/point_cloud_viewer/pull/415 smaller.

This makes the two dangerous/unexpected parts more explicit and doesn't allow them to hide in ..Default::default(), namely:

The attribute data types are duplicated in two places now, but that's necessary for moving towards a state where octrees can have more attributes: The occurence in build_octree() will be removed, while the one in from_data_provider() will stay.

In the end, it's a matter of taste.

nnmm commented 4 years ago

meow @feuerste

feuerste commented 4 years ago

Having the Default implementation actually served the purpose of getting rid of redundant code. How about providing a new(resolution: f64, bounding_box: Aabb3<f64>) method instead?

nnmm commented 4 years ago

Would it be fine to make that HashMap a constant as well?

nnmm commented 4 years ago

Ok, we don't have lazy_static as a dependency, I'll go with the new method.

feuerste commented 4 years ago

Using lazy_static and a const hash map is fine with me, too, whatever you prefer...