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

Hack fix: Gas giant atmosphere workaround+increase 0 K workaround temp+set atmosphere density default values, and avoid unphysical default temps #1788

Closed

Conversation

Ae-2222
Copy link
Contributor

@Ae-2222 Ae-2222 commented Dec 6, 2012

Before I forget.. Quick gas giant workaround.

Since the gas giant surface represents the actual atmosphere, adding an atmosphere on top just hides the details.

The gas giant scale heights are fixed in the WiP PR-ed part of the atmosphere branch, but the complete atmosphere submission will not be ready very soon.

This is a temporary solution.

Very thin white atmosphere to avoid obscuring surface. Visible near the surface.


The scale height and gas giant colour alpha can be tweaked to increase atmosphere size and density, if anyone wants to tweak further.

…htly. Increase gas giant temperatures in the 0 temperature work around.
… temperature to 1 K (avoids unphysical temperatures).
@shadmar
Copy link
Contributor

shadmar commented Dec 7, 2012

Yey this is way better than the non existing atmosphere that is current.

But I'd like to think gas giant should have large "atmospheres" or gases on top you'd fly into.
You can tweak so you get thicker atmosphere without obscuring the planet surface so you'd still have a visible planet. (from what I've tested)

SystemBody::AtmosphereParameters SystemBody::CalcAtmosphereParams() const

if (type==SystemBody::TYPE_PLANET_GAS_GIANT) atmosScaleHeight *= 10.0f;

And in shader (just up the 2.0) to get a more visible surface normal to the eye (this would have to be conditional for gas giants in the shader first) via preprosessor directives

fogFactor = clamp(2.0 / exp(ldprod),0.0,1.0);  //saturate to avoid negative fog.

But then again a method of identifying gas giants in the shader would be needed, wich might be too much for a quickfix like this since there already other work on atmosphere.

@Ae-2222
Copy link
Contributor Author

Ae-2222 commented Dec 7, 2012

And in shader (just up the 2.0) to get a more visible surface normal to the eye (this would have to be conditional for gas giants in the shader first) via preprosessor directives

That would work:)..That effectively gives zero atmosphere thickness until the length density product gets large (how many scattering particles are in the way).
Probably something like this, or perhaps involving the camera-vertex position and the camera-planet center ray will need to be in the replacement scattering thing as well.

The sky shader will probably need a #define sent to it to be consistent, as well. (Eventually I'll add atmosphere effect selection to the atmosphere class in the same way terrain classes can have surface effects, for now just manually checking sbody->type is sufficient).

It's probably overkill for this quick fix:) You can always make a PR based on this branch, and it can be merged when ever it's submitted, if you feel like:)

@johnbartholomew
Copy link
Contributor

Cross-ref: #1395

@johnbartholomew
Copy link
Contributor

I've taken a look at the effect of this, and @shadmar's suggested changes (links to test screenshots are below).

Without shader modifications, there is a trade-off between making the atmosphere visible when close to the planet (which we want), and adding an ugly uniform fog effect when further away (which we don't want).

With the shader modification that @shadmar has suggested the trade-off is easier because the uniform fog effect can be reduced a lot, but it means an extra shader path/variant, and it's really just another hack.

I think that in the longer term the right approach is a much more drastic shader modification for gas giants, and probably modifications to other code in the terrain path, to take into account the fact that the "terrain" geometry is really atmosphere, and the atmosphere's colour should come from that somehow rather than having a uniform white fog on top of a hard shell.

Test screenshots (each of these is an album with three images):

@robn
Copy link
Member

robn commented Apr 2, 2013

I personally prefer screenshot 4 to 3, because the slight white halo around the planet in 4 looks out of place. That's not a strong opinion though.

I don't see any appreciable difference between 1 and 2.

I think adding another shader/material type is a hack (I never liked that arrangement) but at least its damage contained in a known place. I wouldn't be too concerned about it at this point. Its still only really one cleanup, not two.

I agree with @johnbartholomew's long-term approach is right. Better connection between terrain and atmosphere, including the possibly of no terrain for a true gas planet.

@shadmar
Copy link
Contributor

shadmar commented Apr 2, 2013

If we get camera current density information to the shader you can make gas giants look nice and clean from space but thick and foggy when you are entering the atmosphere. That would also solve #1906.
However since this would only apply to gas giants, you'd probably need to set a preprocessor directive #define GAS_GIANT.

If someone knows how to do this suggestion from @robn : #1906 (comment) properly, I think I can do the rest.

@johnbartholomew
Copy link
Contributor

Rebased and merged.

I've decided to merge this as-is, on the basis that I probably won't have time before freeze to hook up a shader variant properly, and a small improvement now does not preclude a larger improvement later on.

@shadmar You're welcome to have a go at hooking up a gas-giant specific shader, or a GAS_GIANT define for the normal geosphere shaders. If you search for EFFECT_GEOSPHERE_TERRAIN_WITH_LAVA you'll find various places that can be extended to add a gas giant variant. It's a bit of a hack: we'd like to find a better way of managing this stuff. As for passing atmosphere density at the camera to the shader, I'm sure we can find a way, but without looking into the render path in more detail I don't know what code it'll touch.

@shadmar
Copy link
Contributor

shadmar commented Apr 3, 2013

Yeah no objection at all :)
Long overdue getting some atmosphere on the thise gaints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants