Skip to content

fix: expose mesh-level shader defines to the fragment shader#8817

Merged
mvaligursky merged 1 commit into
mainfrom
mv-shared-mesh-defines
Jun 1, 2026
Merged

fix: expose mesh-level shader defines to the fragment shader#8817
mvaligursky merged 1 commit into
mainfrom
mv-shared-mesh-defines

Conversation

@mvaligursky
Copy link
Copy Markdown
Contributor

Object/mesh-level shader defines derived from MeshInstance shader defs were only declared in the vertex shader, so user chunk overrides could not branch on them in the fragment shader (issue #8815).

Changes:

  • Mesh-level defines (INSTANCING, SKIN, BATCH, MORPHING/MORPHING_INT/MORPHING_POSITION/MORPHING_NORMAL, SCREENSPACE) are now exposed to both the vertex and fragment shaders.
  • LitShader: added a sharedDefineSet() helper (mirrors fDefineSet) that writes to both the vertex and fragment define maps, and routed the object-level defines through it.
  • ShaderMaterial generator: extracted the duplicated mesh-define logic into a single addSharedDefines() method, now applied to both the vertex and fragment definitions.
  • PIXELSNAP remains vertex-only (a transform-stage option, not a mesh property).

This is purely additive: these defines are only consumed by built-in vertex chunks today, so no existing shader output changes — the symbols simply become available to user chunk overrides (e.g. #if INSTANCING in diffusePS).

Fixes #8815

Object/mesh-level shader defines (INSTANCING, SKIN, BATCH, MORPHING*,
SCREENSPACE) were only declared in the vertex shader, so user chunk
overrides could not branch on them in the fragment shader.

Expose them to both shader stages via a shared helper, and deduplicate
the mesh-define logic in the ShaderMaterial generator. Purely additive:
built-in shader output is unchanged.

Fixes #8815
Copy link
Copy Markdown
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

This PR fixes a limitation in shader generation where mesh/object-level shader defines (derived from MeshInstance shader defs) were only emitted for the vertex stage, preventing user fragment chunk overrides from branching on them (fixes #8815).

Changes:

  • Exposes mesh/object-level defines to both vertex and fragment stages for LitShader programs.
  • Refactors LitShader define emission via a new sharedDefineSet() helper to keep stage define maps in sync.
  • Refactors ShaderMaterial program generation to centralize mesh/object-level define emission and apply it to both vertex and fragment definitions.

Reviewed changes

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

File Description
src/scene/shader-lib/programs/shader-generator-shader.js Adds a shared define helper so ShaderMaterial-generated programs expose mesh/object defines in both VS and FS.
src/scene/shader-lib/programs/lit-shader.js Adds sharedDefineSet() and routes mesh/object-level defines through it so fragment shaders can see them too.

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

Comment thread src/scene/shader-lib/programs/lit-shader.js
@mvaligursky mvaligursky added the area: graphics Graphics related issue label Jun 1, 2026
@mvaligursky mvaligursky merged commit 1b6009f into main Jun 1, 2026
9 checks passed
@mvaligursky mvaligursky deleted the mv-shared-mesh-defines branch June 1, 2026 10:51
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.

INSTANCING define not available in the pixel shader.

2 participants