Warp Renderer OpenGL Lighting Parity by mhasek · Pull Request #1364 · google-deepmind/mujoco_warp

mhasek · 2026-05-19T04:57:59Z

PR Description: Renderer OpenGL Lighting Parity

Summary

This PR brings the MuJoCo Warp RGB renderer much closer to MuJoCo OpenGL lighting behavior. It plumbs missing light and material fields into types.Model, rewrites compute_lighting around MuJoCo-style diffuse/specular/attenuation/spotlight terms, injects the MuJoCo headlight as a camera-relative light, and adds opt-out flags for the main parity features.

The renderer now handles:

MuJoCo vis.headlight ambient/diffuse/specular contribution.
Per-light diffuse, specular, ambient, attenuation, cutoff, and exponent.
Material specular, shininess, and emission.
Binary shadow visibility that matches OpenGL more closely. (Not sure if this should be configurable)
Black-background parity in the renderer parity tests.
Unit tests that look at a simplified SSIM score diff between opengl and warp renders

I realize this is on the longer side and I put this all in a single PR but if need be I can split up into subcomponents to ease the processes.

Visual Parity

Canonical fixture SSIM against mujoco.Renderer improves from roughly 0.3-0.5 before this PR to 0.917-0.994 after this PR.

Renderer parity fixtures before/after/reference

The grid below shows each renderer parity test fixture before this PR, after this PR, and against the MuJoCo OpenGL reference.

Feature visual impact by opt-out flag

The visual comparison below shows the effect of disabling each feature independently on a representative lit scene, with shadows disabled to isolate the lighting terms.

Performance Cost

A reference scene was evaluated for 100 samples where the impact of each feature on SSIM and compute is shown compared to the baseline (origin/main)

Reference Scene

"""
<mujoco>
  <visual>
    <headlight active="1" ambient="0.2 0.2 0.2" diffuse="0.7 0.7 0.7" specular="0.8 0.8 0.8"/>
  </visual>
  <asset>
    <material name="benchmat" specular="0.8" shininess="0.7" emission="0.1" rgba="0.6 0.7 0.9 1"/>
  </asset>
  <worldbody>
    <camera name="cam" pos="0 -3 0.9" xyaxes="1 0 0 0 0.3 1" resolution="256 256"/>
    <light pos="-1 -1.5 2" dir="0.3 0.2 -1" cutoff="35" exponent="15"
           attenuation="1 0.2 0.12" diffuse="1 1 1" specular="1 1 1" ambient="0.1 0.1 0.1"/>
    <geom type="plane" size="4 4 0.1" rgba="0.8 0.8 0.8 1"/>
    <geom type="box" pos="-0.5 0 0.15" size="0.15 0.15 0.15" material="benchmat"/>
    <geom type="sphere" pos="0.0 0 0.25" size="0.25" material="benchmat"/>
    <geom type="capsule" fromto="0.4 -0.2 0.1 0.8 0.3 0.5" size="0.08" material="benchmat"/>
  </worldbody>
</mujoco>
"""

thowell · 2026-05-19T09:24:28Z

@mhasek thanks for adding these features to the batch renderer! the visual comparisons are great and the benchmark results are helpful for understanding the performance of the different features.

tkelestemur

left a few comments. thanks @mhasek

tkelestemur · 2026-05-19T19:08:54Z


  bvh_ngeom = len(geom_enabled_idx)

+  headlight_on = bool(mjm.vis.headlight.active) and headlight_active


why do we need renderer-level argument for this? Can't we just use the mjm.vis.headlight.active for turning the headlight on and off?

I can do either @thowell which one would you guys perfer? I could set it up where the user has absolute control over it or if it is just pulled from the model.

let just use the mjmodel field. thanks!

This also depends on downstream usage too. In mjlab for example, would they be able to add this option easily into their existing spec/compile from config method?

tkelestemur · 2026-05-19T19:11:22Z

  bvh_ngeom = len(geom_enabled_idx)

+  headlight_on = bool(mjm.vis.headlight.active) and headlight_active
+  hl_ambient = np.asarray(mjm.vis.headlight.ambient, dtype=np.float32)


I would name these as headlight_ambient_mjm, headlight_diffuse_mjm etc..

thowell · 2026-05-21T14:13:36Z

+                     False the term is dropped at compile time. Disable for performance
+                     when no emission is present.
+    enable_per_light_ambient: When ambient lighting is enabled, sum each
+                              light's XML `ambient` color into shaded pixels


lets remove 'XML' (could be from mjspec?)

thowell · 2026-05-21T14:17:11Z

+               attenuation="1 0.1 0.05" cutoff="25" exponent="10"
+               ambient="0.1 0.1 0.1" diffuse="0.6 0.5 0.4" specular="0.7 0.7 0.7"
+               bulbradius="0.05"/>
+        <light name="dir" pos="0 0 5" dir="0 0 -1" directional="true"


can we remove name?

thowell · 2026-05-21T14:21:28Z

-      attenuation = attenuation * spot_factor
+    if not default_attenuation:
+      denom = lightatten[0] + lightatten[1] * dist_to_light + lightatten[2] * dist_to_light * dist_to_light
+      atten = 1.0 / wp.max(denom, 1.0e-6)


can we use math.safe_div here?

thowell · 2026-05-21T14:26:00Z

-
-    # Apply lighting and shadows
+      if wp.static(rc.headlight_active):
+        result = result + wp.cw_mul(base_color, wp.static(rc.headlight_ambient))


do we need wp.static here?

yeah I think you do since, other wise it wont compile

thowell · 2026-05-21T14:27:55Z

+        view_dir,
+        mat_spec,
+        mat_shin_exp,
+        wp.static(rc.enable_backface_culling),


do we need wp.static here?

yes we do since it accesses variables from the rc

thowell · 2026-05-21T14:28:04Z

+        mat_spec,
+        mat_shin_exp,
+        wp.static(rc.enable_backface_culling),
+        wp.static(rc.enable_specular),


do we need wp.static here?

thowell · 2026-05-21T14:28:12Z

+        mat_shin_exp,
+        wp.static(rc.enable_backface_culling),
+        wp.static(rc.enable_specular),
+        wp.static(rc.light_attenuation_is_default),


do we need wp.static here?

thowell · 2026-05-21T14:28:23Z

+        wp.static(rc.enable_backface_culling),
+        wp.static(rc.enable_specular),
+        wp.static(rc.light_attenuation_is_default),
+        wp.static(rc.has_spot_lights),


do we need wp.static here?

yes you do since it comes from the rc

thowell · 2026-05-21T14:31:24Z

+    # Apply Headlight
+    if wp.static(rc.headlight_active):
+      cam_pos = ray_origin_world
+      cam_fwd = wp.vec3(-cam_mat_world[0, 2], -cam_mat_world[1, 2], -cam_mat_world[2, 2])


does -cam_mat_world[:, 2] work here?

seems to work

thowell · 2026-05-21T14:31:38Z

+        wp.vec3(1.0, 0.0, 0.0),
+        0.0,
+        0.0,
+        wp.static(rc.headlight_diffuse),


is wp.static necessary here?

unfortunately it is

thowell · 2026-05-21T14:31:47Z

+        0.0,
+        0.0,
+        wp.static(rc.headlight_diffuse),
+        wp.static(rc.headlight_specular),


is wp.static necessary here?

thowell · 2026-05-21T14:32:03Z

+        view_dir,
+        mat_spec,
+        mat_shin_exp,
        wp.static(rc.enable_backface_culling),


is wp.static necessary here?

thowell · 2026-05-21T14:32:11Z

+        mat_spec,
+        mat_shin_exp,
        wp.static(rc.enable_backface_culling),
+        wp.static(rc.enable_specular),


is wp.static necessary here?

thowell · 2026-05-21T14:42:13Z

+  )
+
+  @classmethod
+  def _fixture(cls, name: str) -> str:


lets remove this and provide xmls directly to test_data.fixture

thowell · 2026-05-21T14:49:47Z

+      shaded pixel. When False the term is dropped at compile time.
+    enable_per_light_ambient: when True and `use_ambient_lighting` is also
+      True, sum the per-light `light_ambient` colors into each shaded pixel
+      even when N.L == 0 or the pixel is shadowed. When False the second


what is 'N.L'?

its the dot product between the surface normal vector and the Light vector.

thowell · 2026-05-21T14:55:53Z

would have expected the 'before' and 'all features off' timing results to be about the same since the features should all be statically guarded? are there new features that are not fully guarded with wp.static?

thowell · 2026-05-21T14:57:25Z

@mhasek left some comments throughout the code, great work! please let us know if you have any questions. thanks!

mhasek

I tried to address all the comments. @thowell thanks for the quick review I know it was on the longer side.

mhasek · 2026-05-22T00:15:52Z

  bvh_ngeom = len(geom_enabled_idx)

+  headlight_on = bool(mjm.vis.headlight.active) and headlight_active
+  hl_ambient = np.asarray(mjm.vis.headlight.ambient, dtype=np.float32)


mhasek · 2026-05-22T00:16:46Z

+               attenuation="1 0.1 0.05" cutoff="25" exponent="10"
+               ambient="0.1 0.1 0.1" diffuse="0.6 0.5 0.4" specular="0.7 0.7 0.7"
+               bulbradius="0.05"/>
+        <light name="dir" pos="0 0 5" dir="0 0 -1" directional="true"


mhasek · 2026-05-22T01:08:44Z

+      shaded pixel. When False the term is dropped at compile time.
+    enable_per_light_ambient: when True and `use_ambient_lighting` is also
+      True, sum the per-light `light_ambient` colors into each shaded pixel
+      even when N.L == 0 or the pixel is shadowed. When False the second


its the dot product between the surface normal vector and the Light vector.

mhasek · 2026-05-22T01:20:28Z

+  )
+
+  @classmethod
+  def _fixture(cls, name: str) -> str:


mhasek · 2026-05-22T03:09:29Z

would have expected the 'before' and 'all features off' timing results to be about the same since the features should all be statically guarded? are there new features that are not fully guarded with wp.static?

As would I, I attempted to guard as much as possible but the light accumulation is now in the rgb space before it was a scalar my guess is that is what is contributing to this. I can attempt to guard that too but I feel it would convolute the lighting accumulation.

StafaH

Thanks for the changes! The rendered output definitely looks better. Also great PR description!

Major comment on the tests, I think the added tests are excessive and also unnecessary. For testing if the flags are being passed through correctly, there are tests in io.py that you can follow or add to that can make sure that setting the flags for these lighting options is getting passed through correctly. See similar tests for render_rgb, use_textures etc (test_render_context_with_textures). They are super lightweight and just check that the options are being passed through correctly.

Regarding the actual rendering tests, I would just skip them. The rendering output here is subjective, so if we all agree that the new output looks better or closer to MuJoCo, then we can ship it. In the past we have only added rendering tests for depth and segmentation since we want strict equality in output. This renderer will never be able to match a full renderer from MuJoCo so adding alot of test overhead just to measure how similar the images are is not necessary (which is why we never added these type of tests before as well). I am of the opinion that we can just remove these tests, keep testing lightweight.

StafaH · 2026-05-27T21:40:55Z


  bvh_ngeom = len(geom_enabled_idx)

+  headlight_on = bool(mjm.vis.headlight.active) and headlight_active


This also depends on downstream usage too. In mjlab for example, would they be able to add this option easily into their existing spec/compile from config method?

mhasek

@StafaH Thanks for the review! I tried to address the comments. My only question is I remvoed the additions to the render tests but should I at least and some sanity checks tests to make sure the renderer runs with the separate configs?

mhasek · 2026-05-28T22:03:42Z

+                     False the term is dropped at compile time. Disable for performance
+                     when no emission is present.
+    enable_per_light_ambient: When ambient lighting is enabled, sum each
+                              light's XML `ambient` color into shaded pixels


mhasek · 2026-05-28T22:08:47Z


  bvh_ngeom = len(geom_enabled_idx)

+  headlight_on = bool(mjm.vis.headlight.active) and headlight_active


mhasek

@StafaH Thanks for the review! I tried to address the comments. My only question is I removed the additions to the render tests but should I at least and some sanity checks tests to make sure the renderer runs with the separate configs?

StafaH

LGTM, much cleaner. Thanks!

thowell · 2026-05-29T08:43:21Z

+        if mat_id_for_spec >= 0:
+          if wp.static(rc.enable_specular):
+            mat_spec = mat_specular[worldid % mat_specular.shape[0], mat_id_for_spec]
+            mat_shin_exp = mat_shininess[worldid % mat_shininess.shape[0], mat_id_for_spec] * 128.0


the max shininess value is 128.0, should we add a constant about for this setting and then use the constant here? something like MAX_SHININESS = 128.0 and then DEFAULT_SHININESS_EXPONENT = 0.5 * MAX_SHININESS?

should the 128.0 here be replaced with MAX_SHININESS?

Apologies, I missed that it is fixed now

thowell · 2026-05-29T08:44:58Z

@mhasek left a comment about potentially adding a constant for max shininess, otherwise lgtm

@StafaH thanks for the review!

…th opengl renderer make background color controllable in create_render_context api make light params batchable make material params batchable pr comments PR comments PR comments add render sanity tests remove lighting field prop test make max shininess a constant

mhasek · 2026-06-01T19:21:15Z

@thowell @StafaH Awesome speed up PR! I just rebased the branch fixed the conflicts from it can you guys take a look at it and then merge if its good to go?

thowell · 2026-06-02T17:33:23Z

great work, thanks @mhasek!

thowell requested a review from StafaH May 19, 2026 09:24

tkelestemur reviewed May 19, 2026

View reviewed changes

mhasek changed the title ~~Waro Renderer OpenGL Lighting Parity~~ Warp Renderer OpenGL Lighting Parity May 19, 2026

mhasek force-pushed the renderer-lighting-parity branch from 93601c9 to fa2e0c2 Compare May 20, 2026 15:46