Add GPULimits for vertex inputs#1274
Add GPULimits for vertex inputs#1274kainino0x merged 3 commits intogpuweb:mainfrom jespertheend:add-vertex-attribute-gpulimits
Conversation
|
Hmm, do I need IE status for these checks to pass? I have filled out the Invited Expert application form, but I'm not sure if this is necessary. |
kvark
left a comment
kvark
left a comment
There was a problem hiding this comment.
Thank you for the PR!
There is a few things I believe need fixing, overall looks good!
spec/index.bs
Outdated
| <tr><td><dfn>maxVertexAttributes</dfn> | ||
| <td>{{GPUSize32}} <td>Higher <td>16 | ||
| <tr class=row-continuation><td colspan=4> | ||
| The maximum number of {{GPUVertexBufferLayoutDescriptor/attributes}} when creating a {{GPURenderPipeline}}. |
There was a problem hiding this comment.
this should say "in total across {{GPUVertexStateDescriptor/vertexBuffers}}" somewhere
spec/index.bs
Outdated
|
|
||
| 1. |descriptor|.{{GPUVertexBufferLayoutDescriptor/attributes}}.length is less than or equal to 16. | ||
| 1. |descriptor|.{{GPUVertexBufferLayoutDescriptor/arrayStride}} is less then or equal to 2048. | ||
| 1. |descriptor|.{{GPUVertexBufferLayoutDescriptor/attributes}}.length is less than or equal to {{GPULimits/maxVertexAttributes}}. |
There was a problem hiding this comment.
this check should be replaced by a combined check over the whole pipeline (as opposed to a single vertex buffer)
| unsigned long maxUniformBuffersPerShaderStage = 12; | ||
| unsigned long maxVertexBuffers = 8; | ||
| unsigned long maxVertexAttributes = 16; | ||
| unsigned long maxVertexArrayStride = 2048; |
There was a problem hiding this comment.
I'm not sure if maxVertexArrayStride is the name we want, but the editors can follow-up on that separately, no need to block this PR.
There was a problem hiding this comment.
Should I create a new issue for this?
There was a problem hiding this comment.
We can file an issue during the Monday editing meeting if we don't immediately decide on something.
The ipr pass doesn't have to check for us to land this (it's non-required). That said, I think we could safely mark this contribution as non-substantive. |
|
Adding label for the limit name |
kainino0x
left a comment
kainino0x
left a comment
There was a problem hiding this comment.
No additional comments :)
|
jespertheend marked as non substantive for IPR from ash-nazg. |
1241: Update Extent3d::depth and Limits to latest upstream r=grovesNL a=kvark **Connections** - gpuweb/gpuweb#1390 - gpuweb/gpuweb#1328 - gpuweb/gpuweb#1163 - gpuweb/gpuweb#1274 **Description** Just an API update up to spec. **Testing** Tested on wgpu-rs examples Co-authored-by: Dzmitry Malyshau <[email protected]>
This PR adds unimplemented stub tests for the `atomicStore` builtin. Issue gpuweb#1274
4 participants
Fixes #693.
Preview | Diff