Title: Add studio shading style and edge overlay to GL viewer#2300
Open
AnkaChan wants to merge 16 commits intonewton-physics:mainfrom
Open
Title: Add studio shading style and edge overlay to GL viewer#2300AnkaChan wants to merge 16 commits intonewton-physics:mainfrom
AnkaChan wants to merge 16 commits intonewton-physics:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a registry-driven shading-style system with runtime-selectable styles, replaces prior PBR fragment logic with classic/studio shaders, exposes a shading dropdown and edge-overlay toggle in the viewer UI, and includes an optional edge-overlay render pass plus per-instance material forwarding. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ViewerGL
participant RendererGL
participant STYLE_REGISTRY
participant Shader
User->>ViewerGL: choose shading style / toggle edges
ViewerGL->>RendererGL: set shading_style / draw_edges
RendererGL->>STYLE_REGISTRY: validate & fetch style config
STYLE_REGISTRY-->>RendererGL: ShadingStyleConfig
loop Per-frame
RendererGL->>RendererGL: _build_shader_kwargs()
RendererGL->>STYLE_REGISTRY: read active style overrides
STYLE_REGISTRY-->>RendererGL: overrides
RendererGL->>Shader: select shader from _style_shaders
RendererGL->>Shader: update(view, proj, merged_kwargs)
RendererGL->>RendererGL: _render_scene() using active shader
alt draw_edges enabled
RendererGL->>Shader: bind ShaderEdge and draw edges (GL_LEQUAL)
RendererGL->>RendererGL: restore depth func (GL_LESS)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
c008d5d to
09a4faf
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
newton/_src/viewer/gl/shaders.py (1)
3-14: Redundant license text after SPDX header.Same as in
opengl.py— the SPDX header already identifies the license. Consider removing the full Apache 2.0 boilerplate for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/shaders.py` around lines 3 - 14, The file contains a full Apache 2.0 license block duplicated after an SPDX header; remove the redundant boilerplate and keep only the SPDX license identifier for consistency with opengl.py. Locate the top of newton/_src/viewer/gl/shaders.py where the long Apache 2.0 comment block appears (immediately following the SPDX header) and delete that block so only the SPDX header remains, preserving surrounding imports and any file-level docstring or metadata.newton/_src/viewer/gl/opengl.py (1)
3-14: Redundant license text after SPDX header.The SPDX header on lines 1-2 already identifies the license. The full Apache 2.0 boilerplate is redundant. Consider removing lines 3-14 to keep headers concise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/opengl.py` around lines 3 - 14, Remove the redundant Apache 2.0 boilerplate comment block that follows the SPDX header at the top of the file; keep only the SPDX license identifier and delete the subsequent multi-line Apache License text (the large comment block starting after the SPDX header) so the file header is concise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@newton/_src/viewer/gl/opengl.py`:
- Around line 3-14: Remove the redundant Apache 2.0 boilerplate comment block
that follows the SPDX header at the top of the file; keep only the SPDX license
identifier and delete the subsequent multi-line Apache License text (the large
comment block starting after the SPDX header) so the file header is concise.
In `@newton/_src/viewer/gl/shaders.py`:
- Around line 3-14: The file contains a full Apache 2.0 license block duplicated
after an SPDX header; remove the redundant boilerplate and keep only the SPDX
license identifier for consistency with opengl.py. Locate the top of
newton/_src/viewer/gl/shaders.py where the long Apache 2.0 comment block appears
(immediately following the SPDX header) and delete that block so only the SPDX
header remains, preserving surrounding imports and any file-level docstring or
metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 211789a5-c9a7-460f-ad0c-4e8d3bde0bf3
📒 Files selected for processing (3)
newton/_src/viewer/gl/opengl.pynewton/_src/viewer/gl/shaders.pynewton/_src/viewer/viewer_gl.py
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
09a4faf to
b8d86ae
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
newton/_src/viewer/gl/shaders.py (1)
753-831: Extract the shared GLSL helpers before they diverge.
poissonDisk,rand,ShadowCalculation(), and the checker helpers are now duplicated in both fragment shaders. Centralizing those snippets will keep the next shadow/checker fix from becoming a two-shader edit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/shaders.py` around lines 753 - 831, The fragment-shader helpers poissonDisk, rand, ShadowCalculation, filterwidth, bump and checker are duplicated; extract them into a single shared GLSL snippet (e.g., SHARED_GLSL or SHADER_HELPERS) in newton/_src/viewer/gl/shaders.py and replace the duplicate blocks in both fragment shader sources with that shared snippet (import/concatenate it where the shaders are built), ensuring the symbols poissonDisk, rand, ShadowCalculation, filterwidth, bump and checker remain unchanged so existing calls keep working and remove the duplicated declarations from each shader.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/viewer/gl/shaders.py`:
- Around line 739-750: The studio shader dropped support for per-frame
diffuse/specular knobs: add uniform float declarations for diffuse_scale and
specular_scale (matching the names passed from
RendererGL._build_shader_kwargs()) to the shader (repeat the same fix in the
secondary fragment region around the 875-901 block), and apply them to the
computed lighting terms by multiplying the diffuse term by diffuse_scale and the
specular term by specular_scale where the shader computes final color (ensure
the uniform names exactly match the keys used by
RendererGL._build_shader_kwargs() and update both studio-related fragments).
- Around line 739-750: The fragment shader currently only applies gamma
correction, causing bright linear values to clip when using the "studio" style;
update the shader pipeline so the studio path retains full tone mapping
(exposure + ACES) before gamma, not just gamma correction—modify the fragment
responsible for final color output (the shader code referenced alongside
uniforms view_pos, light_color, sky_color, etc.) and ensure FrameShader's
studio/blit path calls or includes the same exposure + ACES tonemapping steps
(also fix the same logic locations around the other affected block at the region
corresponding to lines ~914-919) so both classic and studio styles use identical
tone mapping then gamma-correct for display.
---
Nitpick comments:
In `@newton/_src/viewer/gl/shaders.py`:
- Around line 753-831: The fragment-shader helpers poissonDisk, rand,
ShadowCalculation, filterwidth, bump and checker are duplicated; extract them
into a single shared GLSL snippet (e.g., SHARED_GLSL or SHADER_HELPERS) in
newton/_src/viewer/gl/shaders.py and replace the duplicate blocks in both
fragment shader sources with that shared snippet (import/concatenate it where
the shaders are built), ensuring the symbols poissonDisk, rand,
ShadowCalculation, filterwidth, bump and checker remain unchanged so existing
calls keep working and remove the duplicated declarations from each shader.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 878eef24-f64d-4abc-8598-2343c79df5e2
📒 Files selected for processing (3)
newton/_src/viewer/gl/opengl.pynewton/_src/viewer/gl/shaders.pynewton/_src/viewer/viewer_gl.py
🚧 Files skipped from review as they are similar to previous changes (2)
- newton/_src/viewer/viewer_gl.py
- newton/_src/viewer/gl/opengl.py
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
b8d86ae to
93d0bd4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
newton/_src/viewer/gl/shaders.py (2)
737-748:⚠️ Potential issue | 🟠 MajorStudio output path is missing exposure + ACES tone mapping.
The studio fragment currently gamma-corrects directly, so bright linear values clip compared to the classic path.
💡 Proposed fix
uniform float shadow_radius; uniform float shadow_extents; uniform int up_axis; +uniform float exposure; @@ - // gamma correction (sRGB) - color = pow(color, vec3(1.0 / 2.2)); + // ACES filmic tone mapping (match classic) + color = color * exposure; + vec3 x = color; + color = (x * (2.51 * x + 0.03)) / (x * (2.43 * x + 0.59) + 0.14); + color = clamp(color, 0.0, 1.0); + color = pow(color, vec3(1.0 / 2.2));Also applies to: 915-917
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/shaders.py` around lines 737 - 748, The fragment shader is missing an exposure uniform and proper ACES tone mapping, so bright linear values are being clipped by direct gamma correction; add a uniform float exposure and implement an ACES filmic tonemap function (e.g., the common RRT+ODT curve or the ACES approximation: color = (color*(a*color+b))/(color*(c*color+d)+e) with appropriate constants), multiply the linear color by exposure before applying the ACES tonemap, then perform the final gamma (pow(color, vec3(1.0/2.2))). Update the uniform declarations (add exposure) near view_pos/light_color and replace the direct gamma step in the fragment output code paths referenced around the other places (the block around lines referencing the final color at 915-917) to use exposure -> ACES tonemap -> gamma so both the studio and classic paths match.
737-748:⚠️ Potential issue | 🟠 MajorStudio shader still ignores
diffuse_scale/specular_scalecontrols.Switching to
"studio"drops existing renderer lighting knobs because the shader never declares or applies those uniforms.💡 Proposed fix
uniform vec3 sun_direction; uniform sampler2D albedo_map; uniform sampler2D shadow_map; uniform mat4 light_space_matrix; uniform float shadow_radius; uniform float shadow_extents; uniform int up_axis; +uniform float diffuse_scale; +uniform float specular_scale; @@ - vec3 diffuse = albedo * light_color * NdotL * 1.10; + vec3 diffuse = albedo * light_color * NdotL * 1.10 * diffuse_scale; @@ - vec3 spec = spec_color * light_color * pow(NdotH, shininess) * NdotL; + vec3 spec = spec_color * light_color * pow(NdotH, shininess) * NdotL * specular_scale;Also applies to: 874-900
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/shaders.py` around lines 737 - 748, The studio shader is missing uniforms for diffuse/specular scaling and never applies them; add uniform float diffuse_scale and uniform float specular_scale declarations alongside the other uniforms (e.g., near view_pos, light_color, etc.) and then multiply the computed diffuse term by diffuse_scale and the computed specular term by specular_scale in the lighting calculation block (also update the same additions/applies in the other shader section referenced around lines 874–900). Ensure the uniform names are exactly diffuse_scale and specular_scale so the renderer's controls will bind correctly and the scaled terms are used when composing final color.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/viewer/gl/opengl.py`:
- Around line 1939-1940: The current check uses style.draw_sky and ignores the
runtime toggle self.draw_sky, so restore runtime behavior by gating the sky draw
on both flags; update the condition around the call to _draw_sky() (the block
that currently reads "if style.draw_sky: self._draw_sky()") to require the
instance runtime flag as well (e.g., require self.draw_sky and style.draw_sky,
or treat self.draw_sky as an override/fallback to preserve intended precedence),
keeping the call target _draw_sky unchanged.
- Around line 1908-1930: The shader kwargs built in _build_shader_kwargs() are
missing the renderer exposure value so changes to RendererGL.exposure don't
reach shaders; add an "exposure" entry to the kwargs dict (e.g., "exposure":
self.exposure) inside _build_shader_kwargs() so the exposure value is forwarded
to shaders, keeping the update order so self._active_style.overrides can still
override it if intended.
- Around line 1053-1062: The PR adds new user-facing ViewerGL APIs (the
shading_style parameter in ViewerGL.__init__ and the shading_style property on
ViewerGL, plus new studio mode and edge overlay features) but omits CHANGELOG
entries; add entries under the [Unreleased] section of CHANGELOG.md using the
appropriate category (e.g., Added) and imperative present tense — e.g., "Add
shading_style parameter and property to ViewerGL", "Add studio mode to
ViewerGL", and "Add edge overlay option to ViewerGL" — ensuring each bullet
references the new public APIs so users see the change.
---
Duplicate comments:
In `@newton/_src/viewer/gl/shaders.py`:
- Around line 737-748: The fragment shader is missing an exposure uniform and
proper ACES tone mapping, so bright linear values are being clipped by direct
gamma correction; add a uniform float exposure and implement an ACES filmic
tonemap function (e.g., the common RRT+ODT curve or the ACES approximation:
color = (color*(a*color+b))/(color*(c*color+d)+e) with appropriate constants),
multiply the linear color by exposure before applying the ACES tonemap, then
perform the final gamma (pow(color, vec3(1.0/2.2))). Update the uniform
declarations (add exposure) near view_pos/light_color and replace the direct
gamma step in the fragment output code paths referenced around the other places
(the block around lines referencing the final color at 915-917) to use exposure
-> ACES tonemap -> gamma so both the studio and classic paths match.
- Around line 737-748: The studio shader is missing uniforms for
diffuse/specular scaling and never applies them; add uniform float diffuse_scale
and uniform float specular_scale declarations alongside the other uniforms
(e.g., near view_pos, light_color, etc.) and then multiply the computed diffuse
term by diffuse_scale and the computed specular term by specular_scale in the
lighting calculation block (also update the same additions/applies in the other
shader section referenced around lines 874–900). Ensure the uniform names are
exactly diffuse_scale and specular_scale so the renderer's controls will bind
correctly and the scaled terms are used when composing final color.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 267daf52-340f-4d16-9df1-683e7ce968f6
📒 Files selected for processing (3)
newton/_src/viewer/gl/opengl.pynewton/_src/viewer/gl/shaders.pynewton/_src/viewer/viewer_gl.py
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/_src/viewer/viewer_gl.py
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
93d0bd4 to
67ae4c5
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
newton/_src/viewer/gl/opengl.py (2)
1908-1930:⚠️ Potential issue | 🟠 Major
exposureis not forwarded to shaders.The
_build_shader_kwargs()method omitsexposure, so changes torenderer.exposurehave no effect on rendering. Per theShaderShape.update()signature (context snippet 1),exposureis an expected parameter.🐛 Proposed fix
kwargs = { "view_matrix": self._view_matrix, "projection_matrix": self._projection_matrix, "view_pos": self.camera.pos, "up_axis": self.camera.up_axis, "sun_direction": tuple(self._sun_direction), "fog_color": self.sky_lower, "sky_color": self.sky_upper, "ground_color": self.sky_lower, "light_color": self._light_color, "enable_shadows": self.draw_shadows, "shadow_texture": self._shadow_texture, "light_space_matrix": self._light_space_matrix, "env_texture": self._env_texture, "env_intensity": self._env_intensity, "shadow_radius": self.shadow_radius, "shadow_extents": self.shadow_extents, "diffuse_scale": self.diffuse_scale, "specular_scale": self.specular_scale, "spotlight_enabled": self.spotlight_enabled, + "exposure": self.exposure, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/opengl.py` around lines 1908 - 1930, The shader kwargs builder (_build_shader_kwargs) is missing the exposure parameter so ShaderShape.update never receives renderer.exposure; add "exposure": self.renderer.exposure to the kwargs dict (placed alongside other lighting/settings entries, before kwargs.update(self._active_style.overrides)) so exposure changes propagate to shaders and can still be overridden by style overrides.
1939-1940:⚠️ Potential issue | 🟠 MajorRuntime
draw_skytoggle is bypassed.Line 1939 checks only
style.draw_sky, ignoring the instance attributeself.draw_sky. This breaks existing behavior where users could disable the sky at runtime viarenderer.draw_sky = False.🐛 Proposed fix
- if style.draw_sky: + if self.draw_sky and style.draw_sky: self._draw_sky()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/opengl.py` around lines 1939 - 1940, The runtime sky toggle is being bypassed because the condition only checks style.draw_sky; update the condition where _draw_sky is invoked so it respects the instance attribute self.draw_sky as well (e.g., require both style.draw_sky and self.draw_sky or use getattr(self, 'draw_sky', True)). Modify the conditional that calls self._draw_sky() to include self.draw_sky so renderer.draw_sky = False will prevent _draw_sky from running.
🧹 Nitpick comments (1)
newton/_src/viewer/gl/opengl.py (1)
45-65: Consider more specific type annotations for dict fields.The
sun_directionsandoverridesfields are typed as baredict, which loses type information. More specific annotations would improve IDE support and documentation.♻️ Suggested improvement
`@dataclass`(frozen=True) class ShadingStyleConfig: ... name: str shader_class: type[ShaderShape] draw_sky: bool - sun_directions: dict - overrides: dict = field(default_factory=dict) + sun_directions: dict[int, np.ndarray] + overrides: dict[str, object] = field(default_factory=dict)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/opengl.py` around lines 45 - 65, ShadingStyleConfig uses bare dict types; update the annotations to be specific (e.g. change sun_directions: dict to sun_directions: Mapping[int, Tuple[float, float, float]] and change overrides: dict to overrides: dict[str, Any] or Mapping[str, Any]) and add the required typing imports (Mapping, Tuple, Any) at the top; preserve the field(default_factory=dict) for overrides and keep the dataclass frozen and other names (ShadingStyleConfig, sun_directions, overrides, shader_class) unchanged so callers still find the same symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@newton/_src/viewer/gl/opengl.py`:
- Around line 1908-1930: The shader kwargs builder (_build_shader_kwargs) is
missing the exposure parameter so ShaderShape.update never receives
renderer.exposure; add "exposure": self.renderer.exposure to the kwargs dict
(placed alongside other lighting/settings entries, before
kwargs.update(self._active_style.overrides)) so exposure changes propagate to
shaders and can still be overridden by style overrides.
- Around line 1939-1940: The runtime sky toggle is being bypassed because the
condition only checks style.draw_sky; update the condition where _draw_sky is
invoked so it respects the instance attribute self.draw_sky as well (e.g.,
require both style.draw_sky and self.draw_sky or use getattr(self, 'draw_sky',
True)). Modify the conditional that calls self._draw_sky() to include
self.draw_sky so renderer.draw_sky = False will prevent _draw_sky from running.
---
Nitpick comments:
In `@newton/_src/viewer/gl/opengl.py`:
- Around line 45-65: ShadingStyleConfig uses bare dict types; update the
annotations to be specific (e.g. change sun_directions: dict to sun_directions:
Mapping[int, Tuple[float, float, float]] and change overrides: dict to
overrides: dict[str, Any] or Mapping[str, Any]) and add the required typing
imports (Mapping, Tuple, Any) at the top; preserve the
field(default_factory=dict) for overrides and keep the dataclass frozen and
other names (ShadingStyleConfig, sun_directions, overrides, shader_class)
unchanged so callers still find the same symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 24b9cb63-ba1e-43ef-a043-24e741584ab3
📒 Files selected for processing (3)
newton/_src/viewer/gl/opengl.pynewton/_src/viewer/gl/shaders.pynewton/_src/viewer/viewer_gl.py
🚧 Files skipped from review as they are similar to previous changes (2)
- newton/_src/viewer/viewer_gl.py
- newton/_src/viewer/gl/shaders.py
shi-eric
reviewed
Apr 3, 2026
shi-eric
reviewed
Apr 3, 2026
eric-heiden
reviewed
Apr 3, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/viewer/gl/opengl.py (1)
912-938:⚠️ Potential issue | 🟠 MajorValidate companion array lengths before launching/uploading.
activecomes only frompoints, butwidths,colors, and the newmaterialsparameter are never checked against that count. A shortwidthsarray will read out of bounds inupdate_vbo_transforms_from_points(), and shortcolors/materialsarrays can resize the GL buffers smaller whilerender()still drawsactive_instances.💡 Suggested guard rails
if active > self.num_instances: raise ValueError("Active point count exceeds allocated capacity. Reallocate before updating.") + if widths is not None and len(widths) != active: + raise ValueError("Number of widths must match number of points") + if colors is not None and len(colors) != active: + raise ValueError("Number of colors must match number of points") + if materials is not None and len(materials) != active: + raise ValueError("Number of materials must match number of points") self.active_instances = active🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/viewer/gl/opengl.py` around lines 912 - 938, The function update_from_points does not validate that companion arrays (widths, colors, materials) match the computed active count, which can cause out-of-bounds reads in update_vbo_transforms_from_points or mismatched GL buffer sizes in _update_vbo; before launching wp.launch or calling _update_vbo, check that if active>0 each of widths, colors, and materials (when not None) has length >= active (and raise a ValueError with a clear message referencing update_from_points and active_instances if not); ensure you reference the same active_instances value used for dim in update_vbo_transforms_from_points and _update_vbo so uploads and render() remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@newton/_src/viewer/gl/opengl.py`:
- Around line 912-938: The function update_from_points does not validate that
companion arrays (widths, colors, materials) match the computed active count,
which can cause out-of-bounds reads in update_vbo_transforms_from_points or
mismatched GL buffer sizes in _update_vbo; before launching wp.launch or calling
_update_vbo, check that if active>0 each of widths, colors, and materials (when
not None) has length >= active (and raise a ValueError with a clear message
referencing update_from_points and active_instances if not); ensure you
reference the same active_instances value used for dim in
update_vbo_transforms_from_points and _update_vbo so uploads and render() remain
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e6fbd4e0-5f3b-4d0b-9f9a-4a2632410047
📒 Files selected for processing (3)
CHANGELOG.mdnewton/_src/viewer/gl/opengl.pynewton/_src/viewer/gl/shaders.py
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- newton/_src/viewer/gl/shaders.py
eric-heiden
reviewed
Apr 4, 2026
eric-heiden
reviewed
Apr 4, 2026
eric-heiden
reviewed
Apr 4, 2026
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
ffb4dda to
1490c28
Compare
eric-heiden
force-pushed
the
horde/studio-shading-v2
branch
from
927089c to
51e0103
Compare
Member
|
This looks great overall! A couple of weeks ago, I started a similar effort to transform the Newton viewer into a lookdev studio, and the main thing I think is missing here is renderer agnosticism (GL, USD, OVRTX, ...). Otherwise we'll end up with nice-looking renders in GL, but if we want to take them to the next level for demos/presentations by rendering them in Kit, we'd be out of luck and would have to do that manually. For reference, sharing the plan drafted with Claude here: Newton Lookdev Studio — Implementation PlanContextTransform the Newton viewer into a lookdev studio — a consistent, beautiful rendering environment for producing publication-quality images without graphics expertise. Background appearance and lighting are decoupled: outdoor presets rely on ambient/reflections from a physically-based sky model, while studio presets with dark backgrounds use explicit light rigs for illumination. Two strictly separate concerns:
A preset sets all values at once (environment + light rig + ground + post-FX + etc.), then each individual value can still be overridden by the user. Key constraint: Lookdev settings = renderer-agnostic data model, portable across GL / USD / OVRTX. Phase 1: Data Model —
|
| Rig | Key | Fill | Rim | Character |
|---|---|---|---|---|
light_rig_sunny_outdoor() |
Warm sun 45° elev, 1.5 | Cool sky-blue opposite, 0.3 | — | Bright outdoor |
light_rig_cloudy_outdoor() |
Soft cool overhead, 0.8 | — | — | Flat overcast |
light_rig_studio_3point() |
Neutral-warm 45°/45°, 1.2 | Cool opposite 30°, 0.4 | Warm back, 0.6 | Classic studio |
light_rig_product() |
Soft warm overhead, 0.8 | Soft sides, 0.5 | — | Catalog clean |
light_rig_dramatic() |
— | — | Strong warm back, 1.5 | Moody silhouette |
Built-in scene presets — factory functions returning SceneEnvironment:
| Preset | Background | Light Rig | Ground | Fog |
|---|---|---|---|---|
preset_outdoor() |
Procedural sky (alt=45°, turb=2.0) | sunny_outdoor |
None | Auto |
preset_golden_hour() |
Procedural sky (alt=8°, turb=3.0) | sunny_outdoor (warm) |
None | Auto |
preset_overcast() |
Procedural sky (alt=60°, turb=8.0) | cloudy_outdoor |
None | Close fog |
preset_studio_light() |
Manual gradient (warm light) | studio_3point |
Shadow catcher | None |
preset_studio_dark() |
Manual gradient (dark gray) | dramatic |
Shadow catcher | None |
preset_neutral() |
Flat mid-gray solid | product |
None | None |
Outdoor presets use PROCEDURAL_SKY — sun position drives colors automatically. Studio presets use SKY_GRADIENT — explicit gradient colors for controlled artistic environments.
PRESETS: dict[str, Callable[[], SceneEnvironment]] — registry for UI dropdown.
Preetham sky utilities in lookdev.py
Python-side helpers for computing Preetham parameters from sun position + turbidity:
def sun_direction_from_angles(altitude_deg: float, azimuth_deg: float) -> tuple[float, float, float]:
"""Convert sun altitude/azimuth to normalized world-space direction vector."""
def preetham_coefficients(turbidity: float) -> tuple[np.ndarray, ...]:
"""Compute Perez A-E luminance distribution coefficients for given turbidity."""
def preetham_zenith(turbidity: float, sun_theta_z: float) -> tuple[float, float, float]:
"""Compute zenith luminance and chrominance (Yxy) for given turbidity and sun zenith angle."""
def preetham_sky_color(direction: np.ndarray, sun_dir: np.ndarray,
turbidity: float) -> np.ndarray:
"""Evaluate Preetham sky model for a single direction. Returns linear RGB."""These are used by:
_apply_environment()inopengl.py— to compute uniform values (sun_dir, perez_A-E, zenith_color, ambient_sky/ground)generate_preetham_hdri()— to rasterize a sky dome for USD export
Modify: newton/viewer.py
Re-export: SceneEnvironment, LightRig, DirectionalLight, BackgroundMode, ToneMapMode, GroundMode.
Phase 2: GL Integration — Multi-Light + Procedural Sky + Gradient Reflections
This phase delivers multi-light shading, a Preetham analytical sky model driven by sun position, and procedural gradient reflections in the shader. No texture generation needed — the sky/gradient that renders as the background also drives ambient fill and specular reflections.
Preetham Sky Model (GLSL)
The Preetham (1999) analytical sky model computes physically-plausible sky colors from just sun position + turbidity. Implemented entirely in the fragment shader:
// Preetham sky model — computes sky radiance for any view direction
// Inputs: sun_direction (from altitude/azimuth), turbidity
// Perez luminance distribution function:
// F(θ, γ) = (1 + A·e^(B/cosθ)) · (1 + C·e^(Dγ) + E·cos²γ)
// where θ = zenith angle of sample, γ = angle between sample and sun
// A-E coefficients are functions of turbidity (lookup table or polynomial)What the Preetham model provides automatically from sun position:
- Zenith color: blue at high sun, orange/pink at sunset, deep blue at twilight
- Horizon color: bright haze at noon, warm glow at golden hour
- Sun disc: position + color + intensity from altitude
- Ambient sky/ground colors: extracted from zenith + horizon for hemispherical ambient
Python-side: SceneEnvironment.sun_altitude + sun_azimuth + turbidity → compute sun direction vector + Preetham A-E coefficients → pass as uniforms. The shader evaluates the full sky per-pixel.
For SKY_GRADIENT mode (studio presets), the existing simple gradient path is used instead — sky_upper/sky_lower uniforms, no Preetham.
Modify: newton/_src/viewer/gl/shaders.py
New uniforms in fragment shader (replace single sun_direction/light_color):
#define MAX_LIGHTS 3
uniform int num_lights;
uniform vec3 light_directions[MAX_LIGHTS];
uniform vec3 light_colors[MAX_LIGHTS]; // pre-multiplied by intensity
uniform int shadow_light_idx; // which light uses shadow map (0 = key)
uniform int spotlight_light_idx; // which light has spotlight (0 = key)
// Environment ambient (derived from sky/background, NOT from rig)
uniform vec3 ambient_sky; // sky zenith color (from Preetham or manual gradient)
uniform vec3 ambient_ground; // horizon/ground color (from Preetham or manual gradient)
uniform float ambient_intensity; // environment ambient fill strength
// Preetham sky model parameters (PROCEDURAL_SKY mode)
uniform vec3 sun_dir; // normalized sun direction (from altitude/azimuth)
uniform float turbidity; // atmospheric clarity [1-10]
uniform vec3 perez_A, perez_B, perez_C, perez_D, perez_E; // Preetham coefficients
uniform vec3 zenith_color; // pre-computed zenith luminance/chrominance
// Sky mode control
uniform int background_mode; // 0=PROCEDURAL_SKY, 1=SKY_GRADIENT, 2=ENV_MAP, 3=SOLID
uniform bool use_env_map; // true when HDRI is loaded, false = procedural sky/gradient
uniform float env_intensity;
uniform float env_rotation; // radians, rotates HDRI sampling around up axis
uniform float bg_blur; // background blur level [0-1]Multi-light loop (replaces single Lo computation at line 374):
vec3 Lo = vec3(0.0);
for (int i = 0; i < num_lights; i++) {
vec3 L = normalize(light_directions[i]);
vec3 H = normalize(V + L);
float NdotL_i = max(dot(N, L), 0.0);
// Recompute D, G, F per-light (H changes per light)
// ... Cook-Torrance as existing code ...
vec3 contrib = (diffuse_i * diffuse_scale + spec_i * specular_scale) * light_colors[i] * NdotL_i;
float shadow_atten = 1.0;
if (i == shadow_light_idx) shadow_atten = 1.0 - ShadowCalculation();
float spot_atten = 1.0;
if (i == spotlight_light_idx) spot_atten = SpotlightAttenuation();
Lo += contrib * shadow_atten * spot_atten;
}Environment ambient (replaces hardcoded * 0.7 at line 381 — ambient fill derived from sky/background, independent of light rig):
// Ambient comes from the environment (sky model or gradient), not the light rig
vec3 ambient = mix(ambient_ground, ambient_sky, sky_fac) * albedo * ambient_intensity;
// Fresnel-weighted ambient specular (existing logic)
vec3 F_ambient = F0 + (F_max - F0) * pow(1.0 - NdotV, 5.0);
vec3 kD_ambient = (1.0 - F_ambient) * (1.0 - metallic);
ambient = kD_ambient * ambient + F_ambient * mix(ambient_ground, ambient_sky, sky_fac) * 0.35;For PROCEDURAL_SKY mode, ambient_sky and ambient_ground are computed Python-side from the Preetham model (evaluate at zenith and horizon directions). For SKY_GRADIENT mode, they come directly from sky_upper/sky_lower.
Procedural gradient reflections (replaces ad-hoc metal-only env reflection, lines 393-399):
// Environment reflections — procedural sky/gradient fallback or HDRI
vec3 R = reflect(-V, N);
vec3 env_color;
if (use_env_map) {
// Sample loaded HDRI (existing env_map texture path)
float env_lod = roughness * 8.0;
env_color = pow(sample_env_map(R, env_lod), vec3(2.2));
} else if (background_mode == 0) {
// PROCEDURAL_SKY: sample Preetham model in reflection direction
env_color = preetham_sky(R, sun_dir, perez_A, perez_B, perez_C, perez_D, perez_E, zenith_color);
} else {
// SKY_GRADIENT / SOLID: sample the gradient used for the background
float r_up = dot(R, up) * 0.5 + 0.5;
env_color = mix(ambient_ground, ambient_sky, r_up);
}
vec3 env_F = F0 + (F_max - F0) * pow(1.0 - NdotV, 5.0);
vec3 env_spec = env_color * env_F * env_intensity;
// Apply to all materials (not just metallic — dielectrics get subtle reflections too)
color += env_spec * mix(0.04, 1.0, metallic);This ensures: (1) outdoor presets show realistic sky reflections that shift with time of day, (2) studio presets with dark backgrounds still show proper gradient reflections on metallic/shiny surfaces.
Environment rotation — apply to HDRI sampling (both reflection and sky shader):
vec2 rotated_equirect_uv(vec3 dir) {
// Rotate direction around up axis by env_rotation
float c = cos(env_rotation), s = sin(env_rotation);
// ... rotate dir.xz (or appropriate plane for up_axis) ...
return equirect_uv(rotated_dir);
}Background blur — in sky shader, when bg_blur > 0 and HDRI is loaded, sample the env map at a mip level proportional to blur:
float blur_lod = bg_blur * 8.0; // max mip levels
vec3 bg_color = textureLod(env_map, uv, blur_lod).rgb;For procedural sky and gradient backgrounds, blur is a no-op (already smooth).
Sky shader update — support both procedural sky and manual gradient:
// sky_fragment_shader: compute background color per-pixel
if (background_mode == 0) {
// PROCEDURAL_SKY: full Preetham evaluation per pixel
vec3 view_dir = normalize(FragPos);
vec3 sky = preetham_sky(view_dir, sun_dir, perez_A, ...perez_E, zenith_color);
// Add sun disc
float sun_angle = acos(max(dot(view_dir, sun_dir), 0.0));
sky += sun_disc(sun_angle, turbidity);
FragColor = vec4(sky, 1.0);
} else if (background_mode == 1) {
// SKY_GRADIENT: simple linear interpolation (studio presets)
float height = ...; // existing height computation
vec3 sky = mix(sky_lower, sky_upper, height);
FragColor = vec4(sky, 1.0);
} else if (background_mode == 2) {
// ENVIRONMENT_MAP: sample HDRI with optional blur
...
}Fog — replace hardcoded 20.0/200.0 (lines 403-404) with uniforms:
uniform float fog_start;
uniform float fog_end;Tone mapping — replace hardcoded ACES (lines 408-412) with switchable:
uniform int tone_map_mode; // 0=ACES, 1=AgX, 2=Reinhard, 3=linear
color = apply_tone_map(color * exposure, tone_map_mode);Add tone_map_aces(), tone_map_agx() (Blender-style polynomial), tone_map_reinhard() functions.
Modify: newton/_src/viewer/gl/opengl.py
- Add
self._environment: SceneEnvironmentproperty onRendererGL - Add
_apply_environment(env)method: mapsSceneEnvironmentfields to renderer state and shader uniforms- For
PROCEDURAL_SKY: compute sun direction fromsun_altitude/sun_azimuth, compute Preetham A-E coefficients fromturbidity, extractambient_sky/ambient_groundby evaluating model at zenith/horizon - For
SKY_GRADIENT: passsky_upper/sky_lowerdirectly asambient_sky/ambient_ground
- For
- Extend
ShapeShader.update()to accept multi-light arrays + ambient params +use_env_mapbool + sky mode - Deprecate
draw_sky→ computed property: getter returnsbackground_mode != SOLID_COLOR, setter issues deprecation warning and setsbackground_modeaccordingly - World-space sun direction: In
PROCEDURAL_SKYmode, derive fromsun_altitude/sun_azimuth. Whenlight_rigis set, userig.key.directionfor shadow matrix computation (replaces camera-relative lazy init at line ~1124). When both areNone, fall back to existing camera-relative behavior. - Shadow map light-space matrix: use key light direction from rig (or sun direction in procedural sky mode)
Modify: newton/_src/viewer/viewer.py
- Add
self.scene_environment: SceneEnvironmentonViewerBase.__init__()
Phase 3: Ground Plane — Shadow Catcher + Reflection Catcher
New shader: ground_catcher_fragment_shader in shaders.py
// Ground catcher: shadows + optional reflections
uniform sampler2D shadow_map;
uniform sampler2D reflection_map; // planar reflection FBO texture
uniform mat4 light_space_matrix;
uniform float shadow_radius;
uniform float shadow_extents;
uniform float shadow_opacity; // parameterized (default 0.5)
uniform float reflection_opacity; // 0 = off, up to 1.0
uniform float reflection_roughness; // blur amount for reflection
uniform vec3 ground_center;
uniform float ground_fade_radius;
uniform int up_axis;
in vec3 FragPos;
in vec4 FragPosLightSpace;
in vec2 ScreenCoord; // for reflection map sampling
out vec4 FragColor;
void main() {
// Radial fade from scene center
vec3 delta = FragPos - ground_center;
if (up_axis == 0) delta.x = 0.0;
else if (up_axis == 1) delta.y = 0.0;
else delta.z = 0.0;
float dist = length(delta);
float fade = 1.0 - smoothstep(ground_fade_radius * 0.5, ground_fade_radius, dist);
// Shadow contribution
float shadow = ShadowCalculation() * shadow_opacity * fade;
// Reflection contribution (from planar reflection FBO)
vec3 refl_color = vec3(0.0);
float refl_alpha = 0.0;
if (reflection_opacity > 0.0) {
// Sample reflection with roughness-based blur (mip level)
float lod = reflection_roughness * 4.0;
refl_color = textureLod(reflection_map, ScreenCoord, lod).rgb;
refl_alpha = reflection_opacity * fade;
}
// Composite: dark shadow + colored reflection
vec3 color = refl_color * refl_alpha;
float alpha = max(shadow, refl_alpha);
if (alpha < 0.001) discard;
FragColor = vec4(color, alpha);
}Planar reflection pass
When ground_reflection_opacity > 0:
- Create a reflection FBO (half scene resolution)
- Render the scene with a mirrored camera (flip across ground plane)
- Generate mip chain on the reflection texture for roughness-based blur
- Sample in ground catcher shader using screen-space UV
This is a standard planar reflection technique — render the scene from a reflected viewpoint into an FBO, then sample it on the ground plane.
RendererGL changes in opengl.py
- Create ground plane quad mesh in
__init__()(large quad, auto-scaled from scene bounds) - Render pipeline with ground plane:
- Shadow pass (existing)
- Reflection pass (new, only when
ground_reflection_opacity > 0): render scene into reflection FBO with mirrored camera + clip plane at ground height - Scene pass (existing)
- Ground plane pass:
SHADOW_CATCHERusesShaderGroundCatcherwith alpha blending,SOLIDusesShapeShaderwith PBR-lit ground albedo
- Auto-detect
ground_heightfrom scene bounds minimum along up axis - Hide collision shapes with
GeoType.PLANEwhen shadow catcher is active
Phase 4: Auto Scene Bounds + Adaptive Parameters
Modify: newton/_src/viewer/viewer.py
Enhance _get_world_extents() → _get_scene_bounds():
def _get_scene_bounds(self) -> tuple[np.ndarray, float] | None:
"""Return (center, max_extent) of visible scene geometry.
Excludes infinite planes. Uses existing Warp kernel.
"""Cache result as self._scene_bounds (updated each log_state()).
Auto-resolve None parameters in _apply_environment()
extent = self._scene_bounds[1] if self._scene_bounds else 10.0
shadow_extents = env.shadow_extents or extent * 1.5
fog_start = env.fog_start or extent * 2.0
fog_end = env.fog_end or extent * 20.0
ground_height = env.ground_height or bounds_min_up - epsilon
ground_fade = env.ground_fade_radius or extent * 3.0Phase 5: Preset Override UX + UI
Modify: newton/_src/viewer/viewer_gl.py
State tracking:
self._active_preset: str = "Outdoor" # from PRESETS keys, or "Custom"Preset dropdown behavior:
- Dropdown shows all preset names. "Custom" entry appears only when
_active_preset == "Custom". - Selecting a preset: calls factory → sets
self._environment→ applies to renderer → resets_active_presetto that name. - Any manual edit (slider, color picker, radio button): sets
_active_preset = "Custom".
Full "Rendering Options" UI layout (replaces current lines 1976-2009):
[Preset: v Outdoor ] ← combo, top of section
-- Background --
(o) Procedural Sky ( ) Gradient ( ) Solid Color ← radio
(if Procedural Sky)
Sun Altitude [slider 0-90°] Azimuth [slider 0-360°]
Turbidity [slider 1-10]
(if Gradient)
Sky Upper: [color] Sky Lower: [color]
-- Lighting --
Key: Dir [x,y,z] Color [color] Intensity [slider]
Fill: [checkbox enabled] Dir/Color/Intensity
Back: [checkbox enabled] Dir/Color/Intensity
[x] Key Shadow [x] Key Spotlight
Ambient Sky [color] Ground [color] Intensity [slider]
-- Environment Map --
Intensity [slider]
Rotation [slider 0-360°]
Blur [slider 0-1]
-- Tone Mapping --
Mode: v ACES Exposure [slider]
-- Ground --
Mode: v None | Shadow Catcher | Solid
(if Solid) Color [color]
Shadow Opacity [slider 0-1]
Reflection Opacity [slider 0-1]
Reflection Rough. [slider 0-1]
-- Shadows --
Radius [slider]
-- Post FX --
[x] AO Intensity [slider] Radius [slider]
[x] DOF Focus [slider] Aperture [slider]
[x] Wireframe [x] VSync
Phase 6: Split-Sum IBL Pipeline (HDRI support)
This phase adds proper IBL for when an HDRI environment map is loaded. The procedural gradient path from Phase 2 remains the default fast path.
New file: newton/_src/viewer/gl/ibl.py
IBLProcessor class — GPU pre-computation via render-to-texture (GL 3.3 core):
class IBLProcessor:
"""Pre-computes IBL textures for split-sum approximation.
Only used when an HDRI environment map is loaded. For gradient-only
backgrounds, the shader uses procedural gradient sampling instead.
"""
def __init__(self, gl):
self._setup_shaders()
self._generate_brdf_lut() # once: 512x512 RG16F
def process(self, source_texture, width, height):
"""Generate irradiance + pre-filtered specular from equirect source."""
self._compute_irradiance(...) # 64x32 equirect
self._compute_prefiltered(...) # 256x128 base, 5 mip levelsThree IBL textures:
- BRDF LUT (TU 5) — 512x512 RG16F. Computed once. Hammersley + importance-sampled GGX →
(F0_scale, F0_bias). - Irradiance map (TU 3) — 64x32 equirect. Cosine-weighted hemisphere convolution.
- Pre-filtered specular (TU 4) — 256x128 equirect, 5 mip levels. GGX importance sampling per roughness.
Texture unit assignments:
| Unit | Texture |
|---|---|
| 0 | shadow_map (existing) |
| 1 | albedo_map (existing) |
| 2 | env_map / raw equirect (existing, for sky shader + non-IBL reflection) |
| 3 | irradiance_map (IBL diffuse, only when HDRI loaded) |
| 4 | prefilter_map (IBL specular, only when HDRI loaded) |
| 5 | brdf_lut (only when HDRI loaded) |
Shader changes: shaders.py
When use_env_map && use_ibl (HDRI loaded + IBL processed), upgrade the reflection path:
uniform bool use_ibl; // true when IBL textures are available
uniform sampler2D irradiance_map;
uniform sampler2D prefilter_map;
uniform sampler2D brdf_lut;
if (use_env_map && use_ibl) {
// Full split-sum IBL
vec3 irradiance = texture(irradiance_map, equirect_uv(N)).rgb;
vec3 ibl_diffuse = kD_ambient * albedo * irradiance;
vec3 R = reflect(-V, N);
float lod = roughness * 4.0;
vec3 prefiltered = textureLod(prefilter_map, equirect_uv(R), lod).rgb;
vec2 env_brdf = texture(brdf_lut, vec2(NdotV, roughness)).rg;
vec3 ibl_specular = prefiltered * (F_ambient * env_brdf.x + env_brdf.y);
color += (ibl_diffuse + ibl_specular) * env_intensity;
} else if (use_env_map) {
// HDRI loaded but IBL not yet processed — simple LOD sampling (existing path)
...
} else {
// Procedural gradient reflections (Phase 2 path — default, no texture)
...
}RendererGL changes
- Create
IBLProcessorlazily when first HDRI is loaded - Manage IBL texture lifecycle: process when HDRI changes, release when cleared
- Bind TU 3/4/5 only when IBL is active
Phase 7: Post-Processing — SSAO + DOF
SSAO (Screen-Space Ambient Occlusion)
Adds contact shadows in crevices and where objects meet, significantly improving visual grounding.
Technique: SAO (Scalable Ambient Obscurance) — a modern SSAO variant that works well at varying scales. Requires a depth buffer (already available from MSAA resolve) and normal reconstruction.
New file: newton/_src/viewer/gl/postprocess.py
class PostProcessPass:
"""Base class for screen-space post-processing passes."""
def __init__(self, gl): ...
def resize(self, width, height): ...
def render(self, color_texture, depth_texture): ...
class SSAOPass(PostProcessPass):
"""Screen-space ambient occlusion using SAO algorithm."""
def __init__(self, gl):
# AO FBO (half-res for performance)
# Blur FBO (bilateral blur to remove noise)
# SAO kernel samples (hemisphere)
def render(self, depth_texture, normal_texture, projection_matrix,
ao_radius, ao_intensity) -> int:
"""Returns AO texture to multiply with scene color."""Render pipeline change:
- Scene pass → color + depth
- SSAO pass: read depth → compute AO → bilateral blur → AO texture
- Composite:
final_color = scene_color * ao_factor
Shader: ssao_fragment_shader
- Reconstruct view-space position from depth
- Sample hemisphere around each fragment (16-32 samples)
- Compare depth of samples vs actual depth → occlusion
- Bilateral blur pass to denoise (preserves edges)
Parameters from SceneEnvironment:
ao_radius: sample radius in world units (None = auto from scene extent * 0.05)ao_intensity: darkening strength multiplier
DOF (Depth of Field)
Simulates camera lens blur for cinematic presentation.
Technique: Separable Gaussian DOF with CoC (Circle of Confusion) computation. Simple but effective for a lookdev tool.
class DOFPass(PostProcessPass):
"""Depth-of-field bokeh blur."""
def render(self, color_texture, depth_texture,
focus_distance, aperture, focal_length) -> int:
"""Returns DOF-blurred color texture."""Shader: dof_fragment_shader
- Compute CoC from depth vs focus distance:
coc = abs(depth - focus) * aperture_factor - Two-pass separable Gaussian blur weighted by CoC
- Near-field and far-field handled separately (near bleeds over focused areas)
Parameters from SceneEnvironment:
dof_focus_distance: world-space focus distance (None = auto, center of scene bounds)dof_aperture: f-stop number (lower = shallower DOF, more blur)
Render pipeline (full, after this phase)
1. Shadow pass → shadow_map
2. Reflection pass → reflection_map (if ground reflection enabled)
3. Scene pass → scene_color + scene_depth
4. SSAO pass (if enabled) → ao_texture
5. Composite AO → color *= ao
6. Ground plane pass → alpha-blended onto color
7. DOF pass (if enabled) → bokeh-blurred color
8. Tone mapping → final output
Phase 8: Material Library
Current state
Materials are set at mesh creation time (Mesh.color, Mesh.roughness, Mesh.metallic). Runtime API only has update_shape_colors() (color only). Instance buffers already carry vec4(roughness, metallic, checker, texture) per shape — so the GPU pipeline already supports per-instance material variation; we just need the Python API.
New dataclass in lookdev.py
@dataclass
class LookdevMaterial:
"""A named PBR material for consistent appearance across scenes."""
color: tuple[float, float, float] = (0.8, 0.8, 0.8)
roughness: float = 0.5
metallic: float = 0.0Built-in material presets
MATERIALS: dict[str, LookdevMaterial] = {
"clay": LookdevMaterial(color=(0.85, 0.82, 0.78), roughness=0.9, metallic=0.0),
"plastic_white": LookdevMaterial(color=(0.95, 0.95, 0.95), roughness=0.4, metallic=0.0),
"plastic_dark": LookdevMaterial(color=(0.15, 0.15, 0.15), roughness=0.4, metallic=0.0),
"rubber": LookdevMaterial(color=(0.12, 0.12, 0.12), roughness=0.95, metallic=0.0),
"chrome": LookdevMaterial(color=(0.95, 0.95, 0.95), roughness=0.05, metallic=1.0),
"brushed_metal": LookdevMaterial(color=(0.7, 0.7, 0.72), roughness=0.35, metallic=1.0),
"gold": LookdevMaterial(color=(1.0, 0.84, 0.0), roughness=0.2, metallic=1.0),
"copper": LookdevMaterial(color=(0.95, 0.64, 0.54), roughness=0.25, metallic=1.0),
"glass": LookdevMaterial(color=(0.95, 0.95, 0.95), roughness=0.05, metallic=0.0),
"wood": LookdevMaterial(color=(0.55, 0.35, 0.18), roughness=0.7, metallic=0.0),
"concrete": LookdevMaterial(color=(0.6, 0.58, 0.55), roughness=0.9, metallic=0.0),
}API: assign materials to shapes
Extend ViewerBase with a full material update method alongside the existing update_shape_colors():
def update_shape_materials(
self,
shape_materials: dict[int, LookdevMaterial | str],
) -> None:
"""Assign materials to shapes by index.
Args:
shape_materials: Maps shape index → LookdevMaterial instance or preset name
(e.g., "chrome", "clay"). Updates color, roughness, and metallic.
"""This extends the existing update_shape_colors() pattern (viewer.py lines 1526-1544) to also update the material vec4 in ShapeInstances.materials. The colors_changed flag triggers GPU buffer re-upload on next frame.
Usage example
import newton
from newton.viewer import ViewerGL, LookdevMaterial, preset_studio_light
viewer = ViewerGL()
viewer.scene_environment = preset_studio_light()
# Assign materials when building the scene
viewer.update_shape_materials({
0: "chrome", # built-in preset by name
1: "clay",
2: LookdevMaterial(color=(0.2, 0.4, 0.8), roughness=0.3, metallic=0.0), # custom
})Public API: newton/viewer.py
Re-export LookdevMaterial and MATERIALS.
Phase 9: USD Lighting Export
Environment and light rig export as separate USD prims — the DomeLight contains only the background/ambient environment, light rigs export as dedicated DistantLight prims.
New utilities in lookdev.py
def generate_gradient_hdri(
sky_upper: tuple[float, float, float],
sky_lower: tuple[float, float, float],
width: int = 512,
height: int = 256,
) -> np.ndarray:
"""Rasterize a sky gradient to an equirectangular HDR image [height, width, 3]."""
t = np.linspace(1.0, 0.0, height)[:, None]
upper, lower = np.array(sky_upper), np.array(sky_lower)
img = lower + t * (upper - lower)
return np.broadcast_to(img, (height, width, 3)).copy()
def generate_preetham_hdri(
sun_altitude: float,
sun_azimuth: float,
turbidity: float = 2.5,
width: int = 512,
height: int = 256,
) -> np.ndarray:
"""Rasterize Preetham sky model to equirectangular HDR image [height, width, 3].
Evaluates the Preetham analytical sky for each pixel direction. Used for
USD DomeLight export when PROCEDURAL_SKY mode is active.
"""Modify: newton/_src/viewer/viewer_usd.py
DomeLight — environment only (sky/gradient or HDRI, no directional lights baked in):
def _setup_dome_light(self, env: SceneEnvironment):
from pxr import UsdLux
dome = UsdLux.DomeLight.Define(self.stage, "/Environment/DomeLight")
if env.environment_map:
dome.CreateTextureFileAttr().Set(env.environment_map)
elif env.background_mode == BackgroundMode.PROCEDURAL_SKY:
# Rasterize Preetham sky to HDRI — contains ONLY sky colors, no lights
hdri = generate_preetham_hdri(env.sun_altitude, env.sun_azimuth, env.turbidity)
path = os.path.join(tempfile.gettempdir(), "newton_dome_env.hdr")
_write_hdr(path, hdri)
dome.CreateTextureFileAttr().Set(path)
else:
# Generate HDRI from manual gradient
hdri = generate_gradient_hdri(env.sky_upper, env.sky_lower)
path = os.path.join(tempfile.gettempdir(), "newton_dome_env.hdr")
_write_hdr(path, hdri)
dome.CreateTextureFileAttr().Set(path)
dome.CreateIntensityAttr().Set(env.environment_intensity)Light rig — exported as separate DistantLight prims:
def _setup_light_rig(self, env: SceneEnvironment):
from pxr import UsdLux, Gf
if env.light_rig is None:
return
for name, light in [("Key", env.light_rig.key),
("Fill", env.light_rig.fill),
("Back", env.light_rig.back)]:
if light is None:
continue
usd_light = UsdLux.DistantLight.Define(self.stage, f"/Environment/{name}Light")
usd_light.CreateIntensityAttr().Set(light.intensity)
usd_light.CreateColorAttr().Set(Gf.Vec3f(*light.color))
# Set direction via xform rotation
...This ensures the environment and light rig remain independently controllable in USD/OVRTX, matching the GL renderer's separation.
Backend Strategy Summary
| Concern | GL (real-time) | USD / OVRTX (offline) |
|---|---|---|
| Background | Sky shader: Preetham sky, manual gradient, or HDRI (with rotation + blur) | DomeLight texture (rasterized Preetham HDRI, gradient HDRI, or user HDRI) |
| Diffuse ambient | Hemispherical ambient from sky model / gradient | DomeLight irradiance |
| Specular reflections | Procedural gradient (default) or HDRI split-sum IBL | DomeLight specular |
| Direct lighting | Multi-light loop (up to 3 directional) | USD lights (future) |
| Shadow catcher | Ground plane quad + alpha-blend shader (param shadow opacity) | USD shadow-catcher material (future) |
| Reflection catcher | Planar reflection FBO + roughness blur | USD reflection plane (future) |
| Post-FX | SSAO (SAO algorithm) + DOF (separable Gaussian) | Renderer-native (future) |
| Runtime control | Instant — all uniforms, no re-bake | Rasterize Preetham / regenerate HDRI on next export |
Files Summary
| File | Action | Phase | Changes |
|---|---|---|---|
newton/_src/viewer/lookdev.py |
CREATE | 1, 8 | SceneEnvironment, LightRig, DirectionalLight, LookdevMaterial, enums, Preetham sky math, preset/rig/material factories, generate_gradient_hdri(), generate_preetham_hdri() |
newton/_src/viewer/gl/ibl.py |
CREATE | 6 | IBLProcessor, BRDF LUT/irradiance/prefilter shaders |
newton/_src/viewer/gl/postprocess.py |
CREATE | 7 | SSAOPass, DOFPass, post-processing framework |
newton/_src/viewer/gl/shaders.py |
MODIFY | 2-7 | Preetham sky model, multi-light loop, procedural sky/gradient reflections, env rotation, bg blur, tone map modes, fog uniforms, ground catcher + reflection shader, SSAO/DOF shaders, IBL split-sum |
newton/_src/viewer/gl/opengl.py |
MODIFY | 2-7 | environment property, _apply_environment(), ground plane + reflection FBO, multi-light uniforms, deprecate draw_sky, IBL textures, post-processing pipeline |
newton/_src/viewer/viewer.py |
MODIFY | 4, 8 | scene_environment on ViewerBase, _get_scene_bounds(), update_shape_materials() |
newton/_src/viewer/viewer_gl.py |
MODIFY | 5 | Full lookdev UI, preset dropdown + "Custom" override, ground/collision plane hiding |
newton/_src/viewer/viewer_usd.py |
MODIFY | 9 | _setup_dome_light(), _setup_light_rig() with USD lights |
newton/viewer.py |
MODIFY | 1, 8 | Re-export lookdev types + LookdevMaterial, MATERIALS |
Verification
- Procedural sky coherence: Outdoor preset — sky colors, ambient fill, sun disc, and metallic reflections all respond to sun altitude/azimuth/turbidity
- Time of day: Slide sun altitude from 90° → 0° — sky transitions from blue noon → orange golden hour → red sunset
- Studio gradient coherence: Studio preset — background gradient, ambient fill, and metallic reflections all derive from the same gradient colors
- Dark background + bright objects:
preset_studio_dark()— dramatic rig lights objects despite dark background - Gradient reflections: Metallic sphere on studio preset reflects the background gradient (no black void)
- Light rig independence: Switch rigs while keeping background — lighting changes, background stays
- Preset switching: Each preset produces distinct, coherent look
- Preset override → "Custom": Edit any param after selecting preset → dropdown shows "Custom"
- Multi-light: Studio 3-point shows key/fill/rim — rotate model to see each
- Environment rotation: Rotate HDRI env → reflections and lighting shift accordingly
- Background blur: Increase blur → HDRI background becomes soft/defocused
- Auto bounds: Small and large scenes auto-adapt shadow extents, fog, ground height
- Tone mapping: Compare ACES/AgX/Reinhard/Linear on same scene
- Shadow catcher: Shadow opacity slider controls darkness; fades at edges
- Reflection catcher: Shiny objects reflected on ground plane with adjustable roughness
- Collision plane hiding: Planes hidden when shadow catcher active
- SSAO: Enable AO → crevices and contact areas darken; disable → no performance cost
- DOF: Focus on subject → foreground/background blur with adjustable aperture
- Material presets: Assign "chrome" to a shape → shiny metallic appearance; assign "clay" → matte diffuse
- Material override:
update_shape_materials({0: "chrome", 1: LookdevMaterial(...)})updates appearance at runtime - HDRI override (Phase 6): Load HDRI → reflections upgrade from gradient to HDRI-based IBL
- USD export: DomeLight contains gradient only (no lights baked in), rig exported as separate
DistantLightprims - Programmatic API:
viewer.scene_environment = preset_studio_dark() - Tests:
uv run --extra dev -m newton.tests -k test_viewer - Pre-commit:
uvx pre-commit run -a
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
2 times, most recently
from
8932005 to
bc0d7df
Compare
Add a "studio" rendering style with soft 3-point lighting, shadow mapping, rim highlights, and a neutral matte ground plane (roughness 0.85). The existing "classic" checker-floor style remains the default. Refactor the shading system into a registry-based design (ShadingStyleConfig + STYLE_REGISTRY) so new styles can be added by registering a config object — no viewer code changes needed. Add an optional edge overlay that draws mesh wireframes on top of solid geometry (renderer.draw_edges = True). Users can switch styles at runtime via the sidebar dropdown or programmatically with viewer.renderer.shading_style = "studio". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add missing `exposure` to _build_shader_kwargs so renderer.exposure reaches the shaders. - Respect runtime `self.draw_sky` toggle (was only checking style config). - Remove redundant Apache 2.0 boilerplate after SPDX headers. - Add CHANGELOG entries for studio shading, edge overlay, and style registry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore the classic fragment shader to upstream's exact Cook-Torrance PBR implementation (GGX distribution, Smith geometry, Schlick Fresnel). The previous commit incorrectly replaced it with Blinn-Phong. Add exposure uniform to the studio shader so renderer.exposure affects both styles. Studio still uses simple clamp+gamma (no ACES) because its lower light multipliers keep values in a moderate range. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Restore MeshGL default roughness to 0.5 (was 0.85 which leaked the studio matte look to all non-instanced meshes including classic mode) - Add enable_shadows, diffuse_scale, specular_scale uniforms to the studio fragment shader so _build_shader_kwargs() values are not silently dropped; gate ShadowCalculation() behind enable_shadows to prevent sampling stale shadow maps when shadows are disabled - Draw sky dome before enabling GL_LINE polygon mode so wireframe toggle no longer rasterizes the sky sphere as wireframe lines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The refactoring in 67ae4c5 replaced the dedicated ambient_sky/ ambient_ground attributes with sky_upper/sky_lower (the visible sky colors), causing a blue tint in the classic shader's hemisphere ambient lighting. Restore the original neutral values so the classic ground plane renders gray instead of blue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Draw a full-screen quad with a vertical color blend before the scene when the active style defines gradient_top/gradient_bottom. The gradient is camera-independent, matching the clean studio backdrop aesthetic Eric requested. Depth writes are disabled during the pass so scene geometry renders normally on top. Studio defaults: near-white (0.96) at top, light gray (0.85) at bottom — mimicking overhead softbox falloff in a photo studio. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch studio from draw_sky=False to draw_sky=True with near-white sky colors (0.96 top, 0.85 bottom) and a zeroed sun direction to suppress the sun flare. The sky dome renders a smooth gradient that responds to camera angle, matching Eric's request for a gradient sky shader instead of a flat clear color. The _draw_sky() method now reads sky_upper, sky_lower, and sky_sun_direction from the active style's overrides dict, falling back to the renderer defaults for classic mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sky_upper, sky_lower, and sky_sun_direction were in the studio overrides dict, which _build_shader_kwargs() merges into kwargs passed to ShaderShape.update(). ShaderShape doesn't accept those parameters, causing a TypeError. Move sky dome colors to dedicated ShadingStyleConfig fields (sky_upper, sky_lower, draw_sun) that _draw_sky() reads directly, keeping the overrides dict clean for shape-shader-only uniforms. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The original near-white colors (0.96 top, 0.85 bottom) were too similar to produce a visible gradient through the dome's height- based interpolation. Widen to (0.96 top, 0.62 bottom) so the gradient from light at zenith to medium gray at the horizon is clearly visible at typical camera angles. Verified: studio shows visible gradient, classic unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Widen sky_lower from 0.62 to 0.55 and sky_upper from 0.96 to 0.98 for a more visible gradient at typical camera angles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the studio fragment shader's ad-hoc Blinn-Phong specular with the same GGX/Cook-Torrance BRDF used by the classic shader: GGX normal distribution, Smith-Schlick geometry, and Schlick Fresnel with roughness dampening. The fill light also gets its own per-light Cook-Torrance evaluation for consistent specular across all three studio lights. Key light multiplier raised from 3.0 to 4.0 and fill from 0.84 to 1.2 to compensate for the energy-conserving PI-normalised diffuse term. Metallic desaturation and F0 computation ported from classic shader. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The checker-enabled ground plane ships with roughness 0.5, which produces visible specular reflections under Cook-Torrance. Clamp roughness to at least 0.85 for checker surfaces in the studio fragment shader so the floor reads as matte concrete/paper rather than wet tile. Classic shader is unaffected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AnkaChan
force-pushed
the
horde/studio-shading-v2
branch
from
bc0d7df to
e8bacd6
Compare
The studio background was much brighter than classic (near-white sky vs dark blue). Darken sky_upper from (0.98) to (0.68), sky_lower from (0.55) to (0.32), and fog_color from (0.93) to (0.55) so the two styles have comparable overall brightness while keeping the studio's neutral gray palette. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Lower key light from 4.0x to 3.0x (same as classic), fill from 1.2 to 0.5, back fill from 0.10 to 0.08, catch light from 0.18 to 0.06, rim from 0.07 to 0.04, and ambient from 0.35 to 0.25. Fill specular weight also reduced from 0.28 to 0.20. The previous values made studio noticeably brighter than classic due to cumulative multi-light energy. These lower values produce a comparable overall intensity while preserving the studio's soft multi-light look. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reduce studio ground_color ambient override from (0.50, 0.45, 0.38) to (0.30, 0.28, 0.25) so the ground plane intensity is closer to classic. In the edge fragment shader, discard fragments with checker_enable > 0 (Material.z) so the ground plane's wireframe is not drawn during the edge overlay pass. Object edges remain unaffected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ground plane (checker-enabled) was reflecting too much direct light in studio mode. Add a ground_dampen factor (0.35) that scales down the combined key/fill/specular direct lighting contribution for checker surfaces, making the ground read as a subdued backdrop rather than a reflective surface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add a "studio" rendering style to the GL viewer with soft 3-point lighting, shadow mapping, rim highlights, and a neutral matte ground plane. The existing "classic"
checker-floor style remains the default.
Refactor the shading system into a registry-based design (ShadingStyleConfig + STYLE_REGISTRY) so new styles can be added by registering a config dict — no viewer code
changes needed. The sidebar dropdown auto-populates from the registry.
Also add an optional edge overlay that draws mesh wireframes on top of solid geometry.
Changes
renderer.draw_edges = Truedraws wireframe edges over solid shadingShadingStyleConfigdataclass +STYLE_REGISTRYdict; adding a new style only requires registering a configViewerGL(shading_style="studio")for headless/programmatic useChecklist
Test plan
Tested with four examples in headless mode (Xvfb), verifying non-black frames for classic, studio, and studio+edges:
cloth_franka — 60 frames @ 60fps ✓
cloth_h1 — 300 frames @ 60fps ✓
robot_anymal_d — 250 frames @ 50fps ✓
cloth_rollers — 300 frames @ 60fps ✓
Runtime style switching verified (single viewer, switching renderer.shading_style between "classic" and "studio").
See https://docs.google.com/document/d/1wdY3Xg2xEXUk8zDkclwjMyTGQ8LfyobCzB69QGh5qkg/edit?usp=sharing for designs and implementation explanation.
Summary by CodeRabbit
New Features
Rendering Improvements
UI/UX Improvements
Documentation