Skip to content

[Visualizers] Fix viewergl fully-black: assign PyVec3 to Newton camera.pos#5547

Merged
hujc7 merged 2 commits intoisaac-sim:developfrom
hujc7:jichuanh/fix-newton-camera-pos-type-bug
May 8, 2026
Merged

[Visualizers] Fix viewergl fully-black: assign PyVec3 to Newton camera.pos#5547
hujc7 merged 2 commits intoisaac-sim:developfrom
hujc7:jichuanh/fix-newton-camera-pos-type-bug

Conversation

@hujc7
Copy link
Copy Markdown
Collaborator

@hujc7 hujc7 commented May 8, 2026

Summary

Root-causes and fixes the long-standing test_visualizer_cartpole_integration::test_cartpole_newton_visualizer_viewergl_rgb_motion 'fully black' failure (skipped as a workaround in #5538).

Refs: isaac-sim/IsaacLab-Internal#890

NewtonVisualizer._apply_camera_pose was assigning the camera position as a wp.vec3:

self._viewer.camera.pos = wp.vec3(*cam_pos)

But Newton's Camera.translate() adds a pyglet.math.Vec3 delta to camera.pos via += every viewer update. warp 1.13's strict __add__ rejects wp.vec3 + pyglet.math.Vec3 with:

TypeError: Built-in functions cannot be called with non-Warp array types, such as lists, tuples, and NumPy arrays. Use a Warp type such as `wp.vec`, `wp.mat`, `wp.quat`, or `wp.transform`.

The exception is silenced by the visualizer's try/except (logger.debug), so renderer.render() never runs — the FBO stays empty and ViewerGL.get_frame reads back all zeros.

This patch assigns pyglet.math.Vec3 instead, matching what Newton uses internally, and re-enables the test that was skipped in #5538.

Why this took a while

  • Same Kit session: tiled_camera_rgb_non_black (Kit RTX) passes, viewergl_rgb_motion (Newton pyglet/EGL) fails — pointed at Newton ViewerGL specifically.
  • Standalone Newton ViewerGL (no Kit) renders fine.
  • Many wrong hypotheses ruled out: rc1→rc2 viewer code, wp.RegisteredGLBuffer ABI change, flakiness, bump cohort alone, missing _make_current(), missing glFinish + wp.synchronize_device, CI Docker image flip, MIG (CI is on AWS L40S, not MIG).
  • Final repro on a non-MIG L40 (Kit 110.0.0 + Newton 1.2.0rc2 + warp 1.13.0) and direct CPU FBO readback proved the FBO was genuinely empty — a clear-to-red on the FBO persisted untouched across all 60 frames, meaning the renderer was not even running its own glClear, let alone drawing geometry.
  • Surfacing the silenced logger.debug exception revealed the TypeError chain into Newton's Camera.translate.

Test plan

  • test_cartpole_newton_visualizer_viewergl_rgb_motion[physx] passes locally (~50 s).
  • test_cartpole_newton_visualizer_viewergl_rgb_motion[newton] passes locally (~50 s).
  • Regression check: temporarily reverting the one-line fix reproduces 'Viewer frame appears fully black' deterministically.
  • CI isaaclab_visualizers job passes on this PR.

NewtonVisualizer._apply_camera_pose was assigning
``self._viewer.camera.pos = wp.vec3(*cam_pos)``, but Newton's
``Camera.translate()`` adds a ``pyglet.math.Vec3`` delta to ``camera.pos``
via ``+=`` on every viewer update.

warp 1.13's strict ``__add__`` rejects ``wp.vec3 + pyglet.math.Vec3``
with::

    TypeError: Built-in functions cannot be called with non-Warp array
    types, such as lists, tuples, and NumPy arrays. Use a Warp type
    such as `wp.vec`, `wp.mat`, `wp.quat`, or `wp.transform`.

The TypeError is silenced by the visualizer's ``try/except``
(``logger.debug``), which then short-circuits before
``renderer.render()`` -- so the framebuffer never gets written and
``ViewerGL.get_frame`` reads back all zeros. The test asserts that
the last frame is non-black, hence "Viewer frame appears fully black."

Same render-path failure manifested across the Newton 1.2.0rc2 +
warp 1.13 cohort. Earlier hypotheses ruled out: viewer-code rc1->rc2
diff, ``wp.RegisteredGLBuffer`` API change, pure flakiness, the bump
cohort alone, ``_make_current()``, and an explicit ``glFinish`` +
``wp.synchronize_device``. Direct CPU readback of the FBO confirmed
it was empty (and a control ``glClear`` to red persisted untouched
across all 60 frames -- proof that nothing was writing to the FBO,
not even the renderer's own ``glClearColor`` + ``glClear``).

Local repro on a non-MIG L40 with Kit 110.0.0 + Newton 1.2.0rc2 +
warp 1.13.0 reproduces the failure deterministically. With
``cam_pos`` assigned as ``pyglet.math.Vec3`` instead, the
``+=`` is type-homogeneous, no exception, ``renderer.render()`` runs,
and both ``[physx]`` and ``[newton]`` parametrizations pass in
~50 s each.

This also re-enables the test that was skipped as a workaround in
PR isaac-sim#5538.
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels May 8, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 8, 2026

Greptile Summary

This PR fixes a fully-black ViewerGL.get_frame output by changing the type assigned to camera.pos from wp.vec3 to pyglet.math.Vec3, matching what Newton's Camera.translate() expects internally. It also re-enables the previously skipped test_cartpole_newton_visualizer_viewergl_rgb_motion test that was skipped as a workaround in #5538.

  • newton_visualizer.py: Replaces wp.vec3(*cam_pos) with pyglet.math.Vec3(...) using an inline deferred import, preventing the TypeError from warp 1.13's strict __add__ when Newton's camera update path does camera.pos += pyglet.math.Vec3(...).
  • test_visualizer_cartpole_integration.py: Removes the @pytest.mark.skip decorator from test_cartpole_newton_visualizer_viewergl_rgb_motion, restoring the test for both physx and newton backends.
  • changelog.d/: Adds a changelog entry documenting the root cause and fix.

Confidence Score: 4/5

Safe to merge — the one-line change is narrowly scoped, directly targets the confirmed root cause, and the previously-skipped test now validates the fix end-to-end.

The fix is minimal and well-justified: assigning a pyglet.math.Vec3 to camera.pos instead of wp.vec3 makes Newton's internal Camera.translate() call homogeneous and avoids the silenced TypeError that was starving the renderer. The only open question is the inline deferred import style, which is a cosmetic nit and does not affect correctness.

No files require special attention; the change is confined to a single assignment in newton_visualizer.py and the test re-enablement in the integration test file.

Important Files Changed

Filename Overview
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py One-line fix replacing wp.vec3 with pyglet.math.Vec3 for camera.pos assignment; inline deferred import is a minor style point but is functionally correct and avoids a top-level import failure when pyglet is absent.
source/isaaclab_visualizers/test/test_visualizer_cartpole_integration.py Removes the @pytest.mark.skip decorator from test_cartpole_newton_visualizer_viewergl_rgb_motion; straightforward re-enablement tied to the upstream bug fix.
source/isaaclab_visualizers/changelog.d/jichuanh-fix-newton-cam-pos-type.rst New changelog entry accurately describing the root cause (wp.vec3 + pyglet.math.Vec3 TypeError) and the fix; no issues.

Sequence Diagram

sequenceDiagram
    participant NV as NewtonVisualizer._apply_camera_pose
    participant Cam as Newton Camera
    participant Renderer as renderer.render()

    Note over NV: Before fix
    NV->>Cam: "camera.pos = wp.vec3(x, y, z)"
    Cam->>Cam: "translate() — pos += pyglet.math.Vec3(dx,dy,dz)"
    Note over Cam: TypeError: wp.vec3 + pyglet.math.Vec3 silenced by try/except
    Cam--xRenderer: render() never called
    Note over Renderer: FBO stays empty — frame reads all zeros

    Note over NV: After fix
    NV->>Cam: "camera.pos = pyglet.math.Vec3(x, y, z)"
    Cam->>Cam: "translate() — pos += pyglet.math.Vec3(dx,dy,dz)"
    Note over Cam: Homogeneous add — no exception
    Cam->>Renderer: render() runs normally
    Note over Renderer: FBO written — non-black frame
Loading

Reviews (1): Last reviewed commit: "Fix Newton viewer fully-black: assign Py..." | Re-trigger Greptile

Comment on lines +474 to +476
from pyglet.math import Vec3 as _PyVec3

self._viewer.camera.pos = _PyVec3(float(cam_pos[0]), float(cam_pos[1]), float(cam_pos[2]))
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.

P2 The from pyglet.math import Vec3 as _PyVec3 import is placed inside the method body and will be re-executed on every call to _apply_camera_pose. Python does cache module imports so the repeated import statement is nearly free, but the underscore-prefixed alias and inline placement are slightly surprising here. Consider hoisting this to the module-level imports alongside the other optional/conditional imports, or at minimum to the top of _apply_camera_pose without the private-style alias name. This is a minor readability point — the logic is correct either way.

Suggested change
from pyglet.math import Vec3 as _PyVec3
self._viewer.camera.pos = _PyVec3(float(cam_pos[0]), float(cam_pos[1]), float(cam_pos[2]))
from pyglet.math import Vec3
self._viewer.camera.pos = Vec3(float(cam_pos[0]), float(cam_pos[1]), float(cam_pos[2]))

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@hujc7 hujc7 force-pushed the jichuanh/fix-newton-camera-pos-type-bug branch from b832ee8 to 6785f7e Compare May 8, 2026 17:04
@hujc7 hujc7 force-pushed the jichuanh/fix-newton-camera-pos-type-bug branch from 6785f7e to ea4db96 Compare May 8, 2026 17:05
@hujc7 hujc7 changed the title Fix viewergl fully-black: assign PyVec3 to NewtonVisualizer camera.pos [Visualizers] Fix viewergl fully-black: assign PyVec3 to Newton camera.pos May 8, 2026
@hujc7 hujc7 merged commit 23ababb into isaac-sim:develop May 8, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants