Skip to content

[Warp Raytrace] Renamed geom to shape#1246

Merged
eric-heiden merged 4 commits into
newton-physics:mainfrom
daniela-hase:dev/rename-geom-to-shape
Dec 12, 2025
Merged

[Warp Raytrace] Renamed geom to shape#1246
eric-heiden merged 4 commits into
newton-physics:mainfrom
daniela-hase:dev/rename-geom-to-shape

Conversation

@daniela-hase

@daniela-hase daniela-hase commented Dec 11, 2025

Copy link
Copy Markdown
Member

Renamed all geom* variables to shape* in the warp rendering code to make it uniform with the rest of Newton.

Newton Migration Guide

Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.

  • The migration guide in docs/migration.rst is up-to date

Before your PR is "Ready for review"

  • Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
  • Documentation is up-to-date
  • Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • Refactor
    • Unified rendering API terminology from "geometry"/"geom" to "shape" across sensors, BVH, raycast, lighting, textures, and examples.
    • Consolidated individual clear parameters into a single ClearData structure for buffer clearing.
    • Standardized world/camera index naming and outputs (shape index image) for consistent coordinate and output handling.
    • Updated public-facing enums/types and function/field names to align with the new "shape" terminology.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Renames geometry-centric identifiers to shape-centric ones across warp_raytrace and tiled camera sensor modules, and consolidates separate clear parameters into a single ClearData structure used throughout the rendering pipeline.

Changes

Cohort / File(s) Summary
Benchmark update
asv/benchmarks/simulation/bench_tiled_camera_sensor.py
Updated two render call sites to replace separate clear_color=None/clear_depth=None args with a single clear_data=None argument.
Public API surface
newton/_src/sensors/warp_raytrace/__init__.py
Export adjustments: removed GeomType/LightType, added ClearData, RenderLightType, RenderShapeType.
Tiled camera sensor
newton/_src/sensors/tiled_camera_sensor.py
Systematic rename geom_*shape_* for fields, buffers, kernels and public APIs; geom_id_imageshape_index_image; DEFAULT_CLEAR_* replaced by DEFAULT_CLEAR_DATA (ClearData); update init, kernels, render calls, outputs.
BVH kernels
newton/_src/sensors/warp_raytrace/bvh.py
Renamed compute_geom_bvh_boundscompute_shape_bvh_bounds; parameters and internal arrays renamed from geom_*shape_*; particle BVH kernel similarly updated.
Lighting
newton/_src/sensors/warp_raytrace/lighting.py
compute_lighting signature and callers updated: bvh_geom_*bvh_shapes_*, geom_*shape_*, use_shadowsenable_shadows, world_idworld_index; added mesh_ids param and reordered args.
Ray helpers
newton/_src/sensors/warp_raytrace/ray.py
ray_mesh_with_bvh and related logic now use and return mesh_shape_id / shape_id instead of mesh_geom_id / geom_id.
Ray cast
newton/_src/sensors/warp_raytrace/ray_cast.py
Renamed constants (NO_HIT_GEOM_IDNO_HIT_SHAPE_ID, etc.), ClosestHit fields (geom_idshape_index, geom_mesh_idshape_mesh_index), functions (closest_hit_geomclosest_hit_shape, first_hit_geomfirst_hit_shape) and all geom_*shape_* parameter/variable renames; world_idworld_index.
Renderer
newton/_src/sensors/warp_raytrace/render.py
Tile/pixel coord returns renamed to world_index/camera_index; render megakernel and wrapper updated: bvh_geom_*bvh_shapes_*, geom_*shape_*, outputs out_shape_index; replaced per-field clear args with clear_data: ClearData.
Render context
newton/_src/sensors/warp_raytrace/render_context.py
Added ClearData dataclass and DEFAULT_CLEAR_DATA; renamed internal/public fields/methods geom_*shape_* (e.g., __init_geom_outputs__init_shape_outputs), create_geom_id_image_outputcreate_shape_index_image_output, render() now accepts clear_data; BVH refit and kernel launches updated.
Textures
newton/_src/sensors/warp_raytrace/textures.py
GeomTypeRenderShapeType; sample_texture_plane(geom_transform)sample_texture_plane(shape_transform); sample_texture signature simplified and renamed params to shape_type, shape_transform, material_index, texture_index.
Examples
newton/examples/sensors/example_sensor_tiled_camera.py
Kernel renames geom_id_to_*shape_index_to_*, param geom_idsshape_indices; calls updated to use create_shape_index_image_output() and shape_index_image; UI label changed to "Show Shape Index Output".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay extra attention to:
    • ray_cast.py and render.py: high-density renames affecting constants, structs, kernel calls and cross-file call sites.
    • render_context.py: new ClearData type and migration of multiple clear-related call sites.
    • lighting.py and textures.py: signature reorderings and parameter reductions that must match all callers.

Possibly related PRs

Suggested reviewers

  • eric-heiden
  • adenzler-nvidia
  • mmacklin

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: a systematic renaming of geom terminology to shape terminology across the warp raytrace codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@daniela-hase daniela-hase marked this pull request as ready for review December 11, 2025 21:45
@codecov

codecov Bot commented Dec 11, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.67442% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
newton/_src/sensors/tiled_camera_sensor.py 95.23% 1 Missing ⚠️
newton/_src/sensors/warp_raytrace/render.py 88.88% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread newton/_src/sensors/tiled_camera_sensor.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
newton/_src/sensors/warp_raytrace/ray.py (2)

42-56: Align map_ray_to_local docstring with actual parameters

The docstring still talks about pos and mat, but the function now takes a single transform. Consider updating the args section for clarity:

-    Args:
-            pos: position of shape frame
-            mat: orientation of shape frame
-            ray_origin_world: starting point of ray in world coordinates
-            ray_direction_world: direction of ray in world coordinates
+    Args:
+        transform: transform of the shape frame in world space
+        ray_origin_world: starting point of the ray in world coordinates
+        ray_direction_world: direction of the ray in world coordinates

This keeps the “shape frame” wording but matches the actual signature.


581-607: ray_mesh_with_bvh rename to mesh_shape_id keeps semantics but check call sites

The function now takes and returns mesh_shape_id, but still uses that value as the index into mesh_bvh_ids. Behavior is preserved relative to the previous mesh_geom_id naming; the only change is the identifier that gets surfaced to callers (last tuple element). Please double‑check that all call sites now pass the ID they want to write into the shape‑index output (vs. a raw mesh index), since that distinction is now encoded in the name.

newton/_src/sensors/tiled_camera_sensor.py (1)

654-662: New shape‑index image output API looks good; consider doc/migration implications

create_shape_index_image_output() wraps the corresponding RenderContext helper and exposes a clear, shape‑centric replacement for any previous geom‑id image output.

Since this is a public API rename, it’s worth checking:

  • Example and user‑facing docs that might still reference the old geom‑id terminology.
  • Whether docs/migration.rst actually needs an entry: prior guidance was that this document is specifically for warp.sim → Newton migrations, and Newton‑only features generally don’t require migration notes. Based on learnings, you may only need to touch it if warp.sim had an analogous geom‑id sensor API.
newton/_src/sensors/warp_raytrace/ray_cast.py (2)

172-174: Consider renaming internal variables for clarity.

The variables gi_global and gi_bvh_local still use the gi (geometry index) prefix even though this function handles particles. Consider renaming to bounds_index_global/bounds_index_local or pi_global/pi_bvh_local for consistency with the shape-focused refactoring.

             while wp.bvh_query_next(query, bounds_nr, closest_hit.distance):
-                gi_global = bounds_nr
-                gi_bvh_local = gi_global - (world_index * bvh_particles_size)
-                si = gi_bvh_local
+                particle_index = bounds_nr - (world_index * bvh_particles_size)

397-399: Same variable naming inconsistency as in closest_hit_particles.

The gi_global/gi_bvh_local variables use geometry-centric naming in a particle-handling function. Consider the same refactor suggested for closest_hit_particles.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56105c0 and 0a87006.

📒 Files selected for processing (11)
  • asv/benchmarks/simulation/bench_tiled_camera_sensor.py (2 hunks)
  • newton/_src/sensors/tiled_camera_sensor.py (15 hunks)
  • newton/_src/sensors/warp_raytrace/__init__.py (1 hunks)
  • newton/_src/sensors/warp_raytrace/bvh.py (2 hunks)
  • newton/_src/sensors/warp_raytrace/lighting.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/ray.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/ray_cast.py (15 hunks)
  • newton/_src/sensors/warp_raytrace/render.py (13 hunks)
  • newton/_src/sensors/warp_raytrace/render_context.py (9 hunks)
  • newton/_src/sensors/warp_raytrace/textures.py (3 hunks)
  • newton/examples/sensors/example_sensor_tiled_camera.py (5 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 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/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).
📚 Learning: 2025-09-09T10:29:35.521Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:86-90
Timestamp: 2025-09-09T10:29:35.521Z
Learning: In Warp's mesh query system, wp.mesh_eval_face_normal returns face normals that point outward from the collider surface by definition. The query.sign from wp.mesh_query_point_sign_normal indicates inside/outside but should not be used to flip the face normal direction, as this would make outward-pointing normals point inward incorrectly.

Applied to files:

  • newton/_src/sensors/warp_raytrace/ray.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/textures.py
  • newton/_src/sensors/warp_raytrace/bvh.py
  • newton/_src/sensors/warp_raytrace/lighting.py
  • newton/_src/sensors/tiled_camera_sensor.py
  • newton/_src/sensors/warp_raytrace/ray_cast.py
  • newton/_src/sensors/warp_raytrace/render.py
  • 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/bvh.py
  • 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 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.py
  • newton/_src/sensors/warp_raytrace/render_context.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/examples/sensors/example_sensor_tiled_camera.py
  • newton/_src/sensors/tiled_camera_sensor.py
  • newton/_src/sensors/warp_raytrace/render.py
📚 Learning: 2025-09-15T13:13:37.976Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:772-783
Timestamp: 2025-09-15T13:13:37.976Z
Learning: In Warp's tiled launch API, using a 2D launch dimension dim=(num_blocks, tile_size) is correct for tiled kernels. The wp.tid() function can either return a 2D tuple (block_index, lane) or just discard the last dimension to return only the block_index when lane information is not needed, making `block_index = wp.tid()` a valid shortcut in tiled kernels.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
🧬 Code graph analysis (8)
newton/_src/sensors/warp_raytrace/__init__.py (1)
newton/_src/sensors/warp_raytrace/render_context.py (2)
  • ClearData (27-31)
  • RenderContext (37-330)
newton/_src/sensors/warp_raytrace/textures.py (1)
newton/_src/sensors/warp_raytrace/types.py (1)
  • GeomType (21-32)
newton/_src/sensors/warp_raytrace/bvh.py (2)
newton/_src/sensors/warp_raytrace/types.py (1)
  • GeomType (21-32)
newton/_src/sensors/tiled_camera_sensor.py (1)
  • compute_mesh_bounds (52-65)
newton/examples/sensors/example_sensor_tiled_camera.py (2)
newton/_src/sensors/tiled_camera_sensor.py (2)
  • create_shape_index_image_output (654-661)
  • flatten_color_image_to_rgba (442-479)
newton/_src/sensors/warp_raytrace/render_context.py (1)
  • create_shape_index_image_output (144-145)
newton/_src/sensors/warp_raytrace/lighting.py (1)
newton/_src/sensors/warp_raytrace/render_context.py (4)
  • particles_position (303-304)
  • particles_position (307-310)
  • particles_radius (313-314)
  • particles_radius (317-320)
newton/_src/sensors/warp_raytrace/ray_cast.py (2)
newton/_src/sensors/warp_raytrace/types.py (1)
  • GeomType (21-32)
newton/_src/sensors/warp_raytrace/ray.py (1)
  • ray_mesh_with_bvh (582-608)
newton/_src/sensors/warp_raytrace/render.py (4)
newton/_src/sensors/warp_raytrace/render_context.py (6)
  • ClearData (27-31)
  • RenderContext (37-330)
  • particles_position (303-304)
  • particles_position (307-310)
  • particles_radius (313-314)
  • particles_radius (317-320)
newton/_src/sensors/warp_raytrace/ray_cast.py (1)
  • closest_hit (216-281)
newton/_src/sensors/warp_raytrace/textures.py (1)
  • sample_texture (88-147)
newton/_src/sensors/warp_raytrace/lighting.py (1)
  • compute_lighting (22-119)
newton/_src/sensors/warp_raytrace/render_context.py (3)
newton/_src/sensors/warp_raytrace/bvh.py (2)
  • compute_bvh_group_roots (237-239)
  • compute_shape_bvh_bounds (145-201)
newton/_src/sensors/tiled_camera_sensor.py (1)
  • render (336-378)
newton/_src/sensors/warp_raytrace/render.py (1)
  • render_megakernel (312-410)
⏰ 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). (2)
  • GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
  • GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (27)
asv/benchmarks/simulation/bench_tiled_camera_sensor.py (1)

89-103: Benchmark render calls correctly opt into the new clear_data API

Passing clear_data=None via keyword while still specifying refit_bvh=False keeps the call compatible with the updated TiledCameraSensor.render signature and cleanly disables per‑frame clears for these tight timing loops. Looks good.

Also applies to: 106-120

newton/_src/sensors/warp_raytrace/__init__.py (1)

16-24: Exporting ClearData from the package is appropriate

Re‑exporting ClearData alongside RenderContext and the enums makes the new clear API discoverable to callers without having to know about render_context.py internals. No issues here.

newton/examples/sensors/example_sensor_tiled_camera.py (1)

77-100: Shape‑index visualization pipeline looks coherent end‑to‑end

The rename from geom‑id to shape‑index is threaded consistently:

  • Kernels now accept shape_indices and either map to semantic colors via semantic_colors or to per‑shape random colors, both writing packed RGBA into the same buffer.
  • tiled_camera_sensor_shape_index_image is allocated via the new create_shape_index_image_output() and passed into TiledCameraSensor.render using the updated keyword.
  • update_texture() uses the shape‑index image both as input and output for the mapping kernels, then feeds it to flatten_color_image_to_rgba, which matches the packed‑RGBA representation.
  • The GUI toggle label “Show Shape Index Output” correctly reflects the new naming.

No correctness issues stand out here.

Also applies to: 211-212, 240-249, 295-335, 371-374

newton/_src/sensors/warp_raytrace/textures.py (1)

36-53: Texture sampling refactor to shape‑centric parameters preserves logic

The plane/mesh branches now consume per‑shape inputs (shape_geom_type, shape_transform, indices) instead of indirect geom IDs, but the core sampling logic and early‑exit checks remain the same. The updated sample_texture_plane call sequence and the PLANE / MESH dispatch look consistent with the new API.

Also applies to: 88-147

newton/_src/sensors/warp_raytrace/lighting.py (1)

22-49: Shape‑based BVH inputs in compute_lighting look consistent; verify first_hit signature

The lighting function now cleanly separates config (enable_shadows, enable_particles) from data (shape_*, mesh_ids, particles) and passes a coherent set of shape‑centric BVH arguments to ray_cast.first_hit. The shadow origin/max‑t handling and visibility reduction are unchanged.

Since this is a fairly wide signature change, please confirm that ray_cast.first_hit has been updated to accept the same parameter order to avoid subtle mismatches.

Also applies to: 79-119

newton/_src/sensors/warp_raytrace/bvh.py (2)

144-202: compute_shape_bvh_bounds correctly switches to shape‑indexed data

The kernel now:

  • Treats shape_enabled as a compact list of enabled shapes (shape_index = shape_enabled[bvh_index_local]).
  • Uses shape_world_index, shape_geom_types, shape_sizes, shape_transforms, and shape_mesh_indices to compute bounds, with the same per‑type logic (sphere, capsule, cylinder, cone, plane, mesh, ellipsoid, box) as before.
  • Writes lowers/uppers and world_index back into the BVH buffers using the local BVH index.

This is a clean terminology and data‑flow update; behavior should remain the same as the previous geom‑based version.


205-234: Particle BVH bounds rename is mechanical

compute_particle_bvh_bounds only renames intermediates (bvh_index_local, shape_index) without changing how world indices or sphere bounds are computed. No functional differences introduced here.

newton/_src/sensors/tiled_camera_sensor.py (7)

24-29: ClearData integration and default clear color are wired sensibly

Importing ClearData and defining a module‑level DEFAULT_CLEAR_DATA = ClearData(0xFF666666) gives a clear, reusable default for callers while still allowing them to pass None to skip clears. Using this constant as the default for render(clear_data=...) is straightforward and keeps behavior explicit.


68-88: Shape enablement helpers match the new shape‑centric RenderContext fields

is_supported_shape_geom_type and compute_enabled_shapes now operate on shape_geom_type / shape_flags and fill:

  • out_shape_enabled as a packed list of visible, supported shapes, and
  • out_mesh_indices as a simple per‑shape index map,

with num_shapes derived from out_shape_enabled_count. This aligns with how compute_shape_bvh_bounds consumes shape_enabled/num_shapes and should be a drop‑in replacement for the previous geom‑based paths.

Also applies to: 90-110


248-308: RenderContext initialization is consistent with shape‑based data and BVH setup

The constructor now:

  • Initializes shape_mesh_indices, mesh_bounds, and all shape_* arrays on render_context.
  • Uses compute_enabled_shapes to populate shape_enabled/num_shapes and compute_mesh_bounds to fill mesh_bounds.
  • For particle/triangle data, keeps the existing pattern of wiring model.particle_* and tri_indices into the context.

This matches the expectations of the updated BVH and render code and keeps the public TiledCameraSensor API unchanged apart from the new shape‑index output.


316-335: State update path correctly switches to shape‑centric transforms

Gating the kernel launch on self.render_context.has_shapes and writing into shape_transforms / shape_sizes mirrors the previous geom‑based update logic and respects the new RenderContext fields. Triangle and particle updates are untouched and still guard on their respective has_* properties.


336-378: render signature and ClearData propagation are coherent

The render method now:

  • Accepts an optional shape_index_image alongside color/depth/normal outputs.
  • Forwards clear_data (defaulting to DEFAULT_CLEAR_DATA) down into render_context.render.
  • Continues to call update_from_state when a state is provided.

Existing call sites using only color/depth still work, and new call sites can opt into shape‑index and customized clear behavior without breaking the API.


563-588: Random color assignment correctly targets shape_colors

Both assign_random_colors_per_world and assign_random_colors_per_shape now populate render_context.shape_colors with per‑shape vec4f colors. The per‑world variant reuses colors based on shape_world, which matches the intended semantics of “colors_per_world”.


605-635: Checkerboard material setup uses the new shape‑material fields correctly

The checkerboard helper enables textures, sets up single‑texture metadata, and then assigns all shapes to material 0 via shape_materials. This is consistent with the shape‑centric material system and should behave identically to the previous geom‑based version.

newton/_src/sensors/warp_raytrace/ray_cast.py (4)

21-24: Constants renamed consistently.

The sentinel constants are properly renamed from *_GEOM_ID to *_SHAPE_ID. The values remain unchanged which ensures backward compatibility for any persisted data.


27-35: Struct field renaming is consistent.

The ClosestHit struct appropriately renames fields from geom_* to shape_*, and changes the suffix from id to index which better reflects that these are array indices rather than unique identifiers.


48-146: Function rename and parameter updates are consistent.

The closest_hit_shape function (renamed from closest_hit_geom) correctly propagates the shape-based naming throughout its parameters and internal logic. The geometry type dispatch logic remains correct.


215-281: Main closest_hit function wiring is correct.

The closest_hit function correctly initializes the ClosestHit struct with the new field names and properly passes all renamed shape parameters to the helper functions.

newton/_src/sensors/warp_raytrace/render_context.py (5)

26-34: ClearData dataclass is well-designed.

The consolidation of clear parameters into a single ClearData dataclass improves the API ergonomics. The use of field(default_factory=...) correctly handles mutable defaults.


83-90: Shape-related field naming is consistent.

The field naming shape_geom_types appropriately indicates that each shape has an associated geometry type. This preserves clarity about what the data represents while aligning with the shape-centric naming convention.


118-126: Private method renamed appropriately.

The __init_shape_outputs method correctly initializes shape-related BVH output buffers with consistent naming.


187-210: Render method signature and implementation updated correctly.

The render() method now accepts shape_index_image and clear_data: ClearData parameters, replacing the previous geometry-centric naming and individual clear parameters. The check for has_shapes is correctly used.


212-230: BVH bounds computation correctly rewired to shape-based naming.

The __compute_bvh_shape_bounds method properly invokes the renamed compute_shape_bvh_bounds kernel with all shape-prefixed inputs and outputs.

newton/_src/sensors/warp_raytrace/render.py (4)

37-49: Coordinate function return values appropriately renamed.

The tid_to_tile_coord function now returns world_index, camera_index instead of world_id, camera_id, which better reflects that these are array indices.


98-109: Kernel parameters renamed consistently with section comments.

The _render_megakernel signature correctly renames all geometry-related parameters to shape-based equivalents. The section comments (# Shapes BVH, # Shapes) are appropriately updated.


163-168: Camera transform access uses consistent indexing.

The camera transforms and rays are correctly accessed using [camera_index, world_index] which aligns with the renamed index variables.


359-370: Kernel launch inputs correctly wired to RenderContext shape fields.

The render_megakernel wrapper correctly passes all renamed rc.shape_* and rc.bvh_shapes* fields to the kernel.

Comment thread newton/_src/sensors/warp_raytrace/render.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
newton/_src/sensors/warp_raytrace/bvh.py (1)

144-201: Inconsistent local variable name: geom_type should be shape_type.

Line 175 still uses geom_type as the local variable name while all other parameters and references have been renamed to use shape_* terminology.

-    geom_type = shape_types[shape_index]
+    shape_type = shape_types[shape_index]

     lower = wp.vec3f()
     upper = wp.vec3f()

-    if geom_type == RenderShapeType.SPHERE:
+    if shape_type == RenderShapeType.SPHERE:
         lower, upper = compute_sphere_bounds(wp.transform_get_translation(transform), size[0])
-    elif geom_type == RenderShapeType.CAPSULE:
+    elif shape_type == RenderShapeType.CAPSULE:
         lower, upper = compute_capsule_bounds(transform, size)
-    elif geom_type == RenderShapeType.CYLINDER:
+    elif shape_type == RenderShapeType.CYLINDER:
         lower, upper = compute_cylinder_bounds(transform, size)
-    elif geom_type == RenderShapeType.CONE:
+    elif shape_type == RenderShapeType.CONE:
         lower, upper = compute_cone_bounds(transform, size)
-    elif geom_type == RenderShapeType.PLANE:
+    elif shape_type == RenderShapeType.PLANE:
         lower, upper = compute_plane_bounds(transform, size)
-    elif geom_type == RenderShapeType.MESH:
+    elif shape_type == RenderShapeType.MESH:
         min_bounds = mesh_bounds[shape_mesh_indices[shape_index], 0]
         max_bounds = mesh_bounds[shape_mesh_indices[shape_index], 1]
         lower, upper = compute_mesh_bounds(transform, size, min_bounds, max_bounds)
-    elif geom_type == RenderShapeType.ELLIPSOID:
+    elif shape_type == RenderShapeType.ELLIPSOID:
         lower, upper = compute_ellipsoid_bounds(transform, size)
-    elif geom_type == RenderShapeType.BOX:
+    elif shape_type == RenderShapeType.BOX:
         lower, upper = compute_box_bounds(transform, size)
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56105c0 and ff15c0b.

📒 Files selected for processing (12)
  • asv/benchmarks/simulation/bench_tiled_camera_sensor.py (2 hunks)
  • newton/_src/sensors/tiled_camera_sensor.py (17 hunks)
  • newton/_src/sensors/warp_raytrace/__init__.py (1 hunks)
  • newton/_src/sensors/warp_raytrace/bvh.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/lighting.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/ray.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/ray_cast.py (15 hunks)
  • newton/_src/sensors/warp_raytrace/render.py (13 hunks)
  • newton/_src/sensors/warp_raytrace/render_context.py (9 hunks)
  • newton/_src/sensors/warp_raytrace/textures.py (4 hunks)
  • newton/_src/sensors/warp_raytrace/types.py (2 hunks)
  • newton/examples/sensors/example_sensor_tiled_camera.py (5 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 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/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).
📚 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__.py
  • newton/_src/sensors/warp_raytrace/textures.py
  • newton/_src/sensors/warp_raytrace/bvh.py
  • newton/_src/sensors/warp_raytrace/types.py
  • newton/_src/sensors/warp_raytrace/lighting.py
  • newton/_src/sensors/warp_raytrace/ray_cast.py
  • newton/_src/sensors/warp_raytrace/render.py
  • newton/_src/sensors/warp_raytrace/render_context.py
📚 Learning: 2025-08-25T20:10:51.009Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 634
File: newton/examples/ik/example_ik_h1.py:0-0
Timestamp: 2025-08-25T20:10:51.009Z
Learning: In Warp, creating a new wp.array() with an existing array and different shape (e.g., wp.array(existing_array, shape=new_shape)) creates a view into the same underlying memory buffer, not a copy. This means modifications through either view are automatically reflected in both arrays.

Applied to files:

  • newton/_src/sensors/warp_raytrace/textures.py
  • 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 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.py
  • 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/bvh.py
  • newton/_src/sensors/warp_raytrace/render_context.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/examples/sensors/example_sensor_tiled_camera.py
  • newton/_src/sensors/warp_raytrace/render.py
  • newton/_src/sensors/tiled_camera_sensor.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/warp_raytrace/render.py
  • newton/_src/sensors/tiled_camera_sensor.py
📚 Learning: 2025-09-15T13:13:37.976Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:772-783
Timestamp: 2025-09-15T13:13:37.976Z
Learning: In Warp's tiled launch API, using a 2D launch dimension dim=(num_blocks, tile_size) is correct for tiled kernels. The wp.tid() function can either return a 2D tuple (block_index, lane) or just discard the last dimension to return only the block_index when lane information is not needed, making `block_index = wp.tid()` a valid shortcut in tiled kernels.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
  • newton/_src/sensors/tiled_camera_sensor.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
📚 Learning: 2025-09-09T10:29:35.521Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:86-90
Timestamp: 2025-09-09T10:29:35.521Z
Learning: In Warp's mesh query system, wp.mesh_eval_face_normal returns face normals that point outward from the collider surface by definition. The query.sign from wp.mesh_query_point_sign_normal indicates inside/outside but should not be used to flip the face normal direction, as this would make outward-pointing normals point inward incorrectly.

Applied to files:

  • newton/_src/sensors/warp_raytrace/ray.py
🧬 Code graph analysis (3)
newton/_src/sensors/warp_raytrace/__init__.py (2)
newton/_src/sensors/warp_raytrace/render_context.py (2)
  • ClearData (27-31)
  • RenderContext (37-330)
newton/_src/sensors/warp_raytrace/types.py (2)
  • RenderLightType (35-42)
  • RenderShapeType (21-32)
newton/_src/sensors/warp_raytrace/textures.py (1)
newton/_src/sensors/warp_raytrace/types.py (1)
  • RenderShapeType (21-32)
newton/_src/sensors/warp_raytrace/ray_cast.py (2)
newton/_src/sensors/warp_raytrace/types.py (1)
  • RenderShapeType (21-32)
newton/_src/sensors/warp_raytrace/ray.py (1)
  • ray_mesh_with_bvh (582-608)
⏰ 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). (1)
  • GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
🔇 Additional comments (51)
newton/_src/sensors/warp_raytrace/types.py (2)

21-32: LGTM - Clean enum rename from GeomType to RenderShapeType.

The rename aligns with the broader shape-centric naming convention. Note that this is a breaking API change. Per the PR description, ensure docs/migration.rst is updated for warp.sim users who may reference the old GeomType name.


35-42: LGTM - Clean enum rename from LightType to RenderLightType.

Consistent with the RenderShapeType naming pattern.

newton/_src/sensors/warp_raytrace/textures.py (3)

18-18: LGTM - Import updated to use RenderShapeType.


36-52: LGTM - sample_texture_plane parameter renamed from geom_transform to shape_transform.

The inverse transform computation on line 45 correctly uses the renamed parameter.


87-124: LGTM - sample_texture signature and logic updated for shape-centric API.

The parameter renames (shape_type, shape_transform, material_index, texture_index) are consistent, and the type comparisons correctly use RenderShapeType.PLANE and RenderShapeType.MESH.

newton/_src/sensors/warp_raytrace/bvh.py (2)

18-18: LGTM - Import updated to use RenderShapeType.


204-233: LGTM - compute_particle_bvh_bounds updated with consistent naming.

The variable renames (bvh_index_local, world_index, shape_index) are consistent with the shape-centric convention.

asv/benchmarks/simulation/bench_tiled_camera_sensor.py (2)

101-102: LGTM - Updated to use new clear_data API.

The change from separate clear_color/clear_depth parameters to the unified clear_data=None aligns with the consolidated ClearData structure introduced in this PR.


118-119: LGTM - Consistent clear_data usage in tiled rendering benchmark.

newton/_src/sensors/warp_raytrace/__init__.py (1)

16-24: LGTM - Public API exports updated for shape-centric naming.

The module now correctly exports ClearData, RenderContext, RenderLightType, and RenderShapeType. This is a breaking change that removes GeomType and LightType from the public API.

newton/_src/sensors/warp_raytrace/ray.py (2)

43-56: LGTM - Docstring updated to reflect shape-centric terminology.

The documentation now correctly references "local shape frame" instead of "local geom frame".


581-608: LGTM - ray_mesh_with_bvh parameter renamed from mesh_geom_id to mesh_shape_id.

The parameter rename is consistent throughout the function body and return statement.

newton/_src/sensors/warp_raytrace/render_context.py (7)

26-34: LGTM - Well-designed ClearData dataclass consolidating clear parameters.

The dataclass cleanly encapsulates all clearing options with sensible defaults:

  • clear_color: 0 (black)
  • clear_depth: 0.0
  • clear_shape_index: 0xFFFFFFFF (sentinel for "no shape")
  • clear_normal: (0, 0, 0)

Using field(default_factory=...) correctly avoids mutable default argument issues with Warp types.


64-90: LGTM - Instance variables renamed from geom_* to shape_*.

All geometry-related fields consistently renamed to shape-centric terminology.


109-126: LGTM - BVH arrays and initialization method renamed to shape-centric naming.

The rename from bvh_geom_* to bvh_shapes_* and __init_geom_outputs to __init_shape_outputs is consistent.


144-145: LGTM - Output creation method renamed from create_geom_id_image_output to create_shape_index_image_output.

Breaking API change - consumers calling the old method name will need to update.


187-210: LGTM - render() method signature updated with shape_index_image and clear_data parameters.

The consolidated clear_data: ClearData | None parameter replaces the previous individual clear parameters, improving API ergonomics.


212-230: LGTM - BVH bounds computation method and kernel call updated for shape-centric naming.

The __compute_bvh_shape_bounds method correctly passes all shape_* arrays to the renamed compute_shape_bvh_bounds kernel.


254-272: LGTM - Properties renamed for shape-centric API.

num_shapes_total and has_shapes correctly replace the previous num_geoms_total and has_geometries.

newton/_src/sensors/warp_raytrace/lighting.py (2)

21-49: LGTM! Function signature updated consistently with shape-centric naming.

The parameter renaming from geom_* to shape_* and world_id to world_index is consistent with the broader refactor. The reordering of parameters (e.g., normal moved after triangle_mesh_id) aligns with the updated ray_cast.first_hit signature.


82-114: Shadow ray casting correctly updated to use shape-based parameters.

The ray_cast.first_hit call now passes the renamed shape-centric parameters in the correct order, matching the updated function signature in ray_cast.py.

newton/examples/sensors/example_sensor_tiled_camera.py (4)

77-99: Kernel renaming from geom_id to shape_index is consistent and correct.

The kernels shape_index_to_semantic_rgb and shape_index_to_random_rgb correctly use shape_index for variable naming internally. The logic remains unchanged.


211-211: Output image creation updated correctly.

The create_shape_index_image_output() call aligns with the renamed API in TiledCameraSensor.


315-334: Kernel launch calls updated consistently with renamed kernels and image outputs.

All references to shape_index_to_semantic_rgb, shape_index_to_random_rgb, and tiled_camera_sensor_shape_index_image are correctly propagated.


373-374: UI label updated appropriately.

"Show Shape Index Output" correctly reflects the renamed output type.

newton/_src/sensors/warp_raytrace/ray_cast.py (9)

19-24: Constants and import updated consistently.

The import of RenderShapeType and renamed constants (NO_HIT_SHAPE_ID, MAX_SHAPE_ID, TRIANGLE_MESH_SHAPE_ID, PARTICLES_SHAPE_ID) align with the shape-centric naming convention.


27-36: ClosestHit struct fields renamed appropriately.

The fields shape_index and shape_mesh_index replace the former geom_id and geom_mesh_id, maintaining semantic clarity.


47-146: closest_hit_shape function correctly refactored from closest_hit_geom.

All parameter names, internal variable names, and RenderShapeType comparisons are consistently updated. The BVH traversal logic and hit detection remain unchanged.


149-189: closest_hit_particles updated with world_index naming.

The particle hit detection correctly uses PARTICLES_SHAPE_ID for the shape index assignment on hit.


192-212: closest_hit_triangle_mesh updated to use shape-centric field names.

Correctly assigns TRIANGLE_MESH_SHAPE_ID and -1 for shape_mesh_index.


215-281: Main closest_hit function updated with shape-centric API.

Initialization uses NO_HIT_SHAPE_ID, and all calls to helper functions use the renamed parameters correctly.


284-371: first_hit_shape function correctly refactored.

The early-out optimization for shadow rays uses shape-centric parameters and RenderShapeType comparisons consistently.


374-411: first_hit_particles updated with world_index naming.

Particle intersection for shadow rays correctly uses the renamed parameters.


429-489: Main first_hit function updated consistently.

All calls to first_hit_shape and first_hit_particles use the renamed shape-centric parameters.

newton/_src/sensors/warp_raytrace/render.py (6)

37-49: Coordinate extraction functions updated with world_index/camera_index naming.

The return values are consistently renamed throughout tile and pixel coordinate calculations.


79-149: Kernel parameters comprehensively renamed to shape-centric naming.

BVH parameters (bvh_shapes_*), shape data (shape_enabled, shape_types, etc.), and output (out_shape_index) are all consistently updated.


163-192: Ray casting and camera transform indexing updated correctly.

The camera_transforms and camera_rays indexing uses camera_index, world_index order, and closest_hit call passes shape-centric parameters.


195-309: Shading, texturing, and lighting paths updated to use shape_index and shape-centric data.

The closest_hit.shape_index is used for color lookup, material access, texture sampling, and final pixel output. All shape array accesses are consistent.


326-336: Clear logic now uses explicit is not None checks.

This correctly addresses the previous review comment about the inconsistent truthiness check for clear_color (which defaults to 0, a falsy value). All clear fields now use is not None for consistency.


312-409: Public render_megakernel function updated with ClearData and shape_index_image.

The API change from individual clear parameters to a single ClearData object is clean, and all kernel inputs are correctly mapped to the renamed RenderContext attributes.

newton/_src/sensors/tiled_camera_sensor.py (11)

26-28: Imports and default clear data updated appropriately.

The import of ClearData, RenderLightType, and RenderShapeType aligns with the module-level type renames. DEFAULT_CLEAR_DATA provides a sensible default gray background.


68-87: is_supported_shape_type updated to use RenderShapeType constants.

All shape type comparisons now reference the renamed enum values consistently.


90-109: compute_enabled_shapes kernel updated with shape-centric output names.

The output arrays out_shape_enabled and out_shape_enabled_count are consistently renamed.


236-238: Class-level type aliases updated.

Exposing RenderLightType and RenderShapeType as class attributes maintains API discoverability.


248-291: __init__ method updated with shape-centric render_context attributes.

All attribute assignments (shape_enabled, shape_types, shape_sizes, shape_transforms, shape_materials, shape_colors, shape_world_index, num_shapes) are consistently renamed.


309-327: update_from_state updated to write to shape-centric attributes.

The kernel outputs now correctly target shape_transforms and shape_sizes.


336-378: render() method API updated with shape_index_image and clear_data.

The parameter rename from geom_id_image to shape_index_image and consolidation of clear parameters into ClearData provides a cleaner API. The docstring is updated accordingly.


563-587: Color assignment methods updated to use shape_colors.

Both assign_random_colors_per_world and assign_random_colors_per_shape correctly write to render_context.shape_colors.


596-603: Default light creation uses RenderLightType.DIRECTIONAL.

The enum reference is updated to match the renamed type.


654-661: create_shape_index_image_output() replaces create_geom_id_image_output().

The method name and docstring are updated to reflect the shape-centric terminology.


605-634: Checkerboard material assignment updated to use shape_materials.

The docstring and implementation correctly reference shapes instead of geometries.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
newton/_src/sensors/tiled_camera_sensor.py (1)

86-86: Minor: Log message still references "geom".

For consistency with the renaming effort, consider updating the message to remove "geom".

-    wp.printf("Unsupported shape geom type: %d\n", shape_type)
+    wp.printf("Unsupported shape type: %d\n", shape_type)
newton/_src/sensors/warp_raytrace/bvh.py (2)

175-175: Minor: Local variable still named geom_type.

For full consistency with the renaming effort, consider renaming this local variable to shape_type.

-    geom_type = shape_types[shape_index]
+    shape_type = shape_types[shape_index]

This would require updating the subsequent comparisons (lines 180-196) to use shape_type instead of geom_type.


204-233: Naming inconsistency: shape_index used for particle index.

In compute_particle_bvh_bounds, the variable shape_index (line 220) is used to index particles, not shapes. This is semantically misleading since particles are distinct from shapes.

Consider using particle_index for clarity:

-    shape_index = bvh_index_local
+    particle_index = bvh_index_local

-    world_index = particle_world_index[shape_index]
+    world_index = particle_world_index[particle_index]
...
-    lower, upper = compute_sphere_bounds(particle_position[shape_index], particle_radius[shape_index])
+    lower, upper = compute_sphere_bounds(particle_position[particle_index], particle_radius[particle_index])
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a87006 and ff15c0b.

📒 Files selected for processing (9)
  • newton/_src/sensors/tiled_camera_sensor.py (17 hunks)
  • newton/_src/sensors/warp_raytrace/__init__.py (1 hunks)
  • newton/_src/sensors/warp_raytrace/bvh.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/lighting.py (3 hunks)
  • newton/_src/sensors/warp_raytrace/ray_cast.py (15 hunks)
  • newton/_src/sensors/warp_raytrace/render.py (13 hunks)
  • newton/_src/sensors/warp_raytrace/render_context.py (9 hunks)
  • newton/_src/sensors/warp_raytrace/textures.py (4 hunks)
  • newton/_src/sensors/warp_raytrace/types.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • newton/_src/sensors/warp_raytrace/init.py
🧰 Additional context used
🧠 Learnings (9)
📓 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/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).
📚 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/types.py
  • newton/_src/sensors/warp_raytrace/render_context.py
  • newton/_src/sensors/warp_raytrace/ray_cast.py
  • newton/_src/sensors/warp_raytrace/render.py
  • newton/_src/sensors/warp_raytrace/textures.py
  • newton/_src/sensors/tiled_camera_sensor.py
  • newton/_src/sensors/warp_raytrace/bvh.py
  • newton/_src/sensors/warp_raytrace/lighting.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
  • 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 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
  • newton/_src/sensors/warp_raytrace/bvh.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/warp_raytrace/render.py
  • newton/_src/sensors/tiled_camera_sensor.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/warp_raytrace/render.py
📚 Learning: 2025-09-15T13:13:37.976Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:772-783
Timestamp: 2025-09-15T13:13:37.976Z
Learning: In Warp's tiled launch API, using a 2D launch dimension dim=(num_blocks, tile_size) is correct for tiled kernels. The wp.tid() function can either return a 2D tuple (block_index, lane) or just discard the last dimension to return only the block_index when lane information is not needed, making `block_index = wp.tid()` a valid shortcut in tiled kernels.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
  • newton/_src/sensors/tiled_camera_sensor.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.

Applied to files:

  • newton/_src/sensors/warp_raytrace/render.py
📚 Learning: 2025-08-25T20:10:51.009Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 634
File: newton/examples/ik/example_ik_h1.py:0-0
Timestamp: 2025-08-25T20:10:51.009Z
Learning: In Warp, creating a new wp.array() with an existing array and different shape (e.g., wp.array(existing_array, shape=new_shape)) creates a view into the same underlying memory buffer, not a copy. This means modifications through either view are automatically reflected in both arrays.

Applied to files:

  • newton/_src/sensors/warp_raytrace/textures.py
  • newton/_src/sensors/warp_raytrace/bvh.py
🧬 Code graph analysis (6)
newton/_src/sensors/warp_raytrace/render_context.py (3)
newton/_src/sensors/warp_raytrace/bvh.py (1)
  • compute_bvh_group_roots (237-239)
newton/_src/sensors/tiled_camera_sensor.py (1)
  • render (336-378)
newton/_src/sensors/warp_raytrace/render.py (1)
  • render_megakernel (312-410)
newton/_src/sensors/warp_raytrace/render.py (3)
newton/_src/sensors/warp_raytrace/render_context.py (6)
  • ClearData (27-31)
  • RenderContext (37-330)
  • particles_position (303-304)
  • particles_position (307-310)
  • particles_radius (313-314)
  • particles_radius (317-320)
newton/_src/sensors/warp_raytrace/ray_cast.py (1)
  • closest_hit (216-281)
newton/_src/sensors/warp_raytrace/textures.py (1)
  • sample_texture (88-147)
newton/_src/sensors/warp_raytrace/textures.py (1)
newton/_src/sensors/warp_raytrace/types.py (1)
  • RenderShapeType (21-32)
newton/_src/sensors/tiled_camera_sensor.py (3)
newton/_src/sensors/warp_raytrace/render_context.py (3)
  • ClearData (27-31)
  • RenderContext (37-330)
  • create_shape_index_image_output (144-145)
newton/_src/sensors/warp_raytrace/types.py (2)
  • RenderLightType (35-42)
  • RenderShapeType (21-32)
newton/_src/sim/builder.py (1)
  • shape_count (932-936)
newton/_src/sensors/warp_raytrace/bvh.py (2)
newton/_src/sensors/warp_raytrace/types.py (1)
  • RenderShapeType (21-32)
newton/_src/sensors/tiled_camera_sensor.py (1)
  • compute_mesh_bounds (52-65)
newton/_src/sensors/warp_raytrace/lighting.py (1)
newton/_src/sensors/warp_raytrace/render_context.py (4)
  • particles_position (303-304)
  • particles_position (307-310)
  • particles_radius (313-314)
  • particles_radius (317-320)
🔇 Additional comments (30)
newton/_src/sensors/tiled_camera_sensor.py (7)

26-28: LGTM: Clean consolidation of clear parameters and type imports.

The import of ClearData, RenderLightType, and RenderShapeType aligns with the shape-centric terminology refactor. The DEFAULT_CLEAR_DATA initialization is straightforward.


90-109: LGTM: Kernel parameter renaming is consistent.

The compute_enabled_shapes kernel correctly renames output parameters from geom_* to shape_* while preserving the original logic.


236-291: LGTM: Class attributes and initialization properly renamed.

All geom_* references in the TiledCameraSensor.__init__ method have been consistently renamed to shape_*, including buffer allocations and kernel launch inputs.


309-334: LGTM: State update method uses consistent shape terminology.

The update_from_state method correctly uses has_shapes guard and shape_transforms/shape_sizes outputs.


654-661: LGTM: Output creation method renamed consistently.

The create_shape_index_image_output method properly delegates to render_context.create_shape_index_image_output() with updated docstring.


605-634: LGTM: Checkerboard material assignment uses correct shape terminology.

Docstring and implementation consistently use "shapes" terminology.


336-378: Verify all call sites are updated for breaking API changes.

The render() method signature has changed significantly:

  1. geom_id_imageshape_index_image
  2. Individual clear_color, clear_depth, clear_geom_id, and clear_normal parameters → consolidated clear_data: ClearData

Ensure all call sites throughout the codebase have been updated to use the new parameter names and consolidated structure.

newton/_src/sensors/warp_raytrace/types.py (2)

21-32: LGTM: Enum renamed to RenderShapeType with preserved values.

The enum correctly maps to GeoType values and the "Render" prefix clearly indicates this is for the raytracer subsystem.


35-42: LGTM: RenderLightType with inline docstrings.

The enum rename and added per-member docstrings improve API documentation.

newton/_src/sensors/warp_raytrace/textures.py (3)

35-52: LGTM: Parameter renamed to shape_transform.

The sample_texture_plane function correctly renames geom_transform to shape_transform with preserved logic.


87-111: LGTM: Simplified sample_texture signature with shape-centric parameters.

The function now takes direct values (shape_type, shape_transform, material_index, texture_index) instead of arrays with index lookups, which is cleaner and aligns with the shape terminology.


113-145: LGTM: Type checks updated to use RenderShapeType.

Both RenderShapeType.PLANE and RenderShapeType.MESH comparisons are correct and consistent with the new enum.

newton/_src/sensors/warp_raytrace/lighting.py (2)

21-49: LGTM: compute_lighting signature updated with shape-centric parameters.

The function signature is comprehensively updated:

  • world_idworld_index for consistency
  • bvh_geom_*bvh_shapes_*
  • geom_*shape_*
  • Added mesh_ids parameter for shape-to-mesh mapping

Parameter reordering groups related inputs logically.


82-114: LGTM: Shadow ray casting uses updated shape-centric parameters.

The ray_cast.first_hit call correctly passes the renamed bvh_shapes_* and shape_* parameters with proper ordering.

newton/_src/sensors/warp_raytrace/bvh.py (2)

144-201: LGTM: Kernel renamed with shape-centric parameters.

The compute_shape_bvh_bounds kernel correctly renames all parameters and uses RenderShapeType for type comparisons. The logic is preserved.


18-18: LGTM: Import updated to RenderShapeType.

Correctly imports the renamed enum from the types module.

newton/_src/sensors/warp_raytrace/ray_cast.py (5)

19-35: LGTM! Consistent renaming of types and constants.

The import and constant renames from geom* to shape* terminology are consistent and maintain the same values. The ClosestHit struct field renames (geom_idshape_index, geom_mesh_idshape_mesh_index) align well with the broader refactoring.


39-146: LGTM! Thorough renaming in ray-shape intersection logic.

The function renaming (closest_hit_geomclosest_hit_shape) and parameter updates are consistent throughout. All internal variables and array accesses have been properly updated to use shape_* terminology, and the intersection logic remains intact.


150-212: LGTM! Particle and triangle mesh hit functions updated consistently.

The special shape IDs (PARTICLES_SHAPE_ID, TRIANGLE_MESH_SHAPE_ID) are correctly assigned to closest_hit.shape_index, and the world_index parameter is properly used throughout. The variable renaming from gi_bvh_local to si improves clarity.


215-282: LGTM! Main closest_hit function properly orchestrates shape-based ray casting.

The wrapper function correctly initializes closest_hit.shape_index to NO_HIT_SHAPE_ID and passes all shape-related parameters to the helper functions. The parameter renaming is complete and consistent.


284-489: LGTM! First-hit detection functions consistently updated.

The first_hit_shape, first_hit_particles, and first_hit functions have been updated with the same consistent shape_* terminology. All parameter renames and function calls are properly aligned with the closest_hit functions.

newton/_src/sensors/warp_raytrace/render.py (5)

24-66: LGTM! Coordinate functions updated with index terminology.

The import of ClearData and the renaming of return values from world_id/camera_id to world_index/camera_index in the coordinate conversion functions are consistent with the broader terminology shift.


79-196: LGTM! Kernel signature and ray casting setup properly updated.

The _render_megakernel function signature has been thoroughly updated with shape_* parameters and render_shape_index/out_shape_index. Ray computation and intersection testing correctly use world_index/camera_index and shape-based parameters throughout.


198-252: LGTM! Output writes and texture sampling use shape indices correctly.

All output array writes properly index with world_index and camera_index. The color, material, and texture lookups correctly use closest_hit.shape_index and closest_hit.shape_mesh_index to access shape-related arrays.


254-309: LGTM! Lighting calculations updated with shape terminology.

The lighting loop correctly passes shape-based BVH and array parameters to compute_lighting, and the final output write properly uses world_index and camera_index.


312-410: LGTM! Wrapper function properly integrated with ClearData and shape terminology.

The render_megakernel wrapper correctly:

  • Uses shape_index_image parameter (renamed from geom_id_image)
  • Implements ClearData with explicit is not None checks for all fields, properly addressing the previous concern about falsy values like wp.int32(0)
  • Passes all shape-based BVH and array parameters to the kernel
newton/_src/sensors/warp_raytrace/render_context.py (4)

18-34: LGTM! ClearData consolidates clear parameters effectively.

The ClearData dataclass properly consolidates separate clear parameters into a single structure with sensible defaults:

  • clear_color: 0 (black)
  • clear_depth: 0.0 (near plane)
  • clear_shape_index: 0xFFFFFFFF (no hit)
  • clear_normal: (0,0,0) (zero vector)

The use of field(default_factory=...) correctly handles mutable defaults, and DEFAULT_CLEAR_DATA provides a convenient default instance.


22-126: LGTM! RenderContext fields and initialization consistently renamed.

All geometry-related fields have been systematically renamed to shape-based equivalents:

  • bvh_geombvh_shapes
  • num_geomsnum_shapes
  • geom_*shape_* for all arrays
  • bvh_geom_*bvh_shapes_* for BVH buffers
  • __init_geom_outputs__init_shape_outputs

The import change to compute_shape_bvh_bounds aligns with updates in the bvh module.


144-210: LGTM! Render methods properly integrated with shape terminology and ClearData.

The output creation, BVH refit, and render methods have been correctly updated:

  • create_shape_index_image_output replaces create_geom_id_image_output
  • BVH refit uses num_shapes_total, bvh_shapes, and shape-related buffers
  • render method signature matches the updates in render_megakernel, accepting shape_index_image and clear_data with DEFAULT_CLEAR_DATA as a convenient default

212-272: LGTM! BVH computation and properties consistently updated.

The BVH bounds computation method (__compute_bvh_shape_bounds) and properties (num_shapes_total, has_shapes) have been properly renamed and updated to use shape-based terminology throughout.

@eric-heiden eric-heiden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@eric-heiden eric-heiden added this pull request to the merge queue Dec 12, 2025
Merged via the queue into newton-physics:main with commit 551f6ee Dec 12, 2025
20 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jan 16, 2026
4 tasks
@daniela-hase daniela-hase deleted the dev/rename-geom-to-shape branch January 21, 2026 00:11
eric-heiden pushed a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Mar 4, 2026
4 tasks
mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026
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