Skip to content

Add GPUCanvasContext getConfiguration() method - #4899

Merged
kainino0x merged 4 commits into
mainfrom
getConfiguration
Sep 27, 2024
Merged

kainino0x merged 4 commits into
mainfrom
getConfiguration

Conversation

@beaufortfrancois

Copy link
Copy Markdown
Contributor

As discussed in #4828, this PR exposes a way for web apps to feature-detect whether WebGPU can do HDR canvas. It does so by exposing a new GPUCanvasContext getConfiguration() method that returns the context configuration which includes device, format, usage, viewFormats, colorSpace, toneMapping, and alphaMode members.

@github-actions

github-actions Bot commented Sep 26, 2024 •

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (3271365):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

Comment thread spec/index.bs Outdated
@kainino0x kainino0x added api resolved Resolved - waiting for a change to the API specification api WebGPU API labels Sep 26, 2024
@kainino0x kainino0x added this to the Milestone 1 milestone Sep 26, 2024

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the updates!

@kainino0x kainino0x 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.

LGTM, that's explained well!

@kainino0x kainino0x 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.

tiny nits

Comment thread spec/index.bs Outdated
Comment thread spec/index.bs Outdated
@kainino0x
kainino0x enabled auto-merge (squash) September 27, 2024 22:29
@kainino0x
kainino0x merged commit 7cffc10 into main Sep 27, 2024
@kainino0x
kainino0x deleted the getConfiguration branch September 27, 2024 23:14
@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Sep 27, 2024
@greggman

Copy link
Copy Markdown
Member
           **Returns:** {{GPUCanvasConfiguration}} or `null`

Sounds like a foot gun. Most people are going to write

const isReallyHDR = context.getConfiguration().toneMapping.mode === 'extended';

and then have their app crash when getConfiguration() returns null.

Also, when does it return null?

@beaufortfrancois

Copy link
Copy Markdown
Contributor Author

and then have their app crash when getConfiguration() returns null.

They should write context.getConfiguration()?.toneMapping?.mode === 'extended'; instead.

We could do the same as with getCurrentTexture() and throw an InvalidStateError if this.[[configuration]] is null but I'm not sure I prefer this.

Also, when does it return null?

It returns null before the app calls configure() and after the app calls unconfigure().

@greggman

Copy link
Copy Markdown
Member

It returns null before the app calls configure() and after the app calls unconfigure().

Thanks. I see that now.

context.getConfiguration()?.toneMapping?.mode === 'extended'

They shouldn't have to write this given that it only returns null before configure and after unconfigure.

@beaufortfrancois

Copy link
Copy Markdown
Contributor Author

context.getConfiguration()?.toneMapping?.mode === 'extended'

They shouldn't have to write this given that it only returns null before configure and after unconfigure.

An external app monitoring the canvas configuration would have to do that but you're right, the app in control of the canvas would know.

aarongable pushed a commit to chromium/chromium that referenced this pull request Sep 30, 2024
This CL adds the GPUCanvasContext getConfiguration() method
bebhind the experimental WebGPUExperimentalFeatures blink runtime
feature. It will allow us to write tests for the WebGPU CTS.

Spec PR: gpuweb/gpuweb#4899

Bug: 370109829
Change-Id: I50aee0a52cd229c5fb1f4a2e518af72d548e1879
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5890176
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Fr <beaufort.francois@gmail.com>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1361869}
Comment thread spec/index.bs
{{GPUCanvasConfiguration/toneMapping}} is implemented and the <l>'@media/dynamic-range'</l> media
query indicates HDR support, then WebGPU canvas **should** render content using the full
HDR range instead of clamping values to the SDR range of the HDR display.
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. I find it a bit odd that the getter for GPUCanvasContext configuration would document behavior of how HDR tone mapping should work. Wouldn't this documentation be better suited to exist in GPUCanvasContext.configure() where these fields are established?

  2. Also, there is

**Returns:** {{GPUCanvasConfiguration}} or `null`

When is this function allowed to return null? Can an implementation return null as a "eh, we didn't care to implement this function. We consider our WebGPU implementation to be a "pure" rendering device implementation (in the same philosophy of [D3DCREATE_PUREDEVICE](https://learn.microsoft.com/en-us/windows/win32/direct3d9/d3dcreate)) - track this information yourself."

  1. Is this function guaranteed to return the exact same fields that were fed to GPUCanvasContext.configure()? Or can there be differences? For example, in the sense of "I configure()d with X, and now I'm calling getConfiguration() to see if I actually got Y" behavior? Can an implementation do that?

@kainino0x kainino0x Sep 12, 2025 •

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.

  1. There are notes in the definitions for both toneMapping and configure() pointing toward it. Fair point it's an odd spot for that note though.
  2. It returns null if and only if the canvas is not configured.
  3. There aren't any steps which modify the configuration passed into configure() so the only change should be that keys not known by the browser are dropped, or if the requested configuration is invalid then configure() will fail and getConfiguration() will return null.

per specs for getConfiguration(), configure()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api resolved Resolved - waiting for a change to the API specification api WebGPU API needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants