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.

Sign up for GitHub

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

Jump to bottom

Upgrade to egui 0.20 #113

Merged
merged 9 commits into from
Dec 13, 2022
Merged

Upgrade to egui 0.20 #113

merged 9 commits into from
Dec 13, 2022

Conversation

TimonPost
Copy link
Member

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Description of Changes

Upgrade egui, eframe to 0.20. And upgrade path version rather then minor version.

@TimonPost TimonPost requested a review from emilk as a code owner December 9, 2022 10:29
@emilk
Copy link
Collaborator

emilk commented Dec 9, 2022 •
edited

I would suggest opting in to eframe features. The new accesskit features pulls in a lot of dependencies, but puffin isn't really design with accessibility in mind anyways.

I would suggest eframe = { version = "0.20.0, default-features = false, features = ["persistence, "wgpu"] }

This also replaces the glow renderer with wgpu, which has some advantages (smoother resizing of the native window, mostly)

@TimonPost
Copy link
Member Author

TimonPost commented Dec 9, 2022 •
edited

Kept glow renderer as dep, adding wgpu seems to include a bunch of deps as well. ~200 locs in cargo.lock.

@emilk
Copy link
Collaborator

emilk commented Dec 9, 2022

The pub fn start(canvas_id: &str) needs to become async

@TimonPost
Copy link
Member Author

TimonPost commented Dec 9, 2022 •
edited

Think i'll also have to use wasm_bindgen_futures. Should we still return AppRunner?

@emilk
Copy link
Collaborator

emilk commented Dec 9, 2022

Yeah, you need to follow the pattern in this docstring that just got fixed: https://github.com/emilk/egui/pull/2422/files

TimonPost
TimonPost commented Dec 9, 2022
View reviewed changes
puffin_egui/Cargo.toml Outdated
macroquad = "0.3"
eframe = { version = "0.20.0", default-features = false, features = ["persistence", "glow", "default_fonts"] }

# Enable examplew when this dependency updated to egui 0.20
Copy link
Member Author

Choose a reason for hiding this comment

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

desirable? if we do enable it it will also add egui 0.19 in the lock which is a bit anoying

@TimonPost
Copy link
Member Author

TimonPost commented Dec 12, 2022 •
edited

@emilk can you do an other review or approve? Would like to do a puffin release with new imgui and egui versions. TY!

emilk
emilk approved these changes Dec 12, 2022
View reviewed changes
Copy link
Collaborator

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Looks great, but I would suggest using egui 0.20.1

puffin_egui/Cargo.toml Outdated Show resolved Hide resolved
@TimonPost @emilk
Update puffin_egui/Cargo.toml
ef42682 
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@TimonPost TimonPost merged commit 3c5e534 into main Dec 13, 2022
@TimonPost TimonPost deleted the timon/egui-0.20 branch December 13, 2022 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@TimonPost @emilk