Fix Newton model not built for sensors added in _setup_scene()#5051
Fix Newton model not built for sensors added in _setup_scene()#5051nblauch wants to merge 1 commit intoisaac-sim:developfrom
Conversation
InteractiveScene.__init__() resolves renderer requirements from sensors declared in the scene config, but DirectRLEnv environments add sensors programmatically in _setup_scene() after the scene is constructed. This means PhysxSceneDataProvider is never told it needs a Newton model, so get_newton_model() returns None and Newton-based renderers/visualizers get a black screen. Add _update_scene_data_requirements_from_sensors() to DirectRLEnv which re-scans all scene sensors after _setup_scene() and merges their renderer requirements into the simulation context before the provider is constructed.
Greptile SummaryThis PR fixes a bug in The fix adds Key points:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant DRL as DirectRLEnv.__init__
participant IS as InteractiveScene.__init__
participant Sim as SimulationContext
participant SDP as SceneDataProvider
DRL->>IS: InteractiveScene(cfg.scene)
IS->>IS: _add_entities_from_cfg()
IS->>IS: _sensor_renderer_types() → [] (no programmatic sensors yet)
IS->>Sim: update_scene_data_requirements(requirements)<br/>requires_newton_model=False
IS->>IS: resolve_visualizer_clone_fn(requirements)<br/>→ None (no Newton model needed)
IS->>IS: clone_environments() [artifact NOT prebuilt]
IS-->>DRL: scene ready
DRL->>DRL: _setup_scene() [programmatic sensors added here]
Note over DRL,Sim: NEW fix — re-scan after _setup_scene()
DRL->>IS: _sensor_renderer_types() → [newton_warp, ...]
DRL->>Sim: get_scene_data_requirements() → current_req
DRL->>DRL: aggregate_requirements([current_req, sensor_req])
DRL->>Sim: update_scene_data_requirements(merged)<br/>requires_newton_model=True ✓
DRL->>Sim: reset()
Sim->>SDP: SceneDataProvider(stage, sim)
SDP->>Sim: get_scene_data_requirements() → requires_newton_model=True
SDP->>SDP: build Newton model ✓
SDP->>Sim: get_scene_data_visualizer_prebuilt_artifact() → None<br/>(artifact was not prebuilt — fallback to USD rebuild)
Last reviewed commit: "Fix Newton model not..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| resolve_scene_data_requirements, | ||
| ) | ||
|
|
||
| renderer_types = self.scene._sensor_renderer_types() |
There was a problem hiding this comment.
self.scene._sensor_renderer_types() calls a name-prefixed protected method on InteractiveScene from outside that class. This creates tight coupling to an implementation detail: any internal refactor of InteractiveScene (e.g., renaming the method, changing its return type, or merging it into another helper) would silently break this fix with no IDE/type-checker warning.
Consider exposing a public sensor_renderer_types() method on InteractiveScene, or a dedicated public helper such as scene.get_renderer_requirements() that returns the already-resolved SceneDataRequirement. That way the contract is stable and tested independently.
| from isaaclab.physics.scene_data_requirements import ( | ||
| aggregate_requirements, | ||
| resolve_scene_data_requirements, | ||
| ) |
There was a problem hiding this comment.
aggregate_requirements and resolve_scene_data_requirements are imported inside the method body. The rest of the file follows the standard pattern of top-level imports. Importing inside a method hides the dependency from tools (linters, type-checkers, dependency scanners) and adds a small repeated cost if the method is ever called more than once.
| from isaaclab.physics.scene_data_requirements import ( | |
| aggregate_requirements, | |
| resolve_scene_data_requirements, | |
| ) | |
| from isaaclab.physics.scene_data_requirements import ( | |
| aggregate_requirements, | |
| resolve_scene_data_requirements, | |
| ) | |
| renderer_types = self.scene._sensor_renderer_types() |
Prefer moving the two imports to the module-level import block at the top of the file alongside the other isaaclab.* imports.
| def _update_scene_data_requirements_from_sensors(self): | ||
| """Re-scan scene sensors and update simulation context requirements. | ||
|
|
||
| ``InteractiveScene.__init__()`` resolves renderer requirements from | ||
| sensors declared in the scene *config*, but sensors added | ||
| programmatically in :meth:`_setup_scene` (the standard ``DirectRLEnv`` | ||
| pattern) are missed. This method re-scans all scene sensors after | ||
| ``_setup_scene()`` returns and merges their renderer requirements into | ||
| the simulation context so that ``PhysxSceneDataProvider`` (constructed | ||
| later) knows whether it needs to build a Newton model. | ||
| """ | ||
| from isaaclab.physics.scene_data_requirements import ( | ||
| aggregate_requirements, | ||
| resolve_scene_data_requirements, | ||
| ) | ||
|
|
||
| renderer_types = self.scene._sensor_renderer_types() | ||
| if not renderer_types: | ||
| return | ||
| sensor_req = resolve_scene_data_requirements(visualizer_types=[], renderer_types=renderer_types) | ||
| current_req = self.sim.get_scene_data_requirements() | ||
| merged = aggregate_requirements([current_req, sensor_req]) | ||
| if merged != current_req: | ||
| self.sim.update_scene_data_requirements(merged) |
There was a problem hiding this comment.
Inside InteractiveScene.__init__(), the visualizer_clone_fn (and thus the Newton-model prebuilt artifact fast path) is resolved from _sensor_renderer_types() before _setup_scene() runs:
# Inside InteractiveScene.__init__() — sensors from _setup_scene() not yet registered
requirements = resolve_scene_data_requirements(
visualizer_types=requested_viz_types,
renderer_types=self._sensor_renderer_types(), # empty for programmatic sensors
)
self.sim.update_scene_data_requirements(requirements)
visualizer_clone_fn = cloner.resolve_visualizer_clone_fn(
requirements=requirements, ...
)This means visualizer_clone_fn is None (or is resolved without requires_newton_model=True) when sensors are added only in _setup_scene(). As a result, set_scene_data_visualizer_prebuilt_artifact is never called with a valid Newton-model artifact, so sim.get_scene_data_visualizer_prebuilt_artifact() returns None.
The new _update_scene_data_requirements_from_sensors() correctly patches sim._scene_data_requirements, ensuring SceneDataProvider is told it needs a Newton model. Whether the provider can build the Newton model correctly from USD when the prebuilt artifact is missing (the fallback path) depends on the SceneDataProvider implementation. If that fallback is always valid, the fix is sufficient. If SceneDataProvider relies on the artifact to correctly wire the multi-env Newton model during initialization, the fix would address the symptom (wrong requirements) but the artifact gap could still cause a black screen in certain backends.
It is worth verifying that SceneDataProvider behaves identically (not just "slightly slower") when get_scene_data_visualizer_prebuilt_artifact() returns None and requires_newton_model=True.
1 participant
InteractiveScene.init() resolves renderer requirements from sensors declared in the scene config, but DirectRLEnv environments add sensors programmatically in _setup_scene() after the scene is constructed. This means PhysxSceneDataProvider is never told it needs a Newton model, so get_newton_model() returns None and Newton-based renderers/visualizers get a black screen.
Add _update_scene_data_requirements_from_sensors() to DirectRLEnv which re-scans all scene sensors after _setup_scene() and merges their renderer requirements into the simulation context before the provider is constructed.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there