sokol icon indicating copy to clipboard operation
sokol copied to clipboard

Continuous Bind Slots Restriction Unnecessary?

Open kayomn opened this issue 1 year ago • 4 comments

I wanted to know why the intentional restriction that shader resource bind slots like storage buffers, images, samplers, etc. need to be used contiguously, as seen in this validation code?

bool storage_buffers_continuous = true;
for (int sbuf_index = 0; sbuf_index < SG_MAX_SHADERSTAGE_STORAGEBUFFERS; sbuf_index++) {
  const sg_shader_storage_buffer_desc* sbuf_desc = &stage_desc->storage_buffers[sbuf_index];
  if (sbuf_desc->used) {
    _SG_VALIDATE(storage_buffers_continuous, VALIDATE_SHADERDESC_NO_CONT_STORAGEBUFFERS);
    _SG_VALIDATE(sbuf_desc->readonly, VALIDATE_SHADERDESC_STORAGEBUFFER_READONLY);
  } else {
    storage_buffers_continuous = false;
  }
}

As far as I know, no API backend would require resource slots to be used in a contiguous manner. This just seems like a way of enforcing more boilerplate in shader programs for rendering pipelines that use some standardized bind points for sending their resources across.

I'd be happy to submit a PR to change this behavior, as being able to use bind points in a non-contiguous manner for images / samplers and storage buffers would be a big benefit to something I'm working on that doesn't use Sokol's shdc workflow.

kayomn avatar Jul 20 '24 12:07 kayomn

I'm planning to make this more flexible in the future:

https://github.com/floooh/sokol/issues/1037

With this change there would be a proper "custom mapping" between the occupied slot indices in an sg_bindings struct and the actual backend shader stage (vertex/fragment/(and later: compute)) and bind slot, and this mapping may also be different for each 3D backend (differences between different shading languages and the various 3D API resource bindings models was the main reason why the current restriction is in place). But once we allow a different bindings-mapping for different shading language and 3D backends, this restriction should no longer apply.

It's not a trivial change though since it spans the sokol-shdc tool, all 3D backends and also needs changes in the public API, it will also make working with sokol-gfx without the sokol-shdc shader compiler a bit more awkward (since more info needs to be provided in the sg_shader_desc struct.

floooh avatar Jul 21 '24 08:07 floooh

What kind of API changes would be needed beyond having to specify a bind slot explicitly on the sg_shader_stage_desc sub-structures?

kayomn avatar Jul 21 '24 15:07 kayomn

The change would go together with getting rid of shader stages in the rest of the API (sg_bindings and sg_apply_uniforms), instead a sokol-gfx bind slot would only be defined by a resource type (image vs sampler vs storage buffer) and a slot index, and such a resource-type + slot index would be mapped to 3D backend API specific resource bind slots (which may follow different rules for each backend).

In the sg_shader_desc, such a sokol-gfx bind slot needs to have a 3D backend specific bindinds information.

Off the top of my head:

  • GL: a binding index (https://www.khronos.org/opengl/wiki/Layout_Qualifier_(GLSL)#Binding_points) - not sure if those are per-shader-stage or not
  • D3D11: a 'bind register index' (register(t3) etc...) and what shader stage the binding applies to
  • Metal: ...a similar per-resource type bind slot index and shader stage identifier
  • WebGPU: a bindgroup index and slot index within the bind group

floooh avatar Jul 21 '24 18:07 floooh

Work happens in this branch: https://github.com/floooh/sokol/tree/issue1037_bindings_cleanup

floooh avatar Sep 13 '24 08:09 floooh

Btw this is now merged, see changelog (and the linked blog post) for details:

https://github.com/floooh/sokol/blob/master/CHANGELOG.md#07-nov-2024

...there's also a new sample which uses the same sg_bindings struct for 3 slightly different shaders which each expect textures and samplers at different bindslots.

https://floooh.github.io/sokol-html5/shared-bindings-sapp.html

floooh avatar Nov 08 '24 16:11 floooh