Implement accessibility APIs via AccessKit

[mwcampbell]

This PR makes egui accessible with assistive technologies such as screen readers. In contrast with egui's existing built-in screen reader, this PR uses AccessKit to implement platform accessibility APIs that are used by existing screen readers and other assistive technologies.

AccessKit is only implemented for Windows so far. I will focus on a Mac implementation over the next week or two.

We should discuss what level of completeness this branch needs to reach before being merged upstream. I haven't yet implemented support for all egui widgets. In general, this branch supports the widgets that were already accessible via WidgetInfo.

Closes #167.

[mwcampbell]

@emilk Are there any particular milestones you want this work to reach before I take the PR out of draft status?

[mwcampbell]

I wonder if I should make AccessKit an optional feature in egui-winit as it is in egui. To do that, I may need to refactor my changes to egui-winit, to initialize the AccessKit winit adapter separately. But by doing this, only eframe would be opinionated about whether to use AccessKit, and I suppose I'd be able to revert my modifications to egui_glium and egui_glow.

[emilk]

@mwcampbell I'll take a proper look as soon as I find time. I have a very busy week, but I'll do my best to squeeze this in!

And thanks so much for working on this 🙏

My general sense is that I would prefer it all to be optional in every crate, but I don't yet have a sense of how much work that is. But this is all pulling in a lot more dependencies: https://github.com/emilk/egui/pull/2294/files#diff-13ee4b2252c9e516a0547f2891aa2105c3ca71c6d7a1e682c69be97998dfc87e

[mwcampbell]

I just modified accesskit_winit so it can be used without requiring an EventLoopProxy and a specific user event type. So I should be able to refactor my changes to eframe to get back to requiring only RequestRepaintEvent. The downside of such a change is that I'll have to put either egui_winit::State or its egui_input field behind a mutex, since an AccessKit ActionHandler implementation may be called from any thread. That might still be preferrable to the changes that I currently have to make to plumb the user event type that accesskit_winit used to require.

[emilk]

I just modified accesskit_winit so it can be used without requiring an EventLoopProxy and a specific user event type. So I should be able to refactor my changes to eframe to get back to requiring only RequestRepaintEvent

Even if you do, I think a pub enum UserEvent { RequestRepaint } still makes sense.

[mwcampbell]

I decided that putting the egui_winit state, or even just the egui_input field of that state, behind a mutex was just as invasive a change as the previous change I made to the user event type, so I decided not to bother with that. However, I did make AccessKit an optional feature in egui-winit, which allowed me to revert my previous changes to egui_glium and egui_glow. So now, only eframe is opinionated about using AccessKit.

I could make AccessKit an optional feature even in eframe, though enabled by default. On the one hand, I'm apprehensive about making it too easy to disable accessibility; I wouldn't want to make it easy for a developer to disable accessibility just because they're idly reducing the size of their executable or trying to improve hypothetical performance, leading to a situation where a disabled person can't use an application they need for their job. On the other hand, I don't want to be so dogmatic about forcing accessibility onto everyone that it leads to a backlash.

[emilk]

I think we should keep this behavior. Tab to select the next button is what I ususally want from a GUI. Selecting labels is only interesting if the screen reader is on

[mwcampbell]

Please see the corresponding change I made in the label widget. Short version: the label widget now adjusts its sense setting based on whether the built-in screen reader is on.

[emilk]

Oh I see. Why did you opt to handle that behavior in Label rather than here?

[mwcampbell]

Since the label widget is the one special case, I thought it made sense to encapsulate the special logic in that widget. And this way, the AccessKit integration code can just look at sense.focus without having to know about the label special case. Does that make sense?

[emilk]

This is looking really nice!

I would still like for accesskit to be an optional (but perhaps default) feature of eframe. Seems like it should be fairly easy.

I made another PR, introducing UserEvent, to reduce the diff of this PR and to lessen the chance for merge conflicts for others working in eframe in parallel.

[mwcampbell]

@emilk I rebased on your recent PR, and now AccessKit is an optional feature even in eframe. You seem unsure about whether it should be enabled by default in eframe. What are your concerns about enabling it by default?

To explain why I think accessibility should be the default at least in the highest-level framework, I'll share an anecdote: My best friend, who is totally blind, bought a piece of hardware that had a companion app to set it up. He couldn't independently use that app, because it was Qt-based, and it was shipped without Qt's optional accessibility support. So apparently the first person to find out that accessibility was missing was the end-user, and you can imagine the layers such a bug report would have to go through before the app developer would even look at it, let alone ship a fix. Now imagine that he had to use the app to do his job. That's why I think that requiring application developers to enable accessibility is a mistake. I can appreciate the reasons for disabling it by default in egui and even egui-winit; properly supporting accessibility in a custom egui integration requires some extra work. But in eframe, it should just work by default.

[emilk]

@mwcampbell my only concern is build times and other potential build problems from extra dependencies, especially since AccessKit is currently only working on Windows, yet is pulled in on all platforms.

But, you make a very good point about it being enabled by default, so let's keep it like that!

[mwcampbell]

I'm eager to minimize all possible barriers to implementing accessibility, so I'm willing to do work to minimize AccessKit's impact on build time, but not necessarily immediately. For now, I need to focus on getting it working at all, across platforms. BTW, I'm currently working on a macOS implementation, but don't yet have anything usable to show.

[mwcampbell]

@emilk I need to write changelog entries for this PR. To acknowledge the funding that Google has provided for both this integration work and the development of AccessKit itself, I propose including something like the following in the entry in the main egui changelog:

Special thanks to the Google Fonts team for funding both this integration and the development of AccessKit itself.

Is this OK? I already cleared it with my main contact at Google.

[mwcampbell]

I'm reconsidering my previous changes to DragValue. I now believe that automatically switching into edit mode when the widget receives keyboard focus is the correct behavior. But it means that because the button itself can never receive keyboard focus, the current code for incrementing and decrementing the value with the keyboard doesn't work. I'm going to address that in a separate issue and PR, then rebase this branch.

[mwcampbell]

This is now blocked on #2342.

[mwcampbell]

I've decided it's time to take this PR out of draft status. There are, of course, more widget types to support, and more things to polish. But I believe the foundation is now solid, and for the sake of keeping the scope of a single PR manageable, I won't do any more work on this one, except for review feedback.

I've done some more refactoring. Most notably, AccessKit support is now lazy. That is, an egui context now only starts generating AccessKit tress once the integration enables AccessKit support in that context. This way, the integration can defer AccessKit initialization until there is actually a request for accessibility info, e.g. from a screen reader. So, even when the AccessKit feature is enabled, the performance impact for users that don't need accessibility is negligible. The downside, at least for egui-winit and eframe, is that the integration has to return a placeholder tree at first, then push a real one on the next repaint. The relevant code in eframe uses egui::Context::request_repaint to request a repaint. In theory, a screen reader could decide how to read a window that has just appeared or received focus based on the (lack of) content in the placeholder tree. But so far, this isn't happening on either Windows or macOS. And there's precedent; Chromium's accessibility implementation has to return a placeholder tree for the web content area at first, then get a real one from the renderer process. I think lazy initialization is critical for wide acceptance of AccessKit, so I'm going to go with this compromise.

[emilk]

This looks amazing - I only have a few small nits - fix those and I'll merge!

[emilk]

Suggested change

	#[cfg(feature = "accesskit")]
	accesskit_update: _, // not currently implemented
	#[cfg(feature = "accesskit")]
	accesskit_update: _, // not currently implemented

[mwcampbell]

The current spacing is what rustfmt wants. If I change it, rustfmt just puts it back.

[emilk]

Oh I see. Why did you opt to handle that behavior in Label rather than here?

[mwcampbell]

@emilk I think that covers everything. Thanks for taking time to review. Looking forward to getting this merged. Also, do you think it would be a good idea to publish egui 0.20 when this is merged?

[emilk]

@mwcampbell merged! 🍾 🥳

Amazing work!

Yes, I'm try to find time to publish egui 0.20 this week!