fix(rendering): add SunLightLayer only if explicitly enabled in options#2785
Open
Neptilo wants to merge 2 commits into
Open
fix(rendering): add SunLightLayer only if explicitly enabled in options#2785Neptilo wants to merge 2 commits into
Neptilo wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add an option in
GlobeViewto decide whether to add aSunLightLayer.Set it to false by default to recreate the default behavior in cases where a sunlight is not needed.
Motivation and Context
The unconditional creation of a Sunlight layer in #2691 modified the default lighting of the scenes by creating a directional sunlight. As a result, the globe views always had a bright side and a dark side, instead of being uniformly lit.
For example, the default placement of potree_3d_map example was completely in the dark.
Technical decisions
To avoid blocking the upcoming release, provide a minimal viable solution, i.e. the additional option creates and adds a
SunLightLayeronly during initialization. If it is not created during initialization, it therefore cannot be enabled afterward.An alternative is to always create it but make it “invisible” immediately afterward if the configuration option is not enabled. In that case, it could be enabled/disabled afterward, but the downside is that a
SunLightLayerwould always be created, even in examples where it is not needed.A long-term solution could of course combine both advantages (creation only upon first activation + ability to modify it afterward), but I do not consider that a minimal solution, and it requires discussions to decide on architectural specifications.