-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 for Primitive entities #3928
Conversation
Given the size of this, are we sure it is worth it since it is likely to not be a widely used feature? Would it be better, to instead, expose very coarse-grained on/off, and keep this parked on this branch until/if it is needed? Can we push CZML updates to a roadmap? |
We could just have hard-coded behavior that models have shadows but nothing else does. I expect that to be the case for 99% of users. We could then have ModelVisualizer just key off of the scene-level shadows property and not even bother with per-model control. If we go that route, I would still leave this branch around until we get feedback |
That would be fine with me. That is the default, I believe.
Agreed. |
Per-model shadows are already in the I'll make a separate PR for the |
Here is a link to the model commit: 5739855 |
All sounds good. |
What is the plan with this pull request? Close it, and link to the branch in the roadmap? |
The Entity and Shadows roadmaps link to this PR now. |
Referenced here: https://groups.google.com/forum/#!topic/cesium-dev/zVdSoscjYqw |
@lilleyse given the forum feedback, do you think we should ship this in 1.23? Or is there a simpler approach with less fine-grained control that we can consider? |
I think the approach here is ok, is there an alternative that still lets you change the shadows per-entity? |
Probably not anything reasonable. Does this PR need additional review? |
I need to make a few changes to support the new types in #3989, I'll update when ready. |
This could also be simplified so that instead of |
I think that's a reasonable compromise. I personally can't think of a real-world use case where you would want one without the other. |
For #2594
I added
castShadows
andreceiveShadows
properties to all the applicable entity types. It supports changing these properties at runtime. Batching based on shadows is handled inGeometryVisualizer
. Each batch, besides the dynamic batch, is now an array of 4 batches (cast+receive, cast, receive, none). This was the cleanest solution for now, but could get unwieldy if we continue to add more bucket types. I tried and gave up on a unified solution for batching so that we wouldn't needStaticOutlineGeometryBatch
,StaticGeometryColorBatch
, and inStaticGeometryPerMaterialBatch
. Maybe this is worth revisiting in a future rewrite.I also added the property
Viewer.terrainShadows
which controls whether the terrain casts shadows. I considered having two properties for cast and receive, but in most cases the user won't be playing around with receive. It seems clearer this way.