Repository navigation
maxBindingsPerBindGroup has arbitrary limit of 640 #3864
Description
I read over the discussion in #3279 but what is the usefulness of having a limit which is arbitrary if the purpose of the limit is to avoid driver issues with large binding indices?
Why not set this limit exactly to (max bindings per shader stage × max shader stages per pipeline) which I believe is 112 with default limits?
Or if there is a reason to use a higher limit, why not select such a value in a non-arbitrary fashion as opposed to something which is funny which seems unprofessional in a publicly documented W3C specification. E.g., at what point do the drivers have issues? > 2^16 - 1 or much lower?
Activity
Kangz commentedon Mar 6, 2023
In general I have a concern about how many limits we are starting to have that intersect each other: it will not be sufficient for developers to raise a single one, like
maxColorAttachments,maxFragmentCombinedOutputResourcesandmaxColorAttachmentBytesPerSample. So we'll be teaching developers to just directly copy the adapter limits into therequestDevicecall and not check / raise individual limits. Setting a "low" value ofmaxBindingsPerBindGroupruns into the same issue, although the current value is still a small-ish number compared to the previous 2^16-1. (a large space helps developers easily choose binding numbers automatically in such a way that they don't conflict).It isn't clear exactly when drivers start having issues, and these issues are something an implementation could paper over by compacting pipeline layouts (and requiring a pipeline layout to generate SPIR-V on these drivers). But to help implementations, while still having a large space for developers, we agreed to a lower limit for now, in between 112 and 2^16-1 so an arbitrary number was picked. It was 640 because it is a number with a relatively known connotation of being arbitrary in comp sci culture.
kainino0x commentedon Mar 8, 2023
There's a possible design for bindless in which the
max bindings per shader stagelimits become extremely large. I don't remember if that's why we decided to add a new limit but I think it is a good reason.Agreed with @Kangz's concerns though, the intersecting limits definitely need more thought.
kainino0x commentedon Mar 15, 2023
Re: 112: also, as was pointed out by someone in the meeting, there is significant value in being able to use sparse binding indices - we just wanted to make the sparse space reasonably-sized.
kainino0x commentedon Mar 15, 2023
Bindless is an argument not to just remove this limit from the limits struct and validate against
max bindings per shader stage * 10or something though.greggman commentedon Mar 15, 2023
I'm probably wrong but it seemed like no matter what is picked, 512, 640, 641, 789, 1000, 1024, the choice will be arbitrary. As such, if people are going to wonder "why this limit?" it seemed like adding a "why" section would be useful. (or not?) That, was my takeaway.
mwyrzykowski commentedon Mar 23, 2023
@litherum and I discussed internally and we would like to propose an arbitrary limit of
1000. Adding the "why" section seems useful as well.kainino0x commentedon Apr 4, 2023
I believe we resolved to use 1000. Moving to 'editorial' to implement that.
https://github.com/gpuweb/gpuweb/wiki/Minutes-2023-03-15