-
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
Fog #3154
Conversation
bagnell
commented
Oct 30, 2015
- Tweak fade distance and density
- Change tile SSE when in fog
- Use preprocessor instead of conditional when enabled.
- Swap shader when a tile is or is not in fog
- Performance
- Tests
- Reference doc including advice on the performance / visual quality trade-off
- Update CHANGES.md
viewModel.endHeight = viewer.scene.fogEndHeight; | ||
|
||
/* | ||
Sandcastle.addToolbarMenu([{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the same views we used for OBBs: http://cesiumjs.org/2015/06/24/Oriented-Bounding-Boxes/
Instead of commenting them out, they could be in a drop down for future testing.
Added a few items to the tasklist
|
var surfaceTile = tile.data; | ||
if (frameState.fogEnabled) { | ||
var distance = this.computeDistanceToTile(tile, frameState); | ||
var scalar = distance * frameState.fogDensity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move the fog equation to a @private
helper function. Keeping it in this file is probably fine for now.
How hard is it to use the ground atmosphere if the ray hits the ellipsoid, but the sky atmosphere if the ray doesn't? |
What are your stats for the default view in the fog Sandcastle example. With ~3K resolution: No fog:
Fog:
Not great. |
For the night ground atmosphere, we might want to use the negative sun vector or something. For example: No fog: Fog: Note that for this view, fog is pretty useful. No fog:
Fog:
|
Overall, I am not impressed with the number of tile reduced. I expected a lot more. Perhaps we can increase the start of the step function and decrease the completely-in-fog distance. I also imagine the VMSSE-style SSE based on fog will be somewhat effective; however, it's effectiveness will depend on how many tiles are partially in fog. Above, I am suggesting to put more tiles completely in fog, but fewer partially in fog... |
…e atmosphere color, always align the position and sun vectors.
When this is done, it would also be interesting to see numbers for when As an aside, is the minimum value for |
Any value You can also think of it as a range. If your |
I'll write up an issue to clarify the doc. We might also want to consider adding a get/set property to enforce non-zero values. |
Default density and SSE factor seem sensible. Increasing the SSE factor can result in significant savings (like another 33%), but, of course, hurts visual quality and makes popping more obvious when the camera moves. |
@bagnell can you do a quick sanity check for when the camera is really low or has a negative altitude? For a horizon view in the dead see, without fog:
With fog:
That seems too aggressive. |
Wow, check out these stats for a horizon view near the ground in FL: No fog:
Fog:
No difference in visual quality. |
Also follow the ISS or GeoEye satellite in the CZML Sandcastle example to see this a bit. |
@pjcozzi I addressed all of your comments. This is ready for another look. |
2D is fixed. Looks like fog is also disabled in Columbus view. Does it need to be? It appeared to work when I tested it last. |
Since we removed the We could just leave this or not draw the atmosphere until the globe renders something. I can't come up with a reason that drawing the atmosphere without the globe is useful. |
Also, for the blog post, can you include FPS (if you can get useful data) in addition to the number of tiles? |
Just those comments and this is ready! |
@pjcozzi This is ready. |
|
Tests look good now.
Good idea.
Also, a good idea.
This is OK with me. The less aggressive we make it, the less performance will improve. This was already tweaked quite a bit, and is a reasonable default. We can change it later if needed. Let me know when this is ready. |
@pjcozzi This is ready. |