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

Mark depthCompare and depthWriteEnabled as optional, and add explanation #36886

Conversation

chrisdavidmills
Copy link
Contributor

Description

In Chrome 120, the depthCompare and depthWriteEnabled properties of the createRenderPipeline/createRenderPipelineAsync descriptors have been made optional when not needed. See https://developer.chrome.com/blog/new-in-webgpu-120#changes_to_depth-stencil_state.

This PR marks both properties as optional and adds an explanation of when they are not needed.

Motivation

Additional details

Related issues and pull requests

Project issue: #36370

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner November 20, 2024 17:35
@chrisdavidmills chrisdavidmills requested review from sideshowbarker and removed request for a team November 20, 2024 17:35
@github-actions github-actions bot added Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed labels Nov 20, 2024
Copy link
Contributor

Copy link
Contributor

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

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

LGTM with nit

@@ -52,7 +52,7 @@ The `depthStencil` object can contain the following properties:
- : A number representing the maximum depth bias of a fragment. If omitted, `depthBiasClamp` defaults to 0.
- `depthBiasSlopeScale` {{optional_inline}}
- : A number representing a depth bias that scales with the fragment's slope. If omitted, `depthBiasSlopeScale` defaults to 0.
- `depthCompare`
- `depthCompare` {{optional_inline}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we don't duplicate the descriptor descriptions on both pages. We just include them in full on the createRenderPipeline() ref page, and then link there from the createRenderPipelineAsync() ref page.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're good then!

Copy link
Contributor

Choose a reason for hiding this comment

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

@sideshowbarker sideshowbarker merged commit 4e696ae into mdn:main Nov 22, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants