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

Shadows use scene near/far #3691

Closed
wants to merge 5 commits into from
Closed

Shadows use scene near/far #3691

wants to merge 5 commits into from

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Mar 9, 2016

For #2594

  • Added a few more demo options
    • terrainCast - toggle terrain casting shadows (off by default now)
    • fitNearFar - use the near and far computed in createPotentiallyVisibleSet to limit the shadow map bounds. This helps a lot with views that face the ground, and helps a little with horizon views because it uses a tighter near plane. There is still the problem where moving the camera causes large terrain tiles to load, which throws off the near and far computed from the draw commands. I've tried a few different approaches to recognize when this happens, but none are ideal yet.
  • Added DrawCommand.receiveShadows. I removed the code that was using this, so nothing is using this right now, but it might be useful to keep. Since DrawCommand.castShadows exists, it doesn't stand out.
  • Only draw debug frustum outlines if in freeze-frame mode, or the shadow map is fixed. Removes some visual clutter this way.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 9, 2016

Added DrawCommand.receiveShadows. I removed the code that was using this, so nothing is using this right now, but it might be useful to keep.

Perhaps put a TODO with it. We'll keep it when shadows is merged to master if we are using it or it proves useful for debugging.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 9, 2016

terrainCast - toggle terrain casting shadows (off by default now)

I'm sure it is outside the scope of this PR, but there are a lot of artifacts for terrain. For a good place to test, use the gecoder to zoom to "Seneca rocks"

Also note that the skirts still cause artifacts:

image

We could turn off skirts for the depth pass. It would also be an interesting optimization in general to render skirts after the surface geometry to take advantage of early z. Likewise, we could also not draw skirts if we know all the neighbor tiles are the same LOD so it is watertight. Food for thought, we don't need to do all of this for shadows.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 9, 2016

There is still the problem where moving the camera causes large terrain tiles to load, which throws off the near and far computed from the draw commands. I've tried a few different approaches to recognize when this happens, but none are ideal yet.

Maybe just use the BV of the tiles that are expected to be loaded, not the low LOD parent tiles. Perhaps try to hack this in first to see how well it works then we can figure out an elegant approach since 3D Tiles would also need changes.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 9, 2016

Just those comments.

@lilleyse lilleyse mentioned this pull request Mar 15, 2016
@lilleyse
Copy link
Contributor Author

Updated. The new approach for finding the shadow near/far is working better.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 15, 2016

Have you tried this on Mac? Here's the default view (note the black shadow map) on my new AMD MacBook Pro:

image

@lilleyse
Copy link
Contributor Author

I've only been testing on Windows, I'll try this out on OSX tomorrow.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 15, 2016

Also, it's not specific to this branch. Here is the shadows branch:
ok

@lilleyse
Copy link
Contributor Author

I'm getting a framebuffer incomplete error in Chrome. When I fallback to the color texture, it works fine. Firefox is working, so maybe this is just a Chrome issue.

@lilleyse
Copy link
Contributor Author

Added a fix to #3711.

@lilleyse
Copy link
Contributor Author

Cherry-picked into #3711

@lilleyse lilleyse closed this Mar 17, 2016
@lilleyse lilleyse deleted the shadows-nearfar branch March 17, 2016 18:37
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.

2 participants