godot icon indicating copy to clipboard operation
godot copied to clipboard

Proof of concept: forward declare shader files so they don't cause recompilation waterfalls

Open clayjohn opened this issue 1 year ago • 2 comments

This is purely a benefit for engine developers. Currently when making a change to a shader file we end up recompiling a bunch of random files due to long include chains. We certainly should improve our include chains so changing one rendering file doesn't require main.cpp to recompile. However, in the meantime, avoiding

This is what a single change to scene_forward_clustered.glsl looks like:

scons: Building targets ...
[  3%] Generating servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl.gen.h ...
[ 99%] progress_finish(["progress_finish"], [])
[100%] Compiling servers/rendering/renderer_rd/effects/tone_mapper.cpp ...
[100%] Compiling servers/rendering/renderer_rd/environment/fog.cpp ...
[100%] Compiling servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp ...
[100%] Compiling servers/rendering/renderer_rd/shader_rd.cpp ...
[100%] Compiling servers/rendering/renderer_rd/effects/copy_effects.cpp ...
[100%] Compiling servers/rendering/renderer_rd/effects/resolve.cpp ...
[100%] Compiling servers/rendering/renderer_rd/renderer_compositor_rd.cpp ...
[100%] Compiling platform/macos/display_server_macos.mm ...
[100%] Compiling servers/rendering/renderer_rd/effects/vrs.cpp ...
[100%] Compiling servers/rendering/renderer_rd/environment/gi.cpp ...
[100%] Compiling servers/rendering/renderer_rd/effects/ss_effects.cpp ...
[100%] Compiling servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp ...
[100%] Compiling servers/rendering/renderer_rd/effects/debug_effects.cpp ...
[100%] Compiling servers/rendering/renderer_rd/storage_rd/particles_storage.cpp ...
[100%] Compiling servers/rendering/renderer_rd/renderer_scene_render_rd.cpp ...
[100%] Compiling servers/rendering/renderer_rd/effects/bokeh_dof.cpp ...
[100%] Compiling servers/rendering/renderer_rd/forward_mobile/scene_shader_forward_mobile.cpp ...
[100%] Compiling servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp ...
[100%] Compiling servers/rendering/renderer_rd/environment/sky.cpp ...
[100%] Linking Static Library servers/libservers.macos.editor.dev.arm64.a ...
[100%] Ranlib Library servers/libservers.macos.editor.dev.arm64.a ...
[100%] Linking Program bin/godot.macos.editor.dev.arm64 ...
[100%] scons: done building targets.
[Time elapsed: 00:00:24.94]
 *  Terminal will be reused by tasks, press any key to close it.

There is no reason we should be recompiling the canvas render file or the mobile render file when we make a small change to the clustered renderer shader.

After this change, it reduces to:

scons: Building targets ...
[ 89%] Generating servers/rendering/renderer_rd/shaders/forward_clustered/scene_forward_clustered.glsl.gen.h ...
[ 99%] progress_finish(["progress_finish"], [])
[100%] Compiling servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp ...
[100%] Compiling servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp ...
[100%] Linking Static Library servers/libservers.macos.editor.dev.arm64.a ...
[100%] Ranlib Library servers/libservers.macos.editor.dev.arm64.a ...
[100%] Linking Program bin/godot.macos.editor.dev.arm64 ...
[100%] scons: done building targets.
[Time elapsed: 00:00:14.70]
 *  Terminal will be reused by tasks, press any key to close it. 

Much better!

Note, in both cases the majority of compile time is taken up by linking.

Opening as draft for now until I move more shaders to this style.

clayjohn avatar Sep 20 '24 23:09 clayjohn

I am not sure if the memory leak is related to the changes so I'll try a restart of the build.

fire avatar Sep 21 '24 09:09 fire

I am not sure if the memory leak is related to the changes so I'll try a restart of the build.

The memory leak comes from this PR. Its from scene_shader not being memdeleted

clayjohn avatar Sep 23 '24 03:09 clayjohn