Atmosphere as entity with transforms#23651
Conversation
|
Thanks! Eagerly waiting for this 😄 |
ecoskey
left a comment
There was a problem hiding this comment.
Ooh nice! Mostly just have some nitpicks and some comments for future work.
Co-authored-by: Emerson Coskey <emerson@coskey.dev>
|
The generated |
There was a problem hiding this comment.
Looks good! Agree that we shouldn't hold this up too long on renaming or other refactors. Approving 🙂
- I do still think we can save three matmuls in
ndc_to_camera_distif the planet's scale is uniform (which it should always be imo) and we keep track of the scale separate from the matrix. - I'm fine with keeping
AtmosphereSettingsas the "driver" component for now, but will note that we moved away from this style in several other effects. i.e.BloomSettingswas renamed toBloomiirc
| #[derive(Clone, Component)] | ||
| #[require(Hdr)] | ||
| #[require(GlobalTransform::default())] | ||
| #[component(on_add = set_default_transform)] |
There was a problem hiding this comment.
It would be nice if there was some way to opt out of setting the Transform for users of non-default transform systems. I don't know if there is a nice solution that doesn't add more complexity.
The only thing I can think of is making the Transform and GlobalTransform optional (not required components), and placing the atmosphere at -6000km in Y if they are not present. That way you have to opt in to manual atmosphere positioning and can use your own components?
There was a problem hiding this comment.
I need to better understand the use case, maybe I should test this change with big_space. is that a good example of a non-default transform system? also, would just changing the lines in set_default_transform to GlobalTransform work for decoupling it?
There was a problem hiding this comment.
I need to better understand the use case, maybe I should test this change with big_space. is that a good example of a non-default transform system?
Not really, it integrates with Transform. It's more about only depending on GlobalTransform instead of Transform everywhere.
would just changing the lines in set_default_transform to GlobalTransform work for decoupling it?
No, because it would just get overwritten by whatever transform system is present, including bevy's. That's why I was suggesting making the atmosphere positioning entirely opt-in: you get good default behavior for bevy out of the box, and don't have any dependency on Transform for arbitrary downstream use cases - the code only reads GlobalTransform.
Objective
Anecdotal feedback:
Solution
scene_units_to_mremoved in favor of usingTransformAtmosphereBuffer/AtmosphereDatastructs to just use the plain extractedGpuAtmospherestruct. this reduces the complexity of the struct in the mesh view bindings. Since atmosphere settings is coupled with the rendering pipeline of the atmosphere this makes sense architecturally.Why not multi atmosphere:
The atmosphere uses multiple LUTs (lookup textures) to accelerate the rendering performance. Some of them are not view dependent:
These can be coupled and rendered for each atmosphere individually. However the remainder of the pipeline is view dependent:
In raymarched rendering mode, these LUTs can be skipped and only the render sky pass runs sampling on all of the atmospheres with a raymarch in screen space.
Further, the Sky View LUT uses a local reference frame to concentrate texel density along the horizon's local up axis. This in turn means it's coupled with both a specific atmosphere's local coordinates as well as the view's transform matrix. We cannot consider rendering both atmospheres into a single LUT for this reason. So it has to be unique for each pair of (view, atmosphere). Given two views and two atmospheres we would need 4 of these Sky View LUTs and at some point, raymarched rendering will become the less expensive option.
Lastly the Render Sky pass needs to happen once per view, we cannot realistically composite them in sequence with simple dual-source blending as we do with the scene, this would result in incorrect scattering integration. This in turn means we need to bind ALL of the luts calculated previously so a single render sky pass and render aerial view lut - perhaps making use of array textures. Rely on unified volumetric ingegration in the raymarching loop: for each light,for each atmosphere, attenuate inscattering and transmittance along the path integral. It is suffice to say this change is overall too complex for the time being and is likely the reason Unreal Engine also do not support multiple atmospheres. For context: our research is based heavily on Sebastian Hillarie's work, one of the Unreal graphics engineers.
That being said about multiple atmospheres - I am thinking of this PR as a segway into unified volumetrics in Bevy. that is: Render the FogVolume and Atmosphere in a single pass! Making use of the frustum aligned voxel grid "froxel" approach to accelerate the rendering. This would drastically increase the performance for scenes wanting to make use of both the atmosphere and local fog volumes.
Testing
examples/3d/atmosphere.rsexample.Showcase
(example screenshot unchanged compared to main.)