[Warp Raytrace] Code Refactoring#1362
Conversation
📝 WalkthroughWalkthroughCentralizes Warp rendering kernels into a new RenderContext utils module and updates SensorTiledCamera to delegate to those utilities. Separates shape counts into Changes
Sequence Diagram(s)sequenceDiagram
participant Sensor as SensorTiledCamera
participant RC as RenderContext
participant Utils as RenderContext.Utils
participant Warp as WarpKernels
Sensor->>RC: call utils API
RC->>Utils: compute_mesh_bounds()
Utils->>Warp: launch compute_mesh_bounds kernel
Warp-->>Utils: mesh bounds written
Sensor->>RC: compute_pinhole_camera_rays(camera_fovs)
RC->>Utils: compute_pinhole_camera_rays(camera_fovs)
Utils->>Warp: launch pinhole rays kernel
Warp-->>Utils: rays buffer
Sensor->>RC: flatten_color/normal/depth(...)
RC->>Utils: flatten_*_to_rgba(...)
Utils->>Warp: launch flatten/find_depth kernels
Warp-->>Utils: flattened RGBA buffers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@newton/_src/sensors/warp_raytrace/debug.py`:
- Around line 1-6: Add the Apache 2.0 license header to the top of this source
file (newton/_src/sensors/warp_raytrace/debug.py) before any imports; ensure the
full standard Apache 2.0 notice and copyright line are included as in the
project’s other files so the header appears above the existing imports
(RenderContext, RenderLightType, numpy, warp).
In `@newton/_src/sensors/warp_raytrace/types.py`:
- Around line 18-31: The fallback GeoType class uses incorrect integer values;
update the fallback constants in the except block so they match the canonical
newton GeoType mapping (e.g. set PLANE=0, SPHERE=2, CAPSULE=3, ELLIPSOID=4,
CYLINDER=5, BOX=6, MESH=7, CONE=9, NONE=11) so that GeoType in this module (and
any dependent RenderShapeType/BVH/ray-casting code) uses the same enum values as
newton._src.geometry.types.GeoType.
In `@newton/_src/sensors/warp_raytrace/utils.py`:
- Line 1: Add the standard Apache 2.0 license header to the top of the module
before any code (i.e., above the existing "import warp as wp" statement) so the
file contains the required license block; ensure the header matches the
project's usual Apache 2.0 text including copyright year and owner.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
newton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/warp_raytrace/utils.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/utils.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/sensor_tiled_camera.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/utils.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/sensor_tiled_camera.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/utils.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/sensor_tiled_camera.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/utils.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/sensor_tiled_camera.py
🧠 Learnings (13)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
Learnt from: daniela-hase
Repo: newton-physics/newton PR: 1044
File: newton/_src/sensors/tiled_camera_sensor.py:451-466
Timestamp: 2025-11-12T23:53:29.478Z
Learning: In newton/_src/sensors/tiled_camera_sensor.py, the convert_camera_to_warp_arrays method extracts [:3, :3] from camera.get_view_matrix().reshape(4, 4) without transposing. This is correct because Pyglet's Mat4.look_at returns a column-major view matrix, and reshaping it as row-major in NumPy creates an implicit transpose, converting the world→camera rotation into the camera→world rotation that the renderer expects.
📚 Learning: 2026-01-13T07:40:13.635Z
Learnt from: CR
Repo: newton-physics/newton PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T07:40:13.635Z
Learning: Applies to newton/**/*.py : Any user-facing class/function/object added under `_src` must be exposed via the public Newton API through re-exports
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/types.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is properly re-exported from the spatial module via "from .spatial import (..., transform_twist, ...)" and included in __all__, making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.py
📚 Learning: 2025-08-14T17:38:36.106Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/__init__.py:25-29
Timestamp: 2025-08-14T17:38:36.106Z
Learning: The Newton project prefers incremental __all__ building using __all__ += [...] pattern to group exports with their related imports, rather than a single consolidated __all__ at the end of the file.
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.py
📚 Learning: 2026-01-13T07:40:13.635Z
Learnt from: CR
Repo: newton-physics/newton PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T07:40:13.635Z
Learning: Applies to newton/examples/**/*.py : Examples must not import from `newton._src`, only from the public Newton API
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/debug.pynewton/_src/sensors/warp_raytrace/utils.pynewton/_src/sensors/warp_raytrace/render.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/types.pynewton/_src/sensors/sensor_tiled_camera.py
📚 Learning: 2025-12-12T17:45:26.847Z
Learnt from: vastsoun
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/geometry/primitives.py:0-0
Timestamp: 2025-12-12T17:45:26.847Z
Learning: In newton/_src/solvers/kamino/geometry/primitives.py, the narrow-phase kernel `_primitive_narrowphase` does not need to handle symmetric shape-pair orderings (e.g., both SPHERE-BOX and BOX-SPHERE) because `kamino.core.builder.ModelBuilder` and `kamino.geometry.primitive.broadphase` guarantee that explicit candidate geometry pairs are always defined with GIDs in ascending order.
Applied to files:
newton/_src/sensors/warp_raytrace/bvh.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/render_context.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/_src/sensors/warp_raytrace/bvh.pynewton/_src/sensors/warp_raytrace/render_context.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/_src/sensors/warp_raytrace/utils.py
📚 Learning: 2025-08-12T18:04:06.577Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:75-75
Timestamp: 2025-08-12T18:04:06.577Z
Learning: The Newton physics framework requires nightly Warp builds, which means compatibility concerns with older stable Warp versions (like missing functions such as wp.spatial_adjoint) are not relevant for this project.
Applied to files:
newton/_src/sensors/warp_raytrace/utils.py
📚 Learning: 2025-11-12T23:53:29.478Z
Learnt from: daniela-hase
Repo: newton-physics/newton PR: 1044
File: newton/_src/sensors/tiled_camera_sensor.py:451-466
Timestamp: 2025-11-12T23:53:29.478Z
Learning: In newton/_src/sensors/tiled_camera_sensor.py, the convert_camera_to_warp_arrays method extracts [:3, :3] from camera.get_view_matrix().reshape(4, 4) without transposing. This is correct because Pyglet's Mat4.look_at returns a column-major view matrix, and reshaping it as row-major in NumPy creates an implicit transpose, converting the world→camera rotation into the camera→world rotation that the renderer expects.
Applied to files:
newton/_src/sensors/sensor_tiled_camera.py
🧬 Code graph analysis (5)
newton/_src/sensors/warp_raytrace/__init__.py (3)
newton/_src/sensors/warp_raytrace/debug.py (1)
RenderDebugUtils(8-53)newton/_src/sensors/warp_raytrace/render_context.py (2)
ClearData(36-40)RenderContext(46-459)newton/_src/sensors/warp_raytrace/types.py (2)
RenderLightType(48-55)RenderShapeType(34-45)
newton/_src/sensors/warp_raytrace/debug.py (3)
newton/_src/sensors/warp_raytrace/render_context.py (1)
RenderContext(46-459)newton/_src/sensors/warp_raytrace/types.py (1)
RenderLightType(48-55)newton/_src/sensors/sensor_tiled_camera.py (4)
assign_random_colors_per_world(343-351)assign_random_colors_per_shape(353-361)create_default_light(363-370)assign_checkerboard_material_to_all_shapes(372-384)
newton/_src/sensors/warp_raytrace/utils.py (2)
newton/_src/sensors/warp_raytrace/render_context.py (2)
compute_mesh_bounds(340-341)compute_pinhole_camera_rays(343-359)newton/_src/sensors/sensor_tiled_camera.py (1)
compute_pinhole_camera_rays(251-280)
newton/_src/sensors/warp_raytrace/render_context.py (2)
newton/_src/sensors/warp_raytrace/bvh.py (4)
compute_bvh_group_roots(237-239)compute_particle_bvh_bounds(205-233)compute_shape_bvh_bounds(145-201)compute_mesh_bounds(22-63)newton/_src/sensors/warp_raytrace/utils.py (6)
compute_mesh_bounds(5-18)compute_pinhole_camera_rays(22-35)find_depth_range(91-96)flatten_color_image(39-61)flatten_depth_image(100-128)flatten_normal_image(65-87)
newton/_src/sensors/warp_raytrace/types.py (1)
newton/_src/geometry/types.py (1)
GeoType(25-67)
🪛 GitHub Actions: License Check
newton/_src/sensors/warp_raytrace/debug.py
[error] 1-1: license-eye: The file does not have a valid license header.
newton/_src/sensors/warp_raytrace/utils.py
[error] 1-1: license-eye: The file does not have a valid license header.
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (30)
newton/_src/sensors/warp_raytrace/utils.py (6)
4-18: LGTM!The mesh bounds computation correctly handles empty meshes (id == 0) by preserving inf/-inf bounds, and properly iterates over all mesh points to compute the axis-aligned bounding box.
21-35: LGTM!The pinhole camera ray computation follows standard conventions: centered pixel sampling, correct aspect ratio handling, and normalized ray directions in camera space (looking down -Z axis).
38-61: LGTM!The color image flattening correctly extracts RGBA channels from packed uint32 values and computes the tiled output layout for multi-world/multi-camera visualization.
64-87: LGTM!The normal image flattening correctly encodes normals from [-1,1] to [0,255] range with alpha set to 255.
90-96: LGTM, but verifydepth_rangeinitialization.The kernel correctly uses atomic operations to find the global depth range. Ensure the caller initializes
depth_rangewith[inf, -inf](or equivalent sentinel values) before launching this kernel to get correct min/max results.
99-128: LGTM!The depth image flattening correctly maps depth values to grayscale (closer=brighter) with division-by-zero protection and reserves value 0 for invalid/no-hit pixels.
newton/_src/sensors/warp_raytrace/render.py (1)
364-364: LGTM!The change to use
rc.num_shapes_enabledinstead of the previousnum_shapescorrectly aligns with the refactor that tracks enabled shapes separately for BVH traversal.newton/_src/sensors/warp_raytrace/bvh.py (1)
146-164: LGTM with a minor observation.The refactor correctly updates the kernel to process only enabled shapes. The shape index lookup via
shape_enabled[bvh_index_local]properly maps local BVH indices to actual shape indices.Note: The bounds check on line 161 (
if bvh_index_local >= num_shapes_enabled) is redundant whentid < num_shapes_enabledsince the modulo on line 160 guaranteesbvh_index_local < num_shapes_enabled. However, this is harmless and may serve as defensive coding if the launch dimension changes in the future.newton/_src/sensors/warp_raytrace/__init__.py (1)
16-20: RenderDebugUtils must be re-exported in the public sensors API if intended for external use.The class is added to the internal
warp_raytracemodule's__all__, but per coding guidelines, any user-facing class under_srcmust be exposed via the public Newton API. Currently,RenderDebugUtilsis not re-exported innewton/sensors.py.If this class is intended for user-facing use, add it to the
__all__list innewton/sensors.pyalong with the other render-related types (ClearData,RenderContext,RenderLightType,RenderShapeType). If it's internal-only, remove it from thewarp_raytracemodule's__all__or clarify via documentation.newton/_src/sensors/warp_raytrace/debug.py (4)
8-15: LGTM!The static method correctly generates seeded random colors and assigns them per world using the shape world index for color lookup.
17-21: LGTM!The method correctly generates unique random colors for each shape.
23-32: LGTM!The method correctly sets up a directional light. The
enable_shadowsparameter controls the global shadow rendering flag whilelights_cast_shadowindicates the light's shadow-casting capability.
34-53: LGTM!The checkerboard texture generation is correct. The procedural pattern uses the sum of row and column checker indices modulo 2 to create alternating squares.
newton/_src/sensors/sensor_tiled_camera.py (5)
25-25: LGTM!Import correctly updated to include
RenderDebugUtilsfrom the warp_raytrace module.
164-167: LGTM!The separation of
num_shapes_totalandnum_shapes_enabledprovides clearer semantics, and delegating mesh bounds computation torender_contextis appropriate for this refactoring.
280-280: LGTM!Delegation to
render_context.compute_pinhole_camera_raysis clean, with input validation appropriately remaining in the public API method.
300-341: LGTM!The flatten methods cleanly delegate to
render_contextwhile preserving the public API signatures and documentation.
351-384: LGTM!The debug utility methods correctly delegate to
RenderDebugUtils, centralizing the implementation while preserving the public API forSensorTiledCamerausers.newton/_src/sensors/warp_raytrace/render_context.py (12)
18-32: LGTM!The new imports for
mathand the utility functions (compute_mesh_bounds,compute_pinhole_camera_rays,find_depth_range, and flatten functions) are appropriate for the new methods being added.
78-79: LGTM!The split of shape counts into
num_shapes_enabled(for BVH operations) andnum_shapes_total(for full shape data) provides clearer semantics and enables proper filtering of invisible/unsupported shapes.
130-138: LGTM!The BVH arrays are correctly sized by
num_shapes_enabledfor per-shape bounds, whilebvh_shapes_group_rootsremains sized bynum_worlds_total.
162-174: LGTM!The
refit_bvhmethod correctly checksnum_shapes_enabledbefore initializing and computing BVH bounds for shapes.
224-242: LGTM!The BVH shape bounds computation correctly uses
num_shapes_enabledfor both the kernel dimension and as an input parameter.
278-280: LGTM!The
has_shapesproperty correctly checksnum_shapes_enabledto determine if there are renderable shapes.
340-341: LGTM!The method correctly launches the
compute_mesh_boundskernel usingmesh_ids.sizeas the kernel dimension.
343-359: LGTM!The method correctly allocates the output array and launches the kernel with appropriate dimensions based on camera count and image resolution.
361-375: LGTM!The helper method correctly calculates grid dimensions for tiled output and handles both buffer creation and reshaping cases.
377-402: LGTM!The method correctly sets up the output buffer and launches the
flatten_color_imagekernel with appropriate dimensions and inputs.
404-429: LGTM!The method follows the same pattern as
flatten_color_image_to_rgba, correctly using theflatten_normal_imagekernel.
431-459: LGTM!The two-pass approach is correct: first computing the depth range using atomic operations, then normalizing the depth values. The initial values (large for min, zero for max) will be properly updated by the kernel's atomic operations.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@newton/_src/sensors/warp_raytrace/render_context.py`:
- Around line 124-132: __init_shape_outputs currently only allocates buffers
when they are None, which allows undersized BVH buffers if num_shapes_enabled
grows at runtime; update __init_shape_outputs to check existing buffer sizes
(for bvh_shapes_lowers, bvh_shapes_uppers, bvh_shapes_groups) against
num_shapes_enabled and reallocate them when their length != num_shapes_enabled,
and also check bvh_shapes_group_roots length against num_worlds_total and
reallocate when mismatched; whenever you reallocate any shape buffer, reset
self.bvh_shapes (to None or equivalent) so the BVH is recreated on next use.
In `@newton/_src/sensors/warp_raytrace/utils.py`:
- Around line 273-278: In assign_random_colors_per_world, guard against
num_shapes_total == 0 to avoid creating colors with length 0 and indexing with
modulo; if self.__render_context.num_shapes_total is zero, set
self.__render_context.shape_colors to an empty wp.array(dtype=wp.vec4f) (or
return early) instead of generating colors and indexing by shape_world_index;
otherwise proceed as before using
colors[self.__render_context.shape_world_index.numpy() % len(colors)] to
populate shape_colors.
🧹 Nitpick comments (2)
newton/_src/sensors/warp_raytrace/utils.py (2)
26-28: Remove stray debug print in TYPE_CHECKING block.
print("MEOW")is a debug artifact and can confuse linters or reviewers; it’s safe to remove.
158-187: Add Google-style docstrings for Utils public methods.Now that
Utilsis a shared interface, adding docstrings for each public method will keep API usage clear and consistent. As per coding guidelines, please use Google-style docstrings.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
newton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/__init__.pynewton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/warp_raytrace/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/_src/sensors/warp_raytrace/init.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
🧠 Learnings (7)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/_src/sensors/warp_raytrace/render_context.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/sensors/warp_raytrace/render_context.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/sensors/warp_raytrace/render_context.pynewton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
📚 Learning: 2025-11-12T23:53:29.478Z
Learnt from: daniela-hase
Repo: newton-physics/newton PR: 1044
File: newton/_src/sensors/tiled_camera_sensor.py:451-466
Timestamp: 2025-11-12T23:53:29.478Z
Learning: In newton/_src/sensors/tiled_camera_sensor.py, the convert_camera_to_warp_arrays method extracts [:3, :3] from camera.get_view_matrix().reshape(4, 4) without transposing. This is correct because Pyglet's Mat4.look_at returns a column-major view matrix, and reshaping it as row-major in NumPy creates an implicit transpose, converting the world→camera rotation into the camera→world rotation that the renderer expects.
Applied to files:
newton/_src/sensors/sensor_tiled_camera.py
📚 Learning: 2025-11-07T01:42:36.906Z
Learnt from: daniela-hase
Repo: newton-physics/newton PR: 1044
File: newton/_src/sensors/tiled_camera_sensor.py:224-256
Timestamp: 2025-11-07T01:42:36.906Z
Learning: In newton/_src/sensors/tiled_camera_sensor.py, the `# noqa: PLC0415` directives on the local PIL imports (lines 224 and 256 in save_color_image and save_depth_image methods) should be kept because they are required by the pre-commit configuration. The local imports are intentional for optional dependency handling.
Applied to files:
newton/_src/sensors/sensor_tiled_camera.pynewton/_src/sensors/warp_raytrace/utils.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/_src/sensors/warp_raytrace/utils.py
🧬 Code graph analysis (3)
newton/_src/sensors/warp_raytrace/render_context.py (1)
newton/_src/sensors/warp_raytrace/utils.py (1)
Utils(158-333)
newton/_src/sensors/sensor_tiled_camera.py (3)
newton/_src/sim/builder.py (1)
shape_count(1135-1139)newton/_src/sensors/warp_raytrace/utils.py (11)
compute_mesh_bounds(32-45)compute_mesh_bounds(162-167)compute_pinhole_camera_rays(49-62)compute_pinhole_camera_rays(169-187)flatten_color_image_to_rgba(189-214)flatten_normal_image_to_rgba(216-241)flatten_depth_image_to_rgba(243-271)assign_random_colors_per_world(273-278)assign_random_colors_per_shape(280-283)create_default_light(285-293)assign_checkerboard_material_to_all_shapes(295-313)newton/_src/sensors/warp_raytrace/bvh.py (1)
compute_mesh_bounds(22-63)
newton/_src/sensors/warp_raytrace/utils.py (3)
newton/_src/sensors/warp_raytrace/types.py (1)
RenderLightType(48-55)newton/_src/sensors/warp_raytrace/render_context.py (1)
RenderContext(38-332)newton/_src/sensors/sensor_tiled_camera.py (8)
compute_pinhole_camera_rays(251-280)flatten_color_image_to_rgba(282-300)flatten_normal_image_to_rgba(302-320)flatten_depth_image_to_rgba(322-341)assign_random_colors_per_world(343-351)assign_random_colors_per_shape(353-361)create_default_light(363-370)assign_checkerboard_material_to_all_shapes(372-384)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-24.04-arm)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (5)
newton/_src/sensors/warp_raytrace/render_context.py (2)
54-74: Clean initialization of shared utils + explicit shape counts.Separation of enabled vs total shapes here makes downstream behavior clearer.
156-223: LGTM: BVH refit + shape gating now follow enabled shapes.Using
num_shapes_enabledfor BVH sizing andhas_shapeschecks makes the render path honor visibility filters.Also applies to: 272-274
newton/_src/sensors/warp_raytrace/utils.py (1)
31-156: Kernel implementations look consistent with the refactor.The ported kernels keep the expected per-world/per-camera layout and data handling.
newton/_src/sensors/sensor_tiled_camera.py (2)
167-167: Nice delegation to render_context.utils.Centralizing mesh bounds, ray generation, and image flattening through
utilssimplifies this sensor and keeps the Warp kernels in one place.Also applies to: 280-384
164-165: No action needed. Verification confirms all references to the renamed fields use the correctnum_shapes_totalandnum_shapes_enablednaming. No stale usages of the oldnum_shapesfield were found in the codebase.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@newton/_src/sensors/warp_raytrace/utils.py`:
- Around line 26-28: Remove the stray debug print inside the TYPE_CHECKING block
in utils.py: delete the print("MEOW") line so the block only contains the
forward type import (from .render_context import RenderContext), leaving the
TYPE_CHECKING guard and its type import intact to avoid stray prints in library
modules.
🧹 Nitpick comments (1)
newton/_src/sensors/warp_raytrace/utils.py (1)
158-160: Add Google-style docstrings for the public Utils API.Please document the class and public methods with parameters/returns for clarity. As per coding guidelines, ...
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/sensors/warp_raytrace/utils.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/sensors/warp_raytrace/utils.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/sensors/warp_raytrace/utils.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/sensors/warp_raytrace/utils.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/sensors/warp_raytrace/utils.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
📚 Learning: 2025-11-07T01:42:36.906Z
Learnt from: daniela-hase
Repo: newton-physics/newton PR: 1044
File: newton/_src/sensors/tiled_camera_sensor.py:224-256
Timestamp: 2025-11-07T01:42:36.906Z
Learning: In newton/_src/sensors/tiled_camera_sensor.py, the `# noqa: PLC0415` directives on the local PIL imports (lines 224 and 256 in save_color_image and save_depth_image methods) should be kept because they are required by the pre-commit configuration. The local imports are intentional for optional dependency handling.
Applied to files:
newton/_src/sensors/warp_raytrace/utils.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.
Applied to files:
newton/_src/sensors/warp_raytrace/utils.py
🧬 Code graph analysis (1)
newton/_src/sensors/warp_raytrace/utils.py (3)
newton/_src/sensors/warp_raytrace/types.py (1)
RenderLightType(48-55)newton/_src/sensors/warp_raytrace/render_context.py (1)
RenderContext(38-332)newton/_src/sensors/warp_raytrace/bvh.py (1)
compute_mesh_bounds(22-63)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (16)
newton/_src/sensors/warp_raytrace/utils.py (16)
31-45: LGTM — mesh bounds kernel is straightforward.
48-62: LGTM — pinhole ray generation math is consistent.
65-88: LGTM — RGBA unpacking and tiling indices look correct.
91-114: LGTM — normal map normalization and alpha channel are correct.
117-123: LGTM — depth range reduction uses atomic min/max on positive depths.
126-155: LGTM — depth normalization and grayscale packing look correct.
162-167: LGTM — kernel launch is sized by mesh_ids.
169-187: LGTM — output allocation and launch dims align with camera_fovs.
189-215: LGTM — buffer reshape and launch dimensions match the image layout.
216-242: LGTM — normal flattening path matches the color path.
243-271: LGTM — two-pass depth range + flatten pipeline is coherent.
273-280: LGTM — zero-shape guard avoids modulo errors.
282-285: LGTM — per-shape random color assignment is straightforward.
297-315: LGTM — checkerboard material setup is consistent.
317-335: LGTM — buffer reshape logic is clear and deterministic.
287-295: Confirm per-light shadow flag respects enable_shadows.
lights_cast_shadowis always set to True; if render kernels rely on this flag, shadows may still render whenenable_shadows=False. Verify whether the globalenable_shadowsflag gates all shadows or if the per-light flag is authoritative, and synchronize accordingly.To investigate, check how the rendering kernel uses
lights_cast_shadowandenable_shadowsto determine if these flags should be wired together.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
0d41de7 to
222e28a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/sensors/warp_raytrace/ray.py (1)
42-61: Stale docstring references non-existent parameters.The docstring references
posandmatparameters that don't exist in the function signature. The actual parameter istransform.📝 Suggested docstring fix
`@wp.func` def map_ray_to_local( transform: wp.transformf, ray_origin_world: wp.vec3f, ray_direction_world: wp.vec3f ) -> tuple[wp.vec3f, wp.vec3f]: """Maps ray to local shape frame coordinates. Args: - pos: position of shape frame - mat: orientation of shape frame + transform: transform of shape frame ray_origin_world: starting point of ray in world coordinates ray_direction_world: direction of ray in world coordinates Returns: 3D point and 3D direction in local shape frame """
🤖 Fix all issues with AI agents
In `@newton/_src/sensors/warp_raytrace/utils.py`:
- Around line 317-335: In __reshape_buffer_for_flatten, guard the case where
num_worlds_and_cameras == 0 to avoid dividing by zero: compute
num_worlds_and_cameras early, and if it's zero return an empty wp.uint8 buffer
shaped (0, 0, 4) (or reshape the provided out_buffer to (0, 0, 4) if out_buffer
is passed) and return num_worlds_per_row as 0; otherwise continue with the
existing math for num_worlds_per_row/num_worlds_per_col and normal reshape
logic.
- Around line 287-295: create_default_light currently sets
self.__render_context.lights_cast_shadow = wp.array([True], dtype=wp.bool) which
ignores the enable_shadows parameter; change it so the per-light shadow flag
uses the enable_shadows value (e.g. wp.array([enable_shadows], dtype=wp.bool))
so calling create_default_light(enable_shadows=False) will mark the created
light as non-casting; adjust only the lights_cast_shadow assignment in
create_default_light to reference enable_shadows while keeping other fields
(lights_active, lights_type, lights_position, lights_orientation) intact.
🧹 Nitpick comments (2)
newton/_src/sensors/warp_raytrace/ray.py (1)
186-186: Consider usingMAXVALfor consistency.This line and similar ones in
ray_cylinder(line 309) use hardcoded1.0e10for initialization, whileMAXVALis defined with the same value. UsingMAXVALwould improve maintainability if the sentinel value ever changes.Similarly, the comparisons at lines 245 and 356 use
1.0e9as the threshold. Consider defining a named constant (e.g.,VALID_HIT_THRESHOLD = 1e9) to clarify the relationship between these values.newton/_src/sensors/warp_raytrace/utils.py (1)
158-160: Confirm public exposure and add API docstrings.If
Utilsis intended to be user-facing, please ensure it’s re-exported via the public Newton API, and add Google-style docstrings for the class and public methods. As per coding guidelines, please ensure any user-facing_srcAPIs are re-exported and documented.
Some refactoring of the Warp Raytracer code.
Moved some things from the TiledCameraSensor class over to the WarpRaytracer code itself, as I needed access to those pieces from TiledCameraSensor unrelated places as well :)
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
Refactor
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.