From 73e0ac26caf222b6a4cb4eefd98add3d2e5329fd Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Mon, 2 Oct 2023 16:22:52 -0500 Subject: [PATCH] Various accessibility API updates. (#9989) # Objective `bevy_a11y` was impossible to integrate into some third-party projects in part because it insisted on managing the accessibility tree on its own. ## Solution The changes in this PR were necessary to get `bevy_egui` working with Bevy's AccessKit integration. They were tested on a fork of 0.11, developed against `bevy_egui`, then ported to main and tested against the `ui` example. ## Changelog ### Changed * Add `bevy_a11y::ManageAccessibilityUpdates` to indicate whether the ECS should manage accessibility tree updates. * Add getter/setter to `bevy_a11y::AccessibilityRequested`. * Add `bevy_a11y::AccessibilitySystem` `SystemSet` for ordering relative to accessibility tree updates. * Upgrade `accesskit` to v0.12.0. ### Fixed * Correctly set initial accessibility focus to new windows on creation. ## Migration Guide ### Change direct accesses of `AccessibilityRequested` to use `AccessibilityRequested.::get()`/`AccessibilityRequested::set()` #### Before ``` use std::sync::atomic::Ordering; // To access accessibility_requested.load(Ordering::SeqCst) // To update accessibility_requested.store(true, Ordering::SeqCst); ``` #### After ``` // To access accessibility_requested.get() // To update accessibility_requested.set(true); ``` --------- Co-authored-by: StaffEngineer <111751109+StaffEngineer@users.noreply.github.com> --- crates/bevy_a11y/Cargo.toml | 2 +- crates/bevy_a11y/src/lib.rs | 72 +++++++++++++++------ crates/bevy_ui/src/accessibility.rs | 4 +- crates/bevy_window/Cargo.toml | 1 + crates/bevy_window/src/lib.rs | 11 +++- crates/bevy_winit/Cargo.toml | 2 +- crates/bevy_winit/src/accessibility.rs | 89 ++++++++++---------------- crates/bevy_winit/src/winit_windows.rs | 13 ++-- 8 files changed, 108 insertions(+), 86 deletions(-) diff --git a/crates/bevy_a11y/Cargo.toml b/crates/bevy_a11y/Cargo.toml index 6a35a0e661..c5da00c51f 100644 --- a/crates/bevy_a11y/Cargo.toml +++ b/crates/bevy_a11y/Cargo.toml @@ -14,4 +14,4 @@ bevy_app = { path = "../bevy_app", version = "0.12.0-dev" } bevy_derive = { path = "../bevy_derive", version = "0.12.0-dev" } bevy_ecs = { path = "../bevy_ecs", version = "0.12.0-dev" } -accesskit = "0.11" +accesskit = "0.12" diff --git a/crates/bevy_a11y/src/lib.rs b/crates/bevy_a11y/src/lib.rs index ed86377e3c..576b920f41 100644 --- a/crates/bevy_a11y/src/lib.rs +++ b/crates/bevy_a11y/src/lib.rs @@ -4,17 +4,18 @@ #![allow(clippy::type_complexity)] #![forbid(unsafe_code)] -use std::{ - num::NonZeroU128, - sync::{atomic::AtomicBool, Arc}, +use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, }; pub use accesskit; -use accesskit::{NodeBuilder, NodeId}; +use accesskit::NodeBuilder; use bevy_app::Plugin; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ prelude::{Component, Entity, Event}, + schedule::SystemSet, system::Resource, }; @@ -30,6 +31,46 @@ pub struct ActionRequest(pub accesskit::ActionRequest); #[derive(Resource, Default, Clone, Debug, Deref, DerefMut)] pub struct AccessibilityRequested(Arc); +impl AccessibilityRequested { + /// Returns `true` if an access technology is active and accessibility tree + /// updates should be sent. + pub fn get(&self) -> bool { + self.load(Ordering::SeqCst) + } + + /// Sets whether accessibility updates were requested by an access technology. + pub fn set(&self, value: bool) { + self.store(value, Ordering::SeqCst); + } +} + +/// Resource whose value determines whether the accessibility tree is updated +/// via the ECS. +/// +/// Set to `false` in cases where an external GUI library is sending +/// accessibility updates instead. Without this, the external library and ECS +/// will generate conflicting updates. +#[derive(Resource, Clone, Debug, Deref, DerefMut)] +pub struct ManageAccessibilityUpdates(bool); + +impl Default for ManageAccessibilityUpdates { + fn default() -> Self { + Self(true) + } +} + +impl ManageAccessibilityUpdates { + /// Returns `true` if the ECS should update the accessibility tree. + pub fn get(&self) -> bool { + self.0 + } + + /// Sets whether the ECS should update the accessibility tree. + pub fn set(&mut self, value: bool) { + self.0 = value; + } +} + /// Component to wrap a [`accesskit::Node`], representing this entity to the platform's /// accessibility API. /// @@ -47,22 +88,16 @@ impl From for AccessibilityNode { } } -/// Extensions to ease integrating entities with [`AccessKit`](https://accesskit.dev). -pub trait AccessKitEntityExt { - /// Convert an entity to a stable [`NodeId`]. - fn to_node_id(&self) -> NodeId; -} - -impl AccessKitEntityExt for Entity { - fn to_node_id(&self) -> NodeId { - let id = NonZeroU128::new(self.to_bits() as u128 + 1); - NodeId(id.unwrap()) - } -} - /// Resource representing which entity has keyboard focus, if any. #[derive(Resource, Default, Deref, DerefMut)] -pub struct Focus(Option); +pub struct Focus(pub Option); + +/// Set enum for the systems relating to accessibility +#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] +pub enum AccessibilitySystem { + /// Update the accessibility tree + Update, +} /// Plugin managing non-GUI aspects of integrating with accessibility APIs. pub struct AccessibilityPlugin; @@ -70,6 +105,7 @@ pub struct AccessibilityPlugin; impl Plugin for AccessibilityPlugin { fn build(&self, app: &mut bevy_app::App) { app.init_resource::() + .init_resource::() .init_resource::(); } } diff --git a/crates/bevy_ui/src/accessibility.rs b/crates/bevy_ui/src/accessibility.rs index 74a8ae5896..50e8836955 100644 --- a/crates/bevy_ui/src/accessibility.rs +++ b/crates/bevy_ui/src/accessibility.rs @@ -124,14 +124,14 @@ fn label_changed( .collect::>(); let name = Some(values.join(" ").into_boxed_str()); if let Some(mut accessible) = accessible { - accessible.set_role(Role::LabelText); + accessible.set_role(Role::StaticText); if let Some(name) = name { accessible.set_name(name); } else { accessible.clear_name(); } } else { - let mut node = NodeBuilder::new(Role::LabelText); + let mut node = NodeBuilder::new(Role::StaticText); if let Some(name) = name { node.set_name(name); } diff --git a/crates/bevy_window/Cargo.toml b/crates/bevy_window/Cargo.toml index 8bffd6a53c..e245bb5fbc 100644 --- a/crates/bevy_window/Cargo.toml +++ b/crates/bevy_window/Cargo.toml @@ -14,6 +14,7 @@ serialize = ["serde"] [dependencies] # bevy +bevy_a11y = { path = "../bevy_a11y", version = "0.12.0-dev" } bevy_app = { path = "../bevy_app", version = "0.12.0-dev" } bevy_ecs = { path = "../bevy_ecs", version = "0.12.0-dev" } bevy_math = { path = "../bevy_math", version = "0.12.0-dev" } diff --git a/crates/bevy_window/src/lib.rs b/crates/bevy_window/src/lib.rs index e62f8bedc6..bb367918db 100644 --- a/crates/bevy_window/src/lib.rs +++ b/crates/bevy_window/src/lib.rs @@ -7,6 +7,8 @@ //! The [`WindowPlugin`] sets up some global window-related parameters and //! is part of the [`DefaultPlugins`](https://docs.rs/bevy/latest/bevy/struct.DefaultPlugins.html). +use bevy_a11y::Focus; + mod cursor; mod event; mod raw_handle; @@ -99,9 +101,14 @@ impl Plugin for WindowPlugin { .add_event::(); if let Some(primary_window) = &self.primary_window { - app.world + let initial_focus = app + .world .spawn(primary_window.clone()) - .insert(PrimaryWindow); + .insert(PrimaryWindow) + .id(); + if let Some(mut focus) = app.world.get_resource_mut::() { + **focus = Some(initial_focus); + } } match self.exit_condition { diff --git a/crates/bevy_winit/Cargo.toml b/crates/bevy_winit/Cargo.toml index ab388a06a7..9deb11411c 100644 --- a/crates/bevy_winit/Cargo.toml +++ b/crates/bevy_winit/Cargo.toml @@ -29,7 +29,7 @@ bevy_tasks = { path = "../bevy_tasks", version = "0.12.0-dev" } # other winit = { version = "0.28.7", default-features = false } -accesskit_winit = { version = "0.14", default-features = false } +accesskit_winit = { version = "0.15", default-features = false } approx = { version = "0.5", default-features = false } raw-window-handle = "0.5" diff --git a/crates/bevy_winit/src/accessibility.rs b/crates/bevy_winit/src/accessibility.rs index f22291af8d..b0f4b6b122 100644 --- a/crates/bevy_winit/src/accessibility.rs +++ b/crates/bevy_winit/src/accessibility.rs @@ -2,25 +2,28 @@ use std::{ collections::VecDeque, - sync::{atomic::Ordering, Arc, Mutex}, + sync::{Arc, Mutex}, }; use accesskit_winit::Adapter; -use bevy_a11y::ActionRequest as ActionRequestWrapper; use bevy_a11y::{ - accesskit::{ActionHandler, ActionRequest, NodeBuilder, NodeClassSet, Role, TreeUpdate}, - AccessKitEntityExt, AccessibilityNode, AccessibilityRequested, Focus, + accesskit::{ + ActionHandler, ActionRequest, NodeBuilder, NodeClassSet, NodeId, Role, TreeUpdate, + }, + AccessibilityNode, AccessibilityRequested, AccessibilitySystem, Focus, }; +use bevy_a11y::{ActionRequest as ActionRequestWrapper, ManageAccessibilityUpdates}; use bevy_app::{App, Plugin, PostUpdate}; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ prelude::{DetectChanges, Entity, EventReader, EventWriter}, query::With, + schedule::IntoSystemConfigs, system::{NonSend, NonSendMut, Query, Res, ResMut, Resource}, }; use bevy_hierarchy::{Children, Parent}; -use bevy_utils::{default, HashMap}; -use bevy_window::{PrimaryWindow, Window, WindowClosed, WindowFocused}; +use bevy_utils::HashMap; +use bevy_window::{PrimaryWindow, Window, WindowClosed}; /// Maps window entities to their `AccessKit` [`Adapter`]s. #[derive(Default, Deref, DerefMut)] @@ -35,34 +38,12 @@ pub struct WinitActionHandlers(pub HashMap); pub struct WinitActionHandler(pub Arc>>); impl ActionHandler for WinitActionHandler { - fn do_action(&self, request: ActionRequest) { + fn do_action(&mut self, request: ActionRequest) { let mut requests = self.0.lock().unwrap(); requests.push_back(request); } } -fn handle_window_focus( - focus: Res, - adapters: NonSend, - mut focused: EventReader, -) { - for event in focused.read() { - if let Some(adapter) = adapters.get(&event.window) { - adapter.update_if_active(|| { - let focus_id = (*focus).unwrap_or_else(|| event.window); - TreeUpdate { - focus: if event.focused { - Some(focus_id.to_node_id()) - } else { - None - }, - ..default() - } - }); - } - } -} - fn window_closed( mut adapters: NonSendMut, mut receivers: ResMut, @@ -86,10 +67,16 @@ fn poll_receivers( } } +fn should_update_accessibility_nodes( + accessibility_requested: Res, + manage_accessibility_updates: Res, +) -> bool { + accessibility_requested.get() && manage_accessibility_updates.get() +} + fn update_accessibility_nodes( adapters: NonSend, focus: Res, - accessibility_requested: Res, primary_window: Query<(Entity, &Window), With>, nodes: Query<( Entity, @@ -99,46 +86,37 @@ fn update_accessibility_nodes( )>, node_entities: Query>, ) { - if !accessibility_requested.load(Ordering::SeqCst) { - return; - } if let Ok((primary_window_id, primary_window)) = primary_window.get_single() { if let Some(adapter) = adapters.get(&primary_window_id) { let should_run = focus.is_changed() || !nodes.is_empty(); if should_run { adapter.update_if_active(|| { let mut to_update = vec![]; - let mut has_focus = false; let mut name = None; if primary_window.focused { - has_focus = true; let title = primary_window.title.clone(); name = Some(title.into_boxed_str()); } - let focus_id = if has_focus { - (*focus).or_else(|| Some(primary_window_id)) - } else { - None - }; + let focus_id = (*focus).unwrap_or_else(|| primary_window_id).to_bits(); let mut root_children = vec![]; for (entity, node, children, parent) in &nodes { let mut node = (**node).clone(); if let Some(parent) = parent { - if node_entities.get(**parent).is_err() { - root_children.push(entity.to_node_id()); + if !node_entities.contains(**parent) { + root_children.push(NodeId(entity.to_bits())); } } else { - root_children.push(entity.to_node_id()); + root_children.push(NodeId(entity.to_bits())); } if let Some(children) = children { for child in children { - if node_entities.get(*child).is_ok() { - node.push_child(child.to_node_id()); + if node_entities.contains(*child) { + node.push_child(NodeId(child.to_bits())); } } } to_update.push(( - entity.to_node_id(), + NodeId(entity.to_bits()), node.build(&mut NodeClassSet::lock_global()), )); } @@ -148,12 +126,12 @@ fn update_accessibility_nodes( } root.set_children(root_children); let root = root.build(&mut NodeClassSet::lock_global()); - let window_update = (primary_window_id.to_node_id(), root); + let window_update = (NodeId(primary_window_id.to_bits()), root); to_update.insert(0, window_update); TreeUpdate { nodes: to_update, - focus: focus_id.map(|v| v.to_node_id()), - ..default() + tree: None, + focus: NodeId(focus_id), } }); } @@ -171,12 +149,13 @@ impl Plugin for AccessibilityPlugin { .add_event::() .add_systems( PostUpdate, - ( - handle_window_focus, - window_closed, - poll_receivers, - update_accessibility_nodes, - ), + (window_closed, poll_receivers).in_set(AccessibilitySystem::Update), + ) + .add_systems( + PostUpdate, + update_accessibility_nodes + .run_if(should_update_accessibility_nodes) + .in_set(AccessibilitySystem::Update), ); } } diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs index 3ca88ed47a..43c0e414b2 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -1,10 +1,9 @@ #![warn(missing_docs)] -use std::sync::atomic::Ordering; use accesskit_winit::Adapter; use bevy_a11y::{ - accesskit::{NodeBuilder, NodeClassSet, Role, Tree, TreeUpdate}, - AccessKitEntityExt, AccessibilityRequested, + accesskit::{NodeBuilder, NodeClassSet, NodeId, Role, Tree, TreeUpdate}, + AccessibilityRequested, }; use bevy_ecs::entity::Entity; @@ -151,17 +150,17 @@ impl WinitWindows { root_builder.set_name(name.into_boxed_str()); let root = root_builder.build(&mut NodeClassSet::lock_global()); - let accesskit_window_id = entity.to_node_id(); + let accesskit_window_id = NodeId(entity.to_bits()); let handler = WinitActionHandler::default(); - let accessibility_requested = (*accessibility_requested).clone(); + let accessibility_requested = accessibility_requested.clone(); let adapter = Adapter::with_action_handler( &winit_window, move || { - accessibility_requested.store(true, Ordering::SeqCst); + accessibility_requested.set(true); TreeUpdate { nodes: vec![(accesskit_window_id, root)], tree: Some(Tree::new(accesskit_window_id)), - focus: None, + focus: accesskit_window_id, } }, Box::new(handler.clone()),