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

Proposal: Allow use of WGPU_BUFFER_BINDING_LAYOUT_INIT/etc for defaulting #450

Merged

Conversation

kainino0x
Copy link
Collaborator

@kainino0x kainino0x commented Nov 28, 2024

An alternative to PR #424. (This proposal prevents us from removing Undefined from the binding type enums, which that PR did.)

This moves the defaulting to BindingNotUsed out to WGPU_BIND_GROUP_LAYOUT_ENTRY_INIT so that it still generates a WGPUBindGroupLayoutEntry with no bindings set, but changes
WGPU_BUFFER_BINDING_LAYOUT_INIT
WGPU_SAMPLER_BINDING_LAYOUT_INIT
WGPU_TEXTURE_BINDING_LAYOUT_INIT
WGPU_STORAGE_TEXTURE_BINDING_LAYOUT_INIT
so that they set their respective types to Undefined instead of BindingNotUsed.

There honestly isn't that much value in this in C, but it is more consistent with JS and should work better for higher-level language bindings. These examples all resolve to
{ binding, visibility, buffer: { type: "uniform", hasDynamicOffset: false, minBindingSize: 0 } }:

In JS:

const entry = { binding, visibility, buffer = {} };

In C:

WGPUBindGroupLayoutEntry entry = WGPU_BIND_GROUP_LAYOUT_ENTRY_INIT;
entry.binding = binding;
entry.visibility = visibility;
entry.buffer = WGPU_BUFFER_BINDING_LAYOUT_INIT;
// or
WGPUBindGroupLayoutEntry entry = {
  .binding = 0,
  .visibility = visibility,
  .buffer = WGPU_BUFFER_BINDING_LAYOUT_INIT,
  // other fields should get zero-initialized I think, which if so is what we want
};

In C++, something like:

wgpu::BindGroupLayoutEntry entry{
  .binding = 0,
  .visibility = visibility,
  .buffer = {},
  // note that unlike in C, gcc/clang with -Wextra do warn on missing fields in designated initializers
};

@kainino0x
Copy link
Collaborator Author

An alternative to PR #424. (This proposal prevents us from removing Undefined from the binding type enums, which that PR did.)

@eliemichel
Copy link
Collaborator

Now that the semantics of Undefined is to mean "use the default value" rather than "this field must be manually defined" (due to #446), this makes a lot of sense. We should note to ourselves somewhere that this relies on NotUsed being 0, as NotUsed is not explicitly stated in WGPU_BIND_GROUP_LAYOUT_ENTRY_INIT.

@kainino0x kainino0x force-pushed the binding-layout-init-undefined branch from 8a0079a to 31e4087 Compare December 3, 2024 04:27
@kainino0x
Copy link
Collaborator Author

Dec 9 meeting:

@kainino0x kainino0x force-pushed the binding-layout-init-undefined branch from c7d455b to e1e67be Compare December 9, 2024 21:49
@kainino0x kainino0x merged commit 60ea8a6 into webgpu-native:main Dec 9, 2024
5 checks passed
@kainino0x kainino0x deleted the binding-layout-init-undefined branch December 9, 2024 22:23
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.

3 participants