Repository navigation
Add multi draw indirect feature #2315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1828,6 +1828,12 @@ A <dfn dfn>supported limits</dfn> object has a value for every limit defined by | |||||||
| <tr class=row-continuation><td colspan=4> | ||||||||
| The maximum value for the arguments of | ||||||||
| {{GPUComputePassEncoder/dispatchWorkgroups(workgroupCountX, workgroupCountY, workgroupCountZ)}}. | ||||||||
|
|
||||||||
| <tr><td><dfn>maxMultiDrawIndirectCount</dfn> | ||||||||
| <td>{{GPUSize32}} <td>[=limit class/maximum=] <td>1 | ||||||||
| <tr class=row-continuation><td colspan=4> | ||||||||
| The maximum value for the `maxDrawCount` argument in | ||||||||
| {{GPURenderCommandsMixin/multiDrawIndirect()}} and {{GPURenderCommandsMixin/multiDrawIndexedIndirect()}}. | ||||||||
| </table> | ||||||||
|
|
||||||||
| <h5 id=gpusupportedlimits data-dfn-type=interface>`GPUSupportedLimits` | ||||||||
|
|
@@ -1874,6 +1880,7 @@ interface GPUSupportedLimits { | |||||||
| readonly attribute unsigned long maxComputeWorkgroupSizeY; | ||||||||
| readonly attribute unsigned long maxComputeWorkgroupSizeZ; | ||||||||
| readonly attribute unsigned long maxComputeWorkgroupsPerDimension; | ||||||||
| readonly attribute unsigned long maxIndirectDrawCount; | ||||||||
| }; | ||||||||
| </script> | ||||||||
|
|
||||||||
|
|
@@ -2847,6 +2854,7 @@ enum GPUFeatureName { | |||||||
| "rg11b10ufloat-renderable", | ||||||||
| "bgra8unorm-storage", | ||||||||
| "float32-filterable", | ||||||||
| "multi-draw-indirect", | ||||||||
| }; | ||||||||
| </script> | ||||||||
|
|
||||||||
|
|
@@ -11516,6 +11524,15 @@ interface mixin GPURenderCommandsMixin { | |||||||
|
|
||||||||
| undefined drawIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset); | ||||||||
| undefined drawIndexedIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset); | ||||||||
|
|
||||||||
| undefined multiDrawIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset, | ||||||||
| GPUSize32 maxDrawCount, | ||||||||
| optional GPUBuffer drawCountBuffer = null, | ||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from @kainino0x: nit for editors: need to check if these GPUBuffers need to be nullable
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To match feature parity with D3D12 and Vulkan the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the question was about optional vs nullable
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To answer this old question I think we should make it optional and not nullable. That should still allow
Suggested change
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); |
||||||||
| optional GPUSize64 drawCountOffset = 0); | ||||||||
| undefined multiDrawIndexedIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset, | ||||||||
| GPUSize32 maxDrawCount, | ||||||||
| optional GPUBuffer drawCountBuffer = null, | ||||||||
| optional GPUSize64 drawCountOffset = 0); | ||||||||
| }; | ||||||||
| </script> | ||||||||
|
|
||||||||
|
|
@@ -11985,6 +12002,132 @@ It must only be included by interfaces which also include those mixins. | |||||||
| with the states from |passState| and |renderState|. | ||||||||
| </div> | ||||||||
| </div> | ||||||||
|
|
||||||||
| : <dfn>multiDrawIndirect(indirectBuffer, indirectOffset, maxDrawCount, drawCountBuffer, drawCountOffset)</dfn> | ||||||||
| :: | ||||||||
| Draws primitives using an array of parameters read from a {{GPUBuffer}}. | ||||||||
| See [[#rendering-operations]] for the detailed specification. | ||||||||
|
|
||||||||
| This method is defined if and only if the {{GPUFeatureName/"multi-draw-indirect"}} [=feature=] is enabled. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||||
|
|
||||||||
| The <dfn dfn for=>indirect multiDraw parameters</dfn> encoded in the |indirectBuffer| must be zero or more | ||||||||
| tightly packed blocks of **four 32-bit unsigned integer values (16 bytes total)**, given in the same | ||||||||
| order as the arguments for {{GPURenderCommandsMixin/draw()}}. For example: | ||||||||
|
|
||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should call out here that |
||||||||
| <pre highlight="js"> | ||||||||
| let drawIndirectParameters = new Uint32Array(8); | ||||||||
| // first draw | ||||||||
| drawIndirectParameters[0] = vertexCount0; | ||||||||
| drawIndirectParameters[1] = instanceCount0; | ||||||||
| drawIndirectParameters[2] = firstVertex0; | ||||||||
| drawIndirectParameters[3] = firstInstance0; | ||||||||
| // second draw | ||||||||
| drawIndirectParameters[4] = vertexCount1; | ||||||||
| drawIndirectParameters[5] = instanceCount1; | ||||||||
| drawIndirectParameters[6] = firstVertex1; | ||||||||
| drawIndirectParameters[7] = firstInstance1; | ||||||||
| </pre> | ||||||||
|
|
||||||||
| The <dfn dfn for=>indirect multiDraw count</dfn> encoded in the |drawCountBuffer| must be a **32-bit unsigned | ||||||||
| integer** giving the number of draws to issue from the |indirectBuffer|. | ||||||||
|
|
||||||||
| <div algorithm="GPURenderCommandsMixin.multiDrawIndirect"> | ||||||||
| **Called on:** {{GPURenderCommandsMixin}} this. | ||||||||
|
|
||||||||
| **Arguments:** | ||||||||
| <pre class=argumentdef for="GPURenderCommandsMixin/multiDrawIndirect(indirectBuffer, indirectOffset, maxDrawCount, drawCountBuffer, drawCountOffset)"> | ||||||||
| |indirectBuffer|: Buffer containing the [=indirect multiDraw parameters=]. | ||||||||
| |indirectOffset|: Offset in bytes into |indirectBuffer| where the drawing data begins. | ||||||||
| |maxDrawCount|: Maximum number of draws or exact number of draws if |drawCountBuffer| is `null`. | ||||||||
| |drawCountBuffer|: Buffer containing the [=indirect multiDraw count=]. | ||||||||
| |drawCountOffset|: Offset in bytes into |drawCountBuffer| where the [=indirect multiDraw count=] is located. | ||||||||
| </pre> | ||||||||
|
|
||||||||
| **Returns:** {{undefined}} | ||||||||
|
|
||||||||
| Issue the following steps on the [=Device timeline=] of |this|.{{GPUObjectBase/[[device]]}}: | ||||||||
| <div class=device-timeline> | ||||||||
| 1. If any of the following conditions are unsatisfied, make |this| [=invalid=] and stop. | ||||||||
| <div class=validusage> | ||||||||
| - It is [$valid to draw$] with |this|. | ||||||||
| - |indirectBuffer| is [$valid to use with$] |this|. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kainino0x WebKit always sets 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's super useful info, thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Then at GPURenderPipelineCreation, an implementation can lookup if the given entry point needs to set |
||||||||
| - |indirectBuffer|.{{GPUBuffer/[[usage]]}} contains {{GPUBufferUsage/INDIRECT}}. | ||||||||
| - |indirectOffset| + sizeof([=indirect multiDraw parameters=]) ≤ | ||||||||
| |indirectBuffer|.{{GPUBuffer/[[size]]}}. | ||||||||
| - |indirectOffset| is a multiple of 4. | ||||||||
| - |maxDrawCount| ≤ limits.{{supported limits/maxMultiDrawIndirectCount}}. | ||||||||
| - |drawCountBuffer| is [$valid to use with$] |this|. | ||||||||
| - |drawCountBuffer|.{{GPUBuffer/[[usage]]}} contains {{GPUBufferUsage/INDIRECT}}. | ||||||||
| - |drawCountOffset| + sizeof([=indirect multiDraw count=]) ≤ | ||||||||
| |drawCountBuffer|.{{GPUBuffer/[[size]]}}. | ||||||||
| - |drawCountOffset| is a multiple of 4. | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to allow any value of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably just keep the validation, but we could also consider using overloads instead so that we can never have a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know what we'll do here but two assorted notes:
|
||||||||
| </div> | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When implementing this for D3D12, the driver / validation layer checks if maxDrawCount exceeds the range of the indirect buffer. Specifically it returns |
||||||||
| 1. Add |indirectBuffer| and |drawCountBuffer| (if given) to the [=usage scope=] as [=internal usage/input=]. | ||||||||
| </div> | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We won't know the exact number of draw calls issued but |
||||||||
| </div> | ||||||||
|
|
||||||||
| : <dfn>multiDrawIndexedIndirect(indirectBuffer, indirectOffset, maxDrawCount, drawCountBuffer, drawCountOffset)</dfn> | ||||||||
| :: | ||||||||
| Draws indexed primitives using an array of parameters read from a {{GPUBuffer}}. | ||||||||
| See [[#rendering-operations]] for the detailed specification. | ||||||||
|
|
||||||||
| The <dfn dfn for=>indirect multiDrawIndexed parameters</dfn> encoded in the |indirectBuffer| must be zero or more | ||||||||
| tightly packed blocks of **five 32-bit unsigned integer values (20 bytes total)**, given in | ||||||||
| the same order as the arguments for {{GPURenderCommandsMixin/drawIndexed()}}. For example: | ||||||||
|
|
||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||||||||
| <pre highlight="js"> | ||||||||
| let drawIndexedIndirectParameters = new Uint32Array(10); | ||||||||
| // first draw | ||||||||
| drawIndexedIndirectParameters[0] = indexCount0; | ||||||||
| drawIndexedIndirectParameters[1] = instanceCount0; | ||||||||
| drawIndexedIndirectParameters[2] = firstIndex0; | ||||||||
| drawIndexedIndirectParameters[3] = baseVertex0; | ||||||||
| drawIndexedIndirectParameters[4] = firstInstance0; | ||||||||
| // second draw | ||||||||
| drawIndexedIndirectParameters[5] = indexCount1; | ||||||||
| drawIndexedIndirectParameters[6] = instanceCount1; | ||||||||
| drawIndexedIndirectParameters[7] = firstIndex1; | ||||||||
| drawIndexedIndirectParameters[8] = baseVertex1; | ||||||||
| drawIndexedIndirectParameters[9] = firstInstance1; | ||||||||
| </pre> | ||||||||
|
|
||||||||
| The <dfn dfn for=>indirect multiDrawIndexed count</dfn> encoded in the |drawCountBuffer| must be a **32-bit unsigned | ||||||||
| integer** giving the number of draws to issue from the |indirectBuffer|. | ||||||||
|
|
||||||||
| <div algorithm="GPURenderCommandsMixin.multiDrawIndexedIndirect"> | ||||||||
| **Called on:** {{GPURenderCommandsMixin}} this. | ||||||||
|
|
||||||||
| **Arguments:** | ||||||||
| <pre class=argumentdef for="GPURenderCommandsMixin/multiDrawIndexedIndirect(indirectBuffer, indirectOffset, maxDrawCount, drawCountBuffer, drawCountOffset)"> | ||||||||
| |indirectBuffer|: Buffer containing the [=indirect multiDrawIndexed parameters=]. | ||||||||
| |indirectOffset|: Offset in bytes into |indirectBuffer| where the drawing data begins. | ||||||||
| |maxDrawCount|: Maximum number of draws or exact number of draws if |drawCountBuffer| is `null`. | ||||||||
| |drawCountBuffer|: Buffer containing the [=indirect multiDrawIndexed count=]. | ||||||||
| |drawCountOffset|: Offset in bytes into |drawCountBuffer| where the [=indirect multiDrawIndexed count=] is located. | ||||||||
| </pre> | ||||||||
|
|
||||||||
| **Returns:** {{undefined}} | ||||||||
|
|
||||||||
| Issue the following steps on the [=Device timeline=] of |this|.{{GPUObjectBase/[[device]]}}: | ||||||||
| <div class=device-timeline> | ||||||||
| 1. If any of the following conditions are unsatisfied, make |this| [=invalid=] and stop. | ||||||||
| <div class=validusage> | ||||||||
| - It is [$valid to draw indexed$] with |this|. | ||||||||
| - |indirectBuffer| is [$valid to use with$] |this|. | ||||||||
| - |indirectBuffer|.{{GPUBuffer/[[usage]]}} contains {{GPUBufferUsage/INDIRECT}}. | ||||||||
| - |indirectOffset| + sizeof([=indirect multiDrawIndexed parameters=]) ≤ | ||||||||
| |indirectBuffer|.{{GPUBuffer/[[size]]}}. | ||||||||
| - |indirectOffset| is a multiple of 4. | ||||||||
| - |maxDrawCount| ≤ limits.{{supported limits/maxMultiDrawIndirectCount}}. | ||||||||
| - |drawCountBuffer| is [$valid to use with$] |this|. | ||||||||
| - |drawCountBuffer|.{{GPUBuffer/[[usage]]}} contains {{GPUBufferUsage/INDIRECT}}. | ||||||||
| - |drawCountOffset| + sizeof([=indirect multiDrawIndexed count=]) ≤ | ||||||||
| |drawCountBuffer|.{{GPUBuffer/[[size]]}}. | ||||||||
| - |drawCountOffset| is a multiple of 4. | ||||||||
| </div> | ||||||||
| 1. Add |indirectBuffer| and |drawCountBuffer| (if given) to the [=usage scope=] as [=internal usage/input=]. | ||||||||
| </div> | ||||||||
| </div> | ||||||||
| </dl> | ||||||||
|
|
||||||||
| <div algorithm> | ||||||||
|
|
@@ -15350,6 +15493,25 @@ This feature adds no [=optional API surfaces=]. | |||||||
| Makes textures with formats {{GPUTextureFormat/"r32float"}}, {{GPUTextureFormat/"rg32float"}}, and | ||||||||
| {{GPUTextureFormat/"rgba32float"}} [=filterable=]. | ||||||||
|
|
||||||||
| <h3 id=multi-draw-indirect data-dfn-type=enum-value data-dfn-for=GPUFeatureName>`"multi-draw-indirect"` | ||||||||
| <span id=dom-gpufeaturename-multi-draw-indirect></span> | ||||||||
| </h3> | ||||||||
|
|
||||||||
| Allows the use of the {{GPURenderCommandsMixin/multiDrawIndirect()}} and | ||||||||
| {{GPURenderCommandsMixin/multiDrawIndexedIndirect()}} methods. | ||||||||
|
|
||||||||
| **Feature Methods** | ||||||||
|
|
||||||||
| The following methods are supported if and only if the {{GPUFeatureName/"multi-draw-indirect"}} | ||||||||
| [=feature=] is enabled. | ||||||||
|
|
||||||||
| <dl> | ||||||||
| : {{GPURenderCommandsMixin}} | ||||||||
| :: | ||||||||
| * {{GPURenderCommandsMixin/multiDrawIndirect()}} | ||||||||
| * {{GPURenderCommandsMixin/multiDrawIndexedIndirect()}} | ||||||||
| </dl> | ||||||||
|
|
||||||||
| # Appendices # {#appendices} | ||||||||
|
|
||||||||
| ## Texture Format Capabilities ## {#texture-format-caps} | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few thoughts here:
drawCountBuffer?maxStorageBufferBindingSizeand then this limit goes away? Not sure how reasonable that is.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realized this is already covered transitively since drawCountBuffer is already limited by maxDrawCount. Nm.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.2**30.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?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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), wheredescis 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would certainly be easier, limiting this functionality to GPURenderPassEncoder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kainino0x What should we do about the
maxMultiDrawIndirectCountlimit?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)