-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add a 3D sky shaders demo #809
base: master
Are you sure you want to change the base?
Conversation
f66c1c4
to
fd28d48
Compare
In the screenshot I am noticing significant striping artifacts in the clouds that look like severe undersampling. How noticeable are they in motion? |
vec3 p = pip; | ||
p.x += time * 10.0 * cloud_time_scale + cloud_time_offset; | ||
float height_fraction = GetHeightFractionForPoint(length(p)); | ||
vec4 n = textureLod(perlworlnoise, p.xyz*0.00008, mip-2.0); | ||
float fbm = n.g*0.625+n.b*0.25+n.a*0.125; | ||
float g = densityHeightGradient(height_fraction, weather.r); | ||
float base_cloud = remap(n.r, -(1.0-fbm), 1.0, 0.0, 1.0); | ||
float weather_coverage = cloud_coverage*weather.b; | ||
base_cloud = remap(base_cloud*g, 1.0-(weather_coverage), 1.0, 0.0, 1.0); | ||
base_cloud *= weather_coverage; | ||
p.xy -= time * 40.0; | ||
vec3 hn = textureLod(worlnoise, p*0.001, mip).rgb; | ||
float hfbm = hn.r*0.625+hn.g*0.25+hn.b*0.125; | ||
hfbm = mix(hfbm, 1.0-hfbm, clamp(height_fraction*4.0, 0.0, 1.0)); | ||
base_cloud = remap(base_cloud, hfbm*0.4 * height_fraction, 1.0, 0.0, 1.0); | ||
return pow(clamp(base_cloud, 0.0, 1.0), (1.0 - height_fraction) * 0.8 + 0.5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add some comments and whitespace here to make this easier to read. If you aren't comfortable doing that let me know and I can add some suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make suggestions here, I'm not 100% sure how that code works. Note that I've force-pushed a commit to make the shader better comply with the style guide.
It looks like this in motion at the default project window size. Note that moving the camera makes artifacts more noticeable than what's shown in the video: simplescreenrecorder-2023-01-02_23.54.55.mp4 I've increased the default value to PS: According to the editor's FPS counter, I've noticed that FPS drops whenever the editor camera is moving (even though the shader doesn't use |
41f0895
to
c2ce73f
Compare
Possible reason I can think of is that the original shader calculated clouds in full res pass and then downscaled them for half res pass, while I moved all the calculations to half res pass. |
@Rytelier The original shader doesn't calculate clouds in full res, it calculates them in half res for the background and quarter res for reflections |
@clayjohn The march() function is called in the full screen pass and only its output is applied on half-res pass. After I moved the march() to half-res pass, the performance became much higher. |
Take a look at the code I linked, all the full screen pass does is read from the HALF_RES_COLOR variable. Which means all it does is read from the half res buffer. The code above that assigns to To illustrate my point, take a look at the ISA code emitted by the following shaders
https://shader-playground.timjones.io/6886ba21c71b03fb08d5c337eb8c5dcc
https://shader-playground.timjones.io/5f6bf0e36efe75b29ec6c2f1e6c3f2a9 And for good measure, here is what happens when HALF_RES_PASS becomes true:
https://shader-playground.timjones.io/0ee23ea5a7f96ce07c87a2370ddfa5cb |
@clayjohn I think there's some misunderstanding, I'll elaborate: notice the code block starting from |
Here are two more analogous examples so you can see what the shader compiler does with this type of code. Note that the ISA output is exactly the same whether the logic is inside or outside the To be even more clear, I tested my shader with the cloud logic inside and outside the
https://shader-playground.timjones.io/90f0f20d12035384352dec124ed78944
https://shader-playground.timjones.io/1101dfe53170e0139243faaad304b448 |
Is there anything actionable I can do to move this PR forward? |
I use the latest Godot 4 .NET and I could not see the sky, perhaps I do not know how to set it up. Is it possible to see what is expected of the screenshot the moment the demo is loaded? |
c2ce73f
to
d3c7ba8
Compare
Fixed the demo to work on 4.0.1. This comment still stands though; I don't know how to make the sky shader look better without decreasing performance too much. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This uses https://github.com/Rytelier/godot-sky-and-volumetric-clouds as a base, with several changes made: - Spheres are now used to represent radiance map reflections of varying roughness and metallic values. - A day/night cycle is now featured. - Mipmaps are enabled on the weather texture as an optimization. - The default radiance map settings are more conservative to account for the real-time sky shader. - This makes reflections lower quality, but it's not too noticeable in most real world scenes (especially if using GI techniques for reflections). - Debanding is now applied in the project settings, rather than on the sky shader. This is significantly faster (over 0.1 ms saved on a RX 6900 XT in 3840×2160). It also has the benefit of working on materials, which can exhibit banding if not textured. - The sky shader has debanding commented out in case it's needed. This debanding also applies to the lower half of the sky as well in this case, as it was required to get rid of noticeable banding on the lower half. - Cloud coverage and density uniform hints now allow values as low as 0.001. Co-authored-by: Rytelier <[email protected]> Co-authored-by: Clay John <[email protected]>
d3c7ba8
to
d07c18d
Compare
I've amended the PR to fix an issue related to radiance size (it's now set correctly after switching from Real-Time to High-Quality or High-Quality Incremental). |
This uses https://github.com/Rytelier/godot-sky-and-volumetric-clouds as a base, with several changes made:
cc @Rytelier
Preview