Rework wgpu::PollType to only two enum variants
#8285
Conversation
|
@cwfitzgerald ping as requested |
| let maintain_result; | ||
| (user_callbacks, maintain_result) = | ||
| device.maintain(fence, wgt::PollType::Wait, snatch_guard); | ||
| device.maintain(fence, wgt::PollType::wait_indefinitely(), snatch_guard); |
There was a problem hiding this comment.
this one is curious. the previous pr changed behavior here without noticing. I do not think this is a concern, but it's an interesting subtlety
There was a problem hiding this comment.
What do you mean? We need to wait for idle before replacing the surface?
There was a problem hiding this comment.
yeah but before that had implicit timeout as well
wgpu-types/src/lib.rs
Outdated
| // <https://github.com/gfx-rs/wgpu/pull/5012> to be split up, as | ||
| // it has meaning in that PR. | ||
| Self::WaitForSubmissionIndex(submission_index) | ||
| pub fn wait_indefinitely() -> Self { |
There was a problem hiding this comment.
thought: Rust's std favors associated constants for cases like this (i.e., {integer}:::MIN,MAX, and this feels sorta the same. I'm not inclined to block, just curious if this had been considered.
There was a problem hiding this comment.
Hmmm true. I'd actually prefer a constant here rather than calling a method. But where I'm coming here is that there was a method before
There was a problem hiding this comment.
Despite that I have no strong opinion, opting for laziness (==sticking closer to status quo) unless someone expresses a clear preference :)
There was a problem hiding this comment.
LGTM with current content, though I'll let @cwfitzgerald give a final once-over, since release is imminent and this PR is in response to his concrete concerns.
|
Thanks for the thorough review @ErichDonGubler, good suggestions/corrections all around! |
| let maintain_result; | ||
| (user_callbacks, maintain_result) = | ||
| device.maintain(fence, wgt::PollType::Wait, snatch_guard); | ||
| device.maintain(fence, wgt::PollType::wait_indefinitely(), snatch_guard); |
There was a problem hiding this comment.
What do you mean? We need to wait for idle before replacing the surface?
3 participants
Connections
Description
Induces a breaking change into
wgpu::PollTypein order to:wgpu::PollTypeTesting
Existing test + (kind of unrelated) new tests with max timeout.
Squash or Rebase?
squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.