Skip to content

Metal: Fix dynamic uniform buffer offset corruption when rebinding sets#114778

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
stuartcarnie:metal_dynamic_offsets
Jan 9, 2026
Merged

Metal: Fix dynamic uniform buffer offset corruption when rebinding sets#114778
akien-mga merged 1 commit into
godotengine:masterfrom
stuartcarnie:metal_dynamic_offsets

Conversation

@stuartcarnie
Copy link
Copy Markdown
Contributor

@stuartcarnie stuartcarnie commented Jan 9, 2026

Closes #114069

When the same uniform set is bound multiple times within a render pass (e.g., OPAQUE pass then ALPHA pass with different instance buffers), the dynamic offset bits were being OR'd together, corrupting the packed frame indices.

For example, if OPAQUE binds set 1 with frame_idx=1 (0x10) and ALPHA binds set 1 with frame_idx=2 (0x20), the OR produces 0x30 instead of the correct 0x20, causing the shader to read from the wrong buffer offset.

This fix clears the relevant bits for each set being bound before OR'ing the new values, ensuring only the latest frame indices are used.

Fixes rendering artefacts in the mobile renderer which uses two dynamic uniforms (uniform buffer + storage buffer) in set 1, unlike the clustered renderer which only has one.

🤖 AI disclosure

I used AI to identify the bug. I have validated the code and verified the fix.

I used the following prompt:

A rendering regression for Metal was introduced by commit 230adb7, which I expect is related to UNIFORM_TYPE_UNIFORM_BUFFER_DYNAMIC or UNIFORM_TYPE_STORAGE_BUFFER_DYNAMIC. It appears there is corruption rendering triangles. The artefacts only appear with the mobile renderer @servers/rendering/renderer_rd/shaders/forward_mobile/ Can you see any issues with how Metal handles dynamic uniforms. Essentially, Godot requests a buffer, and a MTLBuffer is allocated by multiplying the requested size * the number of in-flight frames. Each time the buffer is used for a frame, an index is increased (modulus the number of in-flight frames) to write to that section of the buffer.

@stuartcarnie stuartcarnie requested a review from a team as a code owner January 9, 2026 02:25
When the same uniform set is bound multiple times within a render pass
(e.g., OPAQUE pass then ALPHA pass with different instance buffers),
the dynamic offset bits were being OR'd together, corrupting the packed
frame indices.

For example, if OPAQUE binds set 1 with frame_idx=1 (0x10) and ALPHA
binds set 1 with frame_idx=2 (0x20), the OR produces 0x30 instead of
the correct 0x20, causing the shader to read from the wrong buffer
offset.

This fix clears the relevant bits for each set being bound before
OR'ing the new values, ensuring only the latest frame indices are used.

Fixes rendering artifacts in the mobile renderer which uses two dynamic
uniforms (uniform buffer + storage buffer) in set 1, unlike the clustered
renderer which only has one.
@stuartcarnie stuartcarnie force-pushed the metal_dynamic_offsets branch from 8c4f848 to 541f626 Compare January 9, 2026 05:34
@clayjohn clayjohn added this to the 4.6 milestone Jan 9, 2026
@akien-mga akien-mga merged commit 51117da into godotengine:master Jan 9, 2026
20 checks passed
@akien-mga
Copy link
Copy Markdown
Member

Thanks!

@stuartcarnie stuartcarnie deleted the metal_dynamic_offsets branch January 11, 2026 21:33
rivie13 pushed a commit to rivie13/Phoenix-Agentic-Engine that referenced this pull request Feb 16, 2026
…_offsets

Metal: Fix dynamic uniform buffer offset corruption when rebinding sets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[4.6 Beta1] Strange rendering errors with the Mobile Renderer on macOS

3 participants