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

Add ground atmosphere and day/night shading to Model and 3D Tiles #11765

Closed
wants to merge 19 commits into from

Conversation

ptrgags
Copy link
Contributor

@ptrgags ptrgags commented Jan 16, 2024

Description

⚠️ This PR is built on top of #11744, so make sure that gets merged first.

This PR adds ground atmosphere and day/night shading to the AtmospherePipelineStage for models and 3D Tiles. For world-scale datasets like P3DT, this will make it look a lot nicer!

P3DT without ground atmosphere P3DT with ground atmosphere
image image

Issue number and link

Fixes #11717

Testing plan

Sandcastles for testing:

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have update the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code
  • Other to-dos:
    • Determine why the globe appears black before I toggle dynamic lighting
    • Determine the new condition for adding AtmospherePipelineStage to the model rendering pipeline
    • Figure out better handling for normals (e.g. P3DT doesn't have them!)
    • One of the diffuse terms looks like it's using the wrong light direction (always facing the camera). See if that's related to my normal workaround
    • Add a showGroundAtmosphere setting. Though think about how this will work between Atmosphere and Globe.
    • Try more datasets, e.g. the One World Terrain globe, non-world-scale datasets to see how they look
    • Check through Sandcastle gallery to see if examples look nice or if they need further tweaking.
    • Examine performance of OWT Base Globe

Copy link
Contributor Author

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

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

Some notes

packages/engine/Source/Scene/Model/ModelSceneGraph.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Model/Model.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/FrameState.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Atmosphere.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Atmosphere.js Outdated Show resolved Hide resolved
packages/engine/Source/Renderer/UniformState.js Outdated Show resolved Hide resolved
@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 17, 2024

I tried the OWT base globe, working nicely with styles and custom shaders!

image
image

I am noticing the performance isn't the best for this example, but I'll have to see how much of that is due to the custom shader/styling and how much of that is the added overhead of atmosphere lighting.

Base automatically changed from 11716-visualize-fog to main January 18, 2024 22:03
@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 19, 2024

One of the specs in Model is failing, the render test with fog in sunlight.

I stepped through the code with debugCanvasWidth=400, one weird thing I notice is that the sunny day color seems wrong (I thought I picked a time that would make it orange like a sunset...), it's just a slightly different shade of blue from DynamicAtmosphereLightingType.NONE 🤔. Meanwhile when I use SCENE_LIGHT, then it becomes orange.

I checked the enum values in the uniform map and in the shader, they look correct. I also made sure the atmosphere object was propagated to the frame state. So not sure what's going on yet.

Copy link
Contributor Author

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

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

Some notes

packages/engine/Specs/Scene/Model/ModelSpec.js Outdated Show resolved Hide resolved
Comment on lines +94 to +99
// If the camera is inside the earth, skip rendering the ground atmosphere
vec3 radii = czm_ellipsoidRadii;
float minRadius = min(radii.x, min(radii.y, radii.z));
if (cameraHeight < minRadius) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A couple of specs were failing because a model at the center of the earth was rendering black. I'm guessing some of the lighting math breaks down near the origin.

I wasn't sure if this was the best way to filter out this corner case, but it sounds reasonable since the ground atmosphere is only shown when the camera is far outside the globe.

* @type {Cartesian2}
* @default Cartesian2(1.0e7, 2.0e7)
*/
this.lightingFadeRange = new Cartesian2(1.0e7, 2.0e7);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to @ggetz and @jjhembd: this parameter is a rephrasing of the confusingly-named parameters [globe.lightingFadeOutDistance, globe.lightingFadeInDistance], I figured this way would be easier to explain to users.

* @type {Cartesian2}
* @default Cartesian2(5.0e7, 1.0e7)
*/
this.nightFadeRange = new Cartesian2(1.0e7, 5.0e7);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similarly, this is a rephrasing of [globe.nightFadeOutDistance, globe.nightFadeInDistance]

Comment on lines +111 to +113
const float diffuseMultiplier = 5.0;
const float vertexShadowDarkness = 0.3;
float diffuseIntensity = clamp(lambert * diffuseMultiplier + vertexShadowDarkness, 0.0, 1.0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I wasn't sure how much globe.diffuseMultiplier and globe.vertexShadowDarkness are used, so I only implemented the default settings, not the vertex lighting in GlobeFS.glsl.

Also note that these parameters could also be rephrased as "contrast" and "brightness" (i.e. a scale and shift of the intensity value)

Comment on lines +127 to +137
// Use a dot product (similar to Lambert shading) to create a mask that's positive on the
// daytime side of the globe and 0 on the nighttime side of the globe.
//
// Use this to select the diffuse + ground atmosphere on the daytime side of the globe,
// and just the ground atmosphere (which is much darker) on the nighttime side.
float dayNightMask = clamp(dot(normalWC, atmosphereLightDirectionWC), 0.0, 1.0);
vec3 dayNightColor = mix(groundAtmosphereColor.rgb, diffuseAndGroundAtmosphere, dayNightMask);

// Fade in the day/night color in as the camera height increases towards space.
float nightFade = fade(cameraHeight, czm_atmosphereNightFadeRange, vec2(0.05, 1.0));
finalAtmosphereColor = mix(diffuseAndGroundAtmosphere, dayNightColor, nightFade);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in GlobeFS.glsl, u_nightFadeDistance, I found some of the variable names and usage a bit backwards to my understanding, so I rephrased it and documented the conventions I'm using.

It's mathematically equivalent, I just thought this way would be a little easier to understand.

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 19, 2024

@jjhembd @ggetz oh yikes, it looks like the render fog in sunlight spec fails in main too. Since that's a render test, it doesn't run in CI, so it must have been one of my last commits in #11744 that was incorrect

@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 19, 2024

I fixed it, basically the dark color was exactly black, I thought it was supposed to be slightly lighter than that, so I removed the one expectation.

@ptrgags ptrgags marked this pull request as ready for review January 19, 2024 21:20
@ptrgags
Copy link
Contributor Author

ptrgags commented Jan 19, 2024

@ggetz @jjhembd I only briefly looked at performance of the OWT base globe with the FPS monitor. I would recommend doing more thorough performance testing, but here's my observations so far:

  • with the default view with atmosphere off entirely, the frame rate varies quite a bit from 51-60 FPS. Not sure why that would be for an idling tileset, camera and mouse.
  • Zooming in (again with no atmosphere), the frame rate drops, presumably for rendering many tiles at once.
  • With the default view, turning atmosphere on doesn't have a noticeable impact above the noise
  • Zoomed in however, turning on the ground atmosphere does seem to cause the frame rate to drop a bit more.

@ggetz
Copy link
Contributor

ggetz commented Jan 26, 2024

Thanks @ptrgags for all the work here! We are going to close this for now until we can dedicate someone to finishing up the work the testing an work you suggested here.

@ggetz ggetz closed this Jan 26, 2024
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.

Visualize ground atmosphere for 3D TIles/Models
2 participants