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

Disable shadows on the "simple" headlight #2373

Conversation

marktucker
Copy link
Contributor

When using a "simple" headlight, turn off shadows generated by this light.

Because it is a distant light, and not an actual "headlight" co-located with the camera, interior scenes will be entirely in shadow. Submitting this on behalf of @crydalch spawned from this discussion on usd-interest: https://groups.google.com/g/usd-interest/c/ShnbqGtpi_0

Description of Change(s)

Changed the code in the TaskController that translates a GlfSimpleLight into explicit values stored in the task controller's custom scene delegate so that simple dome lights have shadows enabled, but simple distant light have shadows disabled.

  • [ X ] I have verified that all unit tests pass with the proposed changes
  • [ X ] I have submitted a signed Contributor License Agreement

…ight.

Because it is a distant light, and not an actual "headlight" co-located
with the camera, interior scenes will be entirely in shadow. Submitting
this on behalf of @crydalch spawned from this discussion on usd-interest:
https://groups.google.com/g/usd-interest/c/ShnbqGtpi_0
@marktucker
Copy link
Contributor Author

We gave some consideration to extending GlfSimpleLight to have a flag indicating whether shadows should be turned on or off (for either dome or headlight mode), but opted against it since we felt this simpler change would satisfy most use cases without adding user/UI complexity.

@sunyab
Copy link
Contributor

sunyab commented Apr 5, 2023

Filed as internal issue #USD-8198

@marktucker
Copy link
Contributor Author

Should this PR now be closed? There is some confusion in the commit referenced above (28cefab), where the commit contents are from this PR, but the comment is from a different PR. But if the change here has been merged (and it looks like it has), this should be closed?

@spiffmon
Copy link
Member

Yes, thanks for adding the commit, for posterity, @marktucker

@spiffmon spiffmon closed this Aug 21, 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.

3 participants