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

Chromium 131 validates depth bias on line and point topologies #24811

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Oct 24, 2024

Summary

Chrome 128 deprecates setting depth bias for lines and points (Chrome issue, spec change).

This PR adds a datapoint for this change in behavior to GPUDevice.createRenderPipeline() and GPUDevice.createRenderPipelineAsync().

Test results and supporting details

See the description and data source

Related issues

See related MDN project issue: mdn/content#36341

@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 24, 2024
@caugner
Copy link
Contributor

caugner commented Oct 24, 2024

@chrisdavidmills Are you aware of other spec changes that were documented in BCD in a similar fashion?

My two thoughts are:

  1. This seems to be a rather small spec detail, and I'm not sure that we can maintain compat data at this level of detail (unless maybe if we find tracking bugs for the other engines).
  2. Documenting it under createRenderPipeline() and createRenderPipelineAsync() does not seem ideal, as it looks like its part of the GPUDepthStencilState interface (which neither BCD nor MDN document yet).

PS: It looks like Chrome 131 removes the warning in favor of an error.

@chrisdavidmills
Copy link
Contributor Author

@chrisdavidmills Are you aware of other spec changes that were documented in BCD in a similar fashion?

My two thoughts are:

1. This seems to be a rather small spec detail, and I'm not sure that we can maintain compat data at this level of detail (unless maybe if we find tracking bugs for the other engines).

2. Documenting it under `createRenderPipeline()` and `createRenderPipelineAsync()` does not seem ideal, as it looks like its part of the `GPUDepthStencilState` interface (which neither BCD nor MDN document yet).

PS: It looks like Chrome 131 removes the warning in favor of an error.

Ah right, so it looks like this should be Chrome 131 rather than 128 then.

So, we document GPUDepthStencilState on the createRenderPipeline() page. I really didn't want to have separate pages for all the different levels of WebGPU dictionaries, as it would make the docs way too complicated to understand. If we do end up keeping this, that is where the BCD should go.

But, if it is deemed to be too detailed, we could not include it as a separate data point.

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 modulo question

"status": {
"experimental": true,
"standard_track": true,
"deprecated": false
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the deprecated field used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a feature has been officially deprecated, then we set "deprecated" to true. I know you used "deprecated" to describe what has happened, but when I looked at it, it seemed like a change in behavior rather than a true deprecation. Does that work, or do you think I should reconsider how I've described this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Spec said A, then they changed it to B.
We announced deprecation in Chrome 128 and removed it in Chrome 131.
I'll let you pick what is more appropriate in BCD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think 131 makes more sense — when it starts to produce an error rather than the warning. I've updated it.

api/GPUDevice.json Outdated Show resolved Hide resolved
chrisdavidmills and others added 2 commits October 27, 2024 13:41
Co-authored-by: Claas Augner <495429+caugner@users.noreply.github.com>
@caugner caugner changed the title Add data points for depth bias deprecation on line and point topologies Chromium 131 validates depth bias on line and point topologies Oct 29, 2024
@caugner caugner merged commit 8739c3e into mdn:main Oct 29, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants