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

Update to latest egui, with added error handling in eframe #537

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

emilk
Copy link
Member

@emilk emilk commented Dec 12, 2022

Related egui PR: emilk/egui#2433

This will lead to Err and python exceptions instead of panics for users for which we cannot initialize a rendering context (or at least for some of them).

The update also includes some improved logging during shutdown (at debug log level) and some minor tweaks.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I've added a line to CHANGELOG.md (if this is a big enough change to warrant it)

This will lead to `Err` and python exceptions instead of panics
for users for which we cannot initialize a rendering context
(or at least for some of them).
@@ -1,7 +1,7 @@
use re_log_types::LogMsg;

#[cfg(feature = "re_viewer")]
pub fn show(log_messages: Vec<LogMsg>) {
pub fn show(log_messages: Vec<LogMsg>) -> eframe::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

I was a bit surprised to see an eframe::Result leak out of the SDK. Seems like we should potentially be introducing a rerun_sdk error type.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I was a bit hesitant here too. We should probably wrap it in our own type, but this is an ok start imho

@emilk emilk merged commit cb37a0e into main Dec 12, 2022
@emilk emilk deleted the emilk/egui-update branch December 12, 2022 16:19
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.

2 participants