idiomatic

stelzo · May 13, 2024 · 86e7707 · 86e7707
1 parent 902c766
commit 86e7707
Show file tree

Hide file tree

Showing 9 changed files with 527 additions and 332 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -52,3 +52,6 @@ derive = ["dep:rpcl2_derive", "dep:type-layout"]
 nalgebra = ["dep:nalgebra"]
 
 default = ["derive"]
+
+[package.metadata.docs.rs]
+rustdoc-args = ["--generate-link-to-definition"]
diff --git a/README.md b/README.md
@@ -1,9 +1,10 @@
 ## !! Note !!
+
 This library is currently in development for v1.0.0, for the documentation of v0.4.0 on crates.io, visit the [docs](https://docs.rs/ros_pointcloud2/0.4.0/ros_pointcloud2/).
 
 <p align="center">
   <h3 align="center">ROS PointCloud2</h3>
-  <p align="center">A complete and versatile implementation of PointCloud2.</p>
+  <p align="center">A PointCloud2 message conversion library.</p>
   <p align="center"><a href="https://crates.io/crates/ros_pointcloud2"><img src="https://img.shields.io/crates/v/ros_pointcloud2.svg" alt=""></a> <a href="https://github.com/stelzo/ros_pointcloud2/tree/main/tests"><img src="https://github.com/stelzo/ros_pointcloud2/actions/workflows/tests.yml/badge.svg" alt=""></a>
   </p>
 </p>
@@ -83,6 +84,14 @@ Also, indicate the following dependencies to your linker inside the `package.xml
 
 Please open an issue or PR if you need other integrations.
 
+## Performance
+
+The library offers a speed up when compared to PointCloudLibrary (PCL) conversions but the specific factor depends heavily on the use case and system.
+`vec` conversions are on average ~7.5x faster than PCL while the single core iteration `_iter` APIs are around 2x faster.
+Parallelization with `_par_iter` showcases a 10.5x speed up compared to an OpenMP accelerated PCL pipeline.
+
+The full benchmarks are publicly available in this [repository](https://github.com/stelzo/ros_pcl_conv_bench).
+
 ## License
 
 [MIT](https://choosealicense.com/licenses/mit/)
diff --git a/rpcl2_derive/Cargo.toml b/rpcl2_derive/Cargo.toml
@@ -9,4 +9,4 @@ proc-macro = true
 [dependencies]
 syn = "1"
 quote = "1"
-proc-macro2 = "1"
+proc-macro2 = "1"
diff --git a/rpcl2_derive/src/lib.rs b/rpcl2_derive/src/lib.rs
@@ -21,7 +21,7 @@ fn get_allowed_types() -> HashMap<&'static str, usize> {
 
 // Given a field, get the value of the `rpcl2` renaming attribute like
 // #[rpcl2(name = "new_name")]
-fn get_ros_fields_attribute(attrs: &Vec<syn::Attribute>) -> Option<syn::Lit> {
+fn get_ros_fields_attribute(attrs: &[syn::Attribute]) -> Option<syn::Lit> {
     for attr in attrs {
         if attr.path.is_ident("rpcl2") {
             let meta = attr.parse_meta().unwrap();
@@ -50,12 +50,13 @@ fn struct_field_rename_array(input: &DeriveInput) -> Vec<String> {
         _ => panic!("StructNames can only be derived for structs"),
     };
 
-    fields.iter().map(|field| {
-        let field_name = field.ident.as_ref().unwrap();
-        let ros_fields_attr = get_ros_fields_attribute(&field.attrs);
-        match ros_fields_attr {
-            Some(ros_fields) => {
-                match ros_fields {
+    fields
+        .iter()
+        .map(|field| {
+            let field_name = field.ident.as_ref().unwrap();
+            let ros_fields_attr = get_ros_fields_attribute(&field.attrs);
+            match ros_fields_attr {
+                Some(ros_fields) => match ros_fields {
                     syn::Lit::Str(lit_str) => {
                         let val = lit_str.value();
                         if val.is_empty() {
@@ -66,26 +67,26 @@ fn struct_field_rename_array(input: &DeriveInput) -> Vec<String> {
                     _ => {
                         panic!("Only string literals are allowed for the rpcl2 attribute")
                     }
-                }
+                },
+                None => String::from(field_name.to_token_stream().to_string()),
             }
-            None => {
-                String::from(field_name.to_token_stream().to_string())
-            }
-        }
-    }).collect()
+        })
+        .collect()
 }
 
-#[proc_macro_derive(RosFields, attributes(rpcl2))]
+/// This macro will implement the `Fields` trait for your struct so you can use your point for the PointCloud2 conversion.
+///
+/// Use the rename attribute if your struct field name should be different to the ROS field name.
+#[proc_macro_derive(Fields, attributes(rpcl2))]
 pub fn ros_point_fields_derive(input: TokenStream) -> TokenStream {
-    // Parse the input tokens into a syntax tree
     let input = parse_macro_input!(input as DeriveInput);
-
-    // Get the name of the struct
     let struct_name = &input.ident;
 
-    let field_names = struct_field_rename_array(&input).into_iter().map(|field_name| {
-        quote! { #field_name }
-    });
+    let field_names = struct_field_rename_array(&input)
+        .into_iter()
+        .map(|field_name| {
+            quote! { #field_name }
+        });
 
     let field_names_len = field_names.len();
 
@@ -103,70 +104,12 @@ pub fn ros_point_fields_derive(input: TokenStream) -> TokenStream {
     expanded.into()
 }
 
-/// Derive macro for the `Fields` trait.
-///
-/// Given the ordering from the source code of your struct, this macro will generate an array of field names.
-/// If the fields contain attributes for renaming or skipping, the generated array will reflect the final ordering.
-/*#[proc_macro_derive(RosFields)]
-pub fn ros_point_fields_derive(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
-    let name = input.clone().ident;
-
-    let fields = match input.data {
-        syn::Data::Struct(ref data) => data.fields.clone(),
-        _ => {
-            return syn::Error::new_spanned(input, "Only structs are supported")
-                .to_compile_error()
-                .into()
-        }
-    };
-
-    let allowed_datatypes = get_allowed_types();
-
-    if fields.is_empty() {
-        return syn::Error::new_spanned(input, "No fields found")
-            .to_compile_error()
-            .into();
-    }
-
-    for field in fields.iter() {
-        let ty = field.ty.to_token_stream().to_string();
-        if !allowed_datatypes.contains_key(&ty.as_str()) {
-            return syn::Error::new_spanned(field, "Field type not allowed")
-                .to_compile_error()
-                .into();
-        }
-    }
-
-    let field_len_token: usize = fields.len();
-
-    let field_names = fields
-        .iter()
-        .map(|field| {
-            let field_name = field.ident.as_ref().unwrap();
-            quote! { stringify!(#field_name) }
-        })
-        .collect::<Vec<_>>();
-
-    let field_impl = quote! {
-        impl Fields<#field_len_token> for #name {
-            fn field_names_ordered() -> [&'static str; #field_len_token] {
-                [
-                    #(#field_names,)*
-                ]
-            }
-        }
-    };
-
-    TokenStream::from(field_impl)
-}*/
-
 /// This macro will fully implement the `PointConvertible` trait for your struct so you can use your point for the PointCloud2 conversion.
 ///
 /// Note that the repr(C) attribute is required for the struct to work efficiently with C++ PCL.
 /// With Rust layout optimizations, the struct might not work with the PCL library but the message still conforms to the specification of PointCloud2.
 /// Furthermore, Rust layout can lead to smaller messages to be send over the network.
-#[proc_macro_derive(RosFull)]
+#[proc_macro_derive(PointConvertible)]
 pub fn ros_point_derive(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
     let name = input.clone().ident;
@@ -199,13 +142,11 @@ pub fn ros_point_derive(input: TokenStream) -> TokenStream {
 
     let field_len_token: usize = fields.len();
 
-    let field_names = fields
-        .iter()
-        .map(|field| {
-            let field_name = field.ident.as_ref().unwrap();
-            quote! { stringify!(#field_name) }
-        })
-        .collect::<Vec<_>>();
+    let field_names = struct_field_rename_array(&input)
+        .into_iter()
+        .map(|field_name| {
+            quote! { #field_name }
+        });
 
     let field_impl = quote! {
         impl ros_pointcloud2::Fields<#field_len_token> for #name {

diff --git a/src/convert.rs b/src/convert.rs
@@ -1,3 +1,5 @@
+use std::str::FromStr;
+
 use crate::*;
 
 /// Datatypes from the [PointField message](http://docs.ros.org/en/melodic/api/sensor_msgs/html/msg/PointField.html).
@@ -13,9 +15,8 @@ pub enum FieldDatatype {
     I8,
     I16,
 
-    /// While RGB is not officially supported by ROS, it is used in practice as a packed f32.
-    /// To make it easier to work with and avoid packing code, the
-    /// [`ros_pointcloud2::points::RGB`] union is supported here and handled like a f32.
+    /// While RGB is not officially supported by ROS, it is used in the tooling as a packed f32.
+    /// To make it easy to work with and avoid packing code, the [`ros_pointcloud2::points::RGB`] union is supported here and handled like a f32.
     RGB,
 }
 
@@ -35,11 +36,11 @@ impl FieldDatatype {
     }
 }
 
-impl TryFrom<String> for FieldDatatype {
-    type Error = MsgConversionError;
+impl FromStr for FieldDatatype {
+    type Err = MsgConversionError;
 
-    fn try_from(value: String) -> Result<Self, Self::Error> {
-        match value.to_lowercase().as_str() {
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
             "f32" => Ok(FieldDatatype::F32),
             "f64" => Ok(FieldDatatype::F64),
             "i32" => Ok(FieldDatatype::I32),
@@ -49,7 +50,7 @@ impl TryFrom<String> for FieldDatatype {
             "i8" => Ok(FieldDatatype::I8),
             "i16" => Ok(FieldDatatype::I16),
             "rgb" => Ok(FieldDatatype::RGB),
-            _ => Err(MsgConversionError::UnsupportedFieldType(value)),
+            _ => Err(MsgConversionError::UnsupportedFieldType(s.into())),
         }
     }
 }
@@ -148,6 +149,14 @@ impl From<FieldDatatype> for u8 {
     }
 }
 
+impl TryFrom<&ros_types::PointFieldMsg> for FieldDatatype {
+    type Error = MsgConversionError;
+
+    fn try_from(value: &ros_types::PointFieldMsg) -> Result<Self, Self::Error> {
+        Self::try_from(value.datatype)
+    }
+}
+
 /// Matching field names from each data point.
 /// Always make sure to use the same order as in your conversion implementation to have a correct mapping.
 ///
@@ -372,10 +381,22 @@ impl FromBytes for u8 {
     }
 }
 
+pub enum ByteSimilarity {
+    Equal,
+    Overlapping,
+    Different,
+}
+
 #[derive(Default, Clone, Debug, PartialEq, Copy)]
-pub enum Endianness {
+pub enum Endian {
     Big,
-
     #[default]
     Little,
 }
+
+#[derive(Default, Clone, Debug, PartialEq, Copy)]
+pub enum Denseness {
+    #[default]
+    Dense,
+    Sparse,
+}
diff --git a/src/iterator.rs b/src/iterator.rs
@@ -1,5 +1,5 @@
 use crate::{
-    convert::{Endianness, FieldDatatype},
+    convert::{Endian, FieldDatatype},
     Fields, MsgConversionError, PointCloud2Msg, PointConvertible, PointData, RPCL2Point,
 };
 
@@ -162,7 +162,7 @@ struct ByteBufferView<const N: usize> {
     point_step_size: usize,
     offsets: [usize; N],
     meta: Vec<(String, FieldDatatype)>,
-    endianness: Endianness,
+    endian: Endian,
 }
 
 impl<const N: usize> ByteBufferView<N> {
@@ -173,7 +173,7 @@ impl<const N: usize> ByteBufferView<N> {
         end_point_idx: usize,
         offsets: [usize; N],
         meta: Vec<(String, FieldDatatype)>,
-        endianness: Endianness,
+        endian: Endian,
     ) -> Self {
         Self {
             data: std::sync::Arc::<[u8]>::from(data),
@@ -182,7 +182,7 @@ impl<const N: usize> ByteBufferView<N> {
             point_step_size,
             offsets,
             meta,
-            endianness,
+            endian,
         }
     }
 
@@ -205,7 +205,7 @@ impl<const N: usize> ByteBufferView<N> {
                     &self.data,
                     offset + in_point_offset,
                     *meta_type,
-                    self.endianness,
+                    self.endian,
                 );
             });
 
@@ -221,7 +221,7 @@ impl<const N: usize> ByteBufferView<N> {
             point_step_size: self.point_step_size,
             offsets: self.offsets,
             meta: self.meta.clone(),
-            endianness: self.endianness,
+            endian: self.endian,
         }
     }
 
@@ -303,14 +303,8 @@ where
                 *meta_offset = offset;
             });
 
-        let endian = if cloud.is_bigendian {
-            Endianness::Big
-        } else {
-            Endianness::Little
-        };
-
         let point_step_size = cloud.point_step as usize;
-        let cloud_length = cloud.width as usize * cloud.height as usize;
+        let cloud_length = cloud.dimensions.width as usize * cloud.dimensions.height as usize;
         if point_step_size * cloud_length != cloud.data.len() {
             return Err(MsgConversionError::DataLengthMismatch);
         }
@@ -323,7 +317,7 @@ where
             return Err(MsgConversionError::DataLengthMismatch);
         }
 
-        let cloud_length = cloud.width as usize * cloud.height as usize;
+        let cloud_length = cloud.dimensions.width as usize * cloud.dimensions.height as usize;
 
         let data = ByteBufferView::new(
             cloud.data,
@@ -332,7 +326,7 @@ where
             cloud_length - 1,
             offsets,
             meta,
-            endian,
+            cloud.endian,
         );
 
         Ok(Self {