Tangent Space and Ambient Occlusion #7364
Conversation
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for tangent space computation and ambient occlusion rendering to triangle meshes. The key changes include implementing MikkTSpace for tangent space calculation, enabling normal map transformations between world and tangent space, and adding ambient occlusion texture generation using ray casting.
Key changes:
- Integration of MikkTSpace library for computing tangent space vectors
- New methods for ambient occlusion computation and normal map transformation
- Addition of quasirandom sequence generation and cross product operations for tensor math
- Wayland compatibility fixes for GLFW window positioning
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/open3d/t/geometry/TriangleMesh.cpp | Implements ComputeTangentSpace(), TransformNormalMap(), and ComputeAmbientOcclusion() methods |
| cpp/open3d/t/geometry/kernel/mikktspace.h | Adds MikkTSpace library header for tangent space computation |
| cpp/open3d/t/geometry/kernel/mikktspace.c | Adds MikkTSpace library implementation (1899 lines) |
| cpp/open3d/core/Tensor.cpp | Implements Quasirandom(), Cross(), and Norm() tensor operations |
| cpp/tests/t/geometry/TriangleMesh.cpp | Adds comprehensive tests for tangent space and ambient occlusion |
| cpp/pybind/t/geometry/trianglemesh.cpp | Exposes new mesh methods to Python bindings |
| cpp/open3d/visualization/visualizer/Visualizer.cpp | Adds Wayland platform checks for window positioning |
| cpp/open3d/t/io/file_format/FileASSIMP.cpp | Updates texture export to include ambient occlusion maps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // https://extremelearning.com.au/unreasonable-effectiveness-of-quasirandom-sequences/ | ||
| // for more details. |
There was a problem hiding this comment.
The URL reference should be in a Doxygen comment format. These C++ style comments (//) will not appear in generated documentation. Use /// instead for Doxygen comments.
| // https://extremelearning.com.au/unreasonable-effectiveness-of-quasirandom-sequences/ | |
| // for more details. | |
| /// https://extremelearning.com.au/unreasonable-effectiveness-of-quasirandom-sequences/ | |
| /// for more details. |
| /// t[2, 0:4:2] = np.empty((2, 5), dtype=np.float32) | ||
| /// ``` | ||
| /// | ||
| /// Tensor Cross(const Tensor &b) const; |
There was a problem hiding this comment.
This line appears to be leftover documentation that doesn't belong here. It's placed in the middle of unrelated SetItem documentation and doesn't match the actual Cross() method signature (which takes const Tensor &other and int64_t axis=-1). This should be removed.
| /// Tensor Cross(const Tensor &b) const; |
| // (i.e. broadcastable) shape, and the size of the specified axis must be 3. | ||
| // If the axis is -1 (default), the last axis is used. |
There was a problem hiding this comment.
Inconsistent comment style in Doxygen documentation. Lines 639-640 use // instead of ///. All lines in a Doxygen comment block should use /// for proper documentation generation.
| // (i.e. broadcastable) shape, and the size of the specified axis must be 3. | |
| // If the axis is -1 (default), the last axis is used. | |
| /// (i.e. broadcastable) shape, and the size of the specified axis must be 3. | |
| /// If the axis is -1 (default), the last axis is used. |
| /// \param p The order of the norm. Default is 2.0 (L2 norm). Can be | ||
| /// positive infinity (L-infinity norm). | ||
| /// \return Tensor containing the computed norms. Dtype is | ||
| /// preserved for Float32 and Float64 for L2 norm, It is onverted to Float32 |
There was a problem hiding this comment.
Corrected spelling of 'converted' from 'onverted'.
| /// preserved for Float32 and Float64 for L2 norm, It is onverted to Float32 | |
| /// preserved for Float32 and Float64 for L2 norm, It is converted to Float32 |
| utility::LogError( | ||
| "[Tensor::Cross] Dim axis={} of both tensors must have shape " | ||
| "3. Got {} and {} instead.", | ||
| axis, GetShape(3), other.GetShape(3)); |
There was a problem hiding this comment.
Error message uses hardcoded index 3 instead of the axis parameter. Line 1149 should use GetShape(axis) and other.GetShape(axis) to match the actual dimension being checked.
| axis, GetShape(3), other.GetShape(3)); | |
| axis, GetShape(axis), other.GetShape(axis)); |
| // bool visible = !(flags & FLAG_HIDDEN); | ||
| glfwWindowHint(GLFW_VISIBLE, GLFW_TRUE); |
There was a problem hiding this comment.
The commented-out code for handling window visibility should either be removed or replaced with a TODO comment explaining why it was disabled. Leaving dead code without explanation reduces maintainability.
| //if (!visible) { | ||
| // glfwHideWindow(glfw_window); | ||
| //} else { | ||
| // glfwShowWindow(glfw_window); | ||
| //} |
There was a problem hiding this comment.
Large block of commented-out code without explanation. If this code is no longer needed, it should be removed. If it's kept for reference, add a comment explaining why it's disabled.
| //if (!visible) { | |
| // glfwHideWindow(glfw_window); | |
| //} else { | |
| // glfwShowWindow(glfw_window); | |
| //} |
Type
Motivation and Context
Normal maps are saved in tangent space. This PR adds functions to compute the tangent space of a mesh and convert a normal map between world space and tangent space.
Ambient occlusion allows meshes to be visualized properly without any color information. It can also be used for simple de-lighting.
Checklist:
python util/check_style.py --applyto apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description
This PR adds support for tangent space computation and ambient occlusion rendering to triangle meshes. The key changes include implementing MikkTSpace for tangent space calculation, enabling normal map transformations between world and tangent space, and adding ambient occlusion texture generation using ray casting.
Key changes: