Voxel grid (#185) #310
Voxel grid (#185) #310
Conversation
|
why touch some many files ? |
|
Sorry, i guess i made some mistake, I just added 2 file, voxelgrid.rs in crates and kornia-py folder. Leme try to fix it. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces voxel grid functionality with Python bindings, enabling downsampling of point clouds through a voxel grid structure.
- Added a new PyVoxelGrid class with methods to construct the voxel grid, add points, and downsample the point cloud.
- Updated the PointCloud conversion and integrated voxel grid functionality with corresponding tests.
- Extended the voxelgrid implementation in the underlying Rust crate with basic unit tests.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| kornia-py/src/voxelgrid.rs | Implements Python bindings for voxel grid operations including initialization and downsampling. |
| kornia-py/src/pointcloud.rs | Minor update to support the point cloud conversion used in the Python bindings. |
| crates/kornia-3d/src/voxelgrid.rs | Provides core voxel grid functionality and a downsampling test to validate behavior. |
Comments suppressed due to low confidence (2)
kornia-py/src/voxelgrid.rs:45
- [nitpick] Consider using a more specific Python exception type instead of the generic PyException to provide clearer error messages when array reshaping fails.
.map_err(|e| PyErr::new::<pyo3::exceptions::PyException, _>(format!("{}", e)))?;
kornia-py/src/voxelgrid.rs:41
- [nitpick] Consider verifying that the length of the flattened points vector is divisible by 3 before reshaping, to prevent potential errors with empty point clouds or corrupted data.
let downsampled_points = downsampled.points().iter().flat_map(|p| p.iter().cloned()).collect::<Vec<f64>>();
|
Please rebase on main to get rid of ci issues. Please, send a screenshot showing that local test passed correctly |
There was a problem hiding this comment.
Can you provide what's the source reference for this implementation ? I believe we should provide a similar functionality found here https://github.com/isl-org/Open3D/blob/main/cpp/open3d/geometry/VoxelGrid.h (not copying but re-implementing) put together the minimum pieces for it
crates/kornia-3d/src/voxelgrid.rs
Outdated
| leaf_size: [f64; 3], | ||
| min_bounds: [f64; 3], | ||
| max_bounds: [f64; 3], | ||
| grid: std::collections::HashMap<[i64; 3], Vec<[f64; 3]>>, |
There was a problem hiding this comment.
to follow library and in general Rust good practices, add the import on top of the file
crates/kornia-3d/src/voxelgrid.rs
Outdated
| @@ -0,0 +1,91 @@ | |||
| /// A voxel grid for organizing and downsampling point clouds. | |||
| pub struct VoxelGrid { | |||
| leaf_size: [f64; 3], | |||
There was a problem hiding this comment.
i would use Vec3 from glam-rs as we have been updating the other 3d library components with it
|
open3d voxel implementation has separate function for transform, translate, scale rotate and other operations, I just implemented downsample function. Downsample was done on basis of simple averaging values in each voxel. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new voxel grid feature with Python bindings to the existing 3D voxel grid implementation.
- Introduces Python wrapping for the voxel grid through the PyVoxelGrid class.
- Adds core voxel grid functionality and associated tests in the kornia-3d crate.
- Applies a minor update to the point cloud module.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| kornia-py/src/voxelgrid.rs | Adds Python bindings for creating and manipulating voxel grids. |
| kornia-py/src/pointcloud.rs | Minor update to the point cloud module. |
| crates/kornia-3d/src/voxelgrid.rs | Implements voxel grid functionality and downsampling with tests. |
Comments suppressed due to low confidence (1)
kornia-py/src/voxelgrid.rs:43
- [nitpick] The error message for a coordinate count not divisible by 3 could be enhanced to describe the expected format of the point cloud data.
if downsampled_points.len() % 3 != 0 {
crates/kornia-3d/src/voxelgrid.rs
Outdated
| } | ||
| } | ||
|
|
||
| /// Add points from a point cloud to the voxel grid. |
There was a problem hiding this comment.
add proper docs for public methods
…point and reverse for using glam in voxel since point cloud is using points
|
|
Hi @edgarriba where the changes made correct ? Please suggest changes if any. |
Voxel grid