Skip to content

Newton RGB default fixes #5168

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

Open
nblauch wants to merge 3 commits into
base: develop
Choose a base branch
from
+10 −1
Open

Newton RGB default fixes #5168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c697ccd
Allow ground plane color to be specified from cfg
nblauch Apr 3, 2026
7aeeabb
Make default background color white like in RTX
nblauch Apr 3, 2026
040fdbf
Merge branch 'develop' into newton_rgb_fixes
AntoineRichard Apr 8, 2026
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
8 changes: 7 additions & 1 deletion source/isaaclab/isaaclab/sim/spawners/from_files/from_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,19 @@ def spawn_ground_plane(
# Change the color of the plane
# Warning: This is specific to the default grid plane asset.
if cfg.color is not None:
# change the color
# change the color on the visual grid shader
change_prim_property(
prop_path=f"{prim_path}/Looks/theGrid/Shader.inputs:diffuse_tint",
value=Gf.Vec3f(*cfg.color),
stage=stage,
type_to_create_if_not_exist=Sdf.ValueTypeNames.Color3f,
)
# Also set displayColor on the collision plane so non-RTX renderers
# (e.g. Newton) can pick up the configured color.
if collision_prim is not None:
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot Bot Apr 3, 2026

Choose a reason for hiding this comment

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

Bug: NameError when cfg.physics_material is None

collision_prim is only assigned inside the if cfg.physics_material is not None: block (~line 214). If a caller sets physics_material=None, the variable is never bound, and this line raises NameError — the if collision_prim is not None guard doesn't help because the name itself doesn't exist.

While the default GroundPlaneCfg sets physics_material=RigidBodyMaterialCfg() (non-None), this is a public API — callers can and do override defaults. The simplest fix: look up the collision prim independently here rather than relying on the physics-material block's side effect:

        # Also set displayColor on the collision plane so non-RTX renderers
        # (e.g. Newton) can pick up the configured color.
        color_target_prim = get_first_matching_child_prim(
            prim_path,
            predicate=lambda _prim: _prim.GetTypeName() == "Plane",
            stage=stage,
        )
        if color_target_prim is not None:
            UsdGeom.Gprim(color_target_prim).CreateDisplayColorAttr().Set(
                [Gf.Vec3f(*cfg.color)]
            )

This is slightly more robust than collision_prim = None at function top, because it also works if the physics-material block raises and is caught elsewhere.

Copy link
Copy Markdown
Collaborator

@AntoineRichard AntoineRichard Apr 7, 2026

Choose a reason for hiding this comment

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

@nblauch the bot has a good point here.

UsdGeom.Gprim(collision_prim).CreateDisplayColorAttr().Set(
[Gf.Vec3f(*cfg.color)]
)
Comment on lines +246 to +249
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 3, 2026

Choose a reason for hiding this comment

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

P1 collision_prim may be undefined when cfg.physics_material is None

collision_prim is assigned only inside the if cfg.physics_material is not None: block (lines 210–222). If a caller passes physics_material=None, that block is skipped entirely, so collision_prim is never bound. Reaching if collision_prim is not None: on line 246 then raises a NameError at runtime.

Suggested change
if collision_prim is not None:
UsdGeom.Gprim(collision_prim).CreateDisplayColorAttr().Set(
[Gf.Vec3f(*cfg.color)]
)
collision_prim = get_first_matching_child_prim(
prim_path,
predicate=lambda _prim: _prim.GetTypeName() == "Plane",
stage=stage,
)
if collision_prim is not None:
UsdGeom.Gprim(collision_prim).CreateDisplayColorAttr().Set(
[Gf.Vec3f(*cfg.color)]
)

Alternatively, initialise collision_prim = None before the if cfg.physics_material is not None: block so it is always defined when the color section runs.

Copy link
Copy Markdown
Collaborator

@AntoineRichard AntoineRichard Apr 7, 2026

Choose a reason for hiding this comment

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

They clearly copied their homework...

# Remove the light from the ground plane (USD API, works without Kit/Newton)
# It isn't bright enough and messes up with the user's lighting settings
light_prim = stage.GetPrimAtPath(f"{prim_path}/SphereLight")
Expand Down
3 changes: 3 additions & 0 deletions source/isaaclab_newton/isaaclab_newton/renderers/newton_warp_renderer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ def render(self, render_data: RenderData):
depth_image=render_data.outputs.depth_image,
normal_image=render_data.outputs.normals_image,
shape_index_image=render_data.outputs.instance_segmentation_image,
clear_data=newton.sensors.SensorTiledCamera.ClearData(
clear_color=0xFFFFFFFF, clear_albedo=0xFFFFFFFF,
),
Comment thread
AntoineRichard marked this conversation as resolved.
)

def write_output(self, render_data: RenderData, output_name: str, output_data: torch.Tensor):
Expand Down