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

Cull shadow commands #3819

Merged
merged 54 commits into from
Apr 15, 2016
Merged

Cull shadow commands #3819

merged 54 commits into from
Apr 15, 2016

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Apr 7, 2016

For #2594

  • Updated executeShadowMapCommands to cull commands based on the shadow volume. This will allow for rendering casters that may not be visible to the scene camera. Also culls against individual cascades.
  • Reorganized the debug drawing
  • Added a grid of objects to Shadows.html

To-do:

  • Support terrain culling using Globe.update. Culling based on an orthographic camera seems to have problems.
  • Initially extend the shadow volume backwards to "infinity", find all casters, then clamp the shadow volume to the furthest caster.
  • Don't submit commands if the shadow volume is not visible. Use frustum/frustum and frustum/sphere intersection tests. For cascaded shadows, check that the light source is not occluded by the globe.

@pjcozzi
Copy link
Contributor

pjcozzi commented Apr 7, 2016

@lilleyse is the TODO for this PR? If so, use a Markdown tasklist.

for (var i = 0; i < length; ++i) {
var command = commandList[i];
// Don't insert globe commands with the rest of the scene commands since they are handled separately
if (command.castShadows && (insertAll || (command.pass === Pass.OPAQUE || command.pass === Pass.TRANSLUCENT))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but long-term, we can evaluate avoiding linear searches like this, e.g., "is opaque or translucent", by using separate lists.

@pjcozzi
Copy link
Contributor

pjcozzi commented Apr 11, 2016

Just those comments so far for this PR.

@lilleyse
Copy link
Contributor Author

The most recent code checks if the shadow map is visible or not. For non-cascaded shadow maps, it checks the shadow volume's bounding sphere against the scene camera's frustum. Cascaded shadows go out of view when the camera is camera is zoomed too far from the Earth or when the light is angled below the horizon.

At the same time the code checks if a shadow map needs to update.

@lilleyse
Copy link
Contributor Author

I started bullet two: Initially extend the shadow volume backwards to "infinity", find all casters, then clamp the shadow volume to the furthest caster. but decided to hold off for now. I now extend the shadow volume back by 1000 meters to capture any shadow casters. This seems good enough and doesn't reduce shadow quality.

@pjcozzi
Copy link
Contributor

pjcozzi commented Apr 12, 2016

I started bullet two: Initially extend the shadow volume backwards to "infinity", find all casters, then clamp the shadow volume to the furthest caster. but decided to hold off for now. I now extend the shadow volume back by 1000 meters to capture any shadow casters. This seems good enough and doesn't reduce shadow quality.

I am all for the simple solution and adding the more involved technique to the roadmap. We may need to reevaluate with 3D Tiles buildings, but I think that should be an easier case than terrain.

@@ -1241,7 +1241,7 @@ define([
// When moving the camera low LOD terrain tiles begin to load, whose bounding volumes
// throw off the near/far fitting for the shadow map. Only update shadowNear and shadowFar
// for reasonably sized bounding volumes.
if (!(distances.start < 0.0 && distances.stop > frameState.shadowFar)) {
if (!((distances.start < -100.0) && (distances.stop > 100.0) && (pass === Pass.GLOBE))){
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to add a TODO that 3D Tiles are not part of Pass.GLOBE. However, we can introduce Pass.ENVIRONMENT, etc., but will need to think through opaque vs. translucent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the TODO. I'll have to try it out, but it may not actually be a problem.

@pjcozzi
Copy link
Contributor

pjcozzi commented Apr 12, 2016

Just those comments on the new commits.

@pjcozzi pjcozzi mentioned this pull request Apr 12, 2016
44 tasks
@bagnell
Copy link
Contributor

bagnell commented Apr 15, 2016

Merging this. The TODO list will be added/fixed in separate PRs.

@bagnell bagnell merged commit ebc10e8 into shadows Apr 15, 2016
@bagnell bagnell deleted the shadows-commands branch April 15, 2016 18:22
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