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

[core] PassChannel is not expresive enough to match spec #6700

Closed
sagudev opened this issue Dec 11, 2024 · 2 comments · Fixed by #6716
Closed

[core] PassChannel is not expresive enough to match spec #6700

sagudev opened this issue Dec 11, 2024 · 2 comments · Fixed by #6716
Labels
area: correctness We're behaving incorrectly type: enhancement New feature or request

Comments

@sagudev
Copy link
Contributor

sagudev commented Dec 11, 2024

Is your feature request related to a problem? Please describe.
Spec says that you either specify readonly: true or pass ops:

If format has a depth aspect and this.depthReadOnly is false:
this.depthLoadOp must be provided.
this.depthStoreOp must be provided.
Otherwise:
this.depthLoadOp must not be provided.
this.depthStoreOp must not be provided.

and wgpu-core currently does not support not passing ops.

Describe the solution you'd like
Something that would match specs better:

pub struct PassChannel<V> {
    pub load_op: Option<LoadOp>,
    pub store_op: Option<StoreOp>,
    pub clear_value: V,
    pub read_only: bool,
}

Describe alternatives you've considered
We could use more high level solution, but that would be harder to use in browser impl (not expressive enough for invalid cases, so something like this would make more sense in wgpu):

enum Chan {
    ReadOnly,
    Ops {load_op: LoadOp, store_op: StoreOp}
}

Additional context
Relevant CTS tests: https://gpuweb.github.io/cts/standalone/?q=webgpu:api,validation,render_pass,render_pass_descriptor:depth_stencil_attachment,loadOp_storeOp_match_depthReadOnly_stencilReadOnly:*

@sagudev
Copy link
Contributor Author

sagudev commented Dec 11, 2024

We need to do validation as part of pass creation (beginRenderPass), more specifically in fill_arc_desc, so my plan is to for Arc to have resolved Arc channel (readonly + ops) while unarced (public) desc will have new interface to match the spec.

@teoxoy
Copy link
Member

teoxoy commented Dec 12, 2024

Sounds good to me.

Related: #2944

@cwfitzgerald cwfitzgerald added type: enhancement New feature or request area: correctness We're behaving incorrectly labels Dec 13, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in WebGPU for Firefox Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly type: enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants