这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ccameron-chromium
Copy link
Contributor

@ccameron-chromium ccameron-chromium commented Feb 27, 2024

Add color metadata to canvas configuration, with the ability to specify an extended range.

Move the clarification of the handling out out-of-range values to the subsection on specific values, and add an example of extended range values.

Fixes #4239

@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2024

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

Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

The worked examples are great. A few small comments (and holding "editor" approval until others have reviewed)

@kainino0x
Copy link
Contributor

Fixes #4239

FYI I added this to the PR summary to link the issue. Is it correct that landing this would fix/close that issue?

kainino0x and others added 2 commits February 27, 2024 11:45
From kainino0x review

Co-authored-by: Kai Ninomiya <kainino1@gmail.com>
@ccameron-chromium
Copy link
Contributor Author

Fixes #4239

FYI I added this to the PR summary to link the issue. Is it correct that landing this would fix/close that issue?

Yes, thanks!

@ccameron-chromium
Copy link
Contributor Author

Fixes #4239

FYI I added this to the PR summary to link the issue. Is it correct that landing this would fix/close that issue?

Yes!

@kainino0x
Copy link
Contributor

Friendly review bump @kdashg @mwyrzykowski

mwyrzykowski
mwyrzykowski previously approved these changes Mar 18, 2024
@mwyrzykowski
Copy link

Friendly review bump @kdashg @mwyrzykowski

Sorry! It looks good, I forgot to click approve 👍

kdashg
kdashg previously approved these changes Mar 27, 2024
@kainino0x kainino0x marked this pull request as draft April 1, 2024 22:23
@kainino0x
Copy link
Contributor

kainino0x commented Apr 1, 2024

@ccameron-chromium let me know that CSS is hotly debating gamut mapping right now, and we may need to be less prescriptive in this spec lest we contradict what CSS eventually does. I've converted this to draft so it can't be landed accidentally.

@ccameron-chromium
Copy link
Contributor Author

ccameron-chromium commented Apr 1, 2024

@ccameron-chromium let me know that CSS is hotly debating gamut mapping right now, and we may need to be less prescriptive in this spec lest we contradict what CSS eventually does. I've converted this to draft so it can't be landed accidentally.

We had originally called the modes "default" and "extended", and then made them more descriptive by calling them "clamp-to-standard" and "clamp-to-extended". While this accurately describes the current behavior on all browsers (as did PR 4400), I think we shouldn't be so explicit in the names of the enums.

The core behavior that we want to capture is that

  • in the "standard" behavior, no colors in the SDR gamut of the display will be altered, and all colors outside of the SDR gamut will be projected onto the gamut broder
  • in the "extended" behavior, the same thing, except for the HDR gamut

I could imagine changing the names to "project-to-standard" and "project-to-extended", or just sticking with "standard" and "extended". The behavior of clamping is a projection (the L-infinity error minimizing one, for what that's worth). If there a preference between these two name schemes?

@kainino0x
Copy link
Contributor

I probably should have put this on the agenda for today's meeting, but didn't think about it. We're probably not having another WG meeting for 3 weeks but that should be OK.

I've put this on the editors' agenda for this coming Monday so we can talk about naming and then try to get reapproval.

@kainino0x kainino0x marked this pull request as ready for review April 25, 2024 00:46
@kainino0x kainino0x dismissed stale reviews from kdashg and mwyrzykowski April 25, 2024 00:46

proposal became less strict

Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

The names and language LGTM, just small comments to label non-normative language as non-normative.

@kainino0x
Copy link
Contributor

@kdashg @mwyrzykowski could you re-review with the loosened wording and new naming?

@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 Apr 27, 2024
@kainino0x kainino0x added the api WebGPU API label May 21, 2024
@Kangz
Copy link
Contributor

Kangz commented May 21, 2024

GPU Web WG 2024-05-15
  • KN: The CSS WG is still discussing whether clamping is required, so this PR loosens the verbiage to be more lenient so it can match what CSS does eventually.
  • KN: The two changes are the enums names that say "project" and the text saying it is projected in the color space, but not necessarily by clamping.
  • KG: Will review offline.

@kainino0x
Copy link
Contributor

@kdashg review bump!

@kainino0x kainino0x added the tacit resolution queue Editors have agreed and intend to land if no feedback is given label Jun 12, 2024
@Kangz
Copy link
Contributor

Kangz commented Jun 20, 2024

@kdashg, others, review bump!

To force progress, given that it's a minor revision on an agreed upon version, unless there are new comments to address, we should merge this in two weeks (on 2024-07-04) to finally get to an end state.

@kainino0x kainino0x removed the tacit resolution queue Editors have agreed and intend to land if no feedback is given label Jun 26, 2024
Copy link
Contributor

@kdashg kdashg left a comment

Choose a reason for hiding this comment

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

Thanks!

kainino0x and others added 2 commits June 26, 2024 14:16
@Kangz
Copy link
Contributor

Kangz commented Jul 9, 2024

GPU Web WG 2024-06-26 Atlantic-time
* CW: I’m sorry I keep pinging you on this, KG
* KG: I was going to get to it, and was going to do it last night, but then I saw it wasn’t on the agenda so I didn’t
* KG: I’ll finish it after this. It’s a little frustrating to deal with renames retriggering the review process for me
* KN: It wasn’t just the rename, it was also loosening the spec slightly
* KG: In the future it would help if we landed more things speculatively. This could be in the spec already, and we could be fixing it
* CW: It will be nice when we’re post CR because we can take snapshots and we can look at diffs. Right now, Intel has been working on dual source blending, and has had a month and a half of overhead asking for reviews of stuff.

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

Labels

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.

Extended brightness range rendering

5 participants