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

Cast a reddish tint over the landscape when sun is low. #3858

Merged
merged 12 commits into from
Sep 8, 2024
Merged

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Aug 20, 2024

Description

This may add a yellowish-orange tint to the landscape when the sun is low.
When sun is -3...0° below horizon, tint is smoothly transitioning.
Also when atmosphere is off, there is no tint.

Fixes #3855 (issue)

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Win11
  • Graphics Card: (irrelevant)

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Unfinished: may need better balancing
@gzotti gzotti added feature Entirely new feature importance: low Small problem, rarely visible, no crash labels Aug 20, 2024
@gzotti gzotti added this to the 24.3 milestone Aug 20, 2024
@gzotti gzotti self-assigned this Aug 20, 2024
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@alex-w
Copy link
Member

alex-w commented Aug 21, 2024

The solution is not complete yet...

stellarium-018

@Atque
Copy link
Contributor

Atque commented Aug 21, 2024

I can't currently test; how does it work with an eclipsed Sun? I guess we wouldn't want a red-tinted landscape even with a low eclipsed Sun.

@10110111
Copy link
Contributor

The coloration should be a mixture of the illuminance by the sky and by the Sun. This would make the tint change with the solar altitude non-monotonically. Moreover, the amount of reddening would ideally also depend on the angle between the Sun direction and the normal to the surface being lit.

@gzotti
Copy link
Member Author

gzotti commented Aug 21, 2024

The coloration should be a mixture of the illuminance by the sky and by the Sun. This would make the tint change with the solar altitude non-monotonically.

Yes, this missing sky component is why I feel the tint is too strong. A cheap solution can be some sqrt of the color factor which pulls it closer to 1/1/1. This is still a cheap solution, no global illumination...

Moreover, the amount of reddening would ideally also depend on the angle between the Sun direction and the normal to the surface being lit.

This may be possible to achieve in Scenery3D. However, the landscape pano has no usable normals.

@gzotti
Copy link
Member Author

gzotti commented Aug 21, 2024

I can't currently test; how does it work with an eclipsed Sun? I guess we wouldn't want a red-tinted landscape even with a low eclipsed Sun.

The tint is just taken from the solar color. During eclipse, the landscape is first and foremost darkened. I assume a partially eclipsed low sun will still be reddened enough to tint the landscape. However, such tests can be done when we are happy about the commonday situation.

also fix old_style landscape
- May just look smoother after all.
@gzotti
Copy link
Member Author

gzotti commented Aug 21, 2024

Stupid: tint changes only when sun is in frame... Must do that elsewhere.

@Atque
Copy link
Contributor

Atque commented Aug 23, 2024

I think the second of the options is the best.
4df73d2#diff-147261ff5c272049b26356d320341d2f4045a49692259c91ce08ee058f389682R2869-R2873

stellarium-296

The one currently in use in this PR is too weak, barely altering the color of the scene even with the sun at the horizon:
stellarium-298

@gzotti
Copy link
Member Author

gzotti commented Aug 23, 2024

For some reason the tint in Scenery3D appears much weaker than the tint in regular landscapes. I have not found why so far. Of course, Scenery3D has ambient/diffuse/specular components, and ambient is still white (neutral gray) to simulate the all-sky contribution.

For landscapes, one of the two last solutions looks best to me. I don't want to become too kitschy.

@Atque
Copy link
Contributor

Atque commented Aug 23, 2024

You're right, I hadn't even noticed. Indeed one of the two latter looks best.

I suppose we can't simply split them, so that 3D Sceneries use one tint, and 2D landscapes another? But that might look weird if someone wants to use a landscape photo as a background for their 3D Scenery.

@gzotti
Copy link
Member Author

gzotti commented Aug 23, 2024

Of course some better solution is needed, therefore this is still a draft. Now you see where endless late hours of finetuning go... Where are my weekends?

- now reddening is drawn down to -3° solar altitude with a smooth curve
- Scenery3D: ambient also receives a slight reddening
- Scenery3D: cleanup outdated code (night mode)
@gzotti
Copy link
Member Author

gzotti commented Aug 30, 2024

I think behaviour looks OK now. Maybe the color hue itself needs some more work.

@gzotti gzotti marked this pull request as ready for review September 1, 2024 11:40
@alex-w
Copy link
Member

alex-w commented Sep 4, 2024

Hmm... I see no effect in comparison with version 24.2

@Atque
Copy link
Contributor

Atque commented Sep 4, 2024

Hmm... I see no effect in comparison with version 24.2

The difference is quite subtle compared to the early stages of the PR. Do you see any difference using a snowy landscape panorama, such as Hurricane?

@gzotti
Copy link
Member Author

gzotti commented Sep 4, 2024

It's just a slight change, as I don't want to become too kitschy. Use e.g. any grassy ground, and see reddening with the sun higher than -3°, max. on the horizon and tint fading away as the sun climbs higher. With Scenery3D, I now had also to tint the ambient term slightly. If you really think it is not strong enough, I can step up one level.

@10110111
Copy link
Contributor

10110111 commented Sep 4, 2024

Wouldn't it be better to query Atmosphere for illuminance (including chromaticity)? Maybe even split between the direct solar component and the diffuse sky component.

Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Thanks!

I see the effect now :)

@gzotti
Copy link
Member Author

gzotti commented Sep 4, 2024

Wouldn't it be better to query Atmosphere for illuminance (including chromaticity)? Maybe even split between the direct solar component and the diffuse sky component.

In the long run, sure. This is just a quick first solution. In Scenery3D, someone with enough time could think about global illumination, although we never initially aimed that high. (Earth curvature correction would be more important in its primary field of use.)

The color of the solar "halo" [yes, it's not a halo in the meteorological sense. It's just a variable name] and reddening of sun and moon should take atmospheric extinction factor into account. (If SMS is used, even spectral differences are possible, we had talked about Mars and many other incredible possibilities once somebody could explain and document the generation parameters of SMS and write a guideline how to create other atmosphere settings, including several real-world examples.)

For now, this provides a first improvement. I am open for better ones, now or next year.

@10110111
Copy link
Contributor

10110111 commented Sep 4, 2024

once somebody could explain and document the generation parameters of SMS

It's all documented at https://10110111.github.io/CalcMySky.

write a guideline how to create other atmosphere settings, including several real-world examples

The problem here is that there's not a lot of comprehensive models. Even for the Earth I still haven't found any model of the stratospheric aerosols that produce afterglow, which is present virtually everywhere in clear weather. For Mars there have been some papers that described the efforts to model its atmosphere, but the description there appears to be self-inconsistent (e.g. a formula that contradicts the corresponding chart), so I couldn't reproduce their results yet.

And surely, the parameters are not just turbidity, humidity etc. like in Preetham model, atmospheres are more complicated than that.

Earth curvature correction would be more important in its primary field of use

This should work automatically if the scenery is large enough and not created to be flat. But I haven't seen any sceneries so large that they'd cover hundreds of kilometers.

@gzotti
Copy link
Member Author

gzotti commented Sep 4, 2024

These large scenes unfortunately cannot be made publicly available, my largest just shown in our paper. Once they extend to 80km or so, a difference is notable, and it depends on the application whether it is to be regarded relevant.
Scenes are usually modelled from UTM-based DEM grids. They are unfortunately flat by design. The importer would probably have to convert to planetocentric Cartesian coordinates (double prec.), and during use, find a local vertical, and always rotate the scene so that this local vertical is "up", and true north is "north". However, this needs double precision math which GPUs still don't like very much. But we digress here, it's off-topic to this small-cosmetic PR.

@gzotti
Copy link
Member Author

gzotti commented Sep 7, 2024

Wouldn't it be better to query Atmosphere for illuminance (including chromaticity)? Maybe even split between the direct solar component and the diffuse sky component.

@10110111 where could I ask the atmosphere for chromaticity? If this is not available now, I think this PR should be OK as-is, but as soon as atmosphere can deliver such data, we can further improve here.

@10110111
Copy link
Contributor

10110111 commented Sep 7, 2024

where could I ask the atmosphere for chromaticity?

There's no such API currently.

If this is not available now, I think this PR should be OK as-is

Well it doesn't look realistic to me. The colors look a bit weird, particularly for Guereins the grass becomes too red (I'd expect more of an orange tint than red), and this redness continues into the twilight—why?

@gzotti
Copy link
Member Author

gzotti commented Sep 7, 2024

It just looked bad to just cut off at solar altitude 0 as originally planned. So, now I fade it out 0...-3°. Tell me your favourite solar altitude (or transition altitude) for the cutoff.
The red in early twilight is best arguable when there is some haze or clouds in the sky (which yes, we don't show). Maybe I was also biased, using Preetham sky during tests.

Can you quote your favourite RGB values (1, G, B) for max discoloration?

@gzotti
Copy link
Member Author

gzotti commented Sep 7, 2024

Just tried with SMS. Looks even better. Horizon is reddish (sun side)/pink (opposite) with the sun just below horizon. Anywhere in-between the color that is applied on the grass.

@10110111
Copy link
Contributor

10110111 commented Sep 7, 2024

It just looked bad to just cut off at solar altitude 0 as originally planned.

The illuminance due to the setting sun compared to the illuminance by the diffuse sky glow decreases with decreasing solar elevation. Especially so for the horizontal surfaces. So the tinting should have a maximum somewhere between 0° and the upper cutoff, not at 0°.

The red in early twilight is best arguable when there is some haze or clouds in the sky (which yes, we don't show).

Or the afterglow, but we don't simulate it either (though in real life it has a profound effect on the twilight color of mountains).

Can you quote your favourite RGB values (1, G, B) for max discoloration?

I don't necessarily think the issue is in the RGB, rather in the way the tinting is applied (see above). In the absence of afterglow or red clouds we shouldn't have too much redness after the sun has set.

@10110111
Copy link
Contributor

10110111 commented Sep 7, 2024

Note though that in 3D scenery the reddening of the sunlight should be maximum at 0° (or rather -0.25°), since there, IIUC, the sunlight is separate from ambient skylight.

@gzotti
Copy link
Member Author

gzotti commented Sep 7, 2024

There is no upper cutoff. Reddening goes with airmass. The lower cutoff is currently set arbitrarily to -3°, and looks pretty good IMHO. Definitely better than 0.

Scenery3D: You are cordially invited to improve my attempts. Yes, the ambient term should ideally receive its information from the atmosphere. This is not possible yet. Therefore I have to mix in some of the reddened light, or else the 3D surfaces are not perceivably colored. As soon as the atmosphere can be asked for global chromaticity, this can of course be improved. So, we can have this effect as simple painting effect now, or this PR has to wait until it can immediately be presented physically correct in X years, or have the simple solution now AND the better solution whenever atmosphere API is available.

For a test, setting transition to -1° only. The message is "there is reddening around sunrise". Too much then depends on real-world conditions which we fail to simulate in any case.

@10110111 , @alex-w, @Atque your votes please.

@alex-w
Copy link
Member

alex-w commented Sep 8, 2024

This is acceptable solution for me at the moment

@10110111
Copy link
Contributor

10110111 commented Sep 8, 2024

or have the simple solution now AND the better solution whenever atmosphere API is available.

I'm worried that when the physically-correct solution appears, people will complain that the effect is not as noticeable as it used to be. And then, if we revert to this ad hoc implementation in response, this will mean that we prefer an exaggerated drawing to a simulation.

And from my experiments, the effect on horizontal surfaces is indeed very subtle.

this PR has to wait until it can immediately be presented physically correct in X years

The API to query illuminance is not so hard to create, it can be done in a few weeks.

@Atque
Copy link
Contributor

Atque commented Sep 8, 2024

I also think this is good enough. Not everything needs to be physically correct to be usable.

- a bit more green (yellow)
- docfix
@gzotti
Copy link
Member Author

gzotti commented Sep 8, 2024

I added a bit more green to the color, does not change too much. The effect is noticeable during 20 minutes of sun near horizon, but not too strong. Good enough now, but as everywhere, there will be room for improvement based on atmosphere tuning later. A cloudy sky (whenever this may come) will certainly also have to influence colours.

@gzotti gzotti merged commit 660f90a into master Sep 8, 2024
25 of 26 checks passed
@gzotti gzotti deleted the landscape-tint branch September 8, 2024 07:54
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 8, 2024
Copy link

github-actions bot commented Sep 8, 2024

Hello @gzotti!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Sep 22, 2024
Copy link

Hello @gzotti!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Entirely new feature importance: low Small problem, rarely visible, no crash
Development

Successfully merging this pull request may close these issues.

Tint landscapes orange and red at sunrise/sunset
4 participants