From 191a76cd54b2ed0697395ceff8b7d87db6dc450f Mon Sep 17 00:00:00 2001 From: Ashley Wulber Date: Mon, 19 Aug 2024 12:52:30 -0400 Subject: [PATCH] fix: multiwindow a11y fixes --- core/src/window/id.rs | 10 +- examples/multi_window/Cargo.toml | 2 + examples/multi_window/src/main.rs | 5 +- widget/src/button.rs | 4 +- winit/src/application.rs | 5 +- winit/src/program.rs | 176 ++++++++++++++++++---------- winit/src/program/state.rs | 2 +- winit/src/program/window_manager.rs | 6 +- 8 files changed, 135 insertions(+), 75 deletions(-) diff --git a/core/src/window/id.rs b/core/src/window/id.rs index 31ea92f397..982d31117e 100644 --- a/core/src/window/id.rs +++ b/core/src/window/id.rs @@ -11,8 +11,16 @@ pub struct Id(u64); static COUNT: AtomicU64 = AtomicU64::new(1); impl Id { + /// No window will match this Id + pub const NONE: Id = Id(0); + /// Creates a new unique window [`Id`]. pub fn unique() -> Id { - Id(COUNT.fetch_add(1, atomic::Ordering::Relaxed)) + let id = Id(COUNT.fetch_add(1, atomic::Ordering::Relaxed)); + if id.0 == 0 { + Id(COUNT.fetch_add(1, atomic::Ordering::Relaxed)) + } else { + id + } } } diff --git a/examples/multi_window/Cargo.toml b/examples/multi_window/Cargo.toml index 0cebe43a06..b486e6c6b4 100644 --- a/examples/multi_window/Cargo.toml +++ b/examples/multi_window/Cargo.toml @@ -7,6 +7,8 @@ publish = false [dependencies] iced = { path = "../..", features = [ + "a11y", + "tokio", "debug", "winit", "multi-window", diff --git a/examples/multi_window/src/main.rs b/examples/multi_window/src/main.rs index 5cbf6e123c..da150973f4 100644 --- a/examples/multi_window/src/main.rs +++ b/examples/multi_window/src/main.rs @@ -3,6 +3,7 @@ use iced::widget::{ text_input, }; use iced::window; +use iced::Length::Fill; use iced::{ id::Id, Alignment, Center, Element, Length, Point, Settings, Subscription, Task, Theme, Vector, @@ -28,7 +29,7 @@ struct Window { scale_input: String, current_scale: f64, theme: Theme, - input_id: Id, + input_id: text_input::Id, } #[derive(Debug, Clone)] @@ -166,7 +167,7 @@ impl Window { scale_input: "1.0".to_string(), current_scale: 1.0, theme: Theme::ALL[count % Theme::ALL.len()].clone(), - input_id: Id::unique(), + input_id: text_input::Id::unique(), } } diff --git a/widget/src/button.rs b/widget/src/button.rs index a8aae14ef3..e8a1563650 100644 --- a/widget/src/button.rs +++ b/widget/src/button.rs @@ -382,10 +382,10 @@ where action, iced_accessibility::accesskit::Action::Default )) - .then(|| self.on_press.clone()) + .then(|| self.on_press.as_ref()) { state.is_pressed = false; - shell.publish(on_press); + shell.publish(on_press.get()); } return event::Status::Captured; } diff --git a/winit/src/application.rs b/winit/src/application.rs index 8b01b94d2e..85f24474ea 100644 --- a/winit/src/application.rs +++ b/winit/src/application.rs @@ -625,10 +625,7 @@ async fn run_instance( let mut viewport_version = state.viewport_version(); let physical_size = state.physical_size(); - let mut clipboard = Clipboard::connect( - &window, - crate::proxy::Proxy::new(proxy.raw.clone()).0, - ); + let mut clipboard = Clipboard::connect(&window, proxy.clone()); let mut cache = user_interface::Cache::default(); #[cfg(feature = "a11y")] diff --git a/winit/src/program.rs b/winit/src/program.rs index 1753e54d58..9cb35c6a73 100644 --- a/winit/src/program.rs +++ b/winit/src/program.rs @@ -346,7 +346,6 @@ where if self.boot.is_some() { return; } - self.process_event( event_loop, Event::EventLoopAwakened(winit::event::Event::NewEvents(cause)), @@ -641,7 +640,7 @@ enum Event { }, Dnd(dnd::DndEvent), #[cfg(feature = "a11y")] - Accessibility(iced_accessibility::accesskit_winit::ActionRequestEvent), + Accessibility(iced_accessibility::accesskit::ActionRequest), #[cfg(feature = "a11y")] AccessibilityEnabled(bool), EventLoopAwakened(winit::event::Event), @@ -660,7 +659,7 @@ pub(crate) enum Control { }, Dnd(dnd::DndEvent), #[cfg(feature = "a11y")] - Accessibility(iced_accessibility::accesskit_winit::ActionRequestEvent), + Accessibility(iced_accessibility::accesskit::ActionRequest), #[cfg(feature = "a11y")] AccessibilityEnabled(bool), } @@ -695,43 +694,106 @@ async fn run_instance( let mut actions = 0; #[cfg(feature = "a11y")] - let (window_a11y_id, adapter, mut a11y_enabled) = { + let (window_a11y_id, adapter, mut a11y_enabled) = if let Some(( + main_id, + title, + raw, + )) = + window_manager.ids().next().and_then(|id| { + window_manager + .get(id) + .map(|w| (id, w.state.title.clone(), w.raw.clone())) + }) { let node_id = core::id::window_node_id(); - use iced_accessibility::accesskit::{ - NodeBuilder, NodeId, Role, Tree, TreeUpdate, + ActivationHandler, NodeBuilder, NodeId, Role, Tree, TreeUpdate, }; use iced_accessibility::accesskit_winit::Adapter; - let title = main_window.raw.title().to_string(); - let proxy_clone = proxy.clone(); + pub struct WinitActivationHandler { + pub proxy: mpsc::UnboundedSender, + pub title: String, + } + + impl ActivationHandler for WinitActivationHandler { + fn request_initial_tree( + &mut self, + ) -> Option { + let node_id = core::id::window_node_id(); + + let _ = self + .proxy + .unbounded_send(Control::AccessibilityEnabled(true)); + let mut node = NodeBuilder::new(Role::Window); + node.set_name(self.title.clone()); + let node = node.build(); + let root = NodeId(node_id); + Some(TreeUpdate { + nodes: vec![(root, node)], + tree: Some(Tree::new(root)), + focus: root, + }) + } + } + + let activation_handler = WinitActivationHandler { + proxy: control_sender.clone(), + title: title.clone(), + }; + + pub struct WinitActionHandler { + pub proxy: mpsc::UnboundedSender, + } + + impl iced_accessibility::accesskit::ActionHandler for WinitActionHandler { + fn do_action( + &mut self, + request: iced_accessibility::accesskit::ActionRequest, + ) { + let _ = + self.proxy.unbounded_send(Control::Accessibility(request)); + } + } + + let action_handler = WinitActionHandler { + proxy: control_sender.clone(), + }; + + pub struct WinitDeactivationHandler { + pub proxy: mpsc::UnboundedSender, + } + + impl iced_accessibility::accesskit::DeactivationHandler + for WinitDeactivationHandler + { + fn deactivate_accessibility(&mut self) { + let _ = self + .proxy + .unbounded_send(Control::AccessibilityEnabled(false)); + } + } + + let deactivation_handler = WinitDeactivationHandler { + proxy: control_sender.clone(), + }; ( node_id, - Adapter::new( - &main_window.raw, - move || { - let _ = - proxy_clone.send_event(UserEventWrapper::A11yEnabled); - let mut node = NodeBuilder::new(Role::Window); - node.set_name(title.clone()); - let node = node.build(&mut iced_accessibility::accesskit::NodeClassSet::lock_global()); - let root = NodeId(node_id); - TreeUpdate { - nodes: vec![(root, node)], - tree: Some(Tree::new(root)), - focus: root, - } - }, - proxy.clone(), - ), + Some(Adapter::with_direct_handlers( + &raw, + activation_handler, + action_handler, + deactivation_handler, + )), false, ) + } else { + (Default::default(), None, false) }; let mut ui_caches = FxHashMap::default(); let mut user_interfaces = ManuallyDrop::new(FxHashMap::default()); - let mut cur_dnd_surface = None; + let mut cur_dnd_surface: Option = None; debug.startup_finished(); @@ -746,6 +808,7 @@ async fn run_instance( let Some(event) = event else { break; }; + let mut cur_dnd_surface: Option = None; match event { Event::WindowCreated { @@ -785,7 +848,7 @@ async fn run_instance( } events.push(( - id, + Some(id), core::Event::Window(window::Event::Opened { position: window.position(), size: window.size(), @@ -898,8 +961,8 @@ async fn run_instance( status: core::event::Status::Ignored, }); - let _ = control_sender.start_send(Control::ChangeFlow( - match ui_state { + if let Err(err) = control_sender.start_send( + Control::ChangeFlow(match ui_state { user_interface::State::Updated { redraw_request: Some(redraw_request), } => match redraw_request { @@ -913,11 +976,12 @@ async fn run_instance( } }, _ => ControlFlow::Wait, - }, - )); + }), + ) { + panic!("send error"); + } let physical_size = window.state.physical_size(); - if physical_size.width == 0 || physical_size.height == 0 { continue; @@ -997,7 +1061,6 @@ async fn run_instance( } _ => { debug.render_finished(); - log::error!( "Error {error:?} when \ presenting surface." @@ -1065,7 +1128,7 @@ async fn run_instance( ); } events.push(( - id, + Some(id), core::Event::Window(window::Event::Closed), )); } else { @@ -1080,7 +1143,7 @@ async fn run_instance( window.state.scale_factor(), window.state.modifiers(), ) { - events.push((id, event)); + events.push((Some(id), event)); } } } @@ -1096,7 +1159,7 @@ async fn run_instance( let mut window_events = vec![]; events.retain(|(window_id, event)| { - if *window_id == id { + if *window_id == Some(id) { window_events.push(event.clone()); false } else { @@ -1145,7 +1208,7 @@ async fn run_instance( for (id, event) in events.drain(..) { runtime.broadcast( subscription::Event::Interaction { - window: id, + window: id.unwrap_or(window::Id::NONE), event, status: core::event::Status::Ignored, }, @@ -1265,7 +1328,9 @@ async fn run_instance( redraw_request: Some(redraw_request), } => match redraw_request { window::RedrawRequest::NextFrame => { - ControlFlow::Poll + window.raw.request_redraw(); + + ControlFlow::Wait } window::RedrawRequest::At(at) => { ControlFlow::WaitUntil(at) @@ -1294,7 +1359,6 @@ async fn run_instance( // ), // )); } - event::Event::WindowEvent { event: window_event, window_id, @@ -1322,7 +1386,7 @@ async fn run_instance( } events.push(( - id, + Some(id), core::Event::Window(window::Event::Closed), )); @@ -1343,7 +1407,7 @@ async fn run_instance( window.state.scale_factor(), window.state.modifiers(), ) { - events.push((id, event)); + events.push((Some(id), event)); } } } @@ -1353,13 +1417,7 @@ async fn run_instance( Event::Dnd(e) => { match &e { dnd::DndEvent::Offer(_, dnd::OfferEvent::Leave) => { - let Some(some_cur_dnd_surface) = - cur_dnd_surface.clone() - else { - continue; - }; - events - .push((some_cur_dnd_surface, core::Event::Dnd(e))); + events.push((cur_dnd_surface, core::Event::Dnd(e))); cur_dnd_surface = None; } dnd::DndEvent::Offer( @@ -1385,36 +1443,26 @@ async fn run_instance( ); cur_dnd_surface = window_id; - let Some(cur_dnd_surface) = cur_dnd_surface.clone() - else { - log::error!("Missing Id for Dnd Event {e:?}"); - continue; - }; + events.push((cur_dnd_surface, core::Event::Dnd(e))); } dnd::DndEvent::Offer(..) => { - let Some(cur_dnd_surface) = cur_dnd_surface.clone() - else { - continue; - }; events.push((cur_dnd_surface, core::Event::Dnd(e))); } dnd::DndEvent::Source(_) => { - for id in window_manager.ids() { - events.push((id, core::Event::Dnd(e.clone()))) - } + events.push((None, core::Event::Dnd(e.clone()))) } }; } #[cfg(feature = "a11y")] Event::Accessibility(e) => { - match e.request.action { + match e.action { iced_accessibility::accesskit::Action::Focus => { // TODO send a command for this } _ => {} } - events.push((None, conversion::a11y(e.request))); + events.push((None, conversion::a11y(e))); } #[cfg(feature = "a11y")] Event::AccessibilityEnabled(enabled) => { @@ -1477,7 +1525,7 @@ fn run_action( action: Action, program: &P, compositor: &mut C, - events: &mut Vec<(window::Id, core::Event)>, + events: &mut Vec<(Option, core::Event)>, messages: &mut Vec, clipboard: &mut Clipboard, control_sender: &mut mpsc::UnboundedSender, @@ -1539,7 +1587,7 @@ fn run_action( if window.is_some() { events.push(( - id, + Some(id), core::Event::Window(core::window::Event::Closed), )); } diff --git a/winit/src/program/state.rs b/winit/src/program/state.rs index 68790757cd..4a63ab50e8 100644 --- a/winit/src/program/state.rs +++ b/winit/src/program/state.rs @@ -13,7 +13,7 @@ pub struct State where P::Theme: program::DefaultStyle, { - title: String, + pub(crate) title: String, scale_factor: f64, viewport: Viewport, viewport_version: u64, diff --git a/winit/src/program/window_manager.rs b/winit/src/program/window_manager.rs index 66411a09c7..d23f6a8fc3 100644 --- a/winit/src/program/window_manager.rs +++ b/winit/src/program/window_manager.rs @@ -88,6 +88,10 @@ where self.entries.iter_mut().map(|(k, v)| (*k, v)) } + pub fn get(&self, id: Id) -> Option<&Window> { + self.entries.get(&id) + } + pub fn get_mut(&mut self, id: Id) -> Option<&mut Window> { self.entries.get_mut(&id) } @@ -136,7 +140,7 @@ where P::Theme: DefaultStyle, { pub raw: Arc, - pub state: State

, + pub(crate) state: State

, pub viewport_version: u64, pub exit_on_close_request: bool, pub drag_resize_window_func: Option<