You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
D3D12 ("For more info, see Input Slots. Valid values are between 0 and 15.")
Metal ("Maximum number of entries in the buffer argument table" = 31).
The caveat here is that Metal's table will be shared between vertex buffers, argument buffers, individual buffer descriptors, and any internal buffers we have to provide.
Implementation on Metal has to support at least 4 bind groups. Each will take a slot in the vertex buffer table as an argument buffer, assuming they are supported. If they aren't... this gets much worse.
It has to support at least 8 dynamic uniform buffers and 4 dynamic storage buffers. How are the offsets passed? We were planning on just excluding the dynamic resources from the argument buffer and binding them separately. That would mean at least 12 more bindings taken.
Internally, it may need to bind a buffer with some extra data, say containing the size limits for out-of-bounds checking in the shader. That's one more buffer.
So what we end up is 17 buffers taken by non-vertex-attributes, and only 14 left for the vertex buffers for attributes, at most. Which means, an example implementation can't expose the base limit of 16 that is suggested above. A more complicated implementation could possibly put all the internal things together with dynamic offsets in a single buffer, but the feasibility of this path is yet to be considered in depth (and definitely needs to be discussed).
Another problem is that, if an implementation wants to report a higher limit for any of these, it can't do so for all the limits. I.e. supposing it has an extra buffer. It could either report it as a max bind groups limit of 5, or as a max dynamic storage buffer limit as 5, but not both, and hence - will not be able to run applications that it should be able to run.
I wonder if we can look at it together with #684 and say that:
an implementation may fail to request a device with multiple limits that are above the baseline
That would be totally matching this case here with vertex attributes, and it would discourage users to just blindly request all the limits.
For the maximum stride we have found that 2048 is a good value and required at least by Vulkan. For attribute offsets, Metal requires they are aligned to 4, and that they stay entirely within a stride.
The limited amount of vertex buffers on Metal is a bit of an issue, but maybe this could be worked around by only allowing 14 or even a bit less vertex buffers? I'm not sure I see a usecase where there are more than a handful of vertex buffers.
Yes, setting the base line to 14 would have worked, assuming a Metal implementation would report all the other limits to be the baseline as well, which could certainly be better. I.e. why would an app using 4 vertex buffers have to be limited in 4 bind groups?
Some context about the use cases that need many vertex buffers (namely, glTF import) - gfx-rs/wgpu#558 (comment) .
Would it be possible to have these limits added to GPULimits? So that these limits could be higher on platforms that support this. Or was that the idea already?
@jespertheend yes, we need to add these to GPU limits, whoever got there first. There is an expectation that adding the limits comes with the validation statements about respecting them. If you want to help with a PR, please go for it!
Activity
kvark commentedon Apr 8, 2020
For the vertex attributes, it looks like it would be safe to assume 16 based on the native limits:
The caveat here is that Metal's table will be shared between vertex buffers, argument buffers, individual buffer descriptors, and any internal buffers we have to provide.
kvark commentedon Apr 8, 2020
So here is an unfortunate but potentially valid scenario (numbers taken from https://gpuweb.github.io/gpuweb/#gpulimits).
Implementation on Metal has to support at least 4 bind groups. Each will take a slot in the vertex buffer table as an argument buffer, assuming they are supported. If they aren't... this gets much worse.
It has to support at least 8 dynamic uniform buffers and 4 dynamic storage buffers. How are the offsets passed? We were planning on just excluding the dynamic resources from the argument buffer and binding them separately. That would mean at least 12 more bindings taken.
Internally, it may need to bind a buffer with some extra data, say containing the size limits for out-of-bounds checking in the shader. That's one more buffer.
So what we end up is 17 buffers taken by non-vertex-attributes, and only 14 left for the vertex buffers for attributes, at most. Which means, an example implementation can't expose the base limit of 16 that is suggested above. A more complicated implementation could possibly put all the internal things together with dynamic offsets in a single buffer, but the feasibility of this path is yet to be considered in depth (and definitely needs to be discussed).
Another problem is that, if an implementation wants to report a higher limit for any of these, it can't do so for all the limits. I.e. supposing it has an extra buffer. It could either report it as a max bind groups limit of 5, or as a max dynamic storage buffer limit as 5, but not both, and hence - will not be able to run applications that it should be able to run.
I wonder if we can look at it together with #684 and say that:
That would be totally matching this case here with vertex attributes, and it would discourage users to just blindly request all the limits.
Kangz commentedon Apr 8, 2020
For the maximum stride we have found that 2048 is a good value and required at least by Vulkan. For attribute offsets, Metal requires they are aligned to 4, and that they stay entirely within a stride.
The limited amount of vertex buffers on Metal is a bit of an issue, but maybe this could be worked around by only allowing 14 or even a bit less vertex buffers? I'm not sure I see a usecase where there are more than a handful of vertex buffers.
kvark commentedon Apr 8, 2020
Yes, setting the base line to 14 would have worked, assuming a Metal implementation would report all the other limits to be the baseline as well, which could certainly be better. I.e. why would an app using 4 vertex buffers have to be limited in 4 bind groups?
Some context about the use cases that need many vertex buffers (namely, glTF import) - gfx-rs/wgpu#558 (comment) .
kainino0x commentedon Apr 14, 2020
From the editors meeting: we propose (IIRC):
kainino0x commentedon Jul 20, 2020
Resolved: take that proposal
Kangz commentedon Jul 20, 2020
(with the limit being 8)
jespertheend commentedon Dec 1, 2020
Would it be possible to have these limits added to GPULimits? So that these limits could be higher on platforms that support this. Or was that the idea already?
kvark commentedon Dec 1, 2020
@jespertheend yes, we need to add these to GPU limits, whoever got there first. There is an expectation that adding the limits comes with the validation statements about respecting them. If you want to help with a PR, please go for it!
jespertheend commentedon Dec 2, 2020
@kvark alright I’ll try to have a go at it tomorrow. I have never done a PR for a spec before so bear with me.
kainino0x commentedon Dec 2, 2020
FYI there's a "Limits.md" where we document where the minimums come from; we should add this to that doc (and link to this issue).
jespertheend commentedon Dec 3, 2020
Has a limit for the max attribute offset been chosen yet or should I skip that one for now?
kvark commentedon Dec 3, 2020
I think max attribute offset is not a separate limit, it can be derived from maximum stride (which is 2048)
jespertheend commentedon Dec 3, 2020
Ah I think that's step 3 at https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-gpuvertexbufferlayoutdescriptor then
jespertheend commentedon Dec 3, 2020
Alright I've added #1274, I'm not sure if the limit names are descriptive enough.
Let me know if I missed something.
Fix bugs introduced with memcpy (gpuweb#693)