-
Notifications
You must be signed in to change notification settings - Fork 948
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] Support optional ops in PassChannel #6716
Conversation
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
{"fail_fast": false, "matrix": [{"name": "WebGPU CTS", "workflow": "linux", "wpt_layout": "2020", "profile": "production", "unit_tests": false, "bencher": false, "wpt_args": "_webgpu"}]}
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
CTS:
|
view.same_device(device)?; | ||
check_multiview(view)?; | ||
add_view(view, AttachmentErrorLocation::Depth)?; | ||
|
||
let ds_aspects = view.desc.aspects(); | ||
if ds_aspects.contains(hal::FormatAspects::COLOR) { | ||
return Err(RenderPassErrorInner::InvalidDepthStencilAttachmentFormat( | ||
view.desc.format, | ||
)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks are now part of creation, in a followup I will move even more checks, see #6735 for more info.
depth: if format.has_depth_aspect() { | ||
depth_stencil_attachment.depth.resolve()? | ||
} else { | ||
Default::default() | ||
}, | ||
stencil: if format.has_stencil_aspect() { | ||
depth_stencil_attachment.stencil.resolve()? | ||
} else { | ||
Default::default() | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is rather unfortunate, but I didn't want to touch RenderPassInfo::start
to much (at least not in this PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would be nice to make some rusty types for these after validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Connections
Fixes #6700
Description
When attachment is read only, ops must not be provided (else validation error should be raised). To support this
PassChannel
is generic over L and S, so we can use same struct forLoadOp
orOption<LoadOp>
. Now core users will passPassChannel<V, Option<LoadOp>, Option<StoreOp>>
that will be be resolved toPassChannel<V, LoadOp, StoreOp>
, which is used same as before internally. While resolving we also do validation.Changes needed in servo: sagudev/servo@4aa142c.
Testing
CTS run in servo: #6716 (comment) and existing test.
Checklist
cargo fmt
.taplo format
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
--target wasm32-unknown-emscripten
cargo xtask test
to run tests.CHANGELOG.md
. See simple instructions inside file.