igd-geo / pasture

Rust library for point cloud processing
Apache License 2.0
69 stars 9 forks source link

pasture_algorithms require POSITION_3D attribute to be f64. #23

Open dan-jfisher opened 10 months ago

dan-jfisher commented 10 months ago

It would be nice to generalize this to f32 as well.

My first thought was to accept an AttributeIterator directly in functions like compute_centroid. But that doesn't seem like a good solution.

Maybe it would be useful to have a super trait of PrimitiveType for Vector3<f64> and Vector3<f32>?

I would much rather contribute here than wrap pcl, so I am happy to work on this if necessary.

dan-jfisher commented 10 months ago

Here is an example of what I did to get the behavior I wanted for compute_centroid.

pub fn compute_centroid<'a, T, F>(
    point_cloud: &'a T
) -> Vector3<F>
where
    T: BorrowedBuffer<'a>,
    F: Float + RealField,
    Vector3<F>: PrimitiveType,
{
    if point_cloud.is_empty() {
        panic!("The point cloud is empty!");
    }

    let mut centroid = Vector3::<F>::zeros();
    let mut temp_centroid = Vector3::<F>::zeros();

    if is_dense(point_cloud) {
        // add all points up
        for point in point_cloud.view_attribute::<Vector3<F>>(&POSITION_3D) {
            temp_centroid[0] += point.x;
            temp_centroid[1] += point.y;
            temp_centroid[2] += point.z;
        }

        //normalize over all points
        centroid[0] = temp_centroid[0] / F::from(point_cloud.len()).unwrap();
        centroid[1] = temp_centroid[1] / F::from(point_cloud.len()).unwrap();
        centroid[2] = temp_centroid[2] / F::from(point_cloud.len()).unwrap();
    } else {
        let mut points_in_cloud = 0;
        for point in point_cloud.view_attribute::<Vector3<F>>(&POSITION_3D) {
            if is_finite(&point) {
                // add all points up
                temp_centroid[0] += point.x;
                temp_centroid[1] += point.y;
                temp_centroid[2] += point.z;
                points_in_cloud += 1;
            }
        }

        // normalize over all points
        centroid[0] = temp_centroid[0] / F::from(points_in_cloud).unwrap();
        centroid[1] = temp_centroid[1] / F::from(points_in_cloud).unwrap();
        centroid[2] = temp_centroid[2] / F::from(points_in_cloud).unwrap();
    }

    centroid
}

It would obviously be nice if we could check whether or not the attribute has the right format at compile time, but I don't think that is possible with the current layout structure.

Mortano commented 10 months ago

I agree that the algorithms are not particularly generic at the moment. What you describe is a general theme in pasture due to the fact that PointLayout essentially encodes a type at runtime. So no compile-time checks for layouts, because there are many situations where the layout is not known at compile-time (think reading LAS files with their different point records).

That being said, would the converting attribute views be an option for you? I.e. point_cloud.view_attribute_with_conversion::<Vector3<f64>>(&POSITION_3D). The returned centroid would be Vector3<f64>, but the function itself would work with any point cloud that has a positional attribute that is convertible to Vector3<f64>. You can take a look at calculate_bounds for an example.

If this works for you, I'd be happy to take a PR for this :)

dan-jfisher commented 10 months ago

I appreciate the quick response :)

For context: my company, Fizyr, is in the process of porting our codebase to Rust. If possible, we want to replace our PCL ffi wrappers with Pasture. We are more than happy to contribute if you are open to the features that we would need:

For functions that are going to iterate over every point in the pointcloud, we need the option to run them with f32 rather than converting everything using the AttributeViewConverting.

We would also need a fallible view_attribute_checked function that returns an error rather than panicking if the attribute's type does not match the requested one.

If you have another communication channel where you want to discuss this further, feel free to email me the details. My email is in my bio.

Mortano commented 10 months ago

I sent you a mail :)