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

AccessKit needs Rust 1.61 #1844

Closed
mwcampbell opened this issue Jul 23, 2022 · 9 comments · Fixed by #1846
Closed

AccessKit needs Rust 1.61 #1844

mwcampbell opened this issue Jul 23, 2022 · 9 comments · Fixed by #1846

Comments

@mwcampbell
Copy link
Contributor

The AccessKit Windows implementation needs Rust 1.61+ so it can use a recent version of windows-rs. I suggest updating egui to require Rust 1.61.

@emilk
Copy link
Owner

emilk commented Jul 23, 2022

This is an argument for making egui compatible with 1.61, but I don't see why MSRV for egui needs to be incremented? Every time we bump MSRV we get a lot of users showing up with confusing rustc error messages, so I am hesitant to bump MSRV without a good reason for it!

Granted, 1.61 has been out for two months already, so it is not hot off the presses.

@mwcampbell
Copy link
Contributor Author

I'm asking to bump the MSRV because, if you're willing, I want to integrate AccessKit into egui, so egui can be properly accessible with existing screen readers and other accessibility tools, as opposed to requiring egui applications to provide their own TTS output. The benefits of this have been discussed on #167.

If you don't want to bump egui's MSRV, then I suppose I need to either persuade the windows-rs maintainers to roll back the change that made windows-rs require Rust 1.61 for implementing COM interfaces, or AccessKit will need to be stuck on an old version of windows-rs until you decide to bump the MSRV for egui.

@emilk
Copy link
Owner

emilk commented Jul 24, 2022

Ah, yeah for integrating AccessKit into egui we need to bump the MSRV. I'm not all against bumping the MSRV, I just want to make sure we do it for the right reasons!

That being said, have you considered making the AccessKit integration an add-on onto egui instead? I haven't at all looked into what this would entail, but if it wasn't too much work I would prefer a system where egui emitted enough events (and provided enough control) for that to be possible. However, if that would create too much double-work then it's probably not worth it.

@mwcampbell
Copy link
Contributor Author

I'm not comfortable making accessibility an optional feature. Of course, it's necessary to do so when you implement accessibility by doing your own speech output. But when implementing platform accessibility APIs, as AccessKit does, accessibility should be invisible to users who don't need it. Of course, if you find that AccessKit has too great an impact on performance even when no screen reader or other assistive technology is running, then that's something I need to work on. My concern about making accessibility through something like AccessKit optional is that if it's off by default, too many developers won't turn it on, and if it can be turned off, some developers will, in a misguided effort to reduce code size or marginally increase performance by eliminating something they don't care about. My hope is that, by having something like AccessKit always there, all egui-based applications will be at least partially accessible by default.

You wrote about egui emitting enough events and providing enough control that AccessKit integration could be done outside of egui proper. I do think that would be unnecessary double work. I'm familiar with your current WidgetInfo struct and related events. These don't provide nearly enough information for a full implementation of native accessibility APIs, especially when it comes to text editing. My hope is that AccessKit can become the accessibility abstraction that egui and other Rust GUI toolkits standardize on. Note that the accesskit crate, which is the one that the egui crate itself depends on, is small and has few dependencies. The bulk of the code and dependencies in AccessKit is in other crates, particularly the platform-specific ones like accesskit_windows, and the new accesskit_winit crate which depends on those platform-specific crates. These crates will be used by egui backends, like eframe (actually, egui_winit), but not by the egui crate itself. The accesskit crate basically just defines data structures and traits, so I don't think it makes sense to define equivalent ones in egui.

@mwcampbell
Copy link
Contributor Author

Since you mentioned events, I should also point out that AccessKit is specifically designed to be friendly to immediate-mode GUIs, so it doesn't require you to track state and conditionally emit events when things change. Instead, egui would generate a whole accessibility tree for each frame, just as it generates all the shapes to be rendered. When you push a tree update to AccessKit, whether it's an incremental update or the whole tree, AccessKit will determine what changed and emit the appropriate events. Most of that logic is separate from the platform-specific implementations. So a speech output implementation based on AccessKit could completely replace the speech output support that you have now in egui, meaning you could entirely eliminate WidgetInfo and the related events.

@emilk
Copy link
Owner

emilk commented Jul 25, 2022

I'm not comfortable making accessibility an optional feature. Of course, it's necessary to do so when you implement accessibility by doing your own speech output. But when implementing platform accessibility APIs, as AccessKit does, accessibility should be invisible to users who don't need it. Of course, if you find that AccessKit has too great an impact on performance even when no screen reader or other assistive technology is running, then that's something I need to work on. My concern about making accessibility through something like AccessKit optional is that if it's off by default, too many developers won't turn it on, and if it can be turned off, some developers will, in a misguided effort to reduce code size or marginally increase performance by eliminating something they don't care about. My hope is that, by having something like AccessKit always there, all egui-based applications will be at least partially accessible by default.

I agree with your sentiment here, and for eframe it may make sense. eframe is a "batteries included" framework for egui, and here a lost of features are opt-out (enabled by default), and I definitely think accessibility should be among those. egui proper is a different story, however. egui is purposefully built with a minimum amount of dependencies so that it is light when compiled to WASM, and so that it compiles quickly for all its users. There are many people whom use egui for dev-tools for their personal game engines, and I don't see how they all need AccessKit.

You wrote about egui emitting enough events and providing enough control that AccessKit integration could be done outside of egui proper. I do think that would be unnecessary double work. I'm familiar with your current WidgetInfo struct and related events. These don't provide nearly enough information for a full implementation of native accessibility APIs, especially when it comes to text editing. My hope is that AccessKit can become the accessibility abstraction that egui and other Rust GUI toolkits standardize on. Note that the accesskit crate, which is the one that the egui crate itself depends on, is small and has few dependencies. The bulk of the code and dependencies in AccessKit is in other crates, particularly the platform-specific ones like accesskit_windows, and the new accesskit_winit crate which depends on those platform-specific crates. These crates will be used by egui backends, like eframe (actually, egui_winit), but not by the egui crate itself. The accesskit crate basically just defines data structures and traits, so I don't think it makes sense to define equivalent ones in egui.

You've convinced me! Replacing WidgetInfo with something standard would be aswesome.

Since you mentioned events, I should also point out that AccessKit is specifically designed to be friendly to immediate-mode GUIs, so it doesn't require you to track state and conditionally emit events when things change. Instead, egui would generate a whole accessibility tree for each frame, just as it generates all the shapes to be rendered. When you push a tree update to AccessKit, whether it's an incremental update or the whole tree, AccessKit will determine what changed and emit the appropriate events. Most of that logic is separate from the platform-specific implementations. So a speech output implementation based on AccessKit could completely replace the speech output support that you have now in egui, meaning you could entirely eliminate WidgetInfo and the related events.

That's awesome!

@coderedart
Copy link
Contributor

and if it can be turned off, some developers will, in a misguided effort to reduce code size or marginally increase performance by eliminating something they don't care about

just wanted to point out legitimate issues if i cannot disable a feature.

for example, i cannot compile eframe with --all-features on my machine due to libspeechd linking errors #1125 . so, it literally takes hours for me to add a simple feature. make a small commit -> push -> wait for workflow -> check if there's any errors -> fix them and repeat. ofcourse, i guess egui's repo being a little too complex also slightly contributes some pain (i have no idea which crate depends on which), but the main issue is still the libspeechd linker errors.

idk what libraries accesskit links with (lib-at-spi2-core?), but being able to disable it with a feature to allow things like being able to cross-compile (linux host windows target or windows host linux target) etc.. would be really nice, atleast when i'm still working on a feature and not releasing to users.

if not for that, i am just happy that egui is getting first-class support for accessibility.

@mwcampbell
Copy link
Contributor Author

Difficulty compiling and linking with platform-specific C libraries is a valid problem, and one that I'm trying to minimize in AccessKit. (In general, one of my guiding principles for AccessKit is that I should make it as easy as possible to integrate.)

So far, AccessKit is only implemented for Windows. @DataTriny has started working on a Linux implementation, but we're both focused on other things right now. The plan for the Linux implementation is to use a pure Rust implementation of both D-Bus and AT-SPI, to avoid the problem of compiling and linking with platform libraries. On Windows, AccessKit uses windows-rs (the source of the whole Rust version problem that prompted this issue), which ships its own Windows SDK import libraries, so anyone who can compile a Rust binary for Windows should be able to compile one with AccessKit without difficulty. (I've even verified that cross-compiling for x86_64-pc-windows-gnu on Linux works.) On Mac, I believe AccessKit will only need to link with common system libraries such as objc, CoreFoundation, Foundation, and AppKit, which are already included with Xcode.

@emilk
Copy link
Owner

emilk commented Jul 25, 2022

Off topic, but:

for example, i cannot compile eframe with --all-features on my machine due to libspeechd linking errors #1125 . so, it literally takes hours for me to add a simple feature. make a small commit -> push -> wait for workflow -> check if there's any errors -> fix them and repeat

That sucks, seems like we should really fix #1125. But surely cargo check and cargo cranky still works for you? Most of the time, you don't need to check all features.

i guess egui's repo being a little too complex also slightly contributes some pain (i have no idea which crate depends on which)

it's pretty easy to figure out:

❯ cargo tree -p eframe --all-features | rg egui       
eframe v0.18.0
├── egui v0.18.1
│   ├── epaint v0.18.1
│   │   ├── emath v0.18.0
├── egui-wgpu v0.18.0
│   ├── egui v0.18.1 (*)
├── egui-winit v0.18.0
│   ├── egui v0.18.1 (*)
├── egui_glow v0.18.1
│   ├── egui v0.18.1 (*)

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 a pull request may close this issue.

3 participants