From facf9c032815dd59b1c4df6546866397b719000b Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Sun, 1 Oct 2023 13:34:52 -0500 Subject: [PATCH 1/4] Various accessibility API updates. * Add `bevy_a11y::ManageAccessibilityUpdates` to indicate whether the ECS should manage tree updates. * Add getter/setter to `bevy_a11y::AccessibilityRequested`. * Add `bevy_a11y::AccessibilitySystem` `SystemSet` for ordering relative to accessibility tree updates. * Correctly set initial focus to new windows on creation. --- crates/bevy_a11y/src/lib.rs | 54 +++++++++++++++++++++++++- crates/bevy_window/Cargo.toml | 1 + crates/bevy_window/src/lib.rs | 11 +++++- crates/bevy_winit/src/accessibility.rs | 22 ++++++----- crates/bevy_winit/src/winit_windows.rs | 3 +- 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/crates/bevy_a11y/src/lib.rs b/crates/bevy_a11y/src/lib.rs index ed86377e3ca2c..3e43fd5fb574b 100644 --- a/crates/bevy_a11y/src/lib.rs +++ b/crates/bevy_a11y/src/lib.rs @@ -6,7 +6,10 @@ use std::{ num::NonZeroU128, - sync::{atomic::AtomicBool, Arc}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, }; pub use accesskit; @@ -15,6 +18,7 @@ use bevy_app::Plugin; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ prelude::{Component, Entity, Event}, + schedule::SystemSet, system::Resource, }; @@ -30,6 +34,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. /// @@ -64,12 +108,20 @@ impl AccessKitEntityExt for Entity { #[derive(Resource, Default, Deref, DerefMut)] pub struct Focus(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; impl Plugin for AccessibilityPlugin { fn build(&self, app: &mut bevy_app::App) { app.init_resource::() + .init_resource::() .init_resource::(); } } diff --git a/crates/bevy_window/Cargo.toml b/crates/bevy_window/Cargo.toml index 8bffd6a53c816..e245bb5fbc01e 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 e62f8bedc655a..bb367918db420 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/src/accessibility.rs b/crates/bevy_winit/src/accessibility.rs index f22291af8d533..a5bc85bbd357d 100644 --- a/crates/bevy_winit/src/accessibility.rs +++ b/crates/bevy_winit/src/accessibility.rs @@ -2,20 +2,21 @@ 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, + AccessKitEntityExt, 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}; @@ -48,17 +49,16 @@ fn handle_window_focus( ) { 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 { + if let Some(focus_id) = **focus { + adapter.update_if_active(|| TreeUpdate { focus: if event.focused { Some(focus_id.to_node_id()) } else { None }, ..default() - } - }); + }); + } } } } @@ -91,6 +91,7 @@ fn update_accessibility_nodes( focus: Res, accessibility_requested: Res, primary_window: Query<(Entity, &Window), With>, + manage_accessibility_updates: Res, nodes: Query<( Entity, &AccessibilityNode, @@ -99,7 +100,7 @@ fn update_accessibility_nodes( )>, node_entities: Query>, ) { - if !accessibility_requested.load(Ordering::SeqCst) { + if !accessibility_requested.get() || !manage_accessibility_updates.get() { return; } if let Ok((primary_window_id, primary_window)) = primary_window.get_single() { @@ -176,7 +177,8 @@ impl Plugin for AccessibilityPlugin { window_closed, poll_receivers, 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 3ca88ed47ab0a..ffcc96b5e1aea 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -1,5 +1,4 @@ #![warn(missing_docs)] -use std::sync::atomic::Ordering; use accesskit_winit::Adapter; use bevy_a11y::{ @@ -157,7 +156,7 @@ impl WinitWindows { 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)), From fd8c1cb12cd090f6c5c615009ec647b169cd8b0e Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Sun, 1 Oct 2023 22:16:17 -0500 Subject: [PATCH 2/4] Update `accesskit` and `accesskit_winit`. --- crates/bevy_a11y/Cargo.toml | 2 +- crates/bevy_a11y/src/lib.rs | 24 ++-------- crates/bevy_ui/src/accessibility.rs | 4 +- crates/bevy_winit/Cargo.toml | 2 +- crates/bevy_winit/src/accessibility.rs | 66 +++++++------------------- crates/bevy_winit/src/winit_windows.rs | 10 ++-- 6 files changed, 31 insertions(+), 77 deletions(-) diff --git a/crates/bevy_a11y/Cargo.toml b/crates/bevy_a11y/Cargo.toml index 6a35a0e661a0e..c5da00c51f7a4 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 3e43fd5fb574b..01adf54ab145c 100644 --- a/crates/bevy_a11y/src/lib.rs +++ b/crates/bevy_a11y/src/lib.rs @@ -4,16 +4,13 @@ #![allow(clippy::type_complexity)] #![forbid(unsafe_code)] -use std::{ - num::NonZeroU128, - sync::{ - atomic::{AtomicBool, Ordering}, - 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::{ @@ -91,19 +88,6 @@ 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); diff --git a/crates/bevy_ui/src/accessibility.rs b/crates/bevy_ui/src/accessibility.rs index 74a8ae58963ef..50e883695524a 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_winit/Cargo.toml b/crates/bevy_winit/Cargo.toml index fd7d4d3a373c1..14f486d3159dc 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", 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 a5bc85bbd357d..3e802b5479346 100644 --- a/crates/bevy_winit/src/accessibility.rs +++ b/crates/bevy_winit/src/accessibility.rs @@ -7,8 +7,10 @@ use std::{ use accesskit_winit::Adapter; use bevy_a11y::{ - accesskit::{ActionHandler, ActionRequest, NodeBuilder, NodeClassSet, Role, TreeUpdate}, - AccessKitEntityExt, AccessibilityNode, AccessibilityRequested, AccessibilitySystem, 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}; @@ -20,8 +22,8 @@ use bevy_ecs::{ 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)] @@ -36,33 +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) { - if let Some(focus_id) = **focus { - adapter.update_if_active(|| TreeUpdate { - focus: if event.focused { - Some(focus_id.to_node_id()) - } else { - None - }, - ..default() - }); - } - } - } -} - fn window_closed( mut adapters: NonSendMut, mut receivers: ResMut, @@ -109,37 +90,31 @@ fn update_accessibility_nodes( 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()), )); } @@ -149,12 +124,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), } }); } @@ -172,12 +147,7 @@ impl Plugin for AccessibilityPlugin { .add_event::() .add_systems( PostUpdate, - ( - handle_window_focus, - window_closed, - poll_receivers, - update_accessibility_nodes, - ) + (window_closed, poll_receivers, 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 ffcc96b5e1aea..43c0e414b2956 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -2,8 +2,8 @@ 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; @@ -150,9 +150,9 @@ 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 || { @@ -160,7 +160,7 @@ impl WinitWindows { TreeUpdate { nodes: vec![(accesskit_window_id, root)], tree: Some(Tree::new(accesskit_window_id)), - focus: None, + focus: accesskit_window_id, } }, Box::new(handler.clone()), From a8611f2d0e71e5d9bae1545221d036f926282c12 Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Mon, 2 Oct 2023 07:30:12 -0500 Subject: [PATCH 3/4] Update crates/bevy_a11y/src/lib.rs Co-authored-by: StaffEngineer <111751109+StaffEngineer@users.noreply.github.com> --- crates/bevy_a11y/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_a11y/src/lib.rs b/crates/bevy_a11y/src/lib.rs index 01adf54ab145c..576b920f4183b 100644 --- a/crates/bevy_a11y/src/lib.rs +++ b/crates/bevy_a11y/src/lib.rs @@ -90,7 +90,7 @@ impl From for AccessibilityNode { /// 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)] From e94e6c90586f54aa0d781e9296c4f1073eea3dc4 Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Mon, 2 Oct 2023 07:49:24 -0500 Subject: [PATCH 4/4] Refactor into run condition. --- crates/bevy_winit/src/accessibility.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/crates/bevy_winit/src/accessibility.rs b/crates/bevy_winit/src/accessibility.rs index 3e802b5479346..b0f4b6b1229a0 100644 --- a/crates/bevy_winit/src/accessibility.rs +++ b/crates/bevy_winit/src/accessibility.rs @@ -67,12 +67,17 @@ 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>, - manage_accessibility_updates: Res, nodes: Query<( Entity, &AccessibilityNode, @@ -81,9 +86,6 @@ fn update_accessibility_nodes( )>, node_entities: Query>, ) { - if !accessibility_requested.get() || !manage_accessibility_updates.get() { - 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(); @@ -147,7 +149,12 @@ impl Plugin for AccessibilityPlugin { .add_event::() .add_systems( PostUpdate, - (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), ); }