这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@mrshannon
Copy link
Contributor

@mrshannon mrshannon commented Aug 6, 2021

Combining indirect drawing with render bundles is a powerful way to shift work to the GPU. However, with firstInstance locked to 0 the bundle must use dynamic uniform/storage buffer offsets which cannot be changed once the bundle is encoded. With non-zero values of firstInstance the instance_index can be used to index into uniform/storage arrays inside the shader, allowing the GPU to change what is rendered each frame, greatly adding to the flexibility of render bundles.

This PR addresses adding an indirect-first-instance feature which aims to relax the requirement on firstInstance = 0 for drawIndirect and drawIndexedIndirect on hardware which supports firstInstance > 0.

NOTE: The decision to split this feature off from #1949 was made during the meeting on 2021-08-02

NOTE: This was removed from core (#1878) due to the 36% of Android devices which do not support firstInstance > 0.

The required backend capabilities to implement indirect-first-instance are available on:

  • 99% of Vulkan capable desktop GPUs
  • 64% of Vulkan capable Android devices
  • All DX12 devices
  • All Metal devices

In particular, the required capabilities are core to DX12 and Metal, but requires the drawIndirectFirstInstance feature on Vulkan. The corresponding parameter for each backend API are given in the table below:

Vulkan DX12 Metal
firstInstance StartInstanceLocation baseInstance

Preview | Diff

@kainino0x
Copy link
Contributor

kainino0x commented Aug 10, 2021

@tidoust ipr is blocking this change - does @mrshannon really need to be in the working group to land this change? I thought our model was to do IP rubber-stamping in bulk when the spec is imported from the community group to the working group.

@kainino0x
Copy link
Contributor

For the record: in the meeting we resolved to accept this change.

@tidoust
Copy link
Contributor

tidoust commented Aug 10, 2021

@tidoust ipr is blocking this change - does @mrshannon really need to be in the working group to land this change? I thought our model was to do IP rubber-stamping in bulk when the spec is imported from the community group to the working group.

The notion of "rubber-stamping" tends to irk legal departments... The goal of the IPR bot is to alert the Working Group about situations where it is effectively going to incorporate a substantive contribution from a participant who did not make the exact same IPR commitments as WG participants.

CG participants contributions are covered by the CLA. WG participants contributions are covered by the W3C Patent Policy. The CLA does provide some level of RF guarantees. The W3C Patent Policy is a bit stronger than the CLA. The goal is to secure as many RF guarantees as we can for the final W3C Recommendation.

To solve this:

@mrshannon
Copy link
Contributor Author

The only option that page currently lists is to join the GPU for the Web Working Group. It is very likely I can sign the FSA, but there is currently no means to continue along that route. I have already signed the CLA, when I joined the CG last year.

@mrshannon
Copy link
Contributor Author

I have permission to sign the FSA, but no way to sign it as the IPR is still blocked on WG only.

@kainino0x
Copy link
Contributor

kainino0x commented Aug 23, 2021

@tidoust can you confirm if there should be a way to sign the FSA without joining the working group?

Otherwise we can probably figure out how to get him into the WG as an invited expert? Or we can override the change but we'd want to get WG approval which we don't have a process for.

@Kangz
Copy link
Contributor

Kangz commented Sep 2, 2021

@tidoust how does one override the IPR bot? I see @Ash-Nazg mentioned often but I don't know who owns that github account.

@mrshannon has been blocked on this process for a fairly trivial contribution (adding an optional feature that lifts a restriction in the spec). How can he sign the FSA without joining the WG itself? It seems all that matters is the FSA.

@kdashg
Copy link
Contributor

kdashg commented Sep 2, 2021

FWIW we can always admin-override to merge it, once we know the FSA is signed.

@munrocket
Copy link
Contributor

Someone not from WG signed FSA? If yes, how they do that?

@mrshannon
Copy link
Contributor Author

I hate to bump this again, but is there any update on being able to sign the FSA as a CG member?

@Kangz
Copy link
Contributor

Kangz commented Nov 2, 2021

@tidoust this PR has been blocked for months on CG members not being able to sign the FSA. Any way we can escalate?

@tidoust
Copy link
Contributor

tidoust commented Nov 3, 2021

Oh no! Sorry this has languished for months, mentions of my name did not trigger the notifications I was expecting so I simply hadn't seen this.

There is a way to ask for non-member commitments through the IPR bot with two caveats:

  1. I, or someone else from the team, need to push the right button. So you were right to ping me and I'm very sorry that I missed these repeated pings :/
  2. That is per PR.

I just did that for this PR. @mrshannon, please check your email inbox for a link to the relevant online form.

@mrshannon
Copy link
Contributor Author

Agreement completed. Once this PR is merged I will rebase #1949 on this and ping that one as well. Thanks.

@tidoust
Copy link
Contributor

tidoust commented Nov 3, 2021

And IPR bot turned green as a result, so all good!

@Kangz Kangz force-pushed the add-indirect-first-instance-feature branch from b72313a to 6fb73a5 Compare November 9, 2021 09:14
@Kangz Kangz merged commit d2cbf2b into gpuweb:main Nov 9, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

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

github-actions bot added a commit that referenced this pull request Nov 9, 2021
SHA: d2cbf2b
Reason: push, by @Kangz

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Nov 9, 2021
SHA: d2cbf2b
Reason: push, by @Kangz

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Nov 9, 2021
SHA: d2cbf2b
Reason: push, by @Kangz

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants