Sitelet https://github.com/gpuweb/gpuweb/pull/376
Skip to content

Add a default for GPURenderPassColorAttachmentDescriptor.storeOp - #376

Merged
Kangz merged 1 commit into
gpuweb:masterfrom
kainino0x:storeop-default
Aug 28, 2019
Merged

Kangz merged 1 commit into
gpuweb:masterfrom
kainino0x:storeop-default

Conversation

@kainino0x

@kainino0x kainino0x commented Jul 24, 2019 •

Copy link
Copy Markdown
Contributor

Originally authored by @litherum. Supersedes #268.


Preview | Diff

@kainino0x

Copy link
Copy Markdown
Contributor Author

Re-upload of the remaining change in #268. Uploading this since the old change was stale - I'm not necessarily for (or against) this change.

Right now, I actually don't have much of an opinion on it.

@kdashg kdashg left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weak-no from me, particularly without a default for loadOp/Value.
I think this is a good variable to keep as required. The user has a clear expectation here, and it's not clear (haha) which choice is predominant.

@kainino0x

Copy link
Copy Markdown
Contributor Author

2019-08-26 meeting: uncontroversial [except for @jdashg's comment] but no one seems to particularly care very much. Merge?

@Kangz

Kangz commented Aug 28, 2019

Copy link
Copy Markdown
Contributor

Meeting minutes seem to say this should be merged:

#376 - Add a default for GPURenderPassColorAttachmentDescriptor.storeOp
CW: is this controversial?
MM: great idea, we should do it.
CW: done.

@Kangz
Kangz merged commit f3ea387 into gpuweb:master Aug 28, 2019
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 29, 2019
Following WebGPU spec change at gpuweb/gpuweb#376,
GPURenderPassColorAttachmentDescriptor.storeOp default should be "store".

Bug: 877147
Change-Id: I7dd5db4e3cfcb0ddc4dd132342d8bf145bb8f28e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715186
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#691530}
@kainino0x
kainino0x deleted the storeop-default branch September 11, 2019 01:16
JusSn pushed a commit to JusSn/gpuweb that referenced this pull request Sep 19, 2019
JusSn added a commit that referenced this pull request Sep 19, 2019
* Add a component type for GPUBGLBinding compatiblity (#384)

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.

* Make GPUBGLBinding.textureDimension default to 2d.

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

* Add a component type for GPUBGLBinding compatiblity (#384)

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.

* Make GPUBGLBinding.textureDimension default to 2d.

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

* draft of BindGroupLayout details

* draft of BindGroupLayout details

* polish before PR

* fix typo

* replace u32/i32/u64 with normal int types or specific typedefs (#423)

* Do not require vertexInput in GPURenderPipelineDescriptor (#378)

* Add a default for GPURenderPassColorAttachmentDescriptor.storeOp (#376)

Supersedes #268.

* Initial spec for GPUDevice.createBuffer (#419)

* Start writing spec for device/adapter, introduce internal objects (#422)

* Move validation rules out of algorithm body and better describe GPUBindGroupLayout internal slots

* Include limits for dynamic offset buffers

* Rename 'dynamic' boolean to 'hasDynamicOffsets'

* Fix indentation for ci bot

* More indentation errors

* Fix var typos

* Fix method definition

* Fix enum references

* Missing </dfn> tag

* Missing </dfn> tag

* Remove bad [= =]

* Fix old constant name

* Half-formed new validation rule structure for createBindGroupLayout

* An interface -> the interface

* Remove old 'layout binding' reference

* fix device lost validation reference

* Fix 'dynamic' typo
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
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.

4 participants