Describe validation and creation of GPUBindGroupLayouts #427

JusSn · 2019-08-26T23:15:00Z

Not sure why the two commits from master aren't disappearing when I rebase and merge. I can squash all if needed.

In shaders there are several texture types for each dimensionality depending on their component type. It can be either float, uint or sint, with maybe in the future depth/stencil if WebGPU allows reading such textures. The component type of a GPUTextureView's format must match the component type of its binding in the shader module. This is for several reasons: - Vulkan requires the following: "The Sampled Type of an OpTypeImage declaration must match the numeric format of the corresponding resource in type and signedness, as shown in the SPIR-V Sampled Type column of the Interpretation of Numeric Format table, or the values obtained by reading or sampling from this image are undefined." - It is also required in OpenGL for the texture units to be complete, a uint or sint texture unit used with a non-nearest sampler is incomplete and returns black texels. Similar constraints must exist in other APIs. To encode this compatibility constraint, a new member is added to GPUBindGroupLayoutBinding that is a new enum GPUTextureComponentType that give the component type of the texture.

This is the most common case and avoids having an optional dictionary member with no default value (but that still requires a value for texture bindings).

Kangz · 2019-08-27T10:12:17Z

spec/index.bs

@@ -188,6 +188,11 @@ dictionary GPUExtensions {
 <script type=idl>
 dictionary GPULimits {
    u32 maxBindGroups = 4;
+    u32 maxSampledTexturesPerShaderStage = 16;


Are these meant to be per bindgroup per stage or per pipeline per stage?

Per bind group, per stage. A single pipeline may share some of the same limits (it looks like this is how D3D12 works), but those limits aren't enforceable until createPipelineLayout.

Makes sense thanks for the explanation.

Maybe we can come up with a clearer name here, like PerStagePerBindGroup? verbose though.

this semantics indeed needs to be better reflected in the name (unfortunately for that name)

JusSn · 2019-08-27T19:08:37Z

Additional discussion about GPUBindGroupLayout limits started in #409; I'm going to continue there.

JusSn · 2019-08-27T19:17:07Z

I also want to incorporate conclusions from #406, but this can be done whenever.

Kangz · 2019-08-29T08:16:59Z

spec/index.bs

+
+### {{GPUDevice/createBindGroupLayout()|GPUDevice.createBindGroupLayout(GPUBindGroupLayoutDescriptor)}} ### {#GPUDevice-createBindGroupLayout}
+
+A valid {{GPUBindGroupLayout}} object has an <dfn>internal map</dfn> of |u32| <dfn>binding number</dfn>s to internal <dfn>layout binding</dfn> objects. 


This means that the binding numbers can be arbitrary. That sounds good but our implementation currently requires bindings numbers between 0 and 15 and would likely introduce a map in the BGL from application-facing binding numbers to packed bindings numbers in 0..N.

These numbers exist in an internal map so this isn't the place to specify the range. We could do that on input (GPUBindGroupLayoutDescriptor processing).

Can you clarify if you are going to have a problem with arbitrary binding numbers or not? Our implementation works, and I could think of a case where the users would want the numbers to exceed 15.

We aren't going to have problems with arbitrary numbers, sorry for the confusion (we just need to do a bit more work). I just wanted to make sure this was done being aware that an alternative was to require numbers in a small range corresponding to maxBindingsPerGroup.

@Kangz Do you mean you'd like this text to change at all, or are you just pointing out the work you have to do on your end?

Allowing for arbitrary indices is easier for developers and trivial to reorder inside the implementation.

@JusSn I'm pointing out an alternative that I would like considered if at least for a couple seconds. @litherum just did and I don't have a strong opinion on the subject so being nice to developers sounds good.

Kangz · 2019-08-29T08:18:07Z

spec/index.bs

+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/binding}} is already present in |layout|'s [=internal map=], [=return with error=].
+    1. Let |layoutBinding| be a new internal [=layout binding=] object.
+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/type}} is {{GPUBindingType/uniform-buffer}}:
+        1. If |layout|'s [=internal map=] contains {{GPULimits/maxUniformBuffersPerShaderStage|GPULimits.maxUniformBuffersPerShaderStage}} [=layout binding=]s of [=type=] {{GPUBindingType/uniform-buffer}} that all share any [=visibility=] bit contained in |bindingDescriptor|.{{GPUBindGroupLayoutBinding/visibility}}, [=return with error=].


I'd add these validations at the end of the algorithm even if that would be allowed by implementations under the as-if rule. Basically this makes validation O(n2) instead of O(n)

+1, I think it would be much easier to read too.

it's hard to read those long lines, I'll be happy to take another look once this is refactored according to this suggestion

I've just finished refactoring a bunch of stuff. PTAL!

Kangz

Behavior described LGTM, @kvark and @kainino0x can you TAL too?

spec/index.bs

kainino0x · 2019-08-29T18:47:51Z

spec/index.bs

+
+### {{GPUDevice/createBindGroupLayout()|GPUDevice.createBindGroupLayout(GPUBindGroupLayoutDescriptor)}} ### {#GPUDevice-createBindGroupLayout}
+
+A valid {{GPUBindGroupLayout}} object has an <dfn>internal map</dfn> of |u32| <dfn>binding number</dfn>s to internal <dfn>layout binding</dfn> objects. 


These numbers exist in an internal map so this isn't the place to specify the range. We could do that on input (GPUBindGroupLayoutDescriptor processing).

spec/index.bs

kainino0x · 2019-08-29T21:52:41Z

spec/index.bs

+a {{GPUTextureComponentType}} <dfn>textureComponentType</dfn> which must be initially <code>"float</code>, and a <dfn>multisampled</dfn> boolean which must be initially <code>false</code>.
+
+<div algorithm="GPUDevice.createBindGroupLayout(descriptor)">
+Any time during this algorithm, to <dfn>return with error</dfn> is to:


I think dfns are global to the spec, so this won't end up staying within this algorithm.

Ultimately we SHOULD define this globally though. So ok to keep as is for now.

kainino0x · 2019-08-29T22:01:07Z

spec/index.bs

+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/binding}} is already present in |layout|'s [=internal map=], [=return with error=].
+    1. Let |layoutBinding| be a new internal [=layout binding=] object.
+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/type}} is {{GPUBindingType/uniform-buffer}}:
+        1. If |layout|'s [=internal map=] contains {{GPULimits/maxUniformBuffersPerShaderStage|GPULimits.maxUniformBuffersPerShaderStage}} [=layout binding=]s of [=type=] {{GPUBindingType/uniform-buffer}} that all share any [=visibility=] bit contained in |bindingDescriptor|.{{GPUBindGroupLayoutBinding/visibility}}, [=return with error=].


+1, I think it would be much easier to read too.

spec/index.bs

kainino0x · 2019-08-29T22:07:42Z

spec/index.bs

+        1. Set [=dynamic=] of |layoutBinding| to the value of |bindingDescriptor|.{{GPUBindGroupLayoutBinding/dynamic}}.
+    1. If [=type=] is {{GPUBindingType/sampled-texture}} or {{GPUBindingType/storage-texture}}:
+        1. If [=type=] is {{GPUBindingType/sampled-texture}} and if |layout|'s [=internal map=] contains {{GPULimits/maxSampledTexturesPerShaderStage|GPULimits.maxSampledTexturesPerShaderStage}} [=layout binding=]s of [=type=] {{GPUBindingType/sampled-texture}} that all share any [=visibility=] bit contained in |bindingDescriptor|.{{GPUBindGroupLayoutBinding/visibility}}, [=return with error=].
+        1. If [=type=] is {{GPUBindingType/storage-texture}} and if |layout|'s [=internal map=] contains {{GPULimits/maxStorageTexturesPerShaderStage|GPULimits.maxStorageTexturesPerShaderStage}} [=layout binding=]s of [=type=] {{GPUBindingType/storage-texture}} that all share any [=visibility=] bit contained in |bindingDescriptor|.{{GPUBindGroupLayoutBinding/visibility}}, [=return with error=].


Seems like we should also say "if dynamic is set, return with error", and similar for the other cases too.
(Unfortunate, because it's kind of verbose; maybe it can be condensed.)

IDL wise, I wonder if there's any way we can split GPUBindGroupLayoutBinding into 4 different dictionaries.

I'd also prefer that we split these GPUBindGroupLayoutBinding cases into different sub-classes/dictionaries rather than differentiating by type enum. However, if we keep it as is, I propose we ignore "dynamic" if the resource is not a buffer (maybe issue a warning?), and so on, instead of aborting GPUBindGroupLayout creation. It keeps the API more flexible.

kvark · 2019-09-05T17:26:19Z

spec/index.bs

@@ -188,6 +188,11 @@ dictionary GPUExtensions {
 <script type=idl>
 dictionary GPULimits {
    u32 maxBindGroups = 4;
+    u32 maxSampledTexturesPerShaderStage = 16;


this semantics indeed needs to be better reflected in the name (unfortunately for that name)

spec/index.bs

kvark · 2019-09-05T17:39:15Z

spec/index.bs

+  * {{GPUBindGroupLayoutBinding/textureDimension}}, {{GPUBindGroupLayoutBinding/multisampled}}:
+    The dimensions and multi-sampling properties of texture bindings.
+
+    Note: This allows Metal-based implementations to back the respective bind


might add another note saying that this allows catching the shader-binding mismatches earlier (at the creation of bind groups and pipelines)

kvark · 2019-09-05T17:40:23Z

spec/index.bs

+    groups with `MTLArgumentBuffer` objects that are more efficient to bind at
+    run-time.
+
+  * {{GPUBindGroupLayoutBinding/dynamic}}:


We should probably rename this to something like dynamicOffset. Subsequently, it will be clearer to always say "dynamic offset binding" instead of "dynamic binding".

dynamicOffset sounds like it should be a number; maybe dynamicallyOffset? I still prefer dynamic for a bool.

Agreed re: sounds like a number. hasDynamicOffset?

Renamed to hasDynamicOffsets.

spec/index.bs

kvark · 2019-09-05T17:48:28Z

spec/index.bs

+    1. Generate a validation error in the current scope with appropriate error message.
+    1. Create a new invalid {{GPUBindGroupLayout}} and return the result.
+
+The <dfn method for "GPUDevice">createBindGroupLayout(|descriptor|)</dfn> method attempts to create a valid {{GPUBindGroupLayout}}.


I'm not sure if the wording of "attemtps" and "valid" is good here. Saying that a function "attempts" to do something appears to make sense to me if the return type indicates a possible failure. This is not the case here. Also, "valid" is strange because it's hard to imagine what else it could be doing. Seeing "valid" even makes it confusing to me, since it implies that the implementation is trying really hard to create it in such a way that it's "valid", while in fact the implementation doesn't do anything sophisticated.

I'm not sure yet how else to describe what to expect in a failure case. We use "invalid" to describe other objects in the API so it stands to reason that the opposite of such would be a "valid" object.

The failure cases are described separately as a follow-up: we list the things that are checked and say in which case the result is invalid.
What we shouldn't do is saying that "a method attempts to create a valid XXX", since this implies that the method does more than necessary to make sure it's "valid". We should just say "method creates an XXX" instead.

Text was updated to do so. PTAL

kvark · 2019-09-05T17:55:27Z

spec/index.bs

+
+### {{GPUDevice/createBindGroupLayout()|GPUDevice.createBindGroupLayout(GPUBindGroupLayoutDescriptor)}} ### {#GPUDevice-createBindGroupLayout}
+
+A valid {{GPUBindGroupLayout}} object has an <dfn>internal map</dfn> of |u32| <dfn>binding number</dfn>s to internal <dfn>layout binding</dfn> objects. 


as a radical alternative, we could make it a map on the API side, and then this piece would not need to be as carfully specified in terms of valid inputs :)

kvark · 2019-09-05T17:56:23Z

spec/index.bs

+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/binding}} is already present in |layout|'s [=internal map=], [=return with error=].
+    1. Let |layoutBinding| be a new internal [=layout binding=] object.
+    1. If |bindingDescriptor|.{{GPUBindGroupLayoutBinding/type}} is {{GPUBindingType/uniform-buffer}}:
+        1. If |layout|'s [=internal map=] contains {{GPULimits/maxUniformBuffersPerShaderStage|GPULimits.maxUniformBuffersPerShaderStage}} [=layout binding=]s of [=type=] {{GPUBindingType/uniform-buffer}} that all share any [=visibility=] bit contained in |bindingDescriptor|.{{GPUBindGroupLayoutBinding/visibility}}, [=return with error=].


it's hard to read those long lines, I'll be happy to take another look once this is refactored according to this suggestion

kvark · 2019-09-06T15:54:03Z

spec/index.bs

+    unsigned long maxDynamicUniformBuffersPerPipelineLayout = 8;
+    unsigned long maxDynamicStorageBuffersPerPipelineLayout = 4;
+    unsigned long maxSampledTexturesPerShaderStage = 16;
+    unsigned long maxSamplersPerShaderStage = 16;


is that really addressing the confusion about bind groups? We should still say "PerBindGroup", I think, for clarity.

Actually, these limits apply to the entire pipeline layout as well. So by definition a single bind group must not exceed these limits, but when we combine bind group layouts into pipeline layouts, their combined totals will have to be validated against these same limits.

Saying "per bind group" is also true, but then wouldn't make as much sense as a limit for GPUPipelineLayout validation.

Wait, this seems wrong. If these limits apply to the whole layout, why isn't this expressed here:

There must be {{GPULimits/maxUniformBuffersPerShaderStage|GPULimits.maxUniformBuffersPerShaderStage}} or
fewer |bindingDescriptor|s of type {{GPUBindingType/uniform-buffer}} visible on each shader stage in |descriptor|.

Some of these apply to descriptors per stage, some apply to all descriptors in the entire layout. The one you quoted is for general Uniform buffers, but the one that applies to pipeline layout is DYNAMIC uniform buffers. When validating the descriptor for a dynamic uniform buffer, my understanding is that both of these limits have to be checked separately. Did I understand that correctly?

Ok, now as I re-read it, it makes more sense. So we have shader stages and bind groups as two orthogonal things. The are two kinds of limits here: one kind that is a total across all shader stages and bind groups (XxxPerPipelineLayout), and one that is a total across all bind groups of each shader stage (XxxPerShaderStage).

JusSn · 2019-09-07T01:08:52Z

I have successfully whacked all the moles.

JusSn · 2019-09-07T01:09:29Z

I also want to incorporate conclusions from #406, but this can be done whenever.

This is also done.

kvark · 2019-09-09T19:57:42Z

spec/index.bs

+    unsigned long maxDynamicUniformBuffersPerPipelineLayout = 8;
+    unsigned long maxDynamicStorageBuffersPerPipelineLayout = 4;
+    unsigned long maxSampledTexturesPerShaderStage = 16;
+    unsigned long maxSamplersPerShaderStage = 16;


Wait, this seems wrong. If these limits apply to the whole layout, why isn't this expressed here:

There must be {{GPULimits/maxUniformBuffersPerShaderStage|GPULimits.maxUniformBuffersPerShaderStage}} or
fewer |bindingDescriptor|s of type {{GPUBindingType/uniform-buffer}} visible on each shader stage in |descriptor|.

kvark · 2019-09-09T20:01:09Z

spec/index.bs

-## GPUPipelineLayout ## {#pipeline-layout}
+## GPUBindGroupLayout ## {#bind-group-layout}
+
+A {{GPUBindGroupLayout}} defines an interface between a set of resources bound in a {{GPUBindGroup}} and their accessibility in shader stages.


nit: shouldn't this be "the interface"? Technically, it's a very concrete contract it is defining, not "any" contract.

kvark · 2019-09-09T20:07:50Z

spec/index.bs

+<dl dfn-type=attribute dfn-for="layout binding">
+    : <dfn>\[[binding]]</dfn> of type unsigned long.
+    ::
+        The unique identifier for this [=layout binding=] in shader stages.


would be good to define the scope of this uniqueness, e.g. "unique within this bind group layout"

kvark · 2019-09-09T20:09:13Z

spec/index.bs

+
+    : <dfn>\[[hasDynamicOffsets]]</dfn> of type boolean.
+    ::
+        If `true`, indicates that this [=layout binding=] has dynamic offsets.


Is there a reason for plural here? I don't think we discussed array bindings yet, so this is a bit confusing.

kvark · 2019-09-09T20:11:55Z

spec/index.bs

+    ::
+        If `true`, indicates that this [=layout binding=] has dynamic offsets.
+
+        Ignored if {{layout binding/[[type]]}} is not {{GPUBindingType/uniform-buffer}}, {{GPUBindingType/storage-buffer}}, or {{GPUBindingType/readonly-storage-buffer}}.


Why would we want to allow it to be true for types that don't have dynamic offsets?
Imagine how you'd have to specify the set_bind_group() with this, how you'd define the expected offsets. if we are strict here, then set_bind_group becomes trivial to define: one offset is required for each dynamic-offset binding.

Hmm, I went with the ignore path to make it a little more flexible. If it's only allowed for certain resources, I'm leaning towards splitting GPUBindGroupLayoutBinding into multiple different types, rather than switching on type.

Is there a case where this flexibility would be needed and/or beneficial?

Less code to change if you wanted to update the type of a resource binding, I guess? I'm going to make a new PR to see what different types of GPUBindGroupLayoutBinding would look like.

kvark · 2019-09-09T20:12:46Z

spec/index.bs

+
+    : <dfn>\[[textureDimension]]</dfn> of type {{GPUTextureViewDimension}}.
+    ::
+        The dimensionality of the texture resource described by this [=layout binding=].


Actually, this is the dimensionality of the view, not the texture. It can be different from the texture (e.g. 2D view of a slice of 3D texture).

kvark · 2019-09-09T20:13:51Z

spec/index.bs

+
+    : <dfn>\[[textureComponentType]]</dfn> of type {{GPUTextureComponentType}}.
+    ::
+        The data type of the texture resource described by this [=layout binding=].


This is also technically for the view, but we don't currently allow viewing with a different component type, so it's less of an issue.

kvark · 2019-09-09T20:16:08Z

spec/index.bs

+
+    : <dfn>\[[multisampled]]</dfn> of type boolean.
+    ::
+        If `true`, indicates that the texture resource described by this [=layout binding=] will be multisampled.


We need to reword this. It's written in a way that it uses different tense ("will be") and may read as if "multisample" here is a verb. Could we say "indicates that the resource is multi-sampled"? Or "contains multiple samples per texel"?

kvark · 2019-09-09T20:18:18Z

spec/index.bs

+
+  * {{GPUBindGroupLayoutBinding/hasDynamicOffsets}}:
+    For {{GPUBindingType/uniform-buffer}}, {{GPUBindingType/storage-buffer}}, and {{GPUBindingType/readonly-storage-buffer}} bindings, 
+    indicates that the binding has dynamic offsets. One offset must be passed to 


as noted in my previous comment, one binding can't have multiple dynamic offsets (today)

kainino0x · 2019-09-10T00:58:59Z

spec/index.bs

-</script>
+An internal [=layout binding=] object has the following slots:
+
+<dl dfn-type=attribute dfn-for="layout binding">


This seems like a duplicate of GPUBindGroupLayoutBinding, and would cause duplicate documentation of all of its fields. Why not just have the [[bindingsMap]] values be GPUBindGroupLayoutBindings? It's not exposed, so I think it's ok to use an IDL type for this.

For that matter, [[bindingsMap]] could probably just be a GPUBindGroupLayoutDescriptor. In general I think it would make sense for every object to have a copy of its original descriptor; it's an easy and succinct way to track how it was created.

Agreed. This was originally how I wanted to do this, but wasn't sure if that was correct.

kainino0x · 2019-09-10T01:27:34Z

spec/index.bs

+    fewer |bindingDescriptor|s of type {{GPUBindingType/uniform-buffer}} visible on each shader stage in |descriptor|.
+    * There must be {{GPULimits/maxDynamicUniformBuffersPerPipelineLayout|GPULimits.maxDynamicUniformBuffersPerPipelineLayout}} or
+        fewer |bindingDescriptor|s of type {{GPUBindingType/uniform-buffer}} with {{GPUBindGroupLayoutBinding/hasDynamicOffsets}} set to `true` 
+        visible across <b>all</b> shader stages in |descriptor|.


Does one buffer bound to both VERTEX and FRAGMENT count as 1 or 2? (Clarify in text)

here and below

kainino0x · 2019-09-10T01:34:32Z

spec/index.bs

+* There must be {{GPULimits/maxSamplersPerShaderStage|GPULimits.maxSamplersPerShaderStage}}  or 
+    fewer |bindingDescriptor|s of type {{GPUBindingType/sampler}} visible on each shader stage in |descriptor|.
+
+At any point in the algorithm, if any of the above conditions are violated, run the following steps:


Instead of "at any point", it makes more sense to me to specify when these checks are done. (Otherwise a naive implementation supposedly has to run all of these checks after every step.)

If there's a step that checks if these conditions are validated, these two steps could be inlined under that.

I'm having trouble deciding the best way to reference the very long validation conditions in-line. I'd like to leave them where they are but point to them (e.g. "If [condition 1] is violated, return with error"). How should I link "condition 1" to its definition?

Let's worry about this a little later when we have some more experience writing these rules and then refactor them to be consistent. I think it's clear enough for right now.

In shaders there are several texture types for each dimensionality depending on their component type. It can be either float, uint or sint, with maybe in the future depth/stencil if WebGPU allows reading such textures. The component type of a GPUTextureView's format must match the component type of its binding in the shader module. This is for several reasons: - Vulkan requires the following: "The Sampled Type of an OpTypeImage declaration must match the numeric format of the corresponding resource in type and signedness, as shown in the SPIR-V Sampled Type column of the Interpretation of Numeric Format table, or the values obtained by reading or sampling from this image are undefined." - It is also required in OpenGL for the texture units to be complete, a uint or sint texture unit used with a non-nearest sampler is incomplete and returns black texels. Similar constraints must exist in other APIs. To encode this compatibility constraint, a new member is added to GPUBindGroupLayoutBinding that is a new enum GPUTextureComponentType that give the component type of the texture.

This is the most common case and avoids having an optional dictionary member with no default value (but that still requires a value for texture bindings).

…web#376) Supersedes gpuweb#268.

…uweb#422)

…ndGroupLayout internal slots

kvark

Thank you for addressing my concerns!
I'm not fully satisfied with the relaxed rules for hasDynamicOffset, we can continue here or as a follow-up.

kvark · 2019-09-19T20:23:36Z

spec/index.bs

+    unsigned long maxDynamicUniformBuffersPerPipelineLayout = 8;
+    unsigned long maxDynamicStorageBuffersPerPipelineLayout = 4;
+    unsigned long maxSampledTexturesPerShaderStage = 16;
+    unsigned long maxSamplersPerShaderStage = 16;


Ok, now as I re-read it, it makes more sense. So we have shader stages and bind groups as two orthogonal things. The are two kinds of limits here: one kind that is a total across all shader stages and bind groups (XxxPerPipelineLayout), and one that is a total across all bind groups of each shader stage (XxxPerShaderStage).

JusSn · 2019-09-19T21:42:52Z

Merging, since we said we'd take care of this two weeks ago.

Following WebGPU change at gpuweb/gpuweb#427, this CL renames GPUBindGroupLayoutBinding dynamic to hasDynamicOffset. Bug: 877147 Change-Id: I5dce18178caae102911e64200415c607b65d9767 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819316 Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Kai Ninomiya <kainino@chromium.org> Cr-Commit-Position: refs/heads/master@{#699322}

* Add validation tests for the extensions of Query API - Validation tests for creating timestamp query set with/without timestamp query enabled - Validation tests for creating pipeline statistics query set with/without pipeline statistics query enabled * Update test parameters * Merge queries tests together * Fix typo

Kangz added 2 commits August 26, 2019 16:12

Make GPUBGLBinding.textureDimension default to 2d.

58d16ad

This is the most common case and avoids having an optional dictionary member with no default value (but that still requires a value for texture bindings).

JusSn requested review from kainino0x, kvark and litherum August 26, 2019 23:15

JusSn changed the title ~~Add spec text describing validation and creation of GPUBindGroupLayouts~~ Describe validation and creation of GPUBindGroupLayouts Aug 26, 2019

Kangz reviewed Aug 27, 2019

View reviewed changes

Kangz mentioned this pull request Aug 27, 2019

Provide HTML rendering of specification PRs #428

Closed

Kangz reviewed Aug 29, 2019

View reviewed changes

Kangz approved these changes Aug 29, 2019

View reviewed changes

kainino0x reviewed Aug 29, 2019

View reviewed changes

damyanp approved these changes Sep 4, 2019

View reviewed changes

kvark reviewed Sep 5, 2019

View reviewed changes

kvark reviewed Sep 6, 2019

View reviewed changes

kvark requested changes Sep 9, 2019

View reviewed changes

kainino0x reviewed Sep 10, 2019

View reviewed changes

kvark added this to In testing in Main Sep 19, 2019

kvark removed this from In testing in Main Sep 19, 2019

Kangz and others added 7 commits September 19, 2019 12:32

Make GPUBGLBinding.textureDimension default to 2d.

b1a6f18

This is the most common case and avoids having an optional dictionary member with no default value (but that still requires a value for texture bindings).

unifinished createBindGroupLayout algorithm

79c6df6

draft of BindGroupLayout details

c651c78

draft of BindGroupLayout details

25fa654

polish before PR

a640827

fix typo

af9a1cb

kainino0x and others added 17 commits September 19, 2019 12:32

Add a default for GPURenderPassColorAttachmentDescriptor.storeOp (gpu…

ee0cd4f

…web#376) Supersedes gpuweb#268.

Initial spec for GPUDevice.createBuffer (gpuweb#419)

649a071

Start writing spec for device/adapter, introduce internal objects (gp…

a22f8fe

…uweb#422)

Move validation rules out of algorithm body and better describe GPUBi…

fb5ee96

…ndGroupLayout internal slots

Include limits for dynamic offset buffers

0da7f80

Rename 'dynamic' boolean to 'hasDynamicOffsets'

28a9299

Fix indentation for ci bot

05bae40

More indentation errors

b368560

Fix var typos

882033b

Fix method definition

43d4cf8

Fix enum references

88254c9

Missing </dfn> tag

14f0d17

Missing </dfn> tag

e8900b5

Remove bad [= =]

8f01739

Fix old constant name

5784b77

Half-formed new validation rule structure for createBindGroupLayout

8059767

An interface -> the interface

e1c3acc

JusSn force-pushed the resourceBindings branch from 2cc86e5 to e1c3acc Compare September 19, 2019 19:33

Justin Fan added 3 commits September 19, 2019 12:55

Fix merge conflicts with upstream

73e94b4

Remove old 'layout binding' reference

2bdd1e8

fix device lost validation reference

0c43c44

kvark approved these changes Sep 19, 2019

View reviewed changes

Fix 'dynamic' typo

77003d6

JusSn merged commit 0a2bb28 into gpuweb:master Sep 19, 2019

JusSn deleted the resourceBindings branch September 19, 2019 21:43

beaufortfrancois mentioned this pull request Sep 23, 2019

s/dynamic/hasDynamicOffset/ gpuweb/types#3

Merged

austinEng mentioned this pull request Jun 23, 2020

Maximum and minimum total resources in GPUBindGroup/Layout? #409

Closed


		### {{GPUDevice/createBindGroupLayout()\|GPUDevice.createBindGroupLayout(GPUBindGroupLayoutDescriptor)}} ### {#GPUDevice-createBindGroupLayout}

		A valid {{GPUBindGroupLayout}} object has an <dfn>internal map</dfn> of \|u32\| <dfn>binding number</dfn>s to internal <dfn>layout binding</dfn> objects.

Describe validation and creation of GPUBindGroupLayouts #427

Describe validation and creation of GPUBindGroupLayouts #427

Conversation

JusSn commented Aug 26, 2019 • edited by pr-preview bot

Choose a reason for hiding this comment

JusSn Aug 27, 2019 • edited

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

JusSn commented Aug 27, 2019

JusSn commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kangz Sep 5, 2019 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kangz left a 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

JusSn Sep 5, 2019 • edited

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 Sep 5, 2019 • edited

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

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

Choose a reason for hiding this comment

JusSn commented Sep 7, 2019

JusSn commented Sep 7, 2019

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JusSn Sep 18, 2019 • edited

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

kvark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JusSn commented Sep 19, 2019

JusSn commented Aug 26, 2019 •

edited by pr-preview bot

JusSn Aug 27, 2019 •

edited

Kangz Sep 5, 2019 •

edited

JusSn Sep 5, 2019 •

edited

kainino0x Sep 5, 2019 •

edited

JusSn Sep 18, 2019 •

edited