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

Initial Sky Atmosphere #9649

Merged
merged 49 commits into from Aug 11, 2022

Conversation

AMZN-alexpete
Copy link
Contributor

@AMZN-alexpete AMZN-alexpete commented May 19, 2022

Initial integration of open source unreal sky atmosphere based on https://github.com/sebh/UnrealEngineSkyAtmosphere into Atom/feature/common

image
image
image

Features

  1. Realistic atmospheric scattering
  2. Limited shadowing and ground albedo support
  3. Fast sky optimization for when inside atmosphere
  4. Sun orientation using own transform or optional sun entity component property
  5. Primitive sun radius, falloff, and color controls
  6. Multiple atmosphere supported

Implementation

The atmosphere shader samples along a ray for each pixel in a full-screen shader. To speed this up, a transmission LUT and sky view LUT are calculated, so when the full pipeline for an atmosphere is active you have the following passes

  1. Transmittance LUT writes the transmittance (based on optical depth) into a small LUT 256x64 in a compute shader
  2. A 192x108 SkyView LUT is rendered from inside the atmosphere using the Transmittance LUT
  3. The ray marching full screen pass runs it branches based on
    1. if the current pixel contains sky only, sample the SkyView LUT and we're done
    2. if the current pixel is in the atmosphere and hits something, perform a variable number of samples along a ray from the camera to the world space position of the pixel using the depth buffer. If shadowing is enabled we'll check the shadow state of points along the ray.

There are other minor bits like taking ground albedo into account and some features in the original implementation haven't been integrated yet like the multi-scattering and fast-aerial perspective, which add additional LUTs. Also, we may be able to disable updating the SkyView LUT every frame and only update it when other LUTs change or the sun position changes.

Known issues

  1. Fast aerial perspective not yet integrated
  2. Multi-scattering not yet integrated
  3. Gnarly banding and poor performance with high samples
  4. LUT sizes are hard-coded

Testing

  1. PC only in editor and game using various atmosphere and sun configurations
  2. Ran AtomSampleViewer tests and verified they passed except the parallax tests which have not reliably pass on my machine.

Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
…dev/o3de into Atom/sky/initial

Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>

# Conflicts:
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyAtmosphereCommon.azsli
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyRayMarching.azsl
#	Gems/Atom/Feature/Common/Code/Include/Atom/Feature/SkyAtmosphere/SkyAtmosphereFeatureProcessorInterface.h
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmosphereFeatureProcessor.cpp
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmosphereFeatureProcessor.h
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmospherePass.cpp
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmospherePass.h
#	Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/SkyAtmosphere/SkyAtmosphereComponentConfig.h
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/EditorSkyAtmosphereComponent.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/EditorSkyAtmosphereComponent.h
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/SkyAtmosphereComponentConfig.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/SkyAtmosphereComponentController.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/SkyAtmosphereComponentController.h
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
…dev/o3de into Atom/sky/initial

Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>

# Conflicts:
#	Gems/Atom/Feature/Common/Assets/Passes/OpaqueParent.pass
#	Gems/Atom/Feature/Common/Assets/Passes/PassTemplates.azasset
#	Gems/Atom/Feature/Common/Assets/Passes/SkyAtmosphere.pass
#	Gems/Atom/Feature/Common/Assets/Passes/SkyAtmosphereParent.pass
#	Gems/Atom/Feature/Common/Assets/Passes/SkyRayMarching.pass
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyAtmosphereCommon.azsli
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyRayMarching.azsl
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyRayMarching.shader
#	Gems/Atom/Feature/Common/Assets/Shaders/SkyAtmosphere/SkyTransmittanceLUT.azsl
#	Gems/Atom/Feature/Common/Assets/atom_feature_common_asset_files.cmake
#	Gems/Atom/Feature/Common/Code/Include/Atom/Feature/SkyAtmosphere/SkyAtmosphereFeatureProcessorInterface.h
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmosphereFeatureProcessor.cpp
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmosphereFeatureProcessor.h
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmospherePass.cpp
#	Gems/Atom/Feature/Common/Code/Source/SkyAtmosphere/SkyAtmospherePass.h
#	Gems/AtomLyIntegration/CommonFeatures/Code/Include/AtomLyIntegration/CommonFeatures/SkyAtmosphere/SkyAtmosphereComponentConfig.h
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/EditorSkyAtmosphereComponent.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/SkyAtmosphereComponentConfig.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/Source/SkyAtmosphere/SkyAtmosphereComponentController.cpp
#	Gems/AtomLyIntegration/CommonFeatures/Code/atomlyintegration_commonfeatures_public_files.cmake
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
@AMZN-alexpete AMZN-alexpete force-pushed the Atom/sky/initial branch 2 times, most recently from 7091d31 to 661244f Compare May 19, 2022 05:57
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
@@ -342,21 +342,63 @@
}
]
},
{
"Name": "SkyAtmosphereParentPass",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not be in its own Gem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can easily be moved to a gem, but I thought that if it would be eventually replacing the existing physical sky implementation then perhaps it makes sense to put in the same place.


// Based on https://github.com/sebh/UnrealEngineSkyAtmosphere by Sébastien Hillaire

#define ENABLE_ATMOSPHERE_SHADOWS 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this macro required? it is also declared 3 times and probably should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added to avoid shader compilation issues that occurred because all three passes (raymarching, transmittence, skyview) use the same integration function so some needed the o_shadowsEnabled on and some need it off. I can probably think of a way to ensure I only turn that shader variant on for the raymarching shader.

Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Copy link
Contributor

@antonmic antonmic left a comment

Choose a reason for hiding this comment

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

Awesome work! Sorry for the delayed review

@gadams3 gadams3 requested review from a team July 20, 2022 23:28
Copy link
Contributor

@gadams3 gadams3 left a comment

Choose a reason for hiding this comment

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

Unless someone else chimes in with other objections, I'm improving to unblock development of this feature after addressing @antonmic comments.

Personally, even if this is enabled by default with atom, I strongly feel like this should be in a separate gem. It's a large enough change, an isolated feature, and a fantastic example of how to integrate something new into the engine. Keeping it separate will make it easier for others to dissect and identify any obvious gaps that other features will need.

@DoItForGrandpa
Copy link
Contributor

@gadams3 I agree with this should become a gem, mainly because this engine is known for that capability, and thinking of the future, when/if VR support gets added you do have to keep in mind that having the ability to basically turn off features that are not needed to gain performance and space on the finished project is in high demand.
I am all for adding this straight into the engine right now, and at a future date have another PR for switching it over. This would be a good scenario to point people to after the fact to show how you can turn a full feature in O3DE into a gem that I believe injects into the render pipeline.

…tial

Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>

# Conflicts:
#	Gems/Atom/Feature/Common/Assets/Passes/EnvironmentCubeMapPipeline.pass
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
@AMZN-alexpete AMZN-alexpete merged commit c452f34 into o3de:development Aug 11, 2022
@AMZN-alexpete AMZN-alexpete deleted the Atom/sky/initial branch August 11, 2022 17:37
spham-amzn pushed a commit to aws-lumberyard-dev/o3de that referenced this pull request Aug 12, 2022
Signed-off-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/graphics This item is related to the Atom renderer or graphics. sig/graphics-audio Categorizes an issue or PR as relevant to SIG graphics-audio.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants