Skip to content

WIP: fix: correct SSBO/UBO binding point resolution in material system#2647

Open
zzuegg wants to merge 1 commit intojMonkeyEngine:masterfrom
zzuegg:fix/ssbo-ubo-binding
Open

WIP: fix: correct SSBO/UBO binding point resolution in material system#2647
zzuegg wants to merge 1 commit intojMonkeyEngine:masterfrom
zzuegg:fix/ssbo-ubo-binding

Conversation

@zzuegg
Copy link
Member

@zzuegg zzuegg commented Mar 20, 2026

SSBO and UBO buffers set via the Material system were bound to wrong GL binding points, causing shader data to end up in the wrong slots. This fix queries the actual binding
point from the compiled shader and assigns unique fallback bindings when none is declared, so buffers reliably match their GLSL layout(binding=N) declarations.

This also enables using SSBOs and UBOs through the material system — including via MatParamOverride — without requiring layout(binding=N) in the shader. The renderer
automatically assigns a unique binding point per block, so multiple buffers won't collide. Just set the buffer on the material and it works.

  • GLRenderer.updateShaderBufferBlock() now queries the actual binding from the compiled shader (glGetActiveUniformBlocki for UBOs, glGetProgramResourceiv for SSBOs) and respects layout(binding=N). Falls back to blockIndex when no explicit binding is declared.
  • Remove premature renderer.setShaderStorageBufferObject/ setUniformBufferObject calls from Material.updateShaderMaterialParameter that used a sequential counter as binding point.
  • Cache binding on ShaderBufferBlock (per-shader) instead of BufferObject (per-buffer), since the same buffer can be bound at different points in different shaders.
  • Add glGetActiveUniformBlocki to GL3 and glGetProgramResourceiv + GL_BUFFER_BINDING to GL4 with implementations in both LWJGL backends.
  • BufferObject.initializeEmpty() now calls setUpdateNeeded() so the GPU buffer gets allocated on first use.

The material system was using wrong binding points for shader storage
and uniform buffer objects, causing data to be bound to incorrect slots.

- GLRenderer.updateShaderBufferBlock() now queries the actual binding
  from the compiled shader (glGetActiveUniformBlocki for UBOs,
  glGetProgramResourceiv for SSBOs) and respects layout(binding=N).
  Falls back to blockIndex when no explicit binding is declared.
- Remove premature renderer.setShaderStorageBufferObject/
  setUniformBufferObject calls from Material.updateShaderMaterialParameter
  that used a sequential counter as binding point.
- Cache binding on ShaderBufferBlock (per-shader) instead of BufferObject
  (per-buffer), since the same buffer can be bound at different points
  in different shaders.
- Add glGetActiveUniformBlocki to GL3 and glGetProgramResourceiv +
  GL_BUFFER_BINDING to GL4 with implementations in both LWJGL backends.
- BufferObject.initializeEmpty() now calls setUpdateNeeded() so the GPU
  buffer gets allocated on first use.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves critical issues with Shader Storage Buffer Object (SSBO) and Uniform Buffer Object (UBO) binding within the material system. It introduces a more robust mechanism for resolving GL binding points by dynamically querying compiled shaders, which ensures that buffer data is correctly associated with its intended shader slots. A significant benefit of this change is the enhanced flexibility for developers, who can now utilize SSBOs and UBOs through the material system without needing to explicitly declare layout(binding=N) in their GLSL code, as the renderer intelligently assigns unique binding points to prevent conflicts.

Highlights

  • Corrected SSBO/UBO Binding Resolution: The material system now correctly resolves Shader Storage Buffer Object (SSBO) and Uniform Buffer Object (UBO) binding points by querying the compiled shader, preventing data from ending up in incorrect GL slots.
  • Flexible Buffer Usage: SSBOs and UBOs can now be used through the material system (including via MatParamOverride) without requiring explicit layout(binding=N) declarations in shaders. The renderer automatically assigns unique binding points to avoid collisions.
  • Refactored GLRenderer Logic: GLRenderer.updateShaderBufferBlock() was updated to query the actual binding from the compiled shader (using glGetActiveUniformBlocki for UBOs and glGetProgramResourceiv for SSBOs) and falls back to the blockIndex if no explicit binding is declared.
  • Optimized Material Updates: Premature renderer.setShaderStorageBufferObject and renderer.setUniformBufferObject calls were removed from Material.updateShaderMaterialParameter, as the binding is now handled dynamically.
  • Improved Binding Caching: Binding information is now cached on ShaderBufferBlock (per-shader) instead of BufferObject (per-buffer), allowing the same buffer to be bound at different points in different shaders.
  • New OpenGL API Integrations: Added glGetActiveUniformBlocki to the GL3 interface and glGetProgramResourceiv along with the GL_BUFFER_BINDING constant to the GL4 interface, with implementations in both LWJGL backends.
  • Buffer Initialization Fix: BufferObject.initializeEmpty() now correctly calls setUpdateNeeded(), ensuring the GPU buffer is allocated on its first use.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly refactors the SSBO/UBO binding logic to query binding points from compiled shaders and provides a fallback for shaders without explicit bindings, which is a significant improvement. I've identified one potential bug regarding the handling of layout(binding=0) and a minor performance optimization opportunity. Overall, the changes are well-structured and address the intended problem effectively.

Comment on lines +1503 to +1505
if (bindingPoint == 0) {
bindingPoint = blockIndex;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This logic incorrectly handles layout(binding=0). When a uniform block has an explicit layout(binding=0), the GL query will return 0. This code then overwrites bindingPoint with blockIndex, which might be different from 0, thus ignoring the explicit binding from the shader. This is a bug for shaders that use layout(binding=0). The ambiguity of the return value 0 from the GL functions makes this tricky, but the current implementation doesn't fully respect layout(binding=N) as intended.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think there is a correct solution, if binding trough the material system is allowed, and the direct binding version too. then unfortuantely due to opengl returning 0 for "not defined" creates always a a point where collision can happen.

Comment on lines +1542 to +1544
IntBuffer params = BufferUtils.createIntBuffer(1);
gl4.glGetProgramResourceiv(program, GL4.GL_SHADER_STORAGE_BLOCK, blockIndex, props, null, params);
return params.get(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This method allocates a new IntBuffer for params on every call. To improve performance and reduce garbage, you can reuse the class member intBuf1 which is already available for this purpose.

Suggested change
IntBuffer params = BufferUtils.createIntBuffer(1);
gl4.glGetProgramResourceiv(program, GL4.GL_SHADER_STORAGE_BLOCK, blockIndex, props, null, params);
return params.get(0);
intBuf1.clear();
gl4.glGetProgramResourceiv(program, GL4.GL_SHADER_STORAGE_BLOCK, blockIndex, props, null, intBuf1);
return intBuf1.get(0);

@zzuegg zzuegg changed the title fix: correct SSBO/UBO binding point resolution in material system WIP: fix: correct SSBO/UBO binding point resolution in material system Mar 20, 2026
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