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

separates light entities shaders from light maps shaders #13277

Merged
merged 11 commits into from Feb 16, 2018

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Feb 8, 2018

It is a continuation of #13149

This PR is just a division of the lights shaders in multiples files not affecting in anything the rendering since it is including.
The main cause is to be able to use native properties names like .envMap together with lights shaders and modify values of irradiance, radiance and clearCoatRadiance not need duplicate calculations.

The core modification its about:


lights_pars is separate to:

  • lights_pars -> light entities shaders
  • lights_maps_pars -> light maps (USE_ENVMAP)

lights_template is separate to:

  • begin_lights_fragment -> light entities begin
  • lights_maps_fragment -> light maps (USE_LIGHTMAP, USE_ENVMAP)
  • end_lights_fragment -> light entities end

If add the ShaderChunk in sequence we will have the same result from the previous code, for example:

legacy.lights_pars = lights_pars + lights_maps_pars;
legacy.lights_template = begin_lights_fragment + lights_maps_fragment + end_lights_fragment;

/cc @WestLangley @bhouston

@WestLangley
Copy link
Collaborator

OK. I would suggest you not reuse the lights_pars.glsl filename. Remove it. I am concerned this will cause problems for users who are using existing shader chunks in their code.

I suggest the following name changes:

legacy.lights_pars = begin_lights_pars + maps_lights_pars;
legacy.lights_template = begin_lights_fragment + maps_light_fragment + end_lights_fragment;

@sunag
Copy link
Collaborator Author

sunag commented Feb 9, 2018

@WestLangley Ok! Done.

@mrdoob mrdoob added this to the r90 milestone Feb 10, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 10, 2018

I think lights_fragment_begin, lights_fragment_maps and lights_fragment_end reads better.

@mrdoob mrdoob modified the milestones: r90, r91 Feb 10, 2018
@WestLangley
Copy link
Collaborator

I think lights_fragment_begin, lights_fragment_maps and end_lights_fragment_end reads better.

OK.

Then, for consistency, lights_pars_begin and lights_pars_maps ?

@mrdoob
Copy link
Owner

mrdoob commented Feb 10, 2018

Yep!

@sunag
Copy link
Collaborator Author

sunag commented Feb 10, 2018

Renamed!

@@ -87,7 +87,7 @@ THREE.ShaderTerrain = {

THREE.ShaderChunk[ "common" ],
THREE.ShaderChunk[ "bsdfs" ],
THREE.ShaderChunk[ "lights_pars" ],
THREE.ShaderChunk[ "begin_lights_pars" ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

lights_pars_begin

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed 👍

@mrdoob
Copy link
Owner

mrdoob commented Feb 16, 2018

I think you forgot to remove lights_template.glsl?

@sunag
Copy link
Collaborator Author

sunag commented Feb 16, 2018

Removed!

@mrdoob mrdoob merged commit f0936b0 into mrdoob:dev Feb 16, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 16, 2018

Thanks!

@WestLangley
Copy link
Collaborator

Reminder to note in migration:

lights_pars.glsl -> lights_pars_begin.glsl + lights_pars_maps.glsl
lights_template.glsl -> lights_fragment_begin.glsl + lights_fragment_maps.glsl + lights_fragment_end.glsl

WestLangley added a commit to WestLangley/three.js that referenced this pull request Feb 16, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 16, 2018

Reminder to note in migration:

lights_pars.glsl -> lights_pars_begin.glsl + lights_pars_maps.glsl
lights_template.glsl -> lights_fragment_begin.glsl + lights_fragment_maps.glsl + lights_fragment_end.glsl

Done. Thanks!

mrdoob added a commit that referenced this pull request Feb 17, 2018
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.

None yet

3 participants