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

shadertools: Support mulitple lights in lighting module #2166

Merged
merged 10 commits into from
Aug 8, 2024

Conversation

felixpalmer
Copy link
Collaborator

For visgl/deck.gl#9074

Screen.Recording.2024-08-07.at.16.08.39.mov

Change List

  • Add uniforms to UBO definition to support multiple lights
  • Update getUniforms
  • lighting_getPointLight/lighting_getDirectionalLight picks correct light by index
  • Remove unused lighting_getSpecularLightColor
  • Add light attenuation support
  • Re-use phong shaders in gouraudMaterial
  • Update test app to support gouraudMaterial & phongMaterial

@felixpalmer felixpalmer requested a review from ibgreen August 7, 2024 14:18
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@@ -36,6 +37,10 @@ void main(void) {
vPosition = (app.modelMatrix * vec4(positions, 1.0)).xyz;
vNormal = mat3(app.modelMatrix) * normals;
vUV = texCoords;

#if (defined(LIGHTING_VERTEX))
vColor = lighting_getLightColor(vec3(1.0), app.eyePosition, vPosition, normalize(vNormal));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thought: Maybe we could name the vertex and fragment calculation functions differently and export a trivial stub in one of the modules, so that we can avoid #ifdefs which we don't have naturally in WGSL.

This will come into focus on master where we also need to deal with the WGSL implementation.

vs: GOURAUD_VS,
fs: GOURAUD_FS,
vs: PHONG_FS.replace('phongMaterial', 'gouraudMaterial'),
fs: PHONG_VS.replace('phongMaterial', 'gouraudMaterial'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm... Seems no way to do this that is completely palatable...

Maybe just a single module phongGouraudMaterial with a switch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Given that Gouraud shading is still based on the Phong model, why don't we remove the Gouraud material altogether and provide lighting_getLightColor et al in vs and fs. Then the application code can choose whether to perform lighting at the vertex stage (Gouraud) or fragment (Phong). The UBO will always be called phongMaterial and we won't have to worry about which shader module we are using

@felixpalmer felixpalmer merged commit b787cf3 into 9.0-release Aug 8, 2024
1 check passed
@felixpalmer felixpalmer deleted the felix/multiple-lights branch August 8, 2024 09:15
xinaesthete pushed a commit to xinaesthete/luma.gl that referenced this pull request Aug 11, 2024
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