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.

Sign up for GitHub

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

Jump to bottom

Use argument buffers on Metal #4491

Closed
kanerogers opened this issue Apr 19, 2023 · 9 comments · May be fixed by gfx-rs/naga#2547
Closed

Use argument buffers on Metal #4491

kanerogers opened this issue Apr 19, 2023 · 9 comments · May be fixed by gfx-rs/naga#2547
Labels
area: naga back-end Outputs of naga shader conversion lang: Metal Metal Shading Language naga Shader Translator type: enhancement New feature or request

Comments

@kanerogers
Copy link

kanerogers commented Apr 19, 2023 •
edited
Loading

Background

This is a companion to the wgpu issue #3334

Using argument buffers would simplify the Metal shaders emitted by naga. Instead of having to pass each binding to the shader function as an argument, each binding group can be bundled into a struct which can then be used throughout the shader. For example:

Input wgsl

@group(0) @binding(0) var u_texture : texture_2d<f32>;
@group(0) @binding(1) var u_sampler : sampler;

@fragment
fn frag_main(@location(0) uv : vec2<f32>) -> @location(0) vec4<f32> {
  return textureSample(u_texture, u_sampler, uv);
}

Without argument buffers

fragment frag_mainOutput frag_main(
  frag_mainInput varyings_1 [[stage_in]]
, metal::texture2d<float, metal::access::sample> u_texture [[user(fake0)]]
, metal::sampler u_sampler [[user(fake0)]]
  metal::float4 color = u_texture.sample(u_sampler, uv_1);
  return frag_mainOutput { color };
)

With argument buffers

struct Group_0 {
   metal::texture2d<float, metal::access::sample> u_texture [[id(0)]];
   metal::sampler u_sampler [[id(1)]];
};

fragment frag_mainOutput frag_main(
  frag_mainInput varyings_1 [[stage_in]]
  device Group_0 & group_0 [[buffer(0)]]
) {
  metal::float4 color = group_0.u_texture.sample(group_0.u_sampler, uv_1);
  return frag_mainOutput { color };
}
@teoxoy teoxoy added lang: Metal Metal Shading Language area: naga back-end Outputs of naga shader conversion type: enhancement New feature or request labels Apr 24, 2023
@kanerogers
Copy link
Author

Some updates on this one:

  • I've been struggling to get a solid week blocked off to focus on this issue, so my output has been a little sporadic to date
  • With a hacky, MVP implementation I've been able to get the "hello-cube" example working with wgpu/naga
  • My core struggle has been figuring out how much to keep / how much to throw away with the existing implementation as I'm wary of modifying a large, complex codebase that I'm not familiar with

Some notes (for my reference, or for anyone who wants to pick this up should I get hit by a bus):

  • I am reasonably confident that a 1:1 mapping can be made between a WGPU bind group and an argument buffer. That is, the four kinds of resources defined in the WSGL spec (Uniform Buffers, Storage Buffers, Texture resources and Sampler resources) are all legal members of argument buffers, per section 2.13 of the MSL spec:

Argument buffers extend the basic buffer types to include pointers (buffers), textures, texture buffers, and samplers.

  • With this in mind, I believe it makes more sense to preserve the structure of a bind group on both the wgpu and naga side to more easily populate (on the naga side) and encode (on the wgpu side) the relevant argument buffer.

@kanerogers kanerogers mentioned this issue Sep 15, 2023
Closed
@teoxoy
Copy link
Member

teoxoy commented Sep 15, 2023

Some considerations:

  • argument buffers require MSL 2 (i.e. iOS 11.0+ macOS 10.13+); the WebGPU spec is ok with this and so is Firefox but this might still impact naga/wgpu users
  • argument buffer tier 1 doesn't support writeable textures (ref)

@kanerogers kanerogers mentioned this issue Oct 9, 2023
Draft
@jimblandy
Copy link
Member

@kanerogers The "Without argument buffers" example in the top comment seems mangled - is that valid MSL?

@kanerogers
Copy link
Author

@kanerogers The "Without argument buffers" example in the top comment seems mangled - is that valid MSL?

Ah, good pick up! 😅

No, it's not valid MSL. This was just a rough sketch I wrote, mostly for myself, when I was first looking into the project.

@cwfitzgerald cwfitzgerald added the naga Shader Translator label Oct 25, 2023
@cwfitzgerald cwfitzgerald transferred this issue from gfx-rs/naga Oct 25, 2023
@Patryk27 Patryk27 mentioned this issue Jan 7, 2024
Open
@cwfitzgerald
Copy link
Member

Closing as this is redundant to #3334, now that the repos are merged.

@cwfitzgerald cwfitzgerald closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2024
@mikialex
Copy link

Currently the maxStorageBuffersPerShaderStage on Metal is limited to 31, will the using of argument buffer unlock this limitation to a very large number? I see this issue closed because of the #3334, if the #3334 get implemented, can maxStorageBuffersPerShaderStage be raised a lot?

Background: I have shader that using more than 31 storage buffers so it can not run on Metal now and I'm not able to migrate my shader to using binding_array even if binding_array on Metal is supported.

@cwfitzgerald
Copy link
Member

It will be a different limit (see #6738) but yes, it will be on the order of a million.

@mikialex
Copy link

So I suppose you mean the maxStorageBuffersPerShaderStage will not be raised, but there will be another new millions level limit for bind_array, which means I still need to somehow migrate my code using bindless resource to use so much binging in metal?

@cwfitzgerald
Copy link
Member

Correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: naga back-end Outputs of naga shader conversion lang: Metal Metal Shading Language naga Shader Translator type: enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use argument buffers on Metal leetvr/naga
5 participants
@jimblandy @kanerogers @cwfitzgerald @mikialex @teoxoy