Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving minimum volume OBB matching with Jylänki algorithm. #7138

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

quentin-leboutet
Copy link

This update enhances the OrientedBoundingBox::CreateFromPointsMinimal method to fit a minimum volume oriented bounding box (OBB) to a given point cloud.

Type

  • Bug fix (non-breaking change which fixes an issue):
  • New feature (non-breaking change which adds functionality). Introduces more accurate OBB fitting.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Motivation and Context

Fitting a minimum volume OBB to an arbitrary point cloud is a well-known computational geometry problem. An exact solution to this problem was proposed by O'Rourke in the late 80s, with a cubic time complexity. O’Rourke’s paper also describes a simplified version with quadratic complexity, which is -- I believe -- the variant currently implemented in Open3D. In 2015, Jukka Jylänki proposed a faster algorithm for computing a minimum volume OBB. I used his original implementation as well as the article describing his algorithm to improve the current CreateFromPointsMinimal function. The original approximate approach previously used in CreateFromPointsMinimal has been refactored into a new function named CreateFromPointsMinimalApprox.

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

  • Algorithm Upgrade: The CreateFromPointsMinimal function has been improved using Jylänki's algorithm, resulting in more accurate -- but slower -- OBB fitting.

  • Function Refactoring: The original approximate approach previously used in CreateFromPointsMinimal has been refactored into a new function named CreateFromPointsMinimalApprox. I added a suitable python binding and documented my implementation.

  • Impact: I believe these modifications provide an exact computation of the minimum volume OBB, which can significantly benefit applications that rely on precise spatial bounding computations for large datasets.

Screenshot 2025-01-11 at 14 29 31
Screenshot 2025-01-11 at 14 30 06

Copy link

update-docs bot commented Jan 13, 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 benjaminum January 13, 2025 20:01
@timohl
Copy link
Contributor

timohl commented Jan 14, 2025

Interesting addition.

The tensor-based class o3d.t.geometry.OrientedBoundingBox could also benefit from this (see docs).
Currently, it only supports create_from_points (the PCA variant), but not the former create_from_points_minimal.
It would be nice if create_from_points_approx and create_from_points_minimal could be added there as well.
Right now, it just calls the legacy function, so I think it should be easy to add.

If three create_from_points methods are too much, we could also think about having create_from_points with a mode/algorithm parameter, especially since all three have the same parameters points and robust.

@quentin-leboutet
Copy link
Author

Makes sense. Let me take a closer look at it.
I also noticed that some of the checks failed. I'm gonna investigate that as well.

@quentin-leboutet
Copy link
Author

As suggested I added tensor-based class support.
I still need to investigate why some of the checks failed...

@timohl
Copy link
Contributor

timohl commented Jan 15, 2025

Unit tests for several geometries fail for GetMinimalOrientedBoundingBox which calls OrientedBoundingBox::CreateFromPointsMinimal:

[ RUN      ] TriangleMesh.GetMinimalOrientedBoundingBox
/root/Open3D/cpp/tests/geometry/TriangleMesh.cpp:350: Failure
Expected equality of these values:
  obb.extent_
    Which is: { 1, 3, 2 }
  Eigen::Vector3d(3, 2, 1)
    Which is: { 3, 2, 1 }
[  FAILED  ] TriangleMesh.GetMinimalOrientedBoundingBox (1 ms)

On first view it seems like maybe some dimensions are mixed up, but since your implementation contains over 1000 lines of code, I cannot easily find anything.
Unit test are a good idea for this anyway. 👍
Not sure if this can be simplified or broken down to multiple functions for better testing?
I am usually not a fan of huge functions, but I am also not experienced in adding big algorithms to libraries, nor familiar with this specific algorithm.

Also, there is the question if this should really be used in all those Geometries by default?
How does the performance compare to the previous approximate solution?
Maybe a mode/algorithm parameter would make sense with a default value of the approximate solution?

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