Skip to content

Conversation

uael
Copy link
Contributor

@uael uael commented Jul 30, 2025

Description
This try to handle egl errors instead of panicking out. I left some unwrap and expect where no Result was returned.

Testing
Manually tested.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@uael uael requested a review from a team as a code owner July 30, 2025 06:26
@uael uael force-pushed the uael/egl_less_panics branch 2 times, most recently from a5736e2 to ef685ac Compare July 30, 2025 13:25
@cwfitzgerald cwfitzgerald self-assigned this Jul 30, 2025
@uael uael force-pushed the uael/egl_less_panics branch 2 times, most recently from 8a36522 to dd86d76 Compare August 4, 2025 12:44
@jimblandy jimblandy assigned Wumpf and unassigned cwfitzgerald Aug 6, 2025
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

nice, way better. Some nits, but otherwise good to go

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

ah, sorry one more thing: could you please add a mention in the changelog under bugfixes? thanks!

@uael uael force-pushed the uael/egl_less_panics branch 2 times, most recently from 573fe50 to d4eda9f Compare August 11, 2025 09:23
@uael uael force-pushed the uael/egl_less_panics branch from 4387f91 to e6a83f1 Compare August 11, 2025 09:30
@uael
Copy link
Contributor Author

uael commented Aug 11, 2025

Just to add a bit of context, this is fixing an unwrap failed for some Android devices while creating a context. Unfortunately, the stack-trace I had access to wasn't precise enough to know which one was failing, but after this commit no more crash (nor errors) so I suspect one in configuration dump

Comment on lines +513 to +517
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)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

sounds like an interseting idea, but out of scope for this pr either way

@Wumpf Wumpf enabled auto-merge (squash) August 28, 2025 21:48
@Wumpf Wumpf merged commit cd14b19 into gfx-rs:trunk Aug 28, 2025
40 checks passed
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.

4 participants