Skip to content

[DO NOT MERGE] Test revert of PR #5521 to isolate viewergl failure #5539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hujc7 wants to merge 1 commit into from
+4 −55
Closed

[DO NOT MERGE] Test revert of PR #5521 to isolate viewergl failure #5539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions source/isaaclab/isaaclab/sensors/camera/camera.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ def __init__(self, cfg: CameraCfg):
# IsaacRtxRendererCfg overrides to flip /isaaclab/render/rtx_sensors. The
# flag must be set pre-sim.reset() because SimulationContext.is_rendering
# 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":
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 8, 2026

Choose a reason for hiding this comment

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

P2 Unguarded attribute access on 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.

get_settings_manager().set_bool("/isaaclab/render/rtx_sensors", True)

# Compute camera orientation (convention conversion) and spawn
Expand Down
12 changes: 3 additions & 9 deletions source/isaaclab/isaaclab/sensors/camera/camera_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from dataclasses import MISSING, field
from typing import TYPE_CHECKING, Literal

from isaaclab_physx.renderers import IsaacRtxRendererCfg
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 8, 2026

Choose a reason for hiding this comment

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

P1 Hard dependency on 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.


from isaaclab.renderers import RendererCfg
from isaaclab.sim import FisheyeCameraCfg, PinholeCameraCfg
from isaaclab.utils import configclass
Expand Down Expand Up @@ -189,7 +191,7 @@ class OffsetCfg:
on :attr:`renderer_cfg` instead.
"""

renderer_cfg: RendererCfg = field(default_factory=RendererCfg)
renderer_cfg: RendererCfg = field(default_factory=IsaacRtxRendererCfg)
"""Renderer configuration for camera sensor."""

def __post_init__(self):
Expand All @@ -199,14 +201,6 @@ def __post_init__(self):
:class:`DeprecationWarning` and is copied onto ``self.renderer_cfg``
when that cfg defines the same-named field.
"""
# TODO when Camera.__init__ moves rtx_sensor setting out of camera initialization
# the default renderer config instantiation can be moved into the render factory
# and get_default_render_cfg method can be removed from backend_utils
renderer_type = getattr(self.renderer_cfg, "renderer_type", None)
if renderer_type == "default":
from isaaclab.utils.backend_utils import get_default_renderer_cfg

self.renderer_cfg = get_default_renderer_cfg()
# Forwarded by name: any same-named field on ``renderer_cfg`` will receive the value.
for field_name, default in _DEPRECATED_RENDERER_FIELD_DEFAULTS.items():
value = getattr(self, field_name)
Expand Down
8 changes: 0 additions & 8 deletions source/isaaclab/isaaclab/sensors/camera/tiled_camera_cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ class TiledCameraCfg(CameraCfg):
class_type: type["TiledCamera"] | str = "{DIR}.tiled_camera:TiledCamera"

def __post_init__(self):
# TODO when Camera.__init__ moves rtx_sensor setting out of camera initialization
# the default renderer config instantiation can be moved into the render factory
# and get_default_render_cfg method can be removed from backend_utils
renderer_type = getattr(self.renderer_cfg, "renderer_type", None)
if renderer_type == "default":
from isaaclab.utils.backend_utils import get_default_renderer_cfg

self.renderer_cfg = get_default_renderer_cfg()
warnings.warn(
"TiledCameraCfg is deprecated. Use CameraCfg directly — "
"Camera now includes TiledCamera's vectorized rendering optimizations.",
Expand Down
36 changes: 0 additions & 36 deletions source/isaaclab/isaaclab/utils/backend_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,12 @@
#
# SPDX-License-Identifier: BSD-3-Clause

from __future__ import annotations

import importlib
import logging
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from isaaclab.renderers.renderer_cfg import RendererCfg

logger = logging.getLogger(__name__)


def get_default_renderer_cfg() -> RendererCfg:
"""Return the default :class:`~isaaclab.renderers.renderer_cfg.RendererCfg` for cameras.

Lazily imports :mod:`isaaclab_physx.renderers` and returns a new
:class:`~isaaclab_physx.renderers.IsaacRtxRendererCfg` instance.

Returns:
A new default Isaac RTX renderer configuration.

Raises:
ImportError: If :mod:`isaaclab_physx.renderers` cannot be imported or does not
expose ``IsaacRtxRendererCfg``.
"""
try:
renderers_mod = importlib.import_module("isaaclab_physx.renderers")
except ImportError as e:
raise ImportError(
"The default camera renderer configuration requires the optional 'isaaclab_physx' "
"package (import 'isaaclab_physx.renderers'). Install isaaclab_physx or set "
"CameraCfg.renderer_cfg explicitly."
) from e
try:
default_cls = renderers_mod.IsaacRtxRendererCfg
except AttributeError as e:
raise ImportError(
"Module 'isaaclab_physx.renderers' is available but does not define 'IsaacRtxRendererCfg'."
) from e
return default_cls()


class FactoryBase:
"""A generic factory class that dynamically loads backends."""

Expand Down
Loading