Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OutputShader: Use #include instead of ShaderChunk. #26990

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

CodyJasonBennett
Copy link
Contributor

Related issue: #XXXX

Description

Minor changes from #26912 which were previously needed for tree-shaking better align with surrounding shader chunk consumption.

@@ -35,7 +31,8 @@ const OutputShader = {

uniform sampler2D tDiffuse;

` + ShaderChunk[ 'tonemapping_pars_fragment' ] + ShaderChunk[ 'colorspace_pars_fragment' ] + `
#include <tonemapping_pars_fragment>
#include <colorspace_pars_fragment>
Copy link
Collaborator

@Mugen87 Mugen87 Oct 16, 2023

Choose a reason for hiding this comment

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

Um, I'm not sure why I have written this differently. I could swear previous versions required the ShaderChunk approach otherwise a GLSL error occurred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably related to automatically included chunks when tonemapping is configured on the renderer.

( parameters.toneMapping !== NoToneMapping ) ? ShaderChunk[ 'tonemapping_pars_fragment' ] : '', // this code is required here because it is used by the toneMapping() function defined below

Copy link
Collaborator

@Mugen87 Mugen87 Oct 16, 2023

Choose a reason for hiding this comment

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

Ah, maybe the runtime error occurred because I have used ShaderMaterial at the beginning. I have later switched to RawShaderMaterial and thought includes could not be used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Anyway, let's give this a try. It's indeed the cleaner solution.

@Mugen87 Mugen87 added this to the r158 milestone Oct 16, 2023
@Mugen87 Mugen87 merged commit 5f2e835 into mrdoob:dev Oct 16, 2023
18 checks passed
@Mugen87 Mugen87 changed the title OutputShader: Cleanup. OutputShader: Use #include instead of ShaderChunk. Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants