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] Use higher level LoadOp,StoreOp #6785

Merged
merged 4 commits into from
Dec 23, 2024

Conversation

sagudev
Copy link
Contributor

@sagudev sagudev commented Dec 19, 2024

Connections
#6753 (comment)

Description
In f5bb3f7 I moved Operations, LoadOp, StoreOp for wgpu to wgt, so we can use those abstractions in 5542c3c for ResolvedPassChannel. Then in fa4a5ed I moved PassChannel to also use wgt::LoadOp, wgt::StoreOp; thus removing old LoadOp/StoreOp. We still need somewhat PassChannel in core to be able to express all wrong stuff JS user can do, but when we do all validation we are back to wgpu's abstraction (which prevents all such misuses). It would be interesting future project to avoid some validation in core by passing those safer/strict types directly to core. In the future, hal could also start using higher abstraction for LoadOp.

Changes in servo needed for this PR: sagudev/servo@6125931 (this will be useful for firefox).

Testing
No behaviour change is expected, CTS run in servo confirms that (note that CRASH in webgpu:shader,execution,expression:* are know problem in servo due to cyclic imports), there are also some existing tests covering this.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@sagudev
Copy link
Contributor Author

sagudev commented Dec 19, 2024

cc @ErichDonGubler there are some C related changes, cc @teoxoy for telling me if I went to far.

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

This is so much better; thanks for cleaning this up!

sagudev added a commit to sagudev/servo that referenced this pull request Dec 21, 2024
sagudev added a commit to sagudev/servo that referenced this pull request Dec 21, 2024
{"fail_fast": false, "matrix": [{"name": "WebGPU CTS", "workflow": "linux", "wpt_layout": "2020", "profile": "production", "unit_tests": false, "bencher": false, "wpt_args": "_webgpu"}]}
@sagudev sagudev marked this pull request as ready for review December 21, 2024 08:54
@sagudev sagudev requested review from crowlKats and a team as code owners December 21, 2024 08:54
@cwfitzgerald
Copy link
Member

Merging for expediency - @crowlKats if something is wrong, shout later.

@cwfitzgerald cwfitzgerald merged commit ee3ae0e into gfx-rs:trunk Dec 23, 2024
27 checks passed
sagudev added a commit to sagudev/servo that referenced this pull request Dec 30, 2024
gfx-rs/wgpu@0f5f058

Most notable change is gfx-rs/wgpu#6785

Signed-off-by: sagudev <[email protected]>
@sagudev sagudev mentioned this pull request Dec 30, 2024
3 tasks
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Jan 2, 2025
* Update wgpu

gfx-rs/wgpu@0f5f058

Most notable change is gfx-rs/wgpu#6785

Signed-off-by: sagudev <[email protected]>

* Remove silencing of wgpu logs

I fixed this in upstream: gfx-rs/wgpu#6817

Signed-off-by: Samson <[email protected]>

---------

Signed-off-by: sagudev <[email protected]>
Signed-off-by: Samson <[email protected]>
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Jan 2, 2025
* Update wgpu

gfx-rs/wgpu@0f5f058

Most notable change is gfx-rs/wgpu#6785

Signed-off-by: sagudev <[email protected]>

* Remove silencing of wgpu logs

I fixed this in upstream: gfx-rs/wgpu#6817

Signed-off-by: Samson <[email protected]>

---------

Signed-off-by: sagudev <[email protected]>
Signed-off-by: Samson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants