Page MenuHome

EEVEE: Implement the missing Sky texture
AcceptedPublic

Authored by Szymon Ulatowski (szulat) on Wed, Mar 11, 1:30 PM.
Tags
Tokens
"Mountain of Wealth" token, awarded by franMarz."Love" token, awarded by wo262."Love" token, awarded by monio."Burninate" token, awarded by amonpaike."Mountain of Wealth" token, awarded by fclem."Love" token, awarded by higgsas.

Details

Summary

I'm not sure if the Sky was deliberately left out or was just waiting for a
better moment, but so many I was disappointed that Sky in EEVEE is
completely white.

There are already 2 implementations (osl and gpu) so this is the third one.
Looking at other cases it seems that we are not supposed to share sources
between cycles and the rest? So the new util_sky_model files are just
copies of what is already in cycles, except that the data file uses the RGB
variant of the Hosek/Wilkie model, because we output RGB anyway (but can be
easily changed to XYZ if desired - the results are nearly identical).
I am not sure if it is okay to pass 3*9 float values as 3 mat4 uniforms (I
wanted to use mat3 but it does not work).
Also, should I cache the sky model data between renders if the parameters
do not change?

Diff Detail

Repository
rB Blender
Branch
eevee-sky (branched from master)
Build Status
Buildable 7108
Build 7108: arc lint + arc unit

Event Timeline

Szymon Ulatowski (szulat) retitled this revision from Implement the missing Sky texture in EEVEE to EEVEE: Implement the missing Sky texture.Wed, Mar 11, 1:49 PM
Szymon Ulatowski (szulat) edited the summary of this revision. (Show Details)

I did not test the functionality yet. This is a quick style review.

@Brecht Van Lommel (brecht) are the util files at the right place?

source/blender/gpu/shaders/material/gpu_shader_material_tex_sky.glsl
5

always use parenthesis on ifs

33

parenthesis

48

use consts here

source/blender/nodes/shader/nodes/node_shader_tex_sky.c
57

use cosf

60

use expf

115

remove empty lines

132

for better ubo packing, please use vec4 + float

152

same here

source/blender/nodes/shader/nodes/node_shader_tex_sky.c
122–178

you could use 6*vec4 + 1*vec3 to pass all that data. I know it's not ideal but using mat4 is wasting too much data.

Using more vec4, using cosf()/expf(), formatting.

Szymon Ulatowski (szulat) marked 9 inline comments as done.Wed, Mar 11, 11:53 PM

Looks really good! I wanted to tackle this for a long time already but always had something else to do. Thanks for the patch!

I'm just unsure about the duplication of the sky model files.

Also, should I cache the sky model data between renders if the parameters do not change?

This does not seems to be an issue.

This revision is now accepted and ready to land.Thu, Mar 12, 5:37 PM

I'm just unsure about the duplication of the sky model files.

I think it's ok. We could try to deduplicate it but I'm not sure that really makes anything simpler, especially for Cycles standalone.

source/blender/gpu/shaders/material/gpu_shader_material_tex_sky.glsl
54–56

Don't hardcode this, use imbuf_xyz_to_rgb matrix so it works with different color spaces.