Skip to content
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

getBindGroupLayout #446

Closed
Kangz opened this issue Sep 27, 2019 · 2 comments
Closed

getBindGroupLayout #446

Kangz opened this issue Sep 27, 2019 · 2 comments
Assignees
Projects

Comments

@Kangz
Copy link
Contributor

Kangz commented Sep 27, 2019

Motivation

Today GPUBindGroupLayout and GPUPipelineLayout are the most complicated part of the API. How about we add some defaults to them?

Namely it would change this sample code:

const bindGroupLayout = device.createBindGroupLayout({
  bindings: [
    {binding: 0, visibility: GPUShaderStage.COMPUTE, type: "storage-buffer"},
    {binding: 1, visibility: GPUShaderStage.COMPUTE, type: "storage-buffer"},
    {binding: 2, visibility: GPUShaderStage.COMPUTE, type: "storage-buffer"}
  ]
});

const bindGroup = device.createBindGroup({
  layout: bindGroupLayout,
  bindings: [
    {binding: 0, resource: {buffer: gpuBufferFirstMatrix}},
    {binding: 1, resource: {buffer: gpuBufferSecondMatrix}},
    {binding: 2, resource: {buffer: resultMatrixBuffer}}
  ]
});

const computeShaderCode = `#version 450
  layout(std430, set = 0, binding = 0) readonly buffer FirstMatrix {
  } firstMatrix;

  layout(std430, set = 0, binding = 1) readonly buffer SecondMatrix {
  } secondMatrix;

  layout(std430, set = 0, binding = 2) buffer ResultMatrix {
  } resultMatrix;`;

const computePipeline = device.createComputePipeline({
  layout: device.createPipelineLayout({
    bindGroupLayouts: [bindGroupLayout]
  }),
  computeStage: {
    module: device.createShaderModule({code: theCode}),
    entryPoint: "main"
  }
});

into

const bindGroup = device.createBindGroup({
  bindings: [
    {binding: 0, resource: {buffer: gpuBufferFirstMatrix}},
    {binding: 1, resource: {buffer: gpuBufferSecondMatrix}},
    {binding: 2, resource: {buffer: resultMatrixBuffer}}
  ]
});

const computeShaderCode = `#version 450
  layout(std430, set = 0, binding = 0) readonly buffer FirstMatrix {
  } firstMatrix;

  layout(std430, set = 0, binding = 1) readonly buffer SecondMatrix {
  } secondMatrix;

  layout(std430, set = 0, binding = 2) buffer ResultMatrix {
  } resultMatrix;`;

const computePipeline = device.createComputePipeline({
  computeStage: {
    module: device.createShaderModule({code: theCode}),
    entryPoint: "main"
  }
});

which is much easier to understand for people getting started with WebGPU, and almost 50% less code to type for experienced WebGPU developers hacking with the API.

How would this work

It makes the layout member of GPUPipelineDescriptorBase and GPUBindGroupDescriptor optional and if not present deduces them using the following algorithms.

For GPUBindGroupDescriptor it implicitly creates a GPUBindGroupLayout that one binding (with the same binding number) for each binding in the GPUBindGroupDescriptor. visibility is always all the stage and other member are set like so (if not described, they get their default value):

  • If the resource is a GPUSampler, type is "sampler"
  • If the resource is a GPUTextureView, if the texture has not exactly one of the SAMPLED and STORAGE usages, an error is produced. type is "sampled-texture" if the texture has the SAMPLED usage, "storage-texture" otherwise. textureDimension is set to the texture view dimension of the view, and textureComponentType set to the component type of the texture's format. If the texture has a sampleCount larger than 1, multisampled is set so true.
  • If the resource is a GPUBufferBinding, if the buffer has not exactly one of the UNIFORM and STORAGE usages, an error is produced. type is "uniform-buffer" if the buffer has the UNIFORM usage, "storage-buffer" otherwise.

For GPUComputePipeline and GPURenderPipeline we would deduce an implicit GPUPipelineLayout that has implicit GPUBindGroupLayouts for each set used by the pipeline. visibility is set to all shader stages. Then for each binding of the shader modules of the pipeline a binding is inserted in the correct GPUBindGroupLayout with the matching binding like the following (members not mentioned get their default value):

  • For sampler bindings in the shadermodule, type is set to "sampler".
  • For uniform buffer bindings, type is set to "uniform-buffer".
  • For storage buffer bindings, type is set to "storage-buffer".
  • For texture bindings, if the texture (SPIRV-ism) has the Sampled bit set to 0, type is set to storage-texture, sampled-texture otherwise. textureViewDimension is set to match the texture binding declaration, as is multisampled and textureComponentType.

Note that for render pipeline this would also require checking no two bindings conflict.

@Kangz
Copy link
Contributor Author

Kangz commented Oct 10, 2019

Proposal 2: GPUPipelineBase.getBindGroupLayout

The only change compared to the first proposal is the addition of layout: computePipeline.getBindGroupLayout(0), in the GPUBindGroupDescriptor.

const computeShaderCode = `#version 450
  layout(std430, set = 0, binding = 0) readonly buffer FirstMatrix {
  } firstMatrix;

  layout(std430, set = 0, binding = 1) readonly buffer SecondMatrix {
  } secondMatrix;

  layout(std430, set = 0, binding = 2) buffer ResultMatrix {
  } resultMatrix;`;

const computePipeline = device.createComputePipeline({
  computeStage: {
    module: device.createShaderModule({code: theCode}),
    entryPoint: "main"
  }
});

const bindGroup = device.createBindGroup({
  layout: computePipeline.getBindGroupLayout(0),
  bindings: [
    {binding: 0, resource: {buffer: gpuBufferFirstMatrix}},
    {binding: 1, resource: {buffer: gpuBufferSecondMatrix}},
    {binding: 2, resource: {buffer: resultMatrixBuffer}}
  ]
});

The GPUPipelineLayout if not present is created implicitly as described in the proposal above. A new method is added to GPUPipelineBase that returns the GPUBindGroupLayout at groupIndex for the pipeline.

partial interface GPUPipelineBase {
  GPUBindGroupLayout getBindGroupLayout(unsigned int groupIndex);
};

Open question:

The only difficulty seems to be whether the returned GPUBindGroupLayout is a new JS object wrapping the [[bindGroupLayout]] or if it has to be the same JS object as what was specified in the GPUPipelineLayoutDescriptor used for creation the layout member of GPUPipelineDescriptorBase, if present. And whether deduplication of equivalent bind group layouts is also done at the JS object level.

Deduplication of equivalent BGLs in the client side would make multiprocess implementations of WebGPU extremely sad. For the rest, making JS objects the same as the one in GPUPipelineLayoutDescriptor would be sad but tractable if needed. Making a new JS object wrapper every time would be slightly worse for GC (not that bad imho) but makes less sense for the JS developer using WebGPU.

@kvark
Copy link
Contributor

kvark commented Oct 10, 2019

This is much better, thank you! So the only "guessing" (or deriving) of the BGL entries would be from the shader at the pipeline creation.

I do wonder though, how would this play with WSL shaders? Overall, since we have to write down in the specification the deriving rules from the shading language, this PR might be blocked until we settle on what that language is.

Open question

Can we just return the same object without de-duplication?

@JusSn JusSn added this to Specification in Main Oct 14, 2019
@kainino0x kainino0x moved this from Needs Specification to Needs Testing in Main May 7, 2020
@kainino0x kainino0x changed the title Defaults for BindGroupLayouts May 7, 2020
@Kangz Kangz closed this as completed Jun 4, 2021
@kainino0x kainino0x moved this from Needs CTS Issue to Specification Done in Main Jan 15, 2022
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this issue Sep 6, 2022
* Loosen precision in multisample resolve operation test

Some platforms produce 0x7f and others 0x80 when a pixel is
half-covered.

* rebase / address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants