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.

Sign up for GitHub

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

Jump to bottom

WebGLRenderer: Traverse only visible lights in compile() #20450

Merged
merged 1 commit into from
Oct 5, 2020
Merged

WebGLRenderer: Traverse only visible lights in compile() #20450

merged 1 commit into from
Oct 5, 2020

Conversation

Oletus
Copy link
Contributor

@Oletus Oletus commented Oct 2, 2020

This way the set of lights matches with the ones used during rendering, making sure that the compiled programs have the right parameters for rendering.

@Oletus
WebGLRenderer: Traverse only visible lights in compile()
ae115fe 
This way the set of lights matches with the ones used during rendering, making sure that the compiled programs have the right parameters for rendering.
Mugen87
Mugen87 approved these changes Oct 2, 2020
View reviewed changes
Copy link
Collaborator

@Mugen87 Mugen87 left a comment •
edited

Choose a reason for hiding this comment

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

Makes sense. With this change, compile() produces the same light render state like projectObject() does.

@Mugen87 Mugen87 added this to the r122 milestone Oct 2, 2020
@mrdoob mrdoob merged commit 20497fc into mrdoob:dev Oct 5, 2020
@mrdoob
Copy link
Owner

mrdoob commented Oct 5, 2020

Thanks!

@Oletus
Copy link
Contributor Author

Oletus commented Oct 5, 2020

Thanks for the review!

@Oletus Oletus deleted the fix-compile-masking branch October 5, 2020 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mugen87 Mugen87 Mugen87 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r122
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Oletus @mrdoob @Mugen87