[Visualizers] Fix viewergl fully-black: assign PyVec3 to Newton camera.pos#5547
Conversation
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.
Greptile SummaryThis PR fixes a fully-black
Confidence Score: 4/5Safe 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
Sequence DiagramsequenceDiagram
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
Reviews (1): Last reviewed commit: "Fix Newton viewer fully-black: assign Py..." | Re-trigger Greptile |
| from pyglet.math import Vec3 as _PyVec3 | ||
|
|
||
| self._viewer.camera.pos = _PyVec3(float(cam_pos[0]), float(cam_pos[1]), float(cam_pos[2])) |
There was a problem hiding this comment.
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.
| 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!
b832ee8 to
6785f7e
Compare
6785f7e to
ea4db96
Compare
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_posewas assigning the camera position as awp.vec3:But Newton's
Camera.translate()adds apyglet.math.Vec3delta tocamera.posvia+=every viewer update. warp 1.13's strict__add__rejectswp.vec3 + pyglet.math.Vec3with:The exception is silenced by the visualizer's
try/except(logger.debug), sorenderer.render()never runs — the FBO stays empty andViewerGL.get_framereads back all zeros.This patch assigns
pyglet.math.Vec3instead, matching what Newton uses internally, and re-enables the test that was skipped in #5538.Why this took a while
tiled_camera_rgb_non_black(Kit RTX) passes,viewergl_rgb_motion(Newton pyglet/EGL) fails — pointed at Newton ViewerGL specifically.wp.RegisteredGLBufferABI change, flakiness, bump cohort alone, missing_make_current(), missingglFinish + wp.synchronize_device, CI Docker image flip, MIG (CI is on AWS L40S, not MIG).glClear, let alone drawing geometry.logger.debugexception revealed theTypeErrorchain into Newton'sCamera.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).isaaclab_visualizersjob passes on this PR.