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

ShadowMap fade option #9565

Merged
merged 13 commits into from
Jun 1, 2021
Merged

ShadowMap fade option #9565

merged 13 commits into from
Jun 1, 2021

Conversation

kalosma
Copy link
Contributor

@kalosma kalosma commented May 24, 2021

Fixes #9564

Added an option to shadowMap (shadowMap.fadingEnabled) for enabling/disabling the shadowMap fading when the light source is close to the horizon.
Having the fading disabled is desirable for cases, like the Moon, where there is no atmosphere and the shadows should be solid even if the light source is close to the horizon.

here is a sandcastle example with fading enabled (current behaviour, advance the time in timeline till the shadows fade)

fading enabled (default)

shadow_map_darkness

here is a sandcastle example with the new option to disable the shadows fading

fading disabled (through new option)

shadow_map_darkness_no_fading

@cesium-concierge
Copy link

Thanks for the pull request @kalosma!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@kalosma kalosma changed the title Shadowmap fade opt ShadowMap fade option May 24, 2021
@kalosma kalosma closed this May 24, 2021
@kalosma kalosma reopened this May 24, 2021
@lilleyse lilleyse requested a review from ebogo1 May 25, 2021 12:24
Copy link
Contributor

@ebogo1 ebogo1 left a comment

Choose a reason for hiding this comment

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

Thanks @kalosma! I confirmed these changes work in Sandcastle. A few things -

CHANGES.md Outdated Show resolved Hide resolved
Source/Scene/ShadowMap.js Outdated Show resolved Hide resolved
Specs/Scene/ShadowMapSpec.js Outdated Show resolved Hide resolved
@kalosma
Copy link
Contributor Author

kalosma commented May 27, 2021

Had some problems with the unit test, but it seems it passed now

Copy link
Contributor

@ebogo1 ebogo1 left a comment

Choose a reason for hiding this comment

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

Thanks, this PR looks just about ready. Just a minor change in wording for the test case and a comment I left earlier that you might've missed (I unresolved it). Should be good to go after those changes.

Specs/Scene/ShadowMapSpec.js Outdated Show resolved Hide resolved
kalosma and others added 2 commits May 28, 2021 09:13
CHANGES.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ebogo1 ebogo1 left a comment

Choose a reason for hiding this comment

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

Thanks @kalosma. I pushed a couple changes to CHANGES.md but this is good to go 👍

@ebogo1 ebogo1 merged commit 2711313 into CesiumGS:master Jun 1, 2021
j9liu pushed a commit that referenced this pull request Jun 2, 2021
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.

Disable shadowMap fading
3 participants