I think this is the check I was missing above.

Just an editorial thing but we"ll need to move this to a content-timeline check that throws "TypeError" if the feature isn"t enabled, as WebIDL doesn"t let us dynamically decide whether to define something or not.

nit for editors: need to check if these GPUBuffers need to be nullable

A few thoughts here:

• Shouldn"t this also apply to the value in the drawCountBuffer?
• Does this mean that after enabling the feature you still need to request a higher limit or else will be stuck to 1? It"s workable but seems a bit weird.
• Seems a bit weird that this could be 1 when the feature is not enabled. In fact it seems weird the limit exists at all when the feature isn"t enabled...I don"t think there precedent for feature related limits though.
• @kvark had suggested in a previous comment to " lift the limit entirely...I.e. it becomes limited in the same way as maximum index count is limited." It"s not clear to me how "maximum index count is limited", other than it"s tied to the max buffer size. Maybe the feature should only be available when the implementation max count is >= maxStorageBufferBindingSize and then this limit goes away? Not sure how reasonable that is.

Shouldn"t this also apply to the value in the drawCountBuffer?

Realized this is already covered transitively since drawCountBuffer is already limited by maxDrawCount. Nm.

@Sirtsu55 is working on this feature in Dawn and we got to talking about the limit. I am not sure we need one - if it"s guaranteed to be large on all systems then we could just hardcode a limit, or derive a limit from maxBufferSize.

• Vulkan: has a limit but it"s always either 1 or huge, so when the feature is available we could just have a hardcoded limit as high as 2**30.
• D3D12: not sure yet, currently investingating.
• Metal: There is already a maxBufferSize which imposes some limit. On weaker devices like old iPhones (the ones where maxBufferSize is 256MiB) there"s a good chance this is the limiting factor. However the limiting factor wouldn"t be the size on the indirect buffer (256MiB/16B=16.6M draws or 256MiB/20B=13.4M indexed draws) but the size of the indirect command buffer (ICB) that we generate internally in our draw call validation/rerecording shader. There is a maxCommandCount when you allocate an ICB, but we haven"t found any documented limit on how large this can be. We don"t know what the size of a draw command is so we can"t seem to derive it from maxBufferLength - maybe in practice it"s something like maxBufferLength / 32 bytes (256MiB/32B = 8.3M). We will probably try probing the Metal validation layer, but @mwyrzykowski @litherum would you be able to tell us how to determine this limit - maybe what the validation layer checks and/or whether it would be large enough on all target systems (#1069) that we can hardcode it?

@kainino0x The metal validation layer does not appear to validate maxCommandCount specifically, though you may run into validation errors due to the maxBufferLength.

It appears we need the following to hold device.maxBufferLength <= 40 + maxCommandCount * ((2 + 2 * desc.maxVertexBufferBindCount + desc.maxFragmentBufferBindCount) * 8), where desc is the ICB descriptor.

I will need to check if that is strictly for validation or applies in general, we could start there as a limit.

@mwyrzykowski thanks! Since multidraw will always inherit vertex/fragment buffers, those should be 0, simplifying to 40 + maxCommandCount * 2 * 8. (Also, did you mean the reverse, ... <= maxBufferLength?)

Just want to verify that 16 bytes per command is correct. That is too small for an indexed indirect call (20 bytes) so I"m guessing it is 2 pointers, one which points to the indexed indirect parameters allocated in some other buffer. If so, does the size of that buffer impose a limit? Or will it split into multiple buffers if it doesn"t fit?

I"m also concerned that there will be three huge allocations, and we"ll run out of memory anyway: the user"s draw params, the ICB full of pointers, and Metal"s internal allocation(s) that hold draw params.

This is a great question and I don"t think we have an answer for it. Maybe it would be possible to expand multidraw calls in bundles out, into allocation space for many single-draw calls, which we can write to later? But that would leave (maxDrawCount - actual draw count) of unused draw calls in the bundle which is probably very inefficient.

Maybe we have to say multidraw is not supported in bundles to start.

That would certainly be easier, limiting this functionality to GPURenderPassEncoder

@kainino0x What should we do about the maxMultiDrawIndirectCount limit?

Just to put it out there:

From my investigation of D3D12 in WebGPU the limiting factor is the buffer size. I tried counts up to 2^26 in a native D3D12 app, it does not work. Vulkan has a property and Metal is untested.

I don"t know, I"d propose giving a hardcoded value of 10M or something to punt again on adding limits that are part of features (#4613). Unless we think that will be too small.

Of course we need to test that, and we also need to check that setting maxDrawCount=10M and having a small draw count in the drawCountBuffer can perform passably well. (It should, though in Dawn we need to fix https://crbug.com/367394543.)

from @kainino0x: nit for editors: need to check if these GPUBuffers need to be nullable

To match feature parity with D3D12 and Vulkan the drawCountBuffer should be nullable, in which case maxDrawCount becomes the draw count.

I think the question was about optional vs nullable

To answer this old question I think we should make it optional and not nullable. That should still allow null.

Allowing any of these (I believe) and combinations thereof:

pass.multiDrawIndirect(indirectBuffer, 0, 100);
pass.multiDrawIndirect(indirectBuffer, 0, 100, null);
pass.multiDrawIndirect(indirectBuffer, 0, 100, null, 0);
pass.multiDrawIndirect(indirectBuffer, 0, 100, null, null);
pass.multiDrawIndirect(indirectBuffer, 0, 100, undefined);
pass.multiDrawIndirect(indirectBuffer, 0, 100, undefined, 0);
pass.multiDrawIndirect(indirectBuffer, 0, 100, undefined, undefined);

we should call out here that indirect-first-instance is not required to use firstInstance here, since it is not obvious.

same

When implementing this for D3D12, the driver / validation layer checks if maxDrawCount exceeds the range of the indirect buffer. Specifically it returns E_INVALIDARG when closing the command list. We should add this to the validation. Something along the lines of: indirectOffset + sizeof(indirectDrawParams) * maxDrawCount should not be out of bounds of indirectBuffer

We won"t know the exact number of draw calls issued but maxDrawCount is guaranteed to be the maximum so drawCount should be incremented by it.

Would it make sense to allow any value of drawCountOffset if drawCountBuffer is not set?

I would probably just keep the validation, but we could also consider using overloads instead so that we can never have a drawCountOffset when there is no drawCountBuffer.

@kainino0x FYI I"m asking because of https://github.com/gpuweb/cts/pull/3962/files#diff-faa22eb9a32ec5f94def60bb63172dfef5a8907fd5102d49a8203c762d8ee57cR214

I don"t know what we"ll do here but two assorted notes:

• setVertexBuffer still validates the buffer offset when buffer is null
• I like the overload solution best but we should think about what it will look like in languages without overloads (C webgpu.h). I think we currently only have one overloaded method in the API (setBindGroup) and that overload doesn"t apply to C at all.
  • Two functions? Merge into one function where if drawCountBuffer is null, drawCountOffset {is ignored,must be 0,etc}

(Opening a thread here because the threads on this issue still track all the things we need to do before landing this feature in the spec.)

According to #1354 (comment) there may be limitations on what can be done on a command buffer in Metal if supportIndirectCommandBuffers is set to true. If so we may need to account for them in the spec. When we get a chance we"ll look into the issue/repro on the Chromium issue and see if we think it needs to be accounted for in the spec. (Or maybe someone can find out whether wgpu has this problem and has already found a limitation or workaround.)

@kainino0x WebKit always sets supportIndirectCommandBuffers to YES for all PSOs and we have no such issue problem running three.js"s examples - https://github.com/search?q=repo:WebKit/WebKit supportIndirectCommandBuffers&type=code

I think it is because our implementation is based on Argument Buffers, if Chrome is planning to adopt that then that would be an option without changing the specification.

But setting supportIndirectCommandBuffers on the PSO does have a performance penalty, so I wouldn"t rule out a spec change if we don"t want to always enable this.

That"s super useful info, thank you!

One such option would be extending GPUShaderModuleCompilationHint to indicate whether or not an entry point from a given shader module will be used with MTLIndirectCommandBuffer

If we want to explore that route, it would be nice to apply this to GPURenderBundle as well. I.e., if you set the flag to 0 indicating you will not use ICBs with a given PSO then you can not call executeBundles(...) from the RenderPassEncoder which uses that given entry point.

Then at GPURenderPipelineCreation, an implementation can lookup if the given entry point needs to set supportIndirectCommandBuffers to YES or not and potentially impact codegen as well