godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Remove code binaries from ShaderRD header files (*.glsl.gen.h) #10782

Open sanseiuP opened 1 month ago

sanseiuP commented 1 month ago

Describe the project you are working on

A demo project for testing godot render features

Describe the problem or limitation you are having in your project

When I was debugging shaders, I noticed that some changes were made to auto generated header files (*.glsl.gen.h) after I modify the source glsl file. This results in some C++ code recompilation, which is a little bit annoying. It seems because glsl source code are translated to hard coded char array in a header file. image

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I think it would be better to store the binaries to a separate file, or simply load from source text at engine launch time. So it won't make C++ to recompile. Also, this makes it possible to reload shaders without restarting engine, which makes graphic programmers happy.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Generate binaries to another place. Use get member functions in ShaderRD to load shader code from disk. So header files won't be changed. image

If this enhancement will not be used often, can it be worked around with a few lines of script?

No. It makes a lot of change. But It makes development more efficient in the long run.

Is there a reason why this should be core and not an add-on in the asset library?

It's about core designs.

AThousandShips commented 1 month ago

or simply load from source text at engine launch time

How would this work? You'd have to keep the shader files with the executable constantly, or the separate file

Not having the engine be fully self contained would be a hassle (and a major compatibility breakage)

clayjohn commented 1 month ago

@stuartcarnie Has a build that does this already https://github.com/stuartcarnie/godot/blob/548db8c7d25d2ce9e5d13ab1676c7db85c602c33/servers/rendering/renderer_rd/shader_rd.cpp#L205C2-L205C74

This would be really nice to have for iterating on the core shaders in dev builds. Its not something that we should include outside of dev builds

sanseiuP commented 1 month ago

This would be really nice to have for iterating on the core shaders in dev builds. Its not something that we should include outside of dev builds

I agree. There can be an option to load shaders directly from source code in dev builds.

stuartcarnie commented 1 month ago

@clayjohn do we want to specify changes to how it should work? We could consider generating a new file that generates all the GLOBAL_DEFs for the built-in shaders?