Add multi draw indirect feature #2315

rconde01 · 2021-11-16T22:57:35Z

This PR adds a multi-draw-indirect feature and is meant to replace #1949

It is rebased over the now merged #2022

Fixes #1354

spec/index.bs

github-actions · 2021-11-23T08:34:10Z

Previews, as seen when this build job started (b073087):
WebGPU | IDL
WGSL
Explainer

kvark

Thank you!

spec/index.bs

AmesingFlank · 2022-05-01T22:42:26Z

Hey! Any updates on this? Can we expect this feature to be available in v1.0?

kainino0x · 2022-05-03T18:50:14Z

Since this is an optional feature I don't think it will be a blocking feature for V1 release. After V1 this will be a living spec, and we'll be doing incremental additions like this one (there won't be a wait for a V1.1 or V2).

kainino0x · 2022-09-14T19:38:51Z

Briefly discussed in meeting: We're going to keep this in polish post-v1, won't be able to spend the time to get it into the spec for V1. Will be soon after. Sorry to folks waiting for this!

rconde01 · 2023-10-23T20:27:44Z

@kainino0x We would like to restart this discussion based on your comment above. We're happy to assist in any investigations required to move this forward.

kainino0x · 2023-10-24T19:52:00Z

Let me propose this for Milestone 1 and add it to the agenda. Unfortunately we don't have a meeting this week, but hopefully next week (in the Pacific-timed meeting).

rconde01 · 2023-10-24T19:54:16Z

Sounds good.

spec/index.bs

kainino0x · 2023-10-24T21:01:39Z

spec/index.bs

+        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.


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.

spec/index.bs

kainino0x · 2023-10-24T22:13:48Z

spec/index.bs

+                     optional GPUSize64 drawCountOffset = 0);
+    undefined multiDrawIndexedIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset,
+                     GPUSize32 maxDrawCount,
+                     optional GPUBuffer drawCountBuffer = null,


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

kainino0x · 2023-10-24T22:27:24Z

If you make revisions on this PR, feel free to not deal with doing a rebase (though you can if you want to). We can deal with it later.

rconde01 · 2023-10-25T18:05:23Z

@kainino0x I think the feedback is addressed. I was able to do the rebase, but some of the editor notes got stomped.

rconde01 · 2023-10-25T18:25:53Z

spec/index.bs

@@ -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>


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.

rconde01 · 2023-11-01T20:57:38Z

spec/index.bs

+
+    undefined multiDrawIndirect(GPUBuffer indirectBuffer, GPUSize64 indirectOffset,
+        GPUSize32 maxDrawCount,
+        optional GPUBuffer drawCountBuffer = null,


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

kainino0x · 2024-06-20T18:05:51Z

spec/index.bs

+        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:
+


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

kainino0x · 2024-06-20T18:05:59Z

spec/index.bs

+        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:
+


kainino0x · 2024-06-20T18:06:37Z

^ just dropping some editorial notes for when we pick up this PR again

Sirtsu55 · 2024-06-25T12:43:51Z

spec/index.bs

+                        - |drawCountOffset| + sizeof([=indirect multiDraw count=]) &le;
+                            |drawCountBuffer|.{{GPUBuffer/[[size]]}}.
+                        - |drawCountOffset| is a multiple of 4.
+                    </div>


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

kainino0x reviewed Nov 18, 2021

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

kvark requested changes Feb 7, 2022

View reviewed changes

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

spec/index.bs Outdated Show resolved Hide resolved

kainino0x added this to the Polish post-V1 milestone May 3, 2022

kainino0x added the for webgpu editors meeting label Aug 25, 2022

kainino0x mentioned this pull request Aug 30, 2022

Add multi-draw-indirect feature. #1949

Closed

kainino0x removed the for webgpu editors meeting label Sep 14, 2022

kainino0x marked this pull request as draft September 22, 2022 16:16

kainino0x modified the milestones: Polish post-V1, Milestone 2? Aug 15, 2023

kainino0x modified the milestones: Polish post-V1, Milestone 0, Milestone 1 Oct 24, 2023

kainino0x mentioned this pull request Oct 24, 2023

Investigation: multi-draw-indirect #4349

Closed

kainino0x reviewed Oct 24, 2023

View reviewed changes

Add multi-draw-indirect feature.

a58d222

rconde01 force-pushed the add-multi-draw-indirect-feature branch from 4b20359 to a58d222 Compare October 25, 2023 18:02

rconde01 commented Oct 25, 2023

View reviewed changes

rconde01 commented Nov 1, 2023

View reviewed changes

mwyrzykowski self-requested a review November 1, 2023 23:12

JesseRMeyer mentioned this pull request Nov 8, 2023

DrawIndirectCount #1354

Open

vegarringdal mentioned this pull request Jan 25, 2024

BatchedMesh: Proposal mrdoob/three.js#22376

Closed

RenaudRohlinger mentioned this pull request Mar 18, 2024

WebGPURenderer: Support BatchedMesh. mrdoob/three.js#27937

Merged

kainino0x added the api WebGPU API label May 21, 2024

kainino0x reviewed Jun 20, 2024

View reviewed changes

Sirtsu55 reviewed Jun 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi draw indirect feature #2315

Add multi draw indirect feature #2315

rconde01 commented Nov 16, 2021 •

edited by kainino0x

Loading

github-actions bot commented Nov 23, 2021

kvark left a comment

AmesingFlank commented May 1, 2022

kainino0x commented May 3, 2022

kainino0x commented Sep 14, 2022

rconde01 commented Oct 23, 2023

kainino0x commented Oct 24, 2023

rconde01 commented Oct 24, 2023

kainino0x Oct 24, 2023 •

edited

Loading

kainino0x Oct 24, 2023

kainino0x commented Oct 24, 2023 •

edited

Loading

rconde01 commented Oct 25, 2023

rconde01 Oct 25, 2023

rconde01 Oct 26, 2023 •

edited

Loading

rconde01 Nov 1, 2023

mrshannon Nov 9, 2023

rconde01 Nov 9, 2023

kainino0x Jun 20, 2024

kainino0x Jun 20, 2024

kainino0x commented Jun 20, 2024

Sirtsu55 Jun 25, 2024

Add multi draw indirect feature #2315

Are you sure you want to change the base?

Add multi draw indirect feature #2315

Conversation

rconde01 commented Nov 16, 2021 • edited by kainino0x Loading

github-actions bot commented Nov 23, 2021

kvark left a comment

Choose a reason for hiding this comment

AmesingFlank commented May 1, 2022

kainino0x commented May 3, 2022

kainino0x commented Sep 14, 2022

rconde01 commented Oct 23, 2023

kainino0x commented Oct 24, 2023

rconde01 commented Oct 24, 2023

kainino0x Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kainino0x commented Oct 24, 2023 • edited Loading

rconde01 commented Oct 25, 2023

Choose a reason for hiding this comment

rconde01 Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kainino0x commented Jun 20, 2024

Choose a reason for hiding this comment

rconde01 commented Nov 16, 2021 •

edited by kainino0x

Loading

kainino0x Oct 24, 2023 •

edited

Loading

kainino0x commented Oct 24, 2023 •

edited

Loading

rconde01 Oct 26, 2023 •

edited

Loading