Conversation
Three changes: - As described in gpuweb#145, inputSlot in the WebGPUVertexInputDescriptor should instead be implicitly derived from its position in the inputs field. - The format of each attribute is already present in the shader. We should remove the redundant information. - "Input" is not a very descriptive name. Instead, rename it to be "VertexBuffer".
| u32 inputSlot; | ||
| u32 vertexBufferIndex; | ||
| u32 offset; | ||
| WebGPUVertexFormatEnum format; |
There was a problem hiding this comment.
We need to keep the format because for example a shader taking vec2 as an input can have data stored at in many different ways:
- As two 8/16/32 bit integers that are converted to a float with the same value.
- As two 8/16/32 bit integers that are "normalized" to be turned in a float in the [0, 1] range.
- As two float.
| }; | ||
|
|
||
| dictionary WebGPUVertexInputDescriptor { | ||
| u32 inputSlot; |
There was a problem hiding this comment.
I'd be in favor of keeping this, so that applications can define "holes" in their vertex buffers, which would allow minimizing state setting between two pipeline using very similar but different vertex buffers (they could match on VB 0 to 5 and one of them having VB 6 and the other VB 7).
Actually having holes in interfaces like this is something I also wanted to bring up for the pipeline layout and color attachments as well. We already had at least on project prototyping on Dawn ask if we could have holes in the pipeline layout.
There was a problem hiding this comment.
JavaScript arrays are sparse and heterogeneous, so they don't prevent us from having holes. With the change at [1], we could probably (if the IDL accepts it) have:
{ vertexBuffers: [buffer0, null, buffer2] }or
const vertexBuffers = [];
vertexBuffers[0] = buffer0;
vertexBuffers[2] = buffer2;
{ vertexBuffers }
|
The name change is OK (I would bikeshed for "VertexInputBuffer" but no strong feelings on that). Strong 👎 on the other two changes for rationale discussed as before. |
kainino0x
left a comment
There was a problem hiding this comment.
I think I like VertexInputBuffer or VertexInputIndex (or VertexInputSlot because I don't think it hurts to still make a distinction on the word "index"), because it matches with VertexInputDescriptor.
|
|
||
| sequence<WebGPUVertexAttributeDescriptor> attributes; | ||
| sequence<WebGPUVertexInputDescriptor> inputs; | ||
| sequence<WebGPUVertexBufferDescriptor> vertexBuffers; |
There was a problem hiding this comment.
[1] This would probably have to be sequence<WebGPUVertexBufferDescriptor?>
| }; | ||
|
|
||
| dictionary WebGPUVertexInputDescriptor { | ||
| u32 inputSlot; |
There was a problem hiding this comment.
JavaScript arrays are sparse and heterogeneous, so they don't prevent us from having holes. With the change at [1], we could probably (if the IDL accepts it) have:
{ vertexBuffers: [buffer0, null, buffer2] }or
const vertexBuffers = [];
vertexBuffers[0] = buffer0;
vertexBuffers[2] = buffer2;
{ vertexBuffers }
|
This was discussed in the 10 Dec 2018 meeting |
|
Resolution at the meeting: @litherum will split this into multiple PRs. |
|
Moved to #151 |
Three changes:
WebGPUVertexInputDescriptor should instead be implicitly derived from its
position in the inputs field.
the redundant information.