Sun disk position is not adjusted for `sky_rotation`

ttencate commented 1 year ago

Godot version

4.0.1.stable.arch_linux

System information

Arch Linux x86_64

Issue description

Setting the sky_rotation property of a Sky to a nonzero rotation changes the horizon, but also moves the sun disk along with the rotated sky. I would expect the sun disk to match the global rotation of the directional light in the scene.

This is problematic for games that take place on a sphere, because they need a nonzero sky rotation unless the camera is on the north pole.

Steps to reproduce

Add a Camera3D, DirectionalLight and some geometry so you can see the effect of lighting.
Give the Camera3D an Environment, set its background mode to Sky, and create a ProceduralSkyMaterial or PhysicalSkyMaterial for it.
Enable camera preview mode.
Change the rotation on the sky material, for example the y component. Notice how the sun disk moves in the sky, but the lighting on the geometry remains the same.

For example, here you can see that the sphere is lit from behind and a little to the right, but the sun disk appears to the left:

sky_rotation

Minimal reproduction project

sky_rotation.zip

Calinou commented 1 year ago

I agree it makes sense to keep the sun in a fixed position, regardless of sky rotation. This can likely be fixed by subtracting the sky rotation when drawing the sun disks. The sky rotation vec3 needs to be exposed to sky shaders for this to be done.

clayjohn commented 1 year ago

I'm not sure I understand the use-case well enough to determine if this is a bug or if this is a misuse of a feature.

It sounds like you are rotating the sky in order to keep the "UP" direction of the sky in line with the player's local "UP" direction as they circumnavigate a spherical planet. I wonder if there are other ways to achieve that without rotating the sky (I imagine you are having to manually control a lot of other values like Gravity, Camera-Y-UP, etc.) as this breaks a lot of assumptions the engine makes (primarily the fact that +Y is up). The reason I bring that up is many other rendering effects will also break (e.g. height-based fog, SDFGI) and at a certain point it may be better to revisit your approach.

The intended behavior for sky_rotation is to rotate the entire sky sphere, it is done at mesh draw time (rather than when running the sky shader). If we were to change the sky itself based on the rotation, then we would have to update the radiance cubemap every time that sky_rotation was changed. To be clear, I am not saying that is a reason not to fix this issue. I am just pointing out that there is a performance tradeoff here. The expected behaviour would result in substantial performance loss in the OP's project.

At any rate, this deserves more discussion.

In the meantime, you can work around this issue by using two DirectionalLight3Ds and setting one to "light only" and the other to "sky only"

Calinou commented 1 year ago

The intended behavior for sky_rotation is to rotate the entire sky sphere, it is done at mesh draw time (rather than when running the sky shader). If we were to change the sky itself based on the rotation, then we would have to update the radiance cubemap every time that sky_rotation was changed. To be clear, I am not saying that is a reason not to fix this issue. I am just pointing out that there is a performance tradeoff here. The expected behaviour would result in substantial performance loss in the OP's project.

See https://github.com/godotengine/godot/issues/54397. There was an issue specifically for 4.x too, but it's pretty similar as most people attempting to create a day/night cycle will run into the issue (as the DirectionalLight3D is moved every frame).

ttencate commented 1 year ago

Thank you both for your consideration!

I'm not sure I understand the use-case well enough to determine if this is a bug or if this is a misuse of a feature.

It seems you understood perfectly. If this is not the intended use case of sky_rotation, I wonder what is.

It sounds like you are rotating the sky in order to keep the "UP" direction of the sky in line with the player's local "UP" direction as they circumnavigate a spherical planet. I wonder if there are other ways to achieve that without rotating the sky (I imagine you are having to manually control a lot of other values like Gravity, Camera-Y-UP, etc.) as this breaks a lot of assumptions the engine makes (primarily the fact that +Y is up). The reason I bring that up is many other rendering effects will also break (e.g. height-based fog, SDFGI) and at a certain point it may be better to revisit your approach.

I guess you're right. I'm already working with a floating origin (started this project before precision=double was a thing), so I can work around it by also rotating (in addition to shifting) my world around the player.

If we were to change the sky itself based on the rotation, then we would have to update the radiance cubemap every time that sky_rotation was changed.

This is not a concern in my case, because I'm also going to be rendering dynamic clouds in a sky shader, so I'll be updating the cube map every frame anyway. But for other projects, the tradeoff might be different.

ynot01 commented 1 year ago

Whether the sun should move or not, its position should at least be consistent with the light cast from the directional light that puts it there.

catprisbrey commented 10 months ago

Confirmed this and noted that shadows cast become misaligned with the sun disk as well (which is likely assumed but figured I'd state it anyway)

Aside, my solution for a day/night/weather cycle was to offset each frame an alpha noise sky cover texture imitating clouds moving, instead of the sky itself. Adjusting noise color ramps thickens or clears the sky (for westher changes). then I only rotated the sunlight on the x to keep it simple. Not sure if that's handy

nicola-sorace commented 9 months ago

I've got a similar use-case. Two other things that would be useful along the same lines:

Ability to offset where the horizon is. When very high above a planet, the horizon is below the horizontal. I imagine there are other use-cases where this would be useful for artistic reasons.
Ability to rotate the night-sky image independently from the rest of the sky.

EDIT: And two suns, of course :)

Calinou commented 9 months ago

EDIT: And two suns, of course :)

This is supported with ProceduralSkyMaterial (up to 4 DirectionalLights) but not PhysicalSkyMaterial for performance reasons. You can write a custom shader based on PhysicalSkyMaterial if you need this:

nicola-sorace commented 8 months ago

@Calinou Thanks! That's really useful.

I managed to get what I wanted by adding two parameters to the shader. The up parameter defines the direction of the sky, and horizon_angle defines how far the horizon is from horizontal (in radians):

shader_type sky;
render_mode use_debanding;

uniform float rayleigh : hint_range(0, 64) = 2.0;
uniform vec4 rayleigh_color : source_color = vec4(0.3, 0.405, 0.6, 1.0);
uniform float mie : hint_range(0, 1) = 0.005;
uniform float mie_eccentricity : hint_range(-1, 1) = 0.8;
uniform vec4 mie_color : source_color = vec4(0.69, 0.729, 0.812, 1.0);

uniform float turbidity : hint_range(0, 1000) = 10.0;
uniform float sun_disk_scale : hint_range(0, 360) = 1.0;
uniform vec4 ground_color : source_color = vec4(0.1, 0.07, 0.034, 1.0);
uniform float exposure : hint_range(0, 128) = 1.0;

uniform vec3 up = vec3(0.0, 1.0, 0.0);
uniform float horizon_angle : hint_range(0, 1.5708);

uniform sampler2D night_sky : filter_linear, source_color, hint_default_black;

// Optical length at zenith for molecules.
const float rayleigh_zenith_size = 8.4e3;
const float mie_zenith_size = 1.25e3;

float henyey_greenstein(float cos_theta, float g) {
    const float k = 0.0795774715459;
    return k * (1.0 - g * g) / (pow(1.0 + g * g - 2.0 * g * cos_theta, 1.5));
}

void sky() {
    if (LIGHT0_ENABLED) {
        float angle_fac = (horizon_angle / (PI/2.0));
        float dot_up_dir = (dot(up, EYEDIR) + angle_fac) / (1.0 + angle_fac);

        float zenith_angle = (clamp(dot(up, normalize(LIGHT0_DIRECTION)), -1.0, 1.0) + angle_fac) / (1.0 + angle_fac);
        float sun_energy = max(0.0, 1.0 - exp(-((PI * 0.5) - acos(zenith_angle)))) * LIGHT0_ENERGY;
        float sun_fade = 1.0 - clamp(1.0 - exp(LIGHT0_DIRECTION.y), 0.0, 1.0);

        // Rayleigh coefficients.
        float rayleigh_coefficient = rayleigh - ( 1.0 * ( 1.0 - sun_fade ) );
        vec3 rayleigh_beta = rayleigh_coefficient * rayleigh_color.rgb * 0.0001;
        // mie coefficients from Preetham
        vec3 mie_beta = turbidity * mie * mie_color.rgb * 0.000434;

        // Optical length.
        float zenith = acos(max(0.0, dot_up_dir));
        float optical_mass = 1.0 / (cos(zenith) + 0.15 * pow(93.885 - degrees(zenith), -1.253));
        float rayleigh_scatter = rayleigh_zenith_size * optical_mass;
        float mie_scatter = mie_zenith_size * optical_mass;

        // Light extinction based on thickness of atmosphere.
        vec3 extinction = exp(-(rayleigh_beta * rayleigh_scatter + mie_beta * mie_scatter));

        // In scattering.
        float cos_theta = dot(EYEDIR, normalize(LIGHT0_DIRECTION));

        float rayleigh_phase = (3.0 / (16.0 * PI)) * (1.0 + pow(cos_theta * 0.5 + 0.5, 2.0));
        vec3 betaRTheta = rayleigh_beta * rayleigh_phase;

        float mie_phase = henyey_greenstein(cos_theta, mie_eccentricity);
        vec3 betaMTheta = mie_beta * mie_phase;

        vec3 Lin = pow(sun_energy * ((betaRTheta + betaMTheta) / (rayleigh_beta + mie_beta)) * (1.0 - extinction), vec3(1.5));
        // Hack from https://github.com/mrdoob/three.js/blob/master/examples/jsm/objects/Sky.js
        Lin *= mix(vec3(1.0), pow(sun_energy * ((betaRTheta + betaMTheta) / (rayleigh_beta + mie_beta)) * extinction, vec3(0.5)), clamp(pow(1.0 - zenith_angle, 5.0), 0.0, 1.0));

        // Hack in the ground color.
        Lin  *= mix(ground_color.rgb, vec3(1.0), smoothstep(-0.1, 0.1, dot_up_dir));

        // Solar disk and out-scattering.
        float sunAngularDiameterCos = cos(LIGHT0_SIZE * sun_disk_scale);
        float sunAngularDiameterCos2 = cos(LIGHT0_SIZE * sun_disk_scale*0.5);
        float sundisk = smoothstep(sunAngularDiameterCos, sunAngularDiameterCos2, cos_theta);
        vec3 L0 = (sun_energy * extinction) * sundisk * LIGHT0_COLOR;
        L0 += texture(night_sky, SKY_COORDS).xyz * extinction;

        vec3 color = Lin + L0;
        COLOR = pow(color, vec3(1.0 / (1.2 + (1.2 * sun_fade))));
        COLOR *= exposure;
    } else {
        // There is no sun, so display night_sky and nothing else.
        COLOR = texture(night_sky, SKY_COORDS).xyz;
        COLOR *= exposure;
    }
}

For my use-case an "up" vector was a lot easier to get than Euler angles, so this is actually more convenient.

I still ended up using two lights though, just to artificially dim the directional light to match the sun fading in the sky.

Adding a second sun would be a lot harder. I'll leave that for now.

godotengine / godot