gpu: Added helper functions to BindGroupLayout.Entry #213

2022-04-10T17:51:49Z

michal-z commented

2022-04-10 17:51:49 +00:00

(Migrated from github.com)

Also added default values to *.BindingLayout structures like this is done in webgpu_cpp.h.
Without those methods and default values the code to init BindGroupLayout is very verbose.

With this PR initialization is much cleaner:

    const desc = BindGroupLayout.Descriptor{
        .entries = &.{
            BindGroupLayout.Entry.buffer(0, .{ .vertex = true }, .uniform, true, 0),
            BindGroupLayout.Entry.sampler(1, .{ .vertex = true }, .filtering),
            BindGroupLayout.Entry.texture(2, .{ .fragment = true }, .float, .dimension_2d, false),
            BindGroupLayout.Entry.storageTexture(3, .{ .fragment = true }, .none, .rgba32_float, .dimension_2d),
        },
    };

By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

Also added default values to `*.BindingLayout` structures like this is done in `webgpu_cpp.h`. Without those methods and default values the code to init `BindGroupLayout` is very verbose. With this PR initialization is much cleaner: ```zig const desc = BindGroupLayout.Descriptor{ .entries = &.{ BindGroupLayout.Entry.buffer(0, .{ .vertex = true }, .uniform, true, 0), BindGroupLayout.Entry.sampler(1, .{ .vertex = true }, .filtering), BindGroupLayout.Entry.texture(2, .{ .fragment = true }, .float, .dimension_2d, false), BindGroupLayout.Entry.storageTexture(3, .{ .fragment = true }, .none, .rgba32_float, .dimension_2d), }, }; ``` - [x] By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

emidoots (Migrated from github.com) reviewed

2022-04-10 20:00:30 +00:00

gpu/src/Buffer.zig

				`@ -103,9 +103,9 @@ pub const BindingType = enum(u32) {`

I think we should have this default to .uniform since that's what it's defined as in the spec:

https://www.w3.org/TR/webgpu/#dictdef-gpubufferbindinglayout

I think we should have this default to `.uniform` since that's what it's defined as in the spec: <img width="564" alt="image" src="https://user-images.githubusercontent.com/3173176/162637495-99c752d2-ad6b-4d32-84d9-d562104dc062.png"> https://www.w3.org/TR/webgpu/#dictdef-gpubufferbindinglayout

emidoots (Migrated from github.com) reviewed

2022-04-10 20:01:44 +00:00

gpu/src/Sampler.zig

				`@ -36,7 +36,7 @@ pub const BindingType = enum(u32) {`

Similarly, should default to .filtering

https://www.w3.org/TR/webgpu/#dictdef-gpusamplerbindinglayout

Similarly, should default to `.filtering` <img width="448" alt="image" src="https://user-images.githubusercontent.com/3173176/162637549-8317c3b4-91c8-4103-9cde-083a8ccfba6b.png"> https://www.w3.org/TR/webgpu/#dictdef-gpusamplerbindinglayout

emidoots (Migrated from github.com) reviewed

2022-04-10 20:02:43 +00:00

gpu/src/Texture.zig

					
				@ -217,3 +216,4 @@

				    multisampled: bool = false,

				};

				pub const DataLayout = extern struct {

.float and .dimension_2d

`.float` and `.dimension_2d` <img width="493" alt="image" src="https://user-images.githubusercontent.com/3173176/162637574-750b8c1a-62f5-42a0-9b07-5e7f20f29c35.png">

emidoots (Migrated from github.com) reviewed

2022-04-10 20:03:20 +00:00

gpu/src/structs.zig

				`@ -39,9 +39,9 @@ pub const PrimitiveState = struct {`

.dimension_2d

`.dimension_2d`

emidoots (Migrated from github.com) reviewed

2022-04-10 20:03:28 +00:00

gpu/src/structs.zig

				`@ -39,9 +39,9 @@ pub const PrimitiveState = struct {`

.write_only

`.write_only`

emidoots (Migrated from github.com) reviewed

2022-04-10 20:04:16 +00:00

gpu/src/structs.zig

				`@ -39,9 +39,9 @@ pub const PrimitiveState = struct {`

The spec has this to say - I think we should make this required for now, no default .none:

The spec has this to say - I think we should make this required for now, no default `.none`: <img width="507" alt="image" src="https://user-images.githubusercontent.com/3173176/162637605-b3c65ddd-be43-4fdc-b7f1-a963b484d2e2.png">

emidoots (Migrated from github.com) reviewed

2022-04-10 20:09:22 +00:00

gpu/src/BindGroupLayout.zig

					
				@ -38,10 +40,76 @@ pub const Entry = extern struct {

				    reserved: ?*anyopaque = null,

I actually think these should be optional values, according to the spec, not default to zero values.

I actually think these should be optional values, according to the spec, not default to zero values. <img width="539" alt="image" src="https://user-images.githubusercontent.com/3173176/162637808-1a044a9a-4cad-4ec1-b198-4fcc112f9708.png">

emidoots (Migrated from github.com) reviewed

2022-04-10 20:14:11 +00:00

gpu/src/BindGroupLayout.zig

					
				@ -45,0 +60,4 @@

				                .type = binding_type,

				                .has_dynamic_offset = has_dynamic_offset,

				                .min_binding_size = min_binding_size,

				            },

Could add comments to these to make it clear they're helpers:

/// Helper to create a buffer BindGroupLayout.

Could add comments to these to make it clear they're helpers: ``` /// Helper to create a buffer BindGroupLayout. ```

emidoots commented

2022-04-10 20:14:30 +00:00

(Migrated from github.com)

LGTM otherwise!

michal-z (Migrated from github.com) reviewed

2022-04-10 20:39:52 +00:00

gpu/src/BindGroupLayout.zig

					
				@ -38,10 +40,76 @@ pub const Entry = extern struct {

				    reserved: ?*anyopaque = null,

BindingLayoutGroup.Entry is an extern struct - we can't have optional field for ABI compatibility.

`BindingLayoutGroup.Entry` is an `extern struct` - we can't have optional field for ABI compatibility.

michal-z commented

2022-04-10 21:17:26 +00:00

(Migrated from github.com)

Note that BindGroupLayout.Entry represents an union of various binding types. The idea is that all bindings are undefined by default and then the user sets only the one that is needed.

https://www.w3.org/TR/webgpu/#dictdef-gpubindgrouplayoutentry

Note that `BindGroupLayout.Entry` represents an union of various binding types. The idea is that all bindings are undefined by default and then the user sets only the one that is needed. https://www.w3.org/TR/webgpu/#dictdef-gpubindgrouplayoutentry

👍 1

emidoots (Migrated from github.com) reviewed

2022-04-10 21:27:44 +00:00

gpu/src/BindGroupLayout.zig

					
				@ -38,10 +40,76 @@ pub const Entry = extern struct {

				    reserved: ?*anyopaque = null,

Ahh, you're right!

emidoots (Migrated from github.com) approved these changes

2022-04-10 21:28:20 +00:00

emidoots (Migrated from github.com) left a comment

Perfect, thanks again!

Sign in to join this conversation.

No reviewers

emidoots

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

hexops/mach!213

				`@ -38,10 +40,76 @@ pub const Entry = extern struct {`
				`reserved: ?*anyopaque = null,`

Rows
Columns