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

Invoke a DeviceLostClosure immediately if set on an invalid device. #5358

Merged
merged 8 commits into from
Mar 21, 2024

Conversation

bradwerth
Copy link
Contributor

@bradwerth bradwerth commented Mar 7, 2024

Connections
N/A

Description
This handles another case in device_set_device_lost_closure where a DeviceLostClosure is given to wgpu but never called. The contract with these closures is that wgpu must call them before they are dropped.

Testing
This adds a new test DEVICE_INVALID_THEN_SET_LOST_CALLBACK which invalidates the device and then sets a device lost callback on it. Unfortunately, there was no existing way to make a device invalid other than dropping it, so it also adds a function make_invalid to replace the device with an error in the registry. A few functions had to be touched up to handle this unexpected error object in the registry, representing an invalid device.

Checklist

  • Run cargo fmt.
  • 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.

@bradwerth bradwerth requested a review from a team as a code owner March 7, 2024 01:25
@bradwerth bradwerth force-pushed the lostClosureCallOnInvalid branch 2 times, most recently from 1bf4f79 to ab88eb4 Compare March 7, 2024 17:40
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.

Looks good! I left a few suggestions, I can apply them if you think they are good.

tests/tests/device.rs Outdated Show resolved Hide resolved
wgpu-core/src/device/global.rs Show resolved Hide resolved
wgpu/src/backend/webgpu.rs Show resolved Hide resolved
wgpu/src/backend/wgpu_core.rs Show resolved Hide resolved
wgpu/src/lib.rs Outdated Show resolved Hide resolved
wgpu/src/context.rs Show resolved Hide resolved
wgpu/src/context.rs Show resolved Hide resolved
wgpu/src/context.rs Show resolved Hide resolved
To make the device invalid, this defines an explicit, test-only method
make_invalid. It also modifies calls that expect to always retrieve a
valid device.
@bradwerth

This comment was marked as resolved.

@ErichDonGubler
Copy link
Member

Pushed up bb03319 to abbreviate assert!(a == b, …) to assert_eq!(a, b). assert_eq generally provides superior diagnostics, and, even better, it means you don't have to write your own failure text. 😀

@ErichDonGubler
Copy link
Member

ErichDonGubler commented Mar 15, 2024

Pushed 73d90fb to further clean up tests being added with deriving Eq on DeviceLostReason, so we can leverage assert_eq!(…) s'more.

@ErichDonGubler ErichDonGubler merged commit 00e0e72 into gfx-rs:trunk Mar 21, 2024
27 checks passed
@bradwerth bradwerth deleted the lostClosureCallOnInvalid branch April 1, 2024 16:07
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