[DO NOT MERGE] Test revert of PR #5521 to isolate viewergl failure#5539
[DO NOT MERGE] Test revert of PR #5521 to isolate viewergl failure#5539hujc7 wants to merge 1 commit intoisaac-sim:developfrom
Conversation
Greptile SummaryThis is a hypothesis-test revert of PR #5521 (marked DO NOT MERGE) that restores the eager top-level import of
Confidence Score: 3/5This PR intentionally reintroduces a hard import-time dependency on The top-level import of
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller as Caller (import camera_cfg)
participant CfgModule as camera_cfg.py
participant PhysX as isaaclab_physx.renderers
participant CameraCfg as CameraCfg.__post_init__
participant Camera as Camera.__init__
Note over CfgModule,PhysX: Eager import (this revert)
Caller->>CfgModule: import camera_cfg
CfgModule->>PhysX: from isaaclab_physx.renderers import IsaacRtxRendererCfg
PhysX-->>CfgModule: IsaacRtxRendererCfg class
Note over CfgModule,Camera: Object construction
Caller->>CameraCfg: CameraCfg(...)
CameraCfg->>PhysX: IsaacRtxRendererCfg() [default_factory]
PhysX-->>CameraCfg: renderer_cfg instance
CameraCfg-->>Caller: cfg
Caller->>Camera: Camera(cfg)
Camera->>Camera: "cfg.renderer_cfg.renderer_type == isaac_rtx?"
Camera->>Camera: get_settings_manager().set_bool(/isaaclab/render/rtx_sensors, True)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| from dataclasses import MISSING, field | ||
| from typing import TYPE_CHECKING, Literal | ||
|
|
||
| from isaaclab_physx.renderers import IsaacRtxRendererCfg |
There was a problem hiding this comment.
isaaclab_physx at import time
This top-level eager import makes isaaclab_physx a required package at camera_cfg import time. PR #5521 intentionally replaced this with a lazy-import pattern (get_default_renderer_cfg()) so that environments without isaaclab_physx — or any environment where the import triggers an init-order side-effect — would not fail when camera_cfg is first imported. Reverting to the eager import means that any downstream module that imports from isaaclab.sensors.camera will now pull in isaaclab_physx.renderers unconditionally, and any Omniverse renderer side-effects tied to that import occur at module load rather than at sensor construction time. This is the behavior that the PR description hypothesizes is causing the viewergl test failure.
| # and several env classes read it before the renderer's __init__ runs. | ||
| renderer_type = getattr(self.cfg.renderer_cfg, "renderer_type", None) | ||
| if renderer_type == "isaac_rtx": | ||
| if self.cfg.renderer_cfg.renderer_type == "isaac_rtx": |
There was a problem hiding this comment.
renderer_cfg
The pre-revert code used getattr(self.cfg.renderer_cfg, "renderer_type", None) to safely handle a renderer_cfg object that might not have a renderer_type attribute (e.g., a custom or base RendererCfg subclass). After this revert the direct access self.cfg.renderer_cfg.renderer_type will raise AttributeError if any non-RTX renderer_cfg instance is used that does not define renderer_type. If the base RendererCfg class guarantees that attribute this is benign, but the original defensive pattern was there for a reason.
|
Hypothesis disproven: this revert of #5521 on develop tip still fails Continuing investigation in #5540 with diagnostic instrumentation. |
#5538) ## Summary Two unrelated CI breakages on develop, bundled here so develop turns green in one PR. ### 1. Skip the failing viewergl test `test_cartpole_newton_visualizer_viewergl_rgb_motion[physx,newton]` started returning all-black frames on develop after `nvcr.io/nvidian/isaac-sim:latest-develop` flipped to a Kit 110.1.1 + USD 25.11 base. The failure has been deterministic across multiple PRs (#5523, #5495, #5408, …). Investigation so far has ruled out: - PR #5521 (revert in #5539 still failed) - Newton 1.0 → 1.2.0rc2 viewer code regression (only 7-line addition; ViewerGL alone yields 1.08M nonzero pixels) - warp 1.12 → 1.13 RegisteredGLBuffer ABI (byte-identical) - Module-load side effects of `isaaclab_physx.renderers` - CUDA-GL interop (PR #5540 diagnostic confirms direct CPU FBO readback also returns zeros, with `GL_NO_ERROR`) - GL context-currency (PR #5541 H6 attempt: still fails) - GL/CUDA sync (PR #5542 H4 attempt: still fails) Diagnostic output (PR #5540 v2): ``` [VIZDIAG] fbo=c_uint(8) pbo=None size=600x600 [VIZDIAG] glGetError before: GL_NO_ERROR [VIZDIAG] CPU-readback: nonzero=0/1080000 max=0 err=GL_NO_ERROR [VIZDIAG] PBO-result: nonzero=0/1080000 max=0 ``` The FBO itself is empty — Newton's pyglet/EGL renderer is not depositing pixels under Kit 110.1.1, even though `tiled_camera_rgb_non_black` (Kit RTX path) on the same env passes. Underlying root cause still being chased; this PR ships the skip to unblock develop. ### 2. Fix warp intersphinx 404 in docs build `https://nvidia.github.io/warp/objects.inv` started returning 404 — Warp's `objects.inv` only lives at `/stable/` and `/latest/` now. With Sphinx's `warnings_treated_as_errors`, the broken intersphinx fetch fails the docs build on every PR. Pinning to `/stable/` (matches the existing PyTorch `/docs/2.11/` workaround pattern in the same file). Verified `https://nvidia.github.io/warp/stable/objects.inv` returns 200. ## Test plan - [x] CI `isaaclab_visualizers` on this branch — was passing earlier with the skip; will re-verify with the bundled docs fix - [ ] CI `Build Latest Docs` on this branch — must turn green (was failing on every recent PR before this fix) ## Re-enable plan Once the underlying viewergl bug is identified and fixed, drop the `@pytest.mark.skip` decorator and remove the `jichuanh-disable-viewergl-flaky.skip` fragment.
1 participant
Purpose
Hypothesis test only — do not merge. Validates whether #5521 introduced the `test_cartpole_newton_visualizer_viewergl_rgb_motion` deterministic failure observed across #5523, #5534, #5024, #5495, #5492 (skip in #5538).
If the visualizer test passes here → #5521 camera-cfg lazy-import change is the trigger and the fix lives in adjusting that init order.
If it fails here → #5521 isn't the cause and the regression is elsewhere (likely the daily Isaac Sim image).
Diff
Single revert of `7b44452e9fa` (#5521 merge commit) on top of develop tip.