Skip to content
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

More reflection! #80

Open
Interrupt opened this issue Sep 12, 2024 · 4 comments
Open

More reflection! #80

Interrupt opened this issue Sep 12, 2024 · 4 comments

Comments

@Interrupt
Copy link

So far when building shaders with --reflection for C and Zig it does not make enough functions to be able to build a complete list of stuff like the names and offsets of all of the uniforms in a Uniform Block.

The easy path here would be to add more reflection functions, like one where you could ask for the names of all of the uniforms in a given uniform block. Then you could loop through and use the other reflection functions to get the offsets and such.

Another path here would be to just include a YAML version of the shader inside the built shader file, and let people parse that themselves. That would let people do similar shader reflection parsing between both bare_yaml files and other built in shaders.

Do you have a preference on which way you would prefer for new PRs?

@floooh
Copy link
Owner

floooh commented Sep 12, 2024

I'll just throw a couple of thoughts in here for now, because I've been thinking about similar stuff while working on the resource-bindings-cleanup branch (https://github.com/floooh/sokol/tree/issue1037_bindings_cleanup)

  • For a while I was thinking about whether it makes sense to add reflection-information to sg_shader_desc which would be ignored by sokol-gfx itself, but would be useful as reflection info for user code, in the end I rejected that idea, but I think the general idea to return a separate 'reflection info struct' instead of a bunch of functions would make sense, e.g. instead of a function which returns the number of uniforms in a uniform block, and then another function which returns information about that uniform by index, just have a separate struct which basically contains the same information that we are currently returning in in the YAML file

  • Another thing is that a lot of the current reflection information won't strictly be needed anymore after the bindings cleanup, because one idea is to introduce a flexible mapping between sokol-gfx bind slots, and 3D backend bind slots, to allow the sokol-gfx bind slot structure to be defined by the user, and to allow 'gaps' in the bindings. For instance the user code should be able to say 'the sokol-gfx texture bind slot number 3 is always the normal map for the lighting formula', no matter if a specific shader variation even uses a normal map. This means that no reflection information would be needed anymore to populate the sg_bindings struct, but only for cases like exposing the interior of uniform blocks to a material editor.

The good news is that all of this are breaking changes anyway, so we don't need to hold back with new ideas (like replacing the current set of runtime reflection functions with a struct).

I would suggest that you could create an experimental PR where we can check the 'look and feel' of such a new way to return reflection information, but to hold back that PR until the resource bindings cleanup is nearing completion (and then maybe integrate that PR into the bindings cleanup branch - there will also be one for sokol-shdc, currently there are only branches in the sokol and sokol-samples repo, and I'm very much at the beginning of that work still).

PS: I would try to avoid adding a YAML text blob into the generated code, I think it would be better to use a struct instead.

@Interrupt
Copy link
Author

Yeah, having a fully populated reflection info struct for shaders would be the most ideal and really straightforward to work with - the reflection functions currently are hard to work with when you also don't have the full list of names to pass into them.

It's the uniform blocks currently that are tripping me up. Ideally I could grab the size of a uniform block, allocate that memory, and then lookup the uniform offsets by name to write into that data, ignoring uniforms that don't exist.

@Interrupt
Copy link
Author

Interrupt commented Sep 12, 2024

Looking into this some, it does seem like making a whole new set of structs just for the Shader Reflection info seems like a lot of duplication when so far it looks like all of these structs have the required info:

ShaderAttrDesc
ShaderUniformBlockDesc
ShaderStorageBufferDesc
ShaderImageDesc
ShaderSamplerDesc
ShaderImageSamplerPairDesc
ShaderStageDesc
ShaderDesc

And this one is missing just the offset:
ShaderUniformDesc

Does it make sense to add the offset field to ShaderUniformDesc and re-use that same set of structs between both shader creation as it is now, and for the fully populated reflection info? That way you don't need to keep changes in sync between both when / if things change.

@floooh
Copy link
Owner

floooh commented Sep 13, 2024

That was exactly my thinking when I thought about integrating the reflection info into the shader-desc struct, but after the bindings cleanup those structs will drop some information (like any string names), and other structs would need to be extended with information that's only useful for user code as reflection info, but not needed in sokol-gfx for resource binding.

That's why I think it's better to build a separate reflection info struct, even if it partly overlaps with the shader-desc structs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants