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

sokol_gfx.h resource bindings cleanup (issue #1037) #1111

Merged
merged 86 commits into from
Nov 7, 2024

Conversation

floooh
Copy link
Owner

@floooh floooh commented Sep 16, 2024

See: #1037

TODO:

  • update samples webpage
  • fix other projects
    • doom
    • v6502r
    • chips
    • qoiview
    • cimgui-starter-kit
    • pacman.c
    • pacman.zig
    • sokol-zig-imgui-sample
    • chipz
    • ...?
  • update changelogs:
    • sokol
    • sokol-tools
  • win+d3d11: imgui-usercallback-sapp doesn't render right side (nvm, also doesn't work on the master branch, separate issue)
  • linux+gl: shdfeatures-sapp: /home/floh/projects/sokol/sokol_gfx.h:8631: _sg_gl_create_pipeline: Assertion `pip->cmn.vertex_buffer_layout_active[a_state->buffer_index]' failed. (interesting, doesn't happen on macOS, why?)
  • write blogpost
  • update sokol-gfx documentation
  • do a final language-bindings update after the inline docs have been updated
  • Metal: re-introduce reserved bind slots for uniform and storage buffers (uniforms 0..7, storage 8..16), this allows to bind the uniform buffers only once per pass in sg_begin_pass() instead of rebinding in each sg_apply_pipeline()
  • WebGPU: similar optimization possible? not easily because the default 'max dynamic offsets per pipeline layout' is 8
  • add new shared-bindings-sapp sample to sample webpage
  • revert/remove per-uniform offsets
    • in sokol-gfx
    • in sokol-shdc
  • add a cheap error check in sg_draw() that sg_apply_bindings and/or sg_apply_uniforms has been called after the last sg_apply_pipeline if required by current shader
  • any fixes for 'shared bindings' (e.g. allow the same 'superset' sg_bindings to be used for different shaders even if the shader doesn't use a specific bind slot)
  • ...and add a sample for that case
  • GL: properly handle the new explicit uniform offset: (instead remove the uniform offset again, it doesn't really add any value compared to the existing code
  • figure out why cgltf-sapp renders black, some uniform update confusion?
    • the uniform blocks for material and light params get confused, but funny enough, in the WebGPU inspector the uniform data looks correct!
    • metallic_params => binding=2 and light_param => binding=1 correct
    • metallic_params => binding=1 and light_param => binding=2 incorrect
    • resolved: this is actually correct WebGPU behaviour, even when weird. The dynamic offsets must be ordered by 'binding' not by BindGroupLayout entries.
  • fix WebGPU dynamic offset order confusion
  • rename sg_shader_desc.vertex_attrs back to attrs to be consistent with other sg_pipeline_desc
  • consider removing the trailing _n from the new backend bind slots? no, it's good like that
  • need to differentiate between max number of bindings, and the max binding index:
    • in the WebGPU validation layer:
      • need to fix bindslot collision funcs to use 2x uint64
      • fix backend specific out-of-range check ranges
    • max number of BG0 bindings is 8, but valid indices are 0..15
    • max number of BG1 bijndings is 40 (16 + 16 + 8), but valid indices are 0..127
    • in GL the storage buffer binding (max num storage buffers is 8, but valid indices are 0..15
    • size of wgpu bindgroups index needs to 128
    • size of GL storage buffer bind cache needs to be 16
  • update backends:
    • Metal
    • D3D11
    • GL
    • WebGPU
    • dummy
  • fix sokol tests
  • fix raw samples
    • Metal
    • D3D11
    • GLFW
      • uniformarrays-glfw (needs sokol_debugtext.h)
      • vertexpulling-glfw
      • others
    • Emscripten
    • WebGPU
      • drawcallperf-wgpu (needs max bindings vs max bind slot fix in sokol_gfx.h!)
      • others
  • fix all sapp samples
  • update sokol-shdc:
    • auto-binding (manual @bindslot definitions required now)
    • create ATTR_* defines per program: ATTR_[prg]_[name]
    • rename SLOT_* defines to UB_[prg]_[name], IMG_[prg]_[name], SMP_[prg]_[name] and SBUF_[prg]_[name]
    • get rid of the 'merged reflection info', with the above per-program slot definitions that's no longer needed
    • manual bind slot definition via layout(binding=N)
    • check for bindslot collisions (different resources assigned to the same bindslot per linked program)
    • fix all sokol-shdc test shaders
    • update generators:
      • C
      • Zig
      • Odin
      • Nim
      • Rust
      • D
      • Jai
      • Yaml
    • update sokol-shdc docs
  • util headers:
    • sokol_gfx_imgui.h
    • sokol_imgui.h
      • hlsl
      • others
    • sokol_gl.h
      • hlsl
      • others
    • sokol_nuklear.h
      • hlsl
      • others
    • sokol_debugtext.h
      • hlsl
      • others
    • sokol_spine.h
      • hlsl
      • others
    • sokol_fontstash.h
      • hlsl
      • others
  • fix language bindings samples:
    • zig
    • odin
    • nim
    • rust
    • d
    • jai
  • test all samples on:
    • macOS + Metal
    • macOS + GL
    • iOS + Metal
    • iOS + GLES3
    • Windows + D3D11
    • Windows + GL
    • Linux + GL
    • Android + GLES3
    • WebGL2
    • WebGPU
  • merge:

NOTE: in order to allow combining 'random' vertex and fragment shaders in sokol-shdc, shader dialects with common bind space across stages (e.g. GLSL ssbos and WebGPU), sokol-shdc needs to assign vertex- and fragment-stage bindings to separate ranges. This may require some changes in the GL and WebGPU backends for the size of binding caches.

Q:

  • Support glsl_location_n instead of glsl_name binding for uniforms? This would not be supported in GL 4.1 or WebGL2. not in this PR
  • Support 'sparse vertex attributes'? This would at least require wgsl_location_n and at least a couple of fixes where loops over the vertex attributes stopped on the first empty attribute => in sokol-shdc this is sort-of already supported because layout(location = n) works for vertex attributes not in this PR.

- adds a check that sg_apply_bindings() and sg_apply_uniforms()
  is called between sg_apply_pipeline() and sg_draw() if required
- also some code cleanup around vertex_buffer_layout_active init
@floooh floooh marked this pull request as ready for review November 6, 2024 19:15
@floooh floooh merged commit eed7517 into master Nov 7, 2024
64 checks passed
@floooh floooh deleted the issue1037_bindings_cleanup branch November 7, 2024 16:42
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

Successfully merging this pull request may close these issues.

1 participant