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

Gracefully handle get_next_texture failure #369

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

antonok-edm
Copy link
Contributor

Fixes gfx-rs/wgpu-rs#106

Pretty simple implementation, let me know if this isn't sufficient

@kvark
Copy link
Member

kvark commented Nov 1, 2019

Thank you!
I don't think it fixes the issue though, the unwrap() is still there in wgpu_swap_chain_get_next_texture.

@antonok-edm
Copy link
Contributor Author

Gotcha, same story with #368 then. Didn't realize because I've just been using the Rust API...

I just updated the C API to fill an out param and return a status code - better?

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

That's better, thank you for the update!
One more concern...

ffi/wgpu.h Outdated Show resolved Hide resolved
wgpu-native/src/swap_chain.rs Outdated Show resolved Hide resolved
@antonok-edm
Copy link
Contributor Author

@kvark How's this?

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Alright, this is almost perfect. Just one more step!

wgpu-native/src/id.rs Outdated Show resolved Hide resolved
wgpu-native/src/swap_chain.rs Outdated Show resolved Hide resolved
wgpu-native/src/swap_chain.rs Outdated Show resolved Hide resolved
@antonok-edm
Copy link
Contributor Author

@kvark ok all set :)

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Nov 1, 2019
369: return Result instead of panic r=kvark a=antonok-edm

Fixes gfx-rs/wgpu-rs#106

Pretty simple implementation, let me know if this isn't sufficient

Co-authored-by: Anton Lazarev <antonok35@gmail.com>
@kvark kvark changed the title return Result instead of panic Gracefully handle get_next_texture failure Nov 1, 2019
@bors
Copy link
Contributor

bors bot commented Nov 1, 2019

Build succeeded

@bors bors bot merged commit 027a61a into gfx-rs:master Nov 1, 2019
@antonok-edm
Copy link
Contributor Author

@kvark does it make sense to change the corresponding wgpu-rs function to return a Result now?

@kvark
Copy link
Member

kvark commented Nov 5, 2019

yes, absolutely!

bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Nov 6, 2019
115: Handle error case for SwapChain::get_next_texture r=kvark a=antonok-edm

Updates `SwapChain::get_next_texture` to account for gfx-rs/wgpu#369. Otherwise, an invalid `Id` is returned and may cause confusing errors.

Co-authored-by: Anton Lazarev <antonok35@gmail.com>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
115: Handle error case for SwapChain::get_next_texture r=kvark a=antonok-edm

Updates `SwapChain::get_next_texture` to account for gfx-rs#369. Otherwise, an invalid `Id` is returned and may cause confusing errors.

Co-authored-by: Anton Lazarev <antonok35@gmail.com>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
369: Implement SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING r=kvark a=cwfitzgerald

This implements gfx-rs#715 in wgpu-rs. I haven't changed the example, as I want to actually think up a better example to use and didn't want that to block this. It will change in the future however.

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
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.

Panic when screen is turned off
2 participants