add checked attribute view and float generic in compute centroid #24
Conversation
|
It would probably be better to take an I am happy to go in that direction though. |
There was a problem hiding this comment.
I agree that algorithms over attribute views would probably be better (though some algorithms might require multiple attributes or even mutate data, these would have to take a PointBuffer). If you want to implement it as a PoC for the compute_centroid function that would be fine for me, with the suggested changes (len and is_empty for the views). If not, I'm also fine with merging the changes as is.
Apart from that, for completeness there should be _checked functions for the other view accessors as well, i.e. view, view_attribute_with_conversion and the mutable ones view_mut and view_attribute_mut. Would be great if you could add those.
|
What do you think about making Regardless, I removed that part of PR so that this one is focused on We can handle view_attribute_checked vs unchecked in a separate issue. |
dan-jfisher
force-pushed
the
checked_attribute_view_and_generic_centroid
branch
from
778a1f1
to
94c1e43
Compare
| if point_cloud.is_empty() { | ||
| panic!("The point cloud is empty!"); | ||
| pub fn compute_centroid<'a, 'b, T, F>( | ||
| attribute_view: &AttributeView<'a, 'b, T, Vector3<F>> |
There was a problem hiding this comment.
AttributeView is Copy, so you can pass by value here. This prevents you from having to implement IntoIterator for &AttributeView
#23
Downside of this approach is that it requires the user to specify if their pointcloud has f32 or f64 precision.
If it's important, we can maintain backwards compatibility with some clever naming.
\Edit: for centroid, which I used as an example, we could get around that with a private generic function and a public function that always returns f64 and checks the data_type of the point cloud attribute at runtime. I am not crazy about that solution though.