Skip to content

Conversation

@lmuffang
Copy link

@lmuffang lmuffang commented Oct 1, 2025

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves Translate not implemented for VoxelGrid geometry #3361
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

It was not possible to move around this type of geometry before.

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

This pr implements the Transform, Rotate, Translate and Scale methods for the VoxelGrid object in cpp.
It introduces an origin_transform_ attribute to be able to rotate the grid origin easily and efficiently, keeping all voxels indexes the same.
Python bindings and docs seem to be already done for these methods.

@update-docs
Copy link

update-docs bot commented Oct 1, 2025

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@ssheorey ssheorey requested a review from Copilot October 30, 2025 21:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements geometric transformation methods (Transform, Rotate, Translate, and Scale) for the VoxelGrid class, addressing issue #3361. The implementation introduces an origin_rotation_ attribute to efficiently handle rotations without modifying individual voxel indices.

Key changes:

  • Added origin_rotation_ matrix attribute to VoxelGrid for tracking grid orientation
  • Implemented Transform, Rotate, Translate, and Scale methods using lazy transformation approach
  • Updated PLY I/O to serialize/deserialize the rotation matrix

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cpp/tests/geometry/VoxelGrid.cpp Added comprehensive unit tests for Scale, Translate, Rotate, and Transform operations
cpp/pybind/geometry/voxelgrid.cpp Exposed origin_rotation attribute to Python API
cpp/open3d/visualization/shader/SimpleShader.cpp Updated vertex calculation to incorporate origin rotation matrix
cpp/open3d/visualization/rendering/filament/FilamentGeometryBuffersBuilder.cpp Updated mesh generation to apply rotation transformation
cpp/open3d/io/file_format/FilePLY.cpp Added serialization/deserialization support for origin_rotation matrix
cpp/open3d/geometry/VoxelGrid.h Declared new rotation attribute and GetAllVoxelCorners helper method
cpp/open3d/geometry/VoxelGrid.cpp Implemented transformation methods and updated bound calculations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@ssheorey ssheorey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for adding this missing feature. Can you please rename origin_rotation_ to just rotation_ for clarity?

- also renamed python bindings and ply reader state origin_rotation to rotation for coherence
@lmuffang lmuffang requested a review from ssheorey November 3, 2025 17:40
Copy link
Member

@ssheorey ssheorey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lmuffang Looks good. We can merge once CI passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants