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

Conversation

@kdashg
Copy link
Contributor

@kdashg kdashg commented Apr 7, 2020

Split into two parts, off of #655. This will need to be rebased after #655.

Decorations can't be validated much at parse time, so they shouldn't be tightly specified in the grammar.

"[location 0, builtin 0]" =parsing=> "Error: Failed to parse token after builtin, expected IDENT."

"[location 0, builtin frag_coord]" =parsing=> OK =compilation=> "Error: frag_coord cannot be an output of a vertex entrypoint."

Since having it in the grammar doesn't really reduce the validation load on the compiler, we might as well not constrain it in the grammar. (even if this double-validation is cheap/free)

@kdashg kdashg changed the title No grammar decorations No explicit grammar for decorations Apr 7, 2020
@dj2 dj2 added the wgsl WebGPU Shading Language Issues label Apr 7, 2020
@kvark
Copy link
Contributor

kvark commented Apr 27, 2020

@jdashg could you rebase this please? Would be easier to see the changes independently of #655

@kdashg kdashg force-pushed the no-grammar-decorations branch from 876aa64 to 151565f Compare June 16, 2020 17:54
@kvark kvark changed the base branch from master to main June 23, 2020 13:16
@dj2
Copy link
Member

dj2 commented Jul 21, 2020

Discussed at the WGSL 2020-07-21 meeting with the resolution to accept this change once it's been updated.

@grorg grorg added the wgsl resolved Resolved - waiting for a change to the WGSL specification label Jul 27, 2020
@kdashg kdashg force-pushed the no-grammar-decorations branch 3 times, most recently from c193dcb to 8951c8d Compare August 4, 2020 21:32
@kdashg kdashg requested review from dneto0 and kvark and removed request for kvark August 4, 2020 21:33
Copy link
Contributor

@kvark kvark left a comment

Choose a reason for hiding this comment

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

I see two concerns that you are trying to address here. Please correct if this is wrong.

  1. Free the implementations from the need to validate the same things twice.

As we discussed on the call, no (currently developed) implementations are going to change because of this PR. There is no practical aspect of this for implementations.

  1. Reduce the complexity of the grammar, since it can't enforce the full range of validation rules.

Of course full validation can't happen at the grammar level. I think that's obvious.
So where do we draw the line between stuff that needs to be in the grammar then?

For example, the grammar currently has storage_class. Clearly, not all storage classes are expected in all contexts. Does it mean we should be throwing it away and replacing it with IDENT?

My understanding is that the grammar helps with "first stage" validation. I.e. the validator would work with "here is a list of appropriate decorations for this element" instead of "here is a list of key-value strings".

@kvark
Copy link
Contributor

kvark commented Aug 6, 2020

Sorry for raising these concerns over the PR, I now noticed that it's marked as resolved. I haven't put enough thought into it previously when we were discussing this, but feel free to proceed if the concerns are not ringing any bells :)

Copy link
Contributor

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Talked to @jdashg offline and got convinced that this is fiine :)

@dj2
Copy link
Member

dj2 commented Sep 14, 2020

Can this be landed once it's sync'd?

@grorg
Copy link
Contributor

grorg commented Sep 29, 2020

Discussed at the 2020-09-29 meeting.

@jdashg will address comments

@kdashg kdashg requested a review from dj2 October 5, 2020 18:10
Decorations can't be validated much at parse time, so they shouldn't be
tightly specified in the grammar.

"[location(0), builtin(0)]" =parsing=> "Error: Failed to parse token after
builtin, expected IDENT."

"[location(0), builtin(frag_coord)]" =parsing=> OK =compilation=> "Error:
frag_coord cannot be an output of a vertex entrypoint."

Since having it in the grammar doesn't really reduce the validation load
on the compiler, we might as well not constrain it in the grammar. (even
if this double-validation is cheap/free)
@kdashg kdashg force-pushed the no-grammar-decorations branch from 68d39c9 to 2fc6606 Compare October 8, 2020 17:33
@kdashg kdashg merged commit c7c3f77 into gpuweb:main Oct 8, 2020
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
* Add refttests for different size of canvas and swapchain

This tests covers the cases that user config webgpu back
buffer with larger than/smaller than/equal to canvas size.

And the content should be auto scale to the canvas size and
keep the content.

WebGPU canvas should present things by doing
downscaling/upscaling/no-changes for the back buffer.

* Using CSS to have a correct ref

* Address comments

* add one pixel to make sure image isn't downsampled to canvas element resolution

Co-authored-by: Kai Ninomiya <kainino@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wgsl resolved Resolved - waiting for a change to the WGSL specification wgsl WebGPU Shading Language Issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants