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 accesskit_winit #4219

Closed
mwcampbell opened this issue Mar 23, 2024 · 6 comments
Closed

Update accesskit_winit #4219

mwcampbell opened this issue Mar 23, 2024 · 6 comments
Labels
accessibility More accessible to e.g. the visually impaired dependencies Pull requests that update a dependency file help wanted Extra attention is needed

Comments

@mwcampbell
Copy link
Contributor

accesskit_winit is on 0.18 now, and we have another refactor on the way which will be released in accesskit_winit 0.19. @emilk Do you need help from us to keep egui-winit up to date, or is there something on our end that's blocking the update?

@emilk
Copy link
Owner

emilk commented Mar 29, 2024

The main thing that's been keeping me from updating it is that I don't know how to properly test it. Help would be greatly appreciated!

@emilk emilk added help wanted Extra attention is needed accessibility More accessible to e.g. the visually impaired dependencies Pull requests that update a dependency file labels Mar 29, 2024
@mwcampbell
Copy link
Contributor Author

I updated egui on this branch to work with an AccessKit refactor that I plan to release soon, and I noticed a cargo deny problem: AccessKit uses objc2 0.5.0, icrate 0.1.0, and block2 0.4.0, while winit still uses objc2 0.4.1, icrate 0.0.4, and block2 0.3.0. It was block2 that actually triggered the cargo deny error. Do we need to downgrade AccessKit's dependencies on objc2 and icrate to match winit's, or do you want to add an exception in deny.toml?

@emilk
Copy link
Owner

emilk commented Apr 4, 2024

It depends on how much work it is, I guess.

If the change in accesskit when upgrading was just a version bump (no change in code needed), then you could support multiple version of objc2 etc (objc = ">=0.4.0 <= 0.5.0") which would be the best of two worlds.

Duplicated dependencies are super annoying, as it adds to the already long compile time of eframe

@DataTriny
Copy link
Contributor

It looks like winit 0.30 is coming soon. I'd suggest we update all at once. We need to move away from icrate as quickly as possible because of its number of compile features.

See rust-windowing/winit#3634

emilk added a commit that referenced this issue Apr 23, 2024
These are a replacement to the `objc` and `cocoa` crates.

This PR prevents:
- An extra copy when creating `NSData`
- A memory leak when creating `NSImage`
- A memory leak when creating `NSString`

And is generally a readability improvement.

Note that we define `NSApp` manually for now, the implementation in
`objc2-app-kit` is currently suboptimal and wouldn't allow you to check
whether the NSApplication has been created or not.

Related: #4219, this should nicely
coincide with the Winit `0.30` release.

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@emilk emilk changed the title egui-winit still using accesskit_winit 0.16 Update accesskit_winit May 2, 2024
@torokati44
Copy link
Contributor

Just curious - what's the state of this, now that #4849 has been merged?

@mwcampbell
Copy link
Contributor Author

#4849 fixed this.

hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
These are a replacement to the `objc` and `cocoa` crates.

This PR prevents:
- An extra copy when creating `NSData`
- A memory leak when creating `NSImage`
- A memory leak when creating `NSString`

And is generally a readability improvement.

Note that we define `NSApp` manually for now, the implementation in
`objc2-app-kit` is currently suboptimal and wouldn't allow you to check
whether the NSApplication has been created or not.

Related: emilk#4219, this should nicely
coincide with the Winit `0.30` release.

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility More accessible to e.g. the visually impaired dependencies Pull requests that update a dependency file help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants