[egl] get rid of unwrap where possible
#8024
Conversation
uael
force-pushed
the
uael/egl_less_panics
branch
2 times, most recently
from
a5736e2 to
ef685ac
Compare
uael
force-pushed
the
uael/egl_less_panics
branch
2 times, most recently
from
8a36522 to
dd86d76
Compare
| library.get(c"XOpenDisplay".to_bytes()).unwrap(); | ||
| let func: libloading::Symbol<XOpenDisplayFun> = library | ||
| .get(c"XOpenDisplay".to_bytes()) | ||
| .inspect_err(|err| log::error!("Failed to load XOpenDisplay: {err:?}")) |
There was a problem hiding this comment.
feels like we should have an error category for failing to load methods which we can propagate. But this is already strictly better 🤷
There was a problem hiding this comment.
Are you sure? This changes the semantics a little bit, as if there was no X display that could be opened because standard functions that should theoretically always be there were not found (I doubt that could/should ever happen, hence unwrap()/expect() make sense).
uael
force-pushed
the
uael/egl_less_panics
branch
2 times, most recently
from
573fe50 to
d4eda9f
Compare
Co-authored-by: Andreas Reich <r_andreas2@web.de>
uael
force-pushed
the
uael/egl_less_panics
branch
from
4387f91 to
e6a83f1
Compare
|
Just to add a bit of context, this is fixing an |
| fn instance_err<E: core::error::Error + Send + Sync + 'static>( | ||
| message: impl Into<String>, | ||
| ) -> impl FnOnce(E) -> crate::InstanceError { | ||
| move |e| crate::InstanceError::with_source(message.into(), e) | ||
| } |
There was a problem hiding this comment.
Would be much clearer if wgpu could define an anyhow-like context() trait for every Result. That should make it a lot less likely to loose source errors with other(), that I see a bunch of PRs do.
| library.get(c"XOpenDisplay".to_bytes()).unwrap(); | ||
| let func: libloading::Symbol<XOpenDisplayFun> = library | ||
| .get(c"XOpenDisplay".to_bytes()) | ||
| .inspect_err(|err| log::error!("Failed to load XOpenDisplay: {err:?}")) |
There was a problem hiding this comment.
Are you sure? This changes the semantics a little bit, as if there was no X display that could be opened because standard functions that should theoretically always be there were not found (I doubt that could/should ever happen, hence unwrap()/expect() make sense).
| @@ -91,6 +91,10 @@ By @Vecvec in [#7913](https://github.com/gfx-rs/wgpu/pull/7913). | |||
|
|
|||
| - Fixed a bug where access to matrices with 2 rows would not work in some cases. By @andyleiserson in [#7438](https://github.com/gfx-rs/wgpu/pull/7438). | |||
|
|
|||
| ##### EGL | |||
|
|
|||
| - Fixed unwrap failed in context creation for some Android devices. By @uael in [#8024](https://github.com/gfx-rs/wgpu/pull/8024). | |||
There was a problem hiding this comment.
This does not seem to match the title of the PR. This only intends to add context right? Not fix any actual bugs, this changelog being under ### Bug Fixes?
There was a problem hiding this comment.
Yes, I added after #8024 (review)
Description
This try to handle egl errors instead of panicking out. I left some unwrap and expect where no
Resultwas returned.Testing
Manually tested.
Checklist
cargo fmt.Runtaplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.If this contains user-facing changes, add aCHANGELOG.mdentry.