Skip to content

Follow-up to frontFace PR: fix double-flip, remove twoSidedLightingNegScaleFactor, add setDrawStates#8503

Merged
mvaligursky merged 1 commit intomainfrom
mv-frontface-followup
Mar 5, 2026
Merged

Follow-up to frontFace PR: fix double-flip, remove twoSidedLightingNegScaleFactor, add setDrawStates#8503
mvaligursky merged 1 commit intomainfrom
mv-frontface-followup

Conversation

@mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Mar 5, 2026

Summary

Follow-up to #8448.

  • Fix double-flip bug in setupCullModeAndFrontFace: only flip frontFace (not cullMode) when flipFaces < 0. Cull mode is defined relative to frontFace, so swapping both was a double-correction that culled the wrong triangles.
  • Remove twoSidedLightingNegScaleFactor uniform from renderer.js and both GLSL/WGSL twoSidedLighting shader chunks. With frontFace now corrected at the GPU level, gl_FrontFacing/pcFrontFacing is accurate and the uniform is redundant. The shader logic simplifies to if (!gl_FrontFacing) dTBN[2] = -dTBN[2];.
  • Add setDrawStates() convenience method to GraphicsDevice that sets all draw-related render states (blend, depth, cull, frontFace, stencil) in a single call with sensible defaults for utility rendering. Calling setDrawStates() with no arguments resets to a safe baseline (BlendState.NOBLEND, DepthState.NODEPTH, CULLFACE_NONE, FRONTFACE_CCW, no stencil).
  • Update 8 internal call sites to use setDrawStates(), cleaning up now-unused imports of CULLFACE_NONE, FRONTFACE_CCW, BlendState, DepthState where applicable.
  • Update QuadRender JSDoc to recommend setDrawStates as the primary approach for setting render states before quad rendering.
  • Add hidden two-sided-lighting test example (examples/src/examples/test/two-sided-lighting.example.mjs) with TwoSidedPlane.glb, a skydome, directional light, orbiting omni light with emissive sphere, and orbit camera — useful for visually verifying two-sided lighting behavior.

Public API changes

New method on GraphicsDevice:

device.setDrawStates(blendState, depthState, cullMode, frontFace, stencilFront, stencilBack);

// All parameters are optional with defaults:
device.setDrawStates(); // safe baseline for utility rendering
device.setDrawStates(myBlendState); // custom blend, rest defaults
device.setDrawStates(myBlendState, myDepthState, myCull, myFrontFace, stencilFront, stencilBack); // all custom

…r, add setDrawStates

Follow-up to #8448.

- Fix double-flip bug in setupCullModeAndFrontFace: only flip frontFace
  (not cullMode) when flipFaces < 0, since cull mode is defined relative
  to frontFace.
- Remove the now-redundant twoSidedLightingNegScaleFactor uniform from
  renderer.js and both GLSL/WGSL twoSidedLighting shader chunks. With
  frontFace corrected at the GPU level, gl_FrontFacing/pcFrontFacing is
  accurate and the uniform is no longer needed.
- Add setDrawStates() convenience method to GraphicsDevice that sets all
  draw-related render states (blend, depth, cull, frontFace, stencil) in
  one call with sensible defaults.
- Update 8 internal call sites to use setDrawStates(), cleaning up
  redundant imports.
- Update QuadRender JSDoc to recommend setDrawStates().
- Add hidden two-sided-lighting test example with TwoSidedPlane.glb.

Made-with: Cursor
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Follow-up to prior frontFace pipeline work, this PR fixes incorrect face culling under negative-scale / face-flip conditions, simplifies two-sided lighting shaders by removing a redundant uniform, and introduces a GraphicsDevice#setDrawStates() helper to apply common draw render-state baselines consistently across utility passes.

Changes:

  • Fix setupCullModeAndFrontFace to flip only frontFace (avoiding a double-correction that culled the wrong triangles).
  • Remove twoSidedLightingNegScaleFactor uniform and simplify GLSL/WGSL two-sided lighting logic to rely on gl_FrontFacing / pcFrontFacing.
  • Add GraphicsDevice#setDrawStates() and update internal utility rendering call sites + QuadRender JSDoc; add a hidden two-sided lighting example for visual validation.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/scene/shader-lib/wgsl/chunks/lit/frag/twoSidedLighting.js Removes redundant uniform and simplifies WGSL two-sided lighting basis flip.
src/scene/shader-lib/glsl/chunks/lit/frag/twoSidedLighting.js Removes redundant uniform and simplifies GLSL two-sided lighting basis flip.
src/scene/renderer/renderer.js Fixes cull/frontFace setup logic and removes uniform plumbing.
src/scene/renderer/render-pass-cookie-renderer.js Uses device.setDrawStates() instead of individual state setters.
src/scene/particle-system/gpu-updater.js Uses device.setDrawStates() baseline for GPU particle quad updates.
src/scene/gsplat/gsplat-resolve-sh.js Uses device.setDrawStates() baseline before quad render.
src/scene/gsplat-unified/gsplat-work-buffer-render-pass.js Uses device.setDrawStates() baseline for work-buffer pass.
src/scene/graphics/render-pass-shader-quad.js Consolidates render state setup into a single setDrawStates(...) call.
src/scene/graphics/render-pass-quad.js Uses device.setDrawStates() baseline before quad render.
src/scene/graphics/quad-render.js Updates JSDoc to recommend setDrawStates for pre-quad render state setup.
src/platform/graphics/webgpu/webgpu-clear-renderer.js Uses setDrawStates(blendState, depthState) for clear-quad rendering.
src/platform/graphics/graphics-device.js Adds new convenience API setDrawStates(...) with utility-safe defaults.
src/extras/renderers/outline-renderer.js Uses device.setDrawStates(this.blendState) for outline compositing pass.
examples/src/examples/test/two-sided-lighting.example.mjs Adds a hidden visual test scene for two-sided lighting validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mvaligursky mvaligursky requested a review from a team March 5, 2026 12:10
@mvaligursky mvaligursky merged commit b196b22 into main Mar 5, 2026
12 checks passed
@mvaligursky mvaligursky deleted the mv-frontface-followup branch March 5, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: graphics Graphics related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants