Fm/tianji undistort - #854
Open
farbod-nv wants to merge 1 commit into
Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds native OpenXR cylinder and equirectangular layers alongside generalized quad-layer handling, shared CUDA image mailboxes, backend shape validation, and Python bindings. The camera visualization example gains configurable surface shapes, CloudXR startup handling, gimbal placement, GPU lens undistortion, calibration-based geometry, and runtime statistics. Tests and documentation cover the new APIs, OpenXR constraints, placement behavior, remapping pipeline, and configuration options. Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CameraSource
participant UndistortSource
participant VizRunner
participant VizSession
participant XrBackend
participant OpenXrRuntime
CameraSource->>UndistortSource: Produce calibrated frame
UndistortSource->>VizRunner: Return remapped frame
VizRunner->>VizSession: Submit layer image
VizSession->>XrBackend: Record native layer
XrBackend->>OpenXrRuntime: Submit shaped composition layer
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| "SourceSpec", | ||
| "UndistortSettings", | ||
| "UndistortSource", | ||
| "VizRunner", |
Point a camera at its calibration JSON and the feed is undistorted on the GPU before display. A per-eye sampling LUT is precomputed once from the intrinsics + distortion (+ optional stereo rectification, folded in so pairs come out row-aligned); each frame is then a single bilinear remap kernel on the producer's CUDA stream. Kannala-Brandt fisheye and Brown-Conrady models; the gr00t shw5g ChArUco JSON is accepted verbatim; buffers materialize lazily on the frame's device (multi-GPU hosts). The remap targets the projection matching the display shape — quad → rectilinear, cylinder → cylindrical, equirect → equirectangular — so the shown image is angle-correct end to end. A calibrated camera defaults to shape: cylinder (rectilinear stretches badly at wide FOV and is refused beyond 160°; cylindrical keeps the full FOV at uniform angular resolution — the gr00t WebXR client's in-shader undistort is rectilinear-only). Output FOV is derived from the calibration (Newton inversion of the fisheye polynomial at the image borders) and the layer geometry follows automatically: cylinder arc + aspect = the camera's true FOV, quads sized to the FOV at the configured distance — 1:1 visual angle with no manual tuning; undistort.hfov_deg/vfov_deg/ out_width/out_height override. UndistortSource wraps any FrameSource (all camera types + RTP), so the remap runs exactly once per new frame. 10 new tests (LUT math on CPU, GPU remap round-trip); validated live against CloudXR with the real shw5g stereo calibration (auto FOV 127.4x101.8 deg, zero pipeline loss at 2560x1984 stereo). Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
farbod-nv
force-pushed
the
fm/tianji_undistort
branch
from
July 30, 2026 13:25
b0f22d2 to
cdff588
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/camera_viz/camera_viz.py`:
- Around line 255-264: Update the calibrated size calculation in the
resolve_settings flow so tan-based FOV sizing is used only for quad/rectilinear
shapes with valid rectilinear FOVs. For cylinder/equirect cameras or FOVs
outside the rectilinear range, retain the aspect-derived _DEFAULT_PLANE_WIDTH_M
fallback before assigning spec["size"].
- Around line 534-544: Update the pre-flight shape check around _shape_for to
pass the same calibration presence used by _build_local_entries and
_build_rtp_entries, specifically whether "calib" exists in each camera config.
Keep the existing XR-mode validation and quad exemption unchanged so calibrated
cameras with no explicit shape are correctly treated as cylinders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ac2db08c-e838-4deb-8a01-15c925b968fc
📒 Files selected for processing (44)
docs/source/getting_started/televiz.rstdocs/source/references/camera_streaming.rstexamples/camera_viz/README.mdexamples/camera_viz/camera_viz.pyexamples/camera_viz/configs/oakd.yamlexamples/camera_viz/configs/realsense.yamlexamples/camera_viz/configs/replay.yamlexamples/camera_viz/configs/synthetic.yamlexamples/camera_viz/configs/synthetic_stereo.yamlexamples/camera_viz/configs/v4l2.yamlexamples/camera_viz/configs/zed.yamlexamples/camera_viz/pipeline/__init__.pyexamples/camera_viz/pipeline/runner.pyexamples/camera_viz/pipeline/undistort.pyexamples/camera_viz/placements/lock_modes.pyexamples/camera_viz/scripts/_install_deps.shexamples/camera_viz/sources/zed.pyexamples/camera_viz/tests/test_undistort.pysrc/viz/layers/cpp/CMakeLists.txtsrc/viz/layers/cpp/cylinder_layer.cppsrc/viz/layers/cpp/equirect_layer.cppsrc/viz/layers/cpp/image_layer_base.cppsrc/viz/layers/cpp/inc/viz/layers/cylinder_layer.hppsrc/viz/layers/cpp/inc/viz/layers/equirect_layer.hppsrc/viz/layers/cpp/inc/viz/layers/image_layer_base.hppsrc/viz/layers/cpp/inc/viz/layers/projection_layer.hppsrc/viz/layers/cpp/inc/viz/layers/quad_layer.hppsrc/viz/layers/cpp/quad_layer.cppsrc/viz/layers_tests/cpp/CMakeLists.txtsrc/viz/layers_tests/cpp/test_quad_layer.cppsrc/viz/layers_tests/cpp/test_shaped_layers.cppsrc/viz/python/layers_bindings.cppsrc/viz/python/session_bindings.cppsrc/viz/python/viz_init.pysrc/viz/session/cpp/inc/viz/session/display_backend.hppsrc/viz/session/cpp/inc/viz/session/layer_base.hppsrc/viz/session/cpp/inc/viz/session/viz_session.hppsrc/viz/session/cpp/inc/viz/session/xr_backend.hppsrc/viz/session/cpp/viz_compositor.cppsrc/viz/session/cpp/viz_session.cppsrc/viz/session/cpp/xr_backend.cppsrc/viz/session_tests/cpp/test_quad_milestone.cppsrc/viz/xr/cpp/inc/viz/xr/openxr_session.hppsrc/viz/xr/cpp/openxr_session.cpp
Comment on lines
255
to
+264
| if spec is not None and "size" not in spec: | ||
| spec = { | ||
| **spec, | ||
| "size": [_DEFAULT_PLANE_WIDTH_M, _DEFAULT_PLANE_WIDTH_M * height / width], | ||
| } | ||
| if undistort is not None: | ||
| d = float(spec.get("distance", PlacementConfig.distance)) | ||
| size = [ | ||
| 2.0 * d * math.tan(undistort.hfov_rad / 2.0), | ||
| 2.0 * d * math.tan(undistort.vfov_rad / 2.0), | ||
| ] | ||
| else: | ||
| size = [_DEFAULT_PLANE_WIDTH_M, _DEFAULT_PLANE_WIDTH_M * height / width] | ||
| spec = {**spec, "size": size} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Calibrated size uses tan(fov/2) regardless of shape — negative/degenerate size for wide-FOV cylinder feeds.
resolve_settings only rejects FOV ≥ 160° for rectilinear; a cylinder/equirect camera legitimately keeps hfov_rad > π (see test_rectilinear_rejects_degenerate_fov). 2 d tan(hfov/2) is then negative, and that value is fed into PlacementConfig.size_meters → placement validation blows up (or a garbage quad size is propagated). Gate the angular sizing to the quad shape, or fall back to the aspect-derived size when hfov/vfov exceed the rectilinear range.
🐛 Sketch
- if undistort is not None:
+ if undistort is not None and undistort.projection == "rectilinear":
d = float(spec.get("distance", PlacementConfig.distance))
size = [
2.0 * d * math.tan(undistort.hfov_rad / 2.0),
2.0 * d * math.tan(undistort.vfov_rad / 2.0),
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if spec is not None and "size" not in spec: | |
| spec = { | |
| **spec, | |
| "size": [_DEFAULT_PLANE_WIDTH_M, _DEFAULT_PLANE_WIDTH_M * height / width], | |
| } | |
| if undistort is not None: | |
| d = float(spec.get("distance", PlacementConfig.distance)) | |
| size = [ | |
| 2.0 * d * math.tan(undistort.hfov_rad / 2.0), | |
| 2.0 * d * math.tan(undistort.vfov_rad / 2.0), | |
| ] | |
| else: | |
| size = [_DEFAULT_PLANE_WIDTH_M, _DEFAULT_PLANE_WIDTH_M * height / width] | |
| spec = {**spec, "size": size} | |
| if spec is not None and "size" not in spec: | |
| if undistort is not None and undistort.projection == "rectilinear": | |
| d = float(spec.get("distance", PlacementConfig.distance)) | |
| size = [ | |
| 2.0 * d * math.tan(undistort.hfov_rad / 2.0), | |
| 2.0 * d * math.tan(undistort.vfov_rad / 2.0), | |
| ] | |
| else: | |
| size = [_DEFAULT_PLANE_WIDTH_M, _DEFAULT_PLANE_WIDTH_M * height / width] | |
| spec = {**spec, "size": size} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/camera_viz/camera_viz.py` around lines 255 - 264, Update the
calibrated size calculation in the resolve_settings flow so tan-based FOV sizing
is used only for quad/rectilinear shapes with valid rectilinear FOVs. For
cylinder/equirect cameras or FOVs outside the rectilinear range, retain the
aspect-derived _DEFAULT_PLANE_WIDTH_M fallback before assigning spec["size"].
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/camera_viz/camera_viz.py`:
- Around line 255-264: Update the calibrated size calculation in the
resolve_settings flow so tan-based FOV sizing is used only for quad/rectilinear
shapes with valid rectilinear FOVs. For cylinder/equirect cameras or FOVs
outside the rectilinear range, retain the aspect-derived _DEFAULT_PLANE_WIDTH_M
fallback before assigning spec["size"].
- Around line 534-544: Update the pre-flight shape check around _shape_for to
pass the same calibration presence used by _build_local_entries and
_build_rtp_entries, specifically whether "calib" exists in each camera config.
Keep the existing XR-mode validation and quad exemption unchanged so calibrated
cameras with no explicit shape are correctly treated as cylinders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ac2db08c-e838-4deb-8a01-15c925b968fc
📒 Files selected for processing (44)
docs/source/getting_started/televiz.rstdocs/source/references/camera_streaming.rstexamples/camera_viz/README.mdexamples/camera_viz/camera_viz.pyexamples/camera_viz/configs/oakd.yamlexamples/camera_viz/configs/realsense.yamlexamples/camera_viz/configs/replay.yamlexamples/camera_viz/configs/synthetic.yamlexamples/camera_viz/configs/synthetic_stereo.yamlexamples/camera_viz/configs/v4l2.yamlexamples/camera_viz/configs/zed.yamlexamples/camera_viz/pipeline/__init__.pyexamples/camera_viz/pipeline/runner.pyexamples/camera_viz/pipeline/undistort.pyexamples/camera_viz/placements/lock_modes.pyexamples/camera_viz/scripts/_install_deps.shexamples/camera_viz/sources/zed.pyexamples/camera_viz/tests/test_undistort.pysrc/viz/layers/cpp/CMakeLists.txtsrc/viz/layers/cpp/cylinder_layer.cppsrc/viz/layers/cpp/equirect_layer.cppsrc/viz/layers/cpp/image_layer_base.cppsrc/viz/layers/cpp/inc/viz/layers/cylinder_layer.hppsrc/viz/layers/cpp/inc/viz/layers/equirect_layer.hppsrc/viz/layers/cpp/inc/viz/layers/image_layer_base.hppsrc/viz/layers/cpp/inc/viz/layers/projection_layer.hppsrc/viz/layers/cpp/inc/viz/layers/quad_layer.hppsrc/viz/layers/cpp/quad_layer.cppsrc/viz/layers_tests/cpp/CMakeLists.txtsrc/viz/layers_tests/cpp/test_quad_layer.cppsrc/viz/layers_tests/cpp/test_shaped_layers.cppsrc/viz/python/layers_bindings.cppsrc/viz/python/session_bindings.cppsrc/viz/python/viz_init.pysrc/viz/session/cpp/inc/viz/session/display_backend.hppsrc/viz/session/cpp/inc/viz/session/layer_base.hppsrc/viz/session/cpp/inc/viz/session/viz_session.hppsrc/viz/session/cpp/inc/viz/session/xr_backend.hppsrc/viz/session/cpp/viz_compositor.cppsrc/viz/session/cpp/viz_session.cppsrc/viz/session/cpp/xr_backend.cppsrc/viz/session_tests/cpp/test_quad_milestone.cppsrc/viz/xr/cpp/inc/viz/xr/openxr_session.hppsrc/viz/xr/cpp/openxr_session.cpp
🛑 Comments failed to post (1)
examples/camera_viz/camera_viz.py (1)
534-544: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Pre-flight shape check disagrees with the entry builders — non-quad layers slip into window mode.
_shape_forhere is called withouthas_calib, so a camera withcalib:and no explicitshape:resolves to"quad"and the XR-mode guard is skipped._build_local_entries/_build_rtp_entriespasshas_calib="calib" in camand get"cylinder", so window mode then tries to create a runtime-only cylinder layer — exactly the failure this guard exists to prevent.🐛 Proposed fix
- shape, _, _, _ = _shape_for(cam["name"], placements_cfg) + shape, _, _, _ = _shape_for( + cam["name"], placements_cfg, has_calib="calib" in cam + )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.placements_cfg = cfg.get("display", {}).get("placements", {}) for cam in _enabled_cameras(cfg): shape, _, _, _ = _shape_for( cam["name"], placements_cfg, has_calib="calib" in cam ) if shape == "quad": continue if effective_mode != "xr": raise SystemExit( f"camera_viz: placements.{cam['name']}.shape: {shape} is " "composited by the OpenXR runtime and requires XR mode; " "use --mode xr or shape: quad in window mode." )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/camera_viz/camera_viz.py` around lines 534 - 544, Update the pre-flight shape check around _shape_for to pass the same calibration presence used by _build_local_entries and _build_rtp_entries, specifically whether "calib" exists in each camera config. Keep the existing XR-mode validation and quad exemption unchanged so calibrated cameras with no explicit shape are correctly treated as cylinders.
tengw-nv
reviewed
Jul 30, 2026
| height: 720 | ||
| fps: 60 | ||
| bus_type: usb # usb | gmsl | ||
| # calib: calib/zed_fisheye.json # per-eye K/D (+R_rect) calibration: |
| # out_width: 1920 # output resolution (default: source size) | ||
| # out_height: 1080 | ||
|
|
||
| The calibration JSON carries ``model`` (``fisheye`` for Kannala-Brandt, ``brown``/``pinhole`` |
Contributor
There was a problem hiding this comment.
Should give an example definition. The description here is vague.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #(issue)
Type of change
Testing
Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCOSummary by CodeRabbit
New Features
Documentation
Bug Fixes