Add OpenCV lens distortion rendering to Newton - #6851
Conversation
Use Newton's native fisheye ray generation and add full OpenCV pinhole distortion so calibrated cameras render consistently across backends.
Greptile SummaryThe PR adds OpenCV pinhole and fisheye ray generation to the Newton renderer, including calibrated intrinsics and coefficient muting.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking gap in coverage for the newly advertised rational and thin-prism pinhole coefficients. The renderer dispatch and tested projection paths are coherent, but regressions in k4-k6 and s1-s4 handling would not be detected because all added fixtures disable those terms. Files Needing Attention: source/isaaclab_newton/test/renderers/test_opencv_distortion_rays.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
C[Camera distortion config] --> M{OpenCV model}
M -->|opencvFisheye| F[Newton fisheye ray helper]
M -->|opencvPinhole| P[Warp fixed-point inversion kernel]
F --> R[Per-pixel camera rays]
P --> R
R --> N[Newton tiled renderer]
N --> O[Camera outputs]
Reviews (1): Last reviewed commit: "Add OpenCV lens distortion rendering to ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _PINHOLE_COEFFS = dict( | ||
| k1=0.1, k2=-0.05, k3=0.01, k4=0.0, k5=0.0, k6=0.0, p1=0.001, p2=-0.002, s1=0.0, s2=0.0, s3=0.0, s4=0.0 | ||
| ) |
There was a problem hiding this comment.
The shared nontrivial fixture leaves k4–k6 and s1–s4 at zero, so the round-trip tests never exercise the newly advertised rational-denominator or thin-prism inversion paths. Add nonzero, valid values for these terms to catch coefficient-ordering, sign, and numerical regressions that otherwise produce incorrect edge-of-frame rays without failing this suite.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The Newton backend now consumes existing OpenCV camera calibration configs for pinhole and fisheye ray generation, but the pinhole render-grid mapping introduces a half-pixel calibration offset that should be corrected before merge.
- Design and architecture: The implementation appropriately keeps distortion handling within the Newton renderer, delegates fisheye rays to Newton’s native helper, and uses a dedicated Warp kernel for the full OpenCV pinhole model. The cached ray-field design matches the renderer’s existing camera-ray layout.
- API: No public camera configuration or renderer signatures are changed. Existing OpenCV calibration fields are consumed by Newton, including calibrated intrinsics and the existing apply_lens_distortion flag. The changelog fragment documents the resulting backend behavior change.
- Implementation: The coefficient handling, OpenCV-to-OpenGL basis conversion, ray-buffer allocation, and fixed-point inversion are coherent. However, mapping each render pixel to (px + 0.5) times the calibration scale without subtracting 0.5 shifts rays relative to OpenCV’s integer-centered pixel coordinates, including at native calibration resolution. Use a center-preserving mapping for both axes so the effective principal point matches the authored cx and cy.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
|
|
||
| # Map the render pixel onto the calibrated image grid, then to distorted normalized coordinates. | ||
| # OpenCV image y points down. | ||
| u = ((wp.float32(px) + 0.5) / wp.float32(width)) * image_width |
There was a problem hiding this comment.
🔵 Suggestion · Implementation — Half-pixel offset in calibrated pixel mapping
OpenCV intrinsics are expressed in a frame where integer pixel coordinates are pixel centers, but this maps render pixel px to (px + 0.5) * image_width / width, shifting every ray by half a calibrated pixel and biasing the effective principal point relative to the authored cx/cy. A center-preserving mapping such as (px + 0.5) * image_width / width - 0.5 (and likewise for v) keeps the rendered rays consistent with the calibration this feature exists to honor.
3 participants
Description
PR #6608 introduced renderer-agnostic OpenCV camera calibration configs and native RTX/OVRTX rendering. The Newton path was intentionally left as a documented no-op that emitted a warning and rendered using a centered, square-pixel pinhole projection.
This PR completes backend parity by making
NewtonWarpRendererconsume the same OpenCV calibration and generate distorted per-pixel camera rays.This PR adds native OpenCV lens-distortion rendering to Newton:
k1..k6), tangential (p1,p2), and thin-prism (s1..s4) distortion - conritbuted by @AntoineRichard in Add native OpenCV lens-distortion rendering for the Newton renderer liorbenhorin/IsaacLab#1compute_camera_rays_fisheye_opencvhelper rather than maintaining a duplicate Isaac Lab implementation.fx/fy/cx/cy, including non-square focal lengths and an off-center principal point.apply_lens_distortion=Falsemutes the coefficients while retaining the calibrated intrinsics, matching RTX/OVRTX behavior.This is a stacked follow-up to #6608. It does not change the public camera configuration or USD authoring path introduced there; it only adds Newton renderer consumption of the existing model.
Images
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there