From 6483b45c6da945b4bf885989c8c2bf299924ce81 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 10:24:06 -0600 Subject: [PATCH 01/32] squash before rebase --- CHANGELOG.md | 1 + Cargo.lock | 202 +++++++++++++++++- crates/eframe/CHANGELOG.md | 1 + crates/eframe/Cargo.toml | 5 +- crates/eframe/src/native/epi_integration.rs | 18 ++ crates/eframe/src/native/run.rs | 57 ++++- crates/eframe/src/web/backend.rs | 2 + crates/egui-winit/CHANGELOG.md | 1 + crates/egui-winit/Cargo.toml | 6 + crates/egui-winit/src/lib.rs | 42 ++++ crates/egui/Cargo.toml | 3 +- crates/egui/src/context.rs | 90 +++++++- crates/egui/src/data/input.rs | 4 + crates/egui/src/data/output.rs | 25 +++ crates/egui/src/frame_state.rs | 11 + crates/egui/src/id.rs | 5 + crates/egui/src/input_state.rs | 44 ++++ crates/egui/src/lib.rs | 27 +++ crates/egui/src/memory.rs | 33 ++- crates/egui/src/response.rs | 85 +++++++- crates/egui/src/widgets/drag_value.rs | 23 +- crates/egui/src/widgets/label.rs | 22 +- crates/egui/src/widgets/slider.rs | 94 +++++--- crates/egui/src/widgets/text_edit/builder.rs | 127 ++++++++++- .../src/widgets/text_edit/cursor_range.rs | 2 +- crates/epaint/src/text/cursor.rs | 2 +- examples/hello_world/src/main.rs | 11 +- 27 files changed, 871 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d3ebfc9b63..3983f5adb1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ NOTE: [`epaint`](crates/epaint/CHANGELOG.md), [`eframe`](crates/eframe/CHANGELOG * Added `Area::constrain` and `Window::constrain` which constrains area to the screen bounds. ([#2270](https://github.com/emilk/egui/pull/2270)). * Added `Area::pivot` and `Window::pivot` which controls what part of the window to position. ([#2303](https://github.com/emilk/egui/pull/2303)). * Added support for [thin space](https://en.wikipedia.org/wiki/Thin_space). +* Added optional integration with [AccessKit](https://accesskit.dev/) for implementing platform accessibility APIs. ([#2294](https://github.com/emilk/egui/pull/2294)). ### Changed 🔧 * Panels always have a separator line, but no stroke on other sides. Their spacing has also changed slightly ([#2261](https://github.com/emilk/egui/pull/2261)). diff --git a/Cargo.lock b/Cargo.lock index a4c7b627bd2..02a8185e2be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18,6 +18,68 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a13739d7177fbd22bb0ed28badfff9f372f8bef46c863db4e1c6248f6b223b6e" +[[package]] +name = "accesskit" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2f58dda4ca012077ac19d2062ccc30187fa5588f377961be11674c3ca5f8df1" +dependencies = [ + "enumset", + "kurbo", + "serde", +] + +[[package]] +name = "accesskit_consumer" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9f45191913a5a83cee9e9ec161fe20216d345d0eaa321ec599f69188fa5b0d" +dependencies = [ + "accesskit", + "parking_lot", +] + +[[package]] +name = "accesskit_macos" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbe873b1f135b6538ef3c044424703dc6f381e9232ef84a3f080f4c3731d791b" +dependencies = [ + "accesskit", + "accesskit_consumer", + "objc2", + "once_cell", + "parking_lot", +] + +[[package]] +name = "accesskit_windows" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87a936fb4082e46649748c3c3089bfe5e41f59e1246503ab8e18e1ad81683559" +dependencies = [ + "accesskit", + "accesskit_consumer", + "arrayvec 0.7.2", + "lazy-init", + "parking_lot", + "paste", + "windows 0.42.0", +] + +[[package]] +name = "accesskit_winit" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f036fe9dbb998ddca93fcefb8343988bb799eeb534144b5164f2cdb1056068eb" +dependencies = [ + "accesskit", + "accesskit_macos", + "accesskit_windows", + "parking_lot", + "winit", +] + [[package]] name = "addr2line" version = "0.17.0" @@ -369,6 +431,25 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d8c1fef690941d3e7788d328517591fecc684c084084702d6ff1641e993699a" +[[package]] +name = "block-sys" +version = "0.1.0-beta.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fa55741ee90902547802152aaf3f8e5248aab7e21468089560d4c8840561146" +dependencies = [ + "objc-sys", +] + +[[package]] +name = "block2" +version = "0.2.0-alpha.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8dd9e63c1744f755c2f60332b88de39d341e5e86239014ad839bd71c106dec42" +dependencies = [ + "block-sys", + "objc2-encode", +] + [[package]] name = "bumpalo" version = "3.11.0" @@ -923,8 +1004,18 @@ version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a01d95850c592940db9b8194bc39f4bc0e89dee5c4265e4b1807c34a9aba453c" dependencies = [ - "darling_core", - "darling_macro", + "darling_core 0.13.4", + "darling_macro 0.13.4", +] + +[[package]] +name = "darling" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0dd3cd20dc6b5a876612a6e5accfe7f3dd883db6d07acfbf14c128f61550dfa" +dependencies = [ + "darling_core 0.14.2", + "darling_macro 0.14.2", ] [[package]] @@ -941,13 +1032,37 @@ dependencies = [ "syn", ] +[[package]] +name = "darling_core" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a784d2ccaf7c98501746bf0be29b2022ba41fd62a2e622af997a03e9f972859f" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "darling_macro" version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c972679f83bdf9c42bd905396b6c3588a843a17f0f16dfcfa3e2c5d57441835" dependencies = [ - "darling_core", + "darling_core 0.13.4", + "quote", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7618812407e9402654622dd402b0a89dff9ba93badd6540781526117b92aab7e" +dependencies = [ + "darling_core 0.14.2", "quote", "syn", ] @@ -1165,6 +1280,7 @@ dependencies = [ name = "egui" version = "0.19.0" dependencies = [ + "accesskit", "ahash 0.8.1", "document-features", "epaint", @@ -1193,6 +1309,7 @@ dependencies = [ name = "egui-winit" version = "0.19.0" dependencies = [ + "accesskit_winit", "arboard", "document-features", "egui", @@ -1360,6 +1477,28 @@ dependencies = [ "syn", ] +[[package]] +name = "enumset" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19be8061a06ab6f3a6cf21106c873578bf01bd42ad15e0311a9c76161cb1c753" +dependencies = [ + "enumset_derive", + "serde", +] + +[[package]] +name = "enumset_derive" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03e7b551eba279bf0fa88b83a46330168c1560a52a94f5126f892f0b364ab3e0" +dependencies = [ + "darling 0.14.2", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "epaint" version = "0.19.0" @@ -2094,8 +2233,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a53776d271cfb873b17c618af0298445c88afc52837f3e948fa3fafd131f449" dependencies = [ "arrayvec 0.7.2", + "serde", ] +[[package]] +name = "lazy-init" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f40963626ac12dcaf92afc15e4c3db624858c92fd9f8ba2125eaada3ac2706f" + [[package]] name = "lazy_static" version = "1.4.0" @@ -2340,7 +2486,7 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0df7ac00c4672f9d5aece54ee3347520b7e20f158656c7db2e6de01902eb7a6c" dependencies = [ - "darling", + "darling 0.13.4", "proc-macro-crate", "proc-macro2", "quote", @@ -2501,6 +2647,32 @@ dependencies = [ "objc_id", ] +[[package]] +name = "objc-sys" +version = "0.2.0-beta.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df3b9834c1e95694a05a828b59f55fa2afec6288359cda67146126b3f90a55d7" + +[[package]] +name = "objc2" +version = "0.3.0-beta.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe31e5425d3d0b89a15982c024392815da40689aceb34bad364d58732bcfd649" +dependencies = [ + "block2", + "objc-sys", + "objc2-encode", +] + +[[package]] +name = "objc2-encode" +version = "2.0.0-pre.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "abfcac41015b00a120608fdaa6938c44cb983fee294351cc4bac7638b4e50512" +dependencies = [ + "objc-sys", +] + [[package]] name = "objc_exception" version = "0.1.2" @@ -2625,6 +2797,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "paste" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1de2e551fb905ac83f73f7aedf2f0cb4a0da7e35efa24a202a936269f1f18e1" + [[package]] name = "peeking_take_while" version = "0.1.2" @@ -4365,6 +4543,7 @@ version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0286ba339aa753e70765d521bb0242cc48e1194562bfa2a2ad7ac8a6de28f5d5" dependencies = [ + "windows-implement", "windows_aarch64_gnullvm", "windows_aarch64_msvc 0.42.0", "windows_i686_gnu 0.42.0", @@ -4374,6 +4553,17 @@ dependencies = [ "windows_x86_64_msvc 0.42.0", ] +[[package]] +name = "windows-implement" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9539b6bd3eadbd9de66c9666b22d802b833da7e996bc06896142e09854a61767" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "windows-sys" version = "0.36.1" @@ -4491,9 +4681,9 @@ checksum = "f40009d85759725a34da6d89a94e63d7bdc50a862acf0dbc7c8e488f1edcb6f5" [[package]] name = "winit" -version = "0.27.2" +version = "0.27.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83a8f3e9d742401efcfe833b8f84960397482ff049cb7bf59a112e14a4be97f7" +checksum = "bb796d6fbd86b2fd896c9471e6f04d39d750076ebe5680a3958f00f5ab97657c" dependencies = [ "bitflags", "cocoa", diff --git a/crates/eframe/CHANGELOG.md b/crates/eframe/CHANGELOG.md index 6402ee2a2ed..3759b1ed8ee 100644 --- a/crates/eframe/CHANGELOG.md +++ b/crates/eframe/CHANGELOG.md @@ -18,6 +18,7 @@ NOTE: [`egui-winit`](../egui-winit/CHANGELOG.md), [`egui_glium`](../egui_glium/C * Web: Add `WebInfo::user_agent` ([#2202](https://github.com/emilk/egui/pull/2202)). * Wgpu device/adapter/surface creation has now various configuration options exposed via `NativeOptions/WebOptions::wgpu_options` ([#2207](https://github.com/emilk/egui/pull/2207)). * Fix: Make sure that `native_pixels_per_point` is updated ([#2256](https://github.com/emilk/egui/pull/2256)). +* Added optional, but enabled by default, integration with [AccessKit](https://accesskit.dev/) for implementing platform accessibility APIs. ([#2294](https://github.com/emilk/egui/pull/2294)). ## 0.19.0 - 2022-08-20 diff --git a/crates/eframe/Cargo.toml b/crates/eframe/Cargo.toml index 596e6d72edd..9d3fb34beee 100644 --- a/crates/eframe/Cargo.toml +++ b/crates/eframe/Cargo.toml @@ -20,7 +20,10 @@ all-features = true [features] -default = ["default_fonts", "glow"] +default = ["accesskit", "default_fonts", "glow"] + +## Enable platform accessibility API implementations through [AccessKit](https://accesskit.dev/). +accesskit = ["egui/accesskit", "egui-winit/accesskit"] ## Detect dark mode system preference using [`dark-light`](https://docs.rs/dark-light). ## diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index 2d75d998818..13f6f189f41 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -3,6 +3,10 @@ use winit::event_loop::EventLoopWindowTarget; #[cfg(target_os = "macos")] use winit::platform::macos::WindowBuilderExtMacOS as _; +#[cfg(feature = "accesskit")] +use egui::accesskit; +#[cfg(feature = "accesskit")] +use egui_winit::accesskit_winit; use egui_winit::{native_pixels_per_point, EventResponse, WindowSettings}; use crate::{epi, Theme, WindowInfo}; @@ -262,6 +266,15 @@ impl EpiIntegration { } } + #[cfg(feature = "accesskit")] + pub fn init_accesskit + Send>( + &mut self, + window: &winit::window::Window, + event_loop_proxy: winit::event_loop::EventLoopProxy, + ) { + self.egui_winit.init_accesskit(window, event_loop_proxy); + } + pub fn warm_up(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) { crate::profile_function!(); let saved_memory: egui::Memory = self.egui_ctx.memory().clone(); @@ -301,6 +314,11 @@ impl EpiIntegration { self.egui_winit.on_event(&self.egui_ctx, event) } + #[cfg(feature = "accesskit")] + pub fn on_accesskit_action_request(&mut self, request: accesskit::ActionRequest) { + self.egui_winit.on_accesskit_action_request(request); + } + pub fn update( &mut self, app: &mut dyn epi::App, diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index 5d28e3ea457..0aeabdbbb56 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -4,6 +4,8 @@ use std::time::Duration; use std::time::Instant; +#[cfg(feature = "accesskit")] +use egui_winit::accesskit_winit; use egui_winit::winit; use winit::event_loop::{ ControlFlow, EventLoop, EventLoopBuilder, EventLoopProxy, EventLoopWindowTarget, @@ -15,6 +17,15 @@ use crate::epi; #[derive(Debug)] pub enum UserEvent { RequestRepaint, + #[cfg(feature = "accesskit")] + AccessKitActionRequest(accesskit_winit::ActionRequestEvent), +} + +#[cfg(feature = "accesskit")] +impl From for UserEvent { + fn from(inner: accesskit_winit::ActionRequestEvent) -> Self { + Self::AccessKitActionRequest(inner) + } } // ---------------------------------------------------------------------------- @@ -350,7 +361,9 @@ mod glow_integration { let window_builder = epi_integration::window_builder(native_options, &window_settings) .with_title(title) - .with_visible(false); // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279 + // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279 + // We must also keep the window hidden until AccessKit is initialized. + .with_visible(false); let gl_window = unsafe { glutin::ContextBuilder::new() @@ -397,6 +410,10 @@ mod glow_integration { #[cfg(feature = "wgpu")] None, ); + #[cfg(feature = "accesskit")] + { + integration.init_accesskit(gl_window.window(), self.repaint_proxy.lock().clone()); + } let theme = system_theme.unwrap_or(self.native_options.default_theme); integration.egui_ctx.set_visuals(theme.egui_visuals()); @@ -646,6 +663,21 @@ mod glow_integration { EventResult::Wait } } + #[cfg(feature = "accesskit")] + winit::event::Event::UserEvent(UserEvent::AccessKitActionRequest( + accesskit_winit::ActionRequestEvent { request, .. }, + )) => { + if let Some(running) = &mut self.running { + running + .integration + .on_accesskit_action_request(request.clone()); + // As a form of user input, accessibility actions should + // lead to a repaint. + EventResult::RepaintNext + } else { + EventResult::Wait + } + } _ => EventResult::Wait, } } @@ -738,7 +770,9 @@ mod wgpu_integration { let window_settings = epi_integration::load_window_settings(storage); epi_integration::window_builder(native_options, &window_settings) .with_title(title) - .with_visible(false) // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279 + // Keep hidden until we've painted something. See https://github.com/emilk/egui/pull/2279 + // We must also keep the window hidden until AccessKit is initialized. + .with_visible(false) .build(event_loop) .unwrap() } @@ -794,6 +828,10 @@ mod wgpu_integration { None, wgpu_render_state.clone(), ); + #[cfg(feature = "accesskit")] + { + integration.init_accesskit(&window, self.repaint_proxy.lock().unwrap().clone()); + } let theme = system_theme.unwrap_or(self.native_options.default_theme); integration.egui_ctx.set_visuals(theme.egui_visuals()); @@ -1037,6 +1075,21 @@ mod wgpu_integration { EventResult::Wait } } + #[cfg(feature = "accesskit")] + winit::event::Event::UserEvent(UserEvent::AccessKitActionRequest( + accesskit_winit::ActionRequestEvent { request, .. }, + )) => { + if let Some(running) = &mut self.running { + running + .integration + .on_accesskit_action_request(request.clone()); + // As a form of user input, accessibility actions should + // lead to a repaint. + EventResult::RepaintNext + } else { + EventResult::Wait + } + } _ => EventResult::Wait, } } diff --git a/crates/eframe/src/web/backend.rs b/crates/eframe/src/web/backend.rs index c4fcaf201c0..191709c3058 100644 --- a/crates/eframe/src/web/backend.rs +++ b/crates/eframe/src/web/backend.rs @@ -400,6 +400,8 @@ impl AppRunner { events: _, // already handled mutable_text_under_cursor, text_cursor_pos, + #[cfg(feature = "accesskit")] + accesskit_update: _, // not currently implemented } = platform_output; set_cursor_icon(cursor_icon); diff --git a/crates/egui-winit/CHANGELOG.md b/crates/egui-winit/CHANGELOG.md index 317786e3a32..b9fbdd0c812 100644 --- a/crates/egui-winit/CHANGELOG.md +++ b/crates/egui-winit/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to the `egui-winit` integration will be noted in this file. ## Unreleased * The default features of the `winit` crate are not enabled if the default features of `egui-winit` are disabled too ([#1971](https://github.com/emilk/egui/pull/1971)) * Added new feature `wayland` which enables Wayland support ([#1971](https://github.com/emilk/egui/pull/1971)) +* Added optional integration with [AccessKit](https://accesskit.dev/) for implementing platform accessibility APIs. ([#2294](https://github.com/emilk/egui/pull/2294)). ## 0.19.0 - 2022-08-20 * MSRV (Minimum Supported Rust Version) is now `1.61.0` ([#1846](https://github.com/emilk/egui/pull/1846)). diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index 4667646c0ab..c1d9f46cbee 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -20,6 +20,9 @@ all-features = true [features] default = ["clipboard", "links", "wayland", "winit/default"] +## Enable platform accessibility API implementations through [AccessKit](https://accesskit.dev/). +accesskit = ["accesskit_winit", "egui/accesskit"] + ## [`bytemuck`](https://docs.rs/bytemuck) enables you to cast [`egui::epaint::Vertex`], [`egui::Vec2`] etc to `&[u8]`. bytemuck = ["egui/bytemuck"] @@ -57,6 +60,9 @@ winit = { version = "0.27.2", default-features = false } ## Enable this when generating docs. document-features = { version = "0.2", optional = true } +# feature accesskit +accesskit_winit = { version = "0.6.0", optional = true } + puffin = { version = "0.14", optional = true } serde = { version = "1.0", optional = true, features = ["derive"] } diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 6dde41244a8..a33640f7161 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -11,7 +11,11 @@ use std::os::raw::c_void; +#[cfg(feature = "accesskit")] +pub use accesskit_winit; pub use egui; +#[cfg(feature = "accesskit")] +use egui::accesskit; pub use winit; pub mod clipboard; @@ -86,6 +90,9 @@ pub struct State { /// track ime state input_method_editor_started: bool, + + #[cfg(feature = "accesskit")] + accesskit: Option, } impl State { @@ -114,9 +121,25 @@ impl State { pointer_touch_id: None, input_method_editor_started: false, + + #[cfg(feature = "accesskit")] + accesskit: None, } } + #[cfg(feature = "accesskit")] + pub fn init_accesskit + Send>( + &mut self, + window: &winit::window::Window, + event_loop_proxy: winit::event_loop::EventLoopProxy, + ) { + self.accesskit = Some(accesskit_winit::Adapter::new( + window, + Box::new(egui::accesskit_placeholder_tree_update), + event_loop_proxy, + )); + } + /// Call this once a graphics context has been created to update the maximum texture dimensions /// that egui will use. pub fn set_max_texture_side(&mut self, max_texture_side: usize) { @@ -374,6 +397,16 @@ impl State { } } + /// Call this when there is a new [`accesskit::ActionRequest`]. + /// + /// The result can be found in [`Self::egui_input`] and be extracted with [`Self::take_egui_input`]. + #[cfg(feature = "accesskit")] + pub fn on_accesskit_action_request(&mut self, request: accesskit::ActionRequest) { + self.egui_input + .events + .push(egui::Event::AccessKitActionRequest(request)); + } + fn on_mouse_button_input( &mut self, state: winit::event::ElementState, @@ -592,6 +625,8 @@ impl State { events: _, // handled above mutable_text_under_cursor: _, // only used in eframe web text_cursor_pos, + #[cfg(feature = "accesskit")] + accesskit_update, } = platform_output; self.current_pixels_per_point = egui_ctx.pixels_per_point(); // someone can have changed it to scale the UI @@ -608,6 +643,13 @@ impl State { if let Some(egui::Pos2 { x, y }) = text_cursor_pos { window.set_ime_position(winit::dpi::LogicalPosition { x, y }); } + + #[cfg(feature = "accesskit")] + { + if let Some(accesskit) = self.accesskit.as_ref() { + accesskit.update(accesskit_update); + } + } } fn set_cursor_icon(&mut self, window: &winit::window::Window, cursor_icon: egui::CursorIcon) { diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml index 3e8e3ccce8c..9dec778ca38 100644 --- a/crates/egui/Cargo.toml +++ b/crates/egui/Cargo.toml @@ -52,11 +52,12 @@ mint = ["epaint/mint"] persistence = ["serde", "epaint/serde", "ron"] ## Allow serialization using [`serde`](https://docs.rs/serde). -serde = ["dep:serde", "epaint/serde"] +serde = ["dep:serde", "epaint/serde", "accesskit?/serde"] [dependencies] epaint = { version = "0.19.0", path = "../epaint", default-features = false } +accesskit = { version = "0.8.0", optional = true } ahash = { version = "0.8.1", default-features = false, features = [ "no-rng", # we don't need DOS-protection, so we let users opt-in to it instead "std", diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 8449ca30f58..45e907988ab 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -105,6 +105,25 @@ impl ContextImpl { interactable: true, }, ); + + #[cfg(feature = "accesskit")] + { + let nodes = &mut self.output.accesskit_update.nodes; + assert!(nodes.is_empty()); + let id = crate::accesskit_root_id(); + let accesskit_id = id.accesskit_id(); + let node = accesskit::Node { + role: accesskit::Role::Window, + transform: Some( + accesskit::kurbo::Affine::scale(self.input.pixels_per_point().into()).into(), + ), + ..Default::default() + }; + nodes.push((accesskit_id, Arc::new(node))); + self.frame_state.accesskit_nodes.insert(id, nodes.len() - 1); + assert!(self.output.accesskit_update.tree.is_none()); + self.output.accesskit_update.tree = Some(accesskit::Tree::new(accesskit_id)); + } } /// Load fonts unless already loaded. @@ -132,6 +151,24 @@ impl ContextImpl { } } } + + #[cfg(feature = "accesskit")] + fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { + let nodes = &mut self.output.accesskit_update.nodes; + let node_map = &mut self.frame_state.accesskit_nodes; + let index = node_map.get(&id).copied().unwrap_or_else(|| { + let accesskit_id = id.accesskit_id(); + nodes.push((accesskit_id, Arc::new(Default::default()))); + let index = nodes.len() - 1; + node_map.insert(id, index); + let parent_id = parent_id.unwrap_or_else(crate::accesskit_root_id); + let parent_index = node_map.get(&parent_id).unwrap(); + let parent = Arc::get_mut(&mut nodes[*parent_index].1).unwrap(); + parent.children.push(accesskit_id); + index + }); + Arc::get_mut(&mut nodes[index].1).unwrap() + } } // ---------------------------------------------------------------------------- @@ -436,16 +473,23 @@ impl Context { self.check_for_id_clash(id, rect, "widget"); + #[cfg(feature = "accesskit")] + { + if sense.focusable { + // Make sure anything that can receive focus has an AccessKit node. + // TODO(mwcampbell): For nodes that are filled from widget info, + // some information is written to the node twice. + let mut node = self.accesskit_node(id, None); + response.fill_accesskit_node_common(&mut node); + } + } + let clicked_elsewhere = response.clicked_elsewhere(); let ctx_impl = &mut *self.write(); let memory = &mut ctx_impl.memory; let input = &mut ctx_impl.input; - // We only want to focus labels if the screen reader is on. - let interested_in_focus = - sense.interactive() || sense.focusable && memory.options.screen_reader; - - if interested_in_focus { + if sense.focusable { memory.interested_in_focus(id); } @@ -457,6 +501,15 @@ impl Context { response.clicked[PointerButton::Primary as usize] = true; } + #[cfg(feature = "accesskit")] + { + if sense.click + && input.has_accesskit_action_request(response.id, accesskit::Action::Default) + { + response.clicked[PointerButton::Primary as usize] = true; + } + } + if sense.click || sense.drag { memory.interaction.click_interest |= hovered && sense.click; memory.interaction.drag_interest |= hovered && sense.drag; @@ -983,7 +1036,20 @@ impl Context { textures_delta = ctx_impl.tex_manager.0.write().take_delta(); }; - let platform_output: PlatformOutput = std::mem::take(&mut self.output()); + #[cfg_attr(not(feature = "accesskit"), allow(unused_mut))] + let mut platform_output: PlatformOutput = std::mem::take(&mut self.output()); + + #[cfg(feature = "accesskit")] + { + let has_focus = self.input().raw.has_focus; + platform_output.accesskit_update.focus = has_focus.then(|| { + let focus_id = self.memory().interaction.focus.id; + focus_id.map_or_else( + || crate::accesskit_root_id().accesskit_id(), + |id| id.accesskit_id(), + ) + }); + } // if repaint_requests is greater than zero. just set the duration to zero for immediate // repaint. if there's no repaint requests, then we can use the actual repaint_after instead. @@ -1502,6 +1568,18 @@ impl Context { } } +/// ## Accessibility +impl Context { + #[cfg(feature = "accesskit")] + pub fn accesskit_node( + &self, + id: Id, + parent_id: Option, + ) -> RwLockWriteGuard<'_, accesskit::Node> { + RwLockWriteGuard::map(self.write(), |c| c.accesskit_node(id, parent_id)) + } +} + #[test] fn context_impl_send_sync() { fn assert_send_sync() {} diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index bc5dcffefd8..0c0791621b1 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -268,6 +268,10 @@ pub enum Event { /// The value is in the range from 0.0 (no pressure) to 1.0 (maximum pressure). force: f32, }, + + /// An assistive technology (e.g. screen reader) requested an action. + #[cfg(feature = "accesskit")] + AccessKitActionRequest(accesskit::ActionRequest), } /// Mouse button (or similar for touch input) diff --git a/crates/egui/src/data/output.rs b/crates/egui/src/data/output.rs index 405181a66d7..57bbae31089 100644 --- a/crates/egui/src/data/output.rs +++ b/crates/egui/src/data/output.rs @@ -85,6 +85,9 @@ pub struct PlatformOutput { /// Screen-space position of text edit cursor (used for IME). pub text_cursor_pos: Option, + + #[cfg(feature = "accesskit")] + pub accesskit_update: accesskit::TreeUpdate, } impl PlatformOutput { @@ -121,6 +124,8 @@ impl PlatformOutput { mut events, mutable_text_under_cursor, text_cursor_pos, + #[cfg(feature = "accesskit")] + accesskit_update, } = newer; self.cursor_icon = cursor_icon; @@ -133,6 +138,13 @@ impl PlatformOutput { self.events.append(&mut events); self.mutable_text_under_cursor = mutable_text_under_cursor; self.text_cursor_pos = text_cursor_pos.or(self.text_cursor_pos); + + #[cfg(feature = "accesskit")] + { + // egui produces a complete AccessKit tree for each frame, + // so overwrite rather than appending. + self.accesskit_update = accesskit_update; + } } /// Take everything ephemeral (everything except `cursor_icon` currently) @@ -372,6 +384,19 @@ pub enum OutputEvent { ValueChanged(WidgetInfo), } +impl OutputEvent { + pub fn widget_info(&self) -> &WidgetInfo { + match self { + OutputEvent::Clicked(info) + | OutputEvent::DoubleClicked(info) + | OutputEvent::TripleClicked(info) + | OutputEvent::FocusGained(info) + | OutputEvent::TextSelectionChanged(info) + | OutputEvent::ValueChanged(info) => info, + } + } +} + impl std::fmt::Debug for OutputEvent { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/crates/egui/src/frame_state.rs b/crates/egui/src/frame_state.rs index 3b9589b1d02..106202d52f1 100644 --- a/crates/egui/src/frame_state.rs +++ b/crates/egui/src/frame_state.rs @@ -41,6 +41,9 @@ pub(crate) struct FrameState { /// horizontal, vertical pub(crate) scroll_target: [Option<(RangeInclusive, Option)>; 2], + + #[cfg(feature = "accesskit")] + pub(crate) accesskit_nodes: IdMap, } impl Default for FrameState { @@ -53,6 +56,8 @@ impl Default for FrameState { tooltip_state: None, scroll_delta: Vec2::ZERO, scroll_target: [None, None], + #[cfg(feature = "accesskit")] + accesskit_nodes: Default::default(), } } } @@ -67,6 +72,8 @@ impl FrameState { tooltip_state, scroll_delta, scroll_target, + #[cfg(feature = "accesskit")] + accesskit_nodes, } = self; used_ids.clear(); @@ -76,6 +83,10 @@ impl FrameState { *tooltip_state = None; *scroll_delta = input.scroll_delta; *scroll_target = [None, None]; + #[cfg(feature = "accesskit")] + { + accesskit_nodes.clear(); + } } /// How much space is still available after panels has been added. diff --git a/crates/egui/src/id.rs b/crates/egui/src/id.rs index 682818f9c7c..fee5e259c4d 100644 --- a/crates/egui/src/id.rs +++ b/crates/egui/src/id.rs @@ -69,6 +69,11 @@ impl Id { pub(crate) fn value(&self) -> u64 { self.0 } + + #[cfg(feature = "accesskit")] + pub(crate) fn accesskit_id(&self) -> accesskit::NodeId { + std::num::NonZeroU64::new(self.0).unwrap().into() + } } impl std::fmt::Debug for Id { diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index ab0d321df36..a68583f8931 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -393,6 +393,50 @@ impl InputState { } } } + + #[cfg(feature = "accesskit")] + pub fn accesskit_action_request( + &self, + id: crate::Id, + action: accesskit::Action, + ) -> Option<&accesskit::ActionRequest> { + let accesskit_id = id.accesskit_id(); + for event in &self.events { + if let Event::AccessKitActionRequest(request) = event { + if request.target == accesskit_id && request.action == action { + return Some(request); + } + } + } + None + } + + #[cfg(feature = "accesskit")] + pub fn has_accesskit_action_request(&self, id: crate::Id, action: accesskit::Action) -> bool { + self.accesskit_action_request(id, action).is_some() + } + + #[cfg(feature = "accesskit")] + pub fn num_accesskit_action_requests( + &self, + id: crate::Id, + desired_action: accesskit::Action, + ) -> usize { + let accesskit_id = id.accesskit_id(); + self.events + .iter() + .filter(|event| { + matches!( + event, + Event::AccessKitActionRequest(accesskit::ActionRequest { + target, + action, + .. + }) if *target == accesskit_id && *action == desired_action + ) + }) + .count() + } } // ---------------------------------------------------------------------------- diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index fe4c0491e1e..404b5abec5a 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -324,6 +324,9 @@ pub mod util; pub mod widget_text; pub mod widgets; +#[cfg(feature = "accesskit")] +pub use accesskit; + pub use epaint; pub use epaint::emath; @@ -549,3 +552,27 @@ pub fn __run_test_ui(mut add_contents: impl FnMut(&mut Ui)) { }); }); } + +#[cfg(feature = "accesskit")] +pub fn accesskit_root_id() -> Id { + Id::new("accesskit_root") +} + +#[cfg(feature = "accesskit")] +pub fn accesskit_placeholder_tree_update() -> accesskit::TreeUpdate { + use accesskit::{Node, Role, Tree, TreeUpdate}; + use std::sync::Arc; + + let root_id = accesskit_root_id().accesskit_id(); + TreeUpdate { + nodes: vec![( + root_id, + Arc::new(Node { + role: Role::Window, + ..Default::default() + }), + )], + tree: Some(Tree::new(root_id)), + focus: None, + } +} diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index 73544655d77..a8ae6db6c92 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -166,7 +166,7 @@ pub(crate) struct Interaction { #[derive(Clone, Debug, Default)] pub(crate) struct Focus { /// The widget with keyboard focus (i.e. a text input field). - id: Option, + pub(crate) id: Option, /// What had keyboard focus previous frame? id_previous_frame: Option, @@ -174,6 +174,9 @@ pub(crate) struct Focus { /// Give focus to this widget next frame id_next_frame: Option, + #[cfg(feature = "accesskit")] + id_requested_by_accesskit: Option, + /// If set, the next widget that is interested in focus will automatically get it. /// Probably because the user pressed Tab. give_to_next: bool, @@ -231,6 +234,11 @@ impl Focus { self.id = Some(id); } + #[cfg(feature = "accesskit")] + { + self.id_requested_by_accesskit = None; + } + self.pressed_tab = false; self.pressed_shift_tab = false; for event in &new_input.events { @@ -261,6 +269,18 @@ impl Focus { } } } + + #[cfg(feature = "accesskit")] + { + if let crate::Event::AccessKitActionRequest(accesskit::ActionRequest { + action: accesskit::Action::Focus, + target, + data: None, + }) = event + { + self.id_requested_by_accesskit = Some(*target); + } + } } } @@ -281,6 +301,17 @@ impl Focus { } fn interested_in_focus(&mut self, id: Id) { + #[cfg(feature = "accesskit")] + { + if self.id_requested_by_accesskit == Some(id.accesskit_id()) { + self.id = Some(id); + self.id_requested_by_accesskit = None; + self.give_to_next = false; + self.pressed_tab = false; + self.pressed_shift_tab = false; + } + } + if self.give_to_next && !self.had_focus_last_frame(id) { self.id = Some(id); self.give_to_next = false; diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 8ebf738ab6b..b23372d17a4 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -526,8 +526,91 @@ impl Response { None }; if let Some(event) = event { - self.ctx.output().events.push(event); + self.output_event(event); + } else { + #[cfg(feature = "accesskit")] + { + self.fill_accesskit_node_from_widget_info(make_info()); + } + } + } + + pub fn output_event(&self, event: crate::output::OutputEvent) { + #[cfg(feature = "accesskit")] + self.fill_accesskit_node_from_widget_info(event.widget_info().clone()); + self.ctx.output().events.push(event); + } + + #[cfg(feature = "accesskit")] + pub(crate) fn fill_accesskit_node_common(&self, node: &mut accesskit::Node) { + node.bounds = Some(accesskit::kurbo::Rect { + x0: self.rect.min.x.into(), + y0: self.rect.min.y.into(), + x1: self.rect.max.x.into(), + y1: self.rect.max.y.into(), + }); + if self.sense.focusable { + node.focusable = true; + } + if self.sense.click { + node.default_action_verb = Some(accesskit::DefaultActionVerb::Click); + } + } + + #[cfg(feature = "accesskit")] + fn fill_accesskit_node_from_widget_info(&self, info: crate::WidgetInfo) { + use crate::WidgetType; + use accesskit::{CheckedState, Role}; + + let mut node = self.ctx.accesskit_node(self.id, None); + self.fill_accesskit_node_common(&mut node); + node.role = match info.typ { + WidgetType::Label => Role::StaticText, + WidgetType::Link => Role::Link, + WidgetType::TextEdit => Role::TextField, + WidgetType::Button | WidgetType::ImageButton | WidgetType::CollapsingHeader => { + Role::Button + } + WidgetType::Checkbox => Role::CheckBox, + WidgetType::RadioButton => Role::RadioButton, + WidgetType::SelectableLabel => Role::ToggleButton, + WidgetType::ComboBox => Role::PopupButton, + WidgetType::Slider => Role::Slider, + WidgetType::DragValue => Role::SpinButton, + WidgetType::ColorButton => Role::ColorWell, + WidgetType::Other => Role::Unknown, + }; + if let Some(label) = info.label { + node.name = Some(label.into()); + } + if let Some(value) = info.current_text_value { + node.value = Some(value.into()); } + if let Some(value) = info.value { + node.numeric_value = Some(value); + } + if let Some(selected) = info.selected { + node.checked_state = Some(if selected { + CheckedState::True + } else { + CheckedState::False + }); + } + } + + /// Associate a label with a control for accessibility. + pub fn labelled_by(self, id: Id) -> Self { + #[cfg(feature = "accesskit")] + { + let mut node = self.ctx.accesskit_node(self.id, None); + node.labelled_by.push(id.accesskit_id()); + } + #[cfg(not(feature = "accesskit"))] + { + let _ = id; + } + + self } /// Response to secondary clicks (right-clicks) by showing the given menu. diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 13e727d7424..16739a33ba5 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -425,6 +425,7 @@ impl<'a> Widget for DragValue<'a> { } }; + #[allow(clippy::redundant_clone)] // some clones below are dundant if AccessKit is disabled let mut response = if is_kb_editing { let button_width = ui.spacing().interact_size.x; let mut value_text = ui @@ -444,7 +445,7 @@ impl<'a> Widget for DragValue<'a> { None => value_text.parse().ok(), }; if let Some(parsed_value) = parsed_value { - let parsed_value = clamp_to_range(parsed_value, clamp_range); + let parsed_value = clamp_to_range(parsed_value, clamp_range.clone()); set(&mut get_set_value, parsed_value); } ui.memory().drag_value.edit_string = Some(value_text); @@ -499,7 +500,7 @@ impl<'a> Widget for DragValue<'a> { ); let rounded_new_value = emath::round_to_decimals(rounded_new_value, auto_decimals); - let rounded_new_value = clamp_to_range(rounded_new_value, clamp_range); + let rounded_new_value = clamp_to_range(rounded_new_value, clamp_range.clone()); set(&mut get_set_value, rounded_new_value); drag_state.last_dragged_id = Some(response.id); @@ -514,6 +515,24 @@ impl<'a> Widget for DragValue<'a> { response.changed = get(&mut get_set_value) != old_value; response.widget_info(|| WidgetInfo::drag_value(value)); + + #[cfg(feature = "accesskit")] + { + let mut node = ui.ctx().accesskit_node(response.id, None); + // If either end of the range is unbounded, it's better + // to leave the corresponding AccessKit field set to None, + // to allow for platform-specific default behavior. + if clamp_range.start().is_finite() { + node.min_numeric_value = Some(*clamp_range.start()); + } + if clamp_range.end().is_finite() { + node.max_numeric_value = Some(*clamp_range.end()); + } + // The name field is set to the current value by the button, + // but we don't want it set that way on this widget type. + node.name = None; + } + response } } diff --git a/crates/egui/src/widgets/label.rs b/crates/egui/src/widgets/label.rs index 153a119f478..1addbc7253b 100644 --- a/crates/egui/src/widgets/label.rs +++ b/crates/egui/src/widgets/label.rs @@ -16,7 +16,7 @@ use crate::{widget_text::WidgetTextGalley, *}; pub struct Label { text: WidgetText, wrap: Option, - sense: Sense, + sense: Option, } impl Label { @@ -24,7 +24,7 @@ impl Label { Self { text: text.into(), wrap: None, - sense: Sense::focusable_noninteractive(), + sense: None, } } @@ -60,7 +60,7 @@ impl Label { /// # }); /// ``` pub fn sense(mut self, sense: Sense) -> Self { - self.sense = sense; + self.sense = Some(sense); self } } @@ -68,9 +68,17 @@ impl Label { impl Label { /// Do layout and position the galley in the ui, without painting it or adding widget info. pub fn layout_in_ui(self, ui: &mut Ui) -> (Pos2, WidgetTextGalley, Response) { + let sense = self.sense.unwrap_or_else(|| { + // We only want to focus labels if the screen reader is on. + if ui.memory().options.screen_reader { + Sense::focusable_noninteractive() + } else { + Sense::hover() + } + }); if let WidgetText::Galley(galley) = self.text { // If the user said "use this specific galley", then just use it: - let (rect, response) = ui.allocate_exact_size(galley.size(), self.sense); + let (rect, response) = ui.allocate_exact_size(galley.size(), sense); let pos = match galley.job.halign { Align::LEFT => rect.left_top(), Align::Center => rect.center_top(), @@ -121,10 +129,10 @@ impl Label { let rect = text_galley.galley.rows[0] .rect .translate(vec2(pos.x, pos.y)); - let mut response = ui.allocate_rect(rect, self.sense); + let mut response = ui.allocate_rect(rect, sense); for row in text_galley.galley.rows.iter().skip(1) { let rect = row.rect.translate(vec2(pos.x, pos.y)); - response |= ui.allocate_rect(rect, self.sense); + response |= ui.allocate_rect(rect, sense); } (pos, text_galley, response) } else { @@ -144,7 +152,7 @@ impl Label { }; let text_galley = text_job.into_galley(&ui.fonts()); - let (rect, response) = ui.allocate_exact_size(text_galley.size(), self.sense); + let (rect, response) = ui.allocate_exact_size(text_galley.size(), sense); let pos = match text_galley.galley.job.halign { Align::LEFT => rect.left_top(), Align::Center => rect.center_top(), diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index 59692db8c57..a80d8163757 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -510,7 +510,7 @@ impl<'a> Slider<'a> { SliderOrientation::Horizontal => vec2(ui.spacing().slider_width, thickness), SliderOrientation::Vertical => vec2(thickness, ui.spacing().slider_width), }; - ui.allocate_response(desired_size, Sense::click_and_drag()) + ui.allocate_response(desired_size, Sense::drag()) } /// Just the slider, no text @@ -532,6 +532,9 @@ impl<'a> Slider<'a> { self.set_value(new_value); } + let mut decrement = 0usize; + let mut increment = 0usize; + if response.has_focus() { let (dec_key, inc_key) = match self.orientation { SliderOrientation::Horizontal => (Key::ArrowLeft, Key::ArrowRight), @@ -540,33 +543,39 @@ impl<'a> Slider<'a> { SliderOrientation::Vertical => (Key::ArrowUp, Key::ArrowDown), }; - let decrement = ui.input().num_presses(dec_key); - let increment = ui.input().num_presses(inc_key); - let kb_step = increment as f32 - decrement as f32; - - if kb_step != 0.0 { - let prev_value = self.get_value(); - let prev_position = self.position_from_value(prev_value, position_range.clone()); - let new_position = prev_position + kb_step; - let new_value = match self.step { - Some(step) => prev_value + (kb_step as f64 * step), - None if self.smart_aim => { - let aim_radius = ui.input().aim_radius(); - emath::smart_aim::best_in_range_f64( - self.value_from_position( - new_position - aim_radius, - position_range.clone(), - ), - self.value_from_position( - new_position + aim_radius, - position_range.clone(), - ), - ) - } - _ => self.value_from_position(new_position, position_range.clone()), - }; - self.set_value(new_value); - } + decrement += ui.input().num_presses(dec_key); + increment += ui.input().num_presses(inc_key); + } + + #[cfg(feature = "accesskit")] + { + use accesskit::Action; + decrement += ui + .input() + .num_accesskit_action_requests(response.id, Action::Decrement); + increment += ui + .input() + .num_accesskit_action_requests(response.id, Action::Increment); + } + + let kb_step = increment as f32 - decrement as f32; + + if kb_step != 0.0 { + let prev_value = self.get_value(); + let prev_position = self.position_from_value(prev_value, position_range.clone()); + let new_position = prev_position + kb_step; + let new_value = match self.step { + Some(step) => prev_value + (kb_step as f64 * step), + None if self.smart_aim => { + let aim_radius = ui.input().aim_radius(); + emath::smart_aim::best_in_range_f64( + self.value_from_position(new_position - aim_radius, position_range.clone()), + self.value_from_position(new_position + aim_radius, position_range.clone()), + ) + } + _ => self.value_from_position(new_position, position_range.clone()), + }; + self.set_value(new_value); } // Paint it: @@ -705,12 +714,33 @@ impl<'a> Slider<'a> { } fn add_contents(&mut self, ui: &mut Ui) -> Response { + let old_value = self.get_value(); + let thickness = ui .text_style_height(&TextStyle::Body) .at_least(ui.spacing().interact_size.y); let mut response = self.allocate_slider_space(ui, thickness); self.slider_ui(ui, &response); + let value = self.get_value(); + response.changed = value != old_value; + response.widget_info(|| WidgetInfo::slider(value, self.text.text())); + + #[cfg(feature = "accesskit")] + { + use accesskit::Action; + let mut node = ui.ctx().accesskit_node(response.id, None); + node.min_numeric_value = Some(*self.range.start()); + node.max_numeric_value = Some(*self.range.end()); + let clamp_range = self.clamp_range(); + if value < *clamp_range.end() { + node.actions |= Action::Increment; + } + if value > *clamp_range.start() { + node.actions |= Action::Decrement; + } + } + if self.show_value { let position_range = self.position_range(&response.rect); let value_response = self.value_ui(ui, position_range); @@ -737,18 +767,12 @@ impl<'a> Slider<'a> { impl<'a> Widget for Slider<'a> { fn ui(mut self, ui: &mut Ui) -> Response { - let old_value = self.get_value(); - let inner_response = match self.orientation { SliderOrientation::Horizontal => ui.horizontal(|ui| self.add_contents(ui)), SliderOrientation::Vertical => ui.vertical(|ui| self.add_contents(ui)), }; - let mut response = inner_response.inner | inner_response.response; - let value = self.get_value(); - response.changed = value != old_value; - response.widget_info(|| WidgetInfo::slider(value, self.text.text())); - response + inner_response.inner | inner_response.response } } diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 11cd7be12d1..d4a30c2c1cd 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -648,11 +648,7 @@ impl<'t> TextEdit<'t> { char_range, mask_if_password(password, text.as_str()), ); - response - .ctx - .output() - .events - .push(OutputEvent::TextSelectionChanged(info)); + response.output_event(OutputEvent::TextSelectionChanged(info)); } else { response.widget_info(|| { WidgetInfo::text_edit( @@ -662,6 +658,84 @@ impl<'t> TextEdit<'t> { }); } + #[cfg(feature = "accesskit")] + { + use accesskit::{Role, TextDirection, TextPosition, TextSelection}; + + let parent_id = response.id; + for (i, row) in galley.rows.iter().enumerate() { + let id = parent_id.with(i); + let mut node = ui.ctx().accesskit_node(id, Some(parent_id)); + node.role = Role::InlineTextBox; + let rect = row.rect.translate(text_draw_pos.to_vec2()); + node.bounds = Some(accesskit::kurbo::Rect { + x0: rect.min.x.into(), + y0: rect.min.y.into(), + x1: rect.max.x.into(), + y1: rect.max.y.into(), + }); + node.text_direction = Some(TextDirection::LeftToRight); + // TODO: more info for the whole row + + let glyph_count = row.glyphs.len(); + let mut value = String::new(); + value.reserve(glyph_count); + let mut character_lengths = Vec::::new(); + character_lengths.reserve(glyph_count); + let mut character_positions = Vec::::new(); + character_positions.reserve(glyph_count); + let mut character_widths = Vec::::new(); + character_widths.reserve(glyph_count); + let mut word_lengths = Vec::::new(); + let mut was_at_word_end = false; + let mut last_word_start = 0usize; + + for glyph in &row.glyphs { + let is_word_char = is_word_char(glyph.chr); + if is_word_char && was_at_word_end { + word_lengths.push((character_lengths.len() - last_word_start) as _); + last_word_start = character_lengths.len(); + } + was_at_word_end = !is_word_char; + let old_len = value.len(); + value.push(glyph.chr); + character_lengths.push((value.len() - old_len) as _); + character_positions.push(glyph.pos.x - row.rect.min.x); + character_widths.push(glyph.size.x); + } + + if row.ends_with_newline { + value.push('\n'); + character_lengths.push(1); + character_positions.push(row.rect.max.x - row.rect.min.x); + character_widths.push(0.0); + } + word_lengths.push((character_lengths.len() - last_word_start) as _); + + node.value = Some(value.into()); + node.character_lengths = character_lengths.into(); + node.character_positions = Some(character_positions.into()); + node.character_widths = Some(character_widths.into()); + node.word_lengths = word_lengths.into(); + } + + if let Some(cursor_range) = &cursor_range { + let mut node = ui.ctx().accesskit_node(parent_id, None); + let anchor = &cursor_range.secondary.rcursor; + let focus = &cursor_range.primary.rcursor; + node.text_selection = Some(TextSelection { + anchor: TextPosition { + node: parent_id.with(anchor.row).accesskit_id(), + character_index: anchor.column, + }, + focus: TextPosition { + node: parent_id.with(focus.row).accesskit_id(), + character_index: focus.column, + }, + }); + } + } + TextEditOutput { response, galley, @@ -689,6 +763,28 @@ fn mask_if_password(is_password: bool, text: &str) -> String { // ---------------------------------------------------------------------------- +#[cfg(feature = "accesskit")] +fn ccursor_from_accesskit_text_position( + id: Id, + galley: &Galley, + position: &accesskit::TextPosition, +) -> Option { + let mut total_length = 0usize; + for (i, row) in galley.rows.iter().enumerate() { + let row_id = id.with(i); + if row_id.accesskit_id() == position.node { + return Some(CCursor { + index: total_length + position.character_index, + prefer_next_row: !(position.character_index == row.glyphs.len() + && !row.ends_with_newline + && (i + 1) < galley.rows.len()), + }); + } + total_length += row.glyphs.len() + (row.ends_with_newline as usize); + } + None +} + /// Check for (keyboard) events to edit the cursor and/or text. #[allow(clippy::too_many_arguments)] fn events( @@ -849,6 +945,27 @@ fn events( } } + #[cfg(feature = "accesskit")] + Event::AccessKitActionRequest(accesskit::ActionRequest { + action: accesskit::Action::SetTextSelection, + target, + data: Some(accesskit::ActionData::SetTextSelection(selection)), + }) => { + if id.accesskit_id() == *target { + let primary = + ccursor_from_accesskit_text_position(id, galley, &selection.focus); + let secondary = + ccursor_from_accesskit_text_position(id, galley, &selection.anchor); + if let (Some(primary), Some(secondary)) = (primary, secondary) { + Some(CCursorRange { primary, secondary }) + } else { + None + } + } else { + None + } + } + _ => None, }; diff --git a/crates/egui/src/widgets/text_edit/cursor_range.rs b/crates/egui/src/widgets/text_edit/cursor_range.rs index 360260b876e..10668a562c1 100644 --- a/crates/egui/src/widgets/text_edit/cursor_range.rs +++ b/crates/egui/src/widgets/text_edit/cursor_range.rs @@ -1,7 +1,7 @@ use epaint::text::cursor::*; /// A selected text range (could be a range of length zero). -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct CursorRange { /// When selecting with a mouse, this is where the mouse was released. diff --git a/crates/epaint/src/text/cursor.rs b/crates/epaint/src/text/cursor.rs index b2078739491..fcce2d47c5a 100644 --- a/crates/epaint/src/text/cursor.rs +++ b/crates/epaint/src/text/cursor.rs @@ -113,7 +113,7 @@ impl PartialEq for PCursor { /// They all point to the same place, but in their own different ways. /// pcursor/rcursor can also point to after the end of the paragraph/row. /// Does not implement `PartialEq` because you must think which cursor should be equivalent. -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct Cursor { pub ccursor: CCursor, diff --git a/examples/hello_world/src/main.rs b/examples/hello_world/src/main.rs index a1f91f058b4..a96e47d9ada 100644 --- a/examples/hello_world/src/main.rs +++ b/examples/hello_world/src/main.rs @@ -33,10 +33,15 @@ impl eframe::App for MyApp { egui::CentralPanel::default().show(ctx, |ui| { ui.heading("My egui Application"); ui.horizontal(|ui| { - ui.label("Your name: "); - ui.text_edit_singleline(&mut self.name); + let name_label = ui.label("Your name: "); + ui.text_edit_singleline(&mut self.name) + .labelled_by(name_label.id); }); - ui.add(egui::Slider::new(&mut self.age, 0..=120).text("age")); + ui.add( + egui::Slider::new(&mut self.age, 0..=120) + .step_by(1.0) + .text("age"), + ); if ui.button("Click each year").clicked() { self.age += 1; } From 6dd21180dac268d4be6ba328d265ec63a6156045 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 10:33:14 -0600 Subject: [PATCH 02/32] Update AccessKit, introducing support for editable spinners on Windows and an important fix for navigation order on macOS --- Cargo.lock | 16 ++++++++-------- crates/egui-winit/Cargo.toml | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02a8185e2be..865490abdd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -31,9 +31,9 @@ dependencies = [ [[package]] name = "accesskit_consumer" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9f45191913a5a83cee9e9ec161fe20216d345d0eaa321ec599f69188fa5b0d" +checksum = "7c22c90fa269beac30826b50ff07c4e0fb1dbc7a6dd80555e1d61f9c599db752" dependencies = [ "accesskit", "parking_lot", @@ -41,9 +41,9 @@ dependencies = [ [[package]] name = "accesskit_macos" -version = "0.1.1" +version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbe873b1f135b6538ef3c044424703dc6f381e9232ef84a3f080f4c3731d791b" +checksum = "64dbd336bb7fc43fa3bfc8b59e5cbab880421983dd043d540ce3d6db3f099e4c" dependencies = [ "accesskit", "accesskit_consumer", @@ -54,9 +54,9 @@ dependencies = [ [[package]] name = "accesskit_windows" -version = "0.9.1" +version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87a936fb4082e46649748c3c3089bfe5e41f59e1246503ab8e18e1ad81683559" +checksum = "40846a52cc28969f8e9f10fdcc946c2119121e38e9388c42cbdf1a4b6c0a7aef" dependencies = [ "accesskit", "accesskit_consumer", @@ -69,9 +69,9 @@ dependencies = [ [[package]] name = "accesskit_winit" -version = "0.6.0" +version = "0.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f036fe9dbb998ddca93fcefb8343988bb799eeb534144b5164f2cdb1056068eb" +checksum = "c5d8630e275f2958ddab357e7dfc11abe41d28b1eba114e0d80cf3df672426fd" dependencies = [ "accesskit", "accesskit_macos", diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index c1d9f46cbee..b1d5556441b 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -61,7 +61,7 @@ winit = { version = "0.27.2", default-features = false } document-features = { version = "0.2", optional = true } # feature accesskit -accesskit_winit = { version = "0.6.0", optional = true } +accesskit_winit = { version = "0.6.6", optional = true } puffin = { version = "0.14", optional = true } serde = { version = "1.0", optional = true, features = ["derive"] } From de62604b0d53796a817f9e09dfa49b23609203ef Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 11:01:15 -0600 Subject: [PATCH 03/32] Restore support for increment and decrement actions in DragValue --- crates/egui/src/widgets/drag_value.rs | 65 ++++++++++++++++++--------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 16739a33ba5..08df4d2aaa9 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -369,15 +369,16 @@ impl<'a> Widget for DragValue<'a> { } = self; let shift = ui.input().modifiers.shift_only(); - let is_slow_speed = shift && ui.memory().is_being_dragged(ui.next_auto_id()); + // The widget has the same ID whether it's in edit or button mode. + let id = ui.next_auto_id(); + let is_slow_speed = shift && ui.memory().is_being_dragged(id); - let kb_edit_id = ui.next_auto_id(); // The following call ensures that when a `DragValue` receives focus, // it is immediately rendered in edit mode, rather than being rendered // in button mode for just one frame. This is important for // screen readers. - ui.memory().interested_in_focus(kb_edit_id); - let is_kb_editing = ui.memory().has_focus(kb_edit_id); + ui.memory().interested_in_focus(id); + let is_kb_editing = ui.memory().has_focus(id); let old_value = get(&mut get_set_value); let mut value = old_value; @@ -388,24 +389,37 @@ impl<'a> Widget for DragValue<'a> { let max_decimals = max_decimals.unwrap_or(auto_decimals + 2); let auto_decimals = auto_decimals.clamp(min_decimals, max_decimals); - if is_kb_editing { + let change = { + let mut change = 0.0; let mut input = ui.input_mut(); - // This deliberately doesn't listen for left and right arrow keys, - // because when editing, these are used to move the caret. - // This behavior is consistent with other editable spinner/stepper - // implementations, such as Chromium's (for HTML5 number input). - // It is also normal for such controls to go directly into edit mode - // when they receive keyboard focus, and some screen readers - // assume this behavior, so having a separate mode for incrementing - // and decrementing, that supports all arrow keys, would be - // problematic. - let change = input.count_and_consume_key(Modifiers::NONE, Key::ArrowUp) as f64 - - input.count_and_consume_key(Modifiers::NONE, Key::ArrowDown) as f64; - - if change != 0.0 { - value += speed * change; - value = emath::round_to_decimals(value, auto_decimals); + + if is_kb_editing { + // This deliberately doesn't listen for left and right arrow keys, + // because when editing, these are used to move the caret. + // This behavior is consistent with other editable spinner/stepper + // implementations, such as Chromium's (for HTML5 number input). + // It is also normal for such controls to go directly into edit mode + // when they receive keyboard focus, and some screen readers + // assume this behavior, so having a separate mode for incrementing + // and decrementing, that supports all arrow keys, would be + // problematic. + change += input.count_and_consume_key(Modifiers::NONE, Key::ArrowUp) as f64 + - input.count_and_consume_key(Modifiers::NONE, Key::ArrowDown) as f64; + } + + #[cfg(feature = "accesskit")] + { + use accesskit::Action; + change += input.num_accesskit_action_requests(id, Action::Increment) as f64 + - input.num_accesskit_action_requests(id, Action::Decrement) as f64; } + + change + }; + + if change != 0.0 { + value += speed * change; + value = emath::round_to_decimals(value, auto_decimals); } value = clamp_to_range(value, clamp_range.clone()); @@ -436,7 +450,7 @@ impl<'a> Widget for DragValue<'a> { .unwrap_or(value_text); let response = ui.add( TextEdit::singleline(&mut value_text) - .id(kb_edit_id) + .id(id) .desired_width(button_width) .font(TextStyle::Monospace), ); @@ -472,7 +486,7 @@ impl<'a> Widget for DragValue<'a> { } if response.clicked() { - ui.memory().request_focus(kb_edit_id); + ui.memory().request_focus(id); } else if response.dragged() { ui.output().cursor_icon = CursorIcon::ResizeHorizontal; @@ -518,6 +532,7 @@ impl<'a> Widget for DragValue<'a> { #[cfg(feature = "accesskit")] { + use accesskit::Action; let mut node = ui.ctx().accesskit_node(response.id, None); // If either end of the range is unbounded, it's better // to leave the corresponding AccessKit field set to None, @@ -528,6 +543,12 @@ impl<'a> Widget for DragValue<'a> { if clamp_range.end().is_finite() { node.max_numeric_value = Some(*clamp_range.end()); } + if value < *clamp_range.end() { + node.actions |= Action::Increment; + } + if value > *clamp_range.start() { + node.actions |= Action::Decrement; + } // The name field is set to the current value by the button, // but we don't want it set that way on this widget type. node.name = None; From a08282f7af12d9965957d6db328ba878c98038c0 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 11:26:23 -0600 Subject: [PATCH 04/32] Avoid VoiceOver race condition bug --- crates/egui/src/widgets/drag_value.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 08df4d2aaa9..a54bdbaaaf3 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -447,7 +447,7 @@ impl<'a> Widget for DragValue<'a> { .drag_value .edit_string .take() - .unwrap_or(value_text); + .unwrap_or(value_text.clone()); let response = ui.add( TextEdit::singleline(&mut value_text) .id(id) @@ -467,7 +467,7 @@ impl<'a> Widget for DragValue<'a> { } else { ui.memory().drag_value.edit_string = None; let button = Button::new( - RichText::new(format!("{}{}{}", prefix, value_text, suffix)).monospace(), + RichText::new(format!("{}{}{}", prefix, value_text.clone(), suffix)).monospace(), ) .wrap(false) .sense(Sense::click_and_drag()) @@ -552,6 +552,28 @@ impl<'a> Widget for DragValue<'a> { // The name field is set to the current value by the button, // but we don't want it set that way on this widget type. node.name = None; + // Always expose the value as a string. This makes the widget + // more stable to accessibility users as it switches + // between edit and button modes. This is particularly important + // for VoiceOver on macOS; if the value is not exposed as a string + // when the widget is in button mode, then VoiceOver speaks + // the value (or a percentage if the widget has a clamp range) + // when the widget loses focus, overriding the announcement + // of the newly focused widget. This is certainly a VoiceOver bug, + // but it's good to make our software work as well as possible + // with existing assistive technology. However, if the widget + // has a prefix and/or suffix, expose those when in button mode, + // just as they're exposed on the screen. This triggers the + // VoiceOver bug just described, but exposing all information + // is more important, and at least we can avoid the bug + // for instances of the widget with no prefix or suffix. + // + // The value is exposed as a string by the text edit widget + // when in edit mode. + if !is_kb_editing { + let value_text = format!("{}{}{}", prefix, value_text, suffix); + node.value = Some(value_text.into()); + } } response From a472d147d62e0fb533aa4e6f5bc71e2e89539fa1 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 11:35:44 -0600 Subject: [PATCH 05/32] fix clippy lint --- crates/egui/src/widgets/drag_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index a54bdbaaaf3..e0227ccba62 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -447,7 +447,7 @@ impl<'a> Widget for DragValue<'a> { .drag_value .edit_string .take() - .unwrap_or(value_text.clone()); + .unwrap_or_else(|| value_text.clone()); let response = ui.add( TextEdit::singleline(&mut value_text) .id(id) From 9e108221729bd000f985538b77b877eea859c08c Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 12:16:11 -0600 Subject: [PATCH 06/32] Tell AccessKit that the default action for a text edit (equivalent to a click) is to set the focus. This matters to some platform adapters. --- crates/egui/src/response.rs | 2 +- crates/egui/src/widgets/text_edit/builder.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index b23372d17a4..500358fcb28 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -552,7 +552,7 @@ impl Response { if self.sense.focusable { node.focusable = true; } - if self.sense.click { + if self.sense.click && node.default_action_verb.is_none() { node.default_action_verb = Some(accesskit::DefaultActionVerb::Click); } } diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index d4a30c2c1cd..981c1b6d62c 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -719,8 +719,9 @@ impl<'t> TextEdit<'t> { node.word_lengths = word_lengths.into(); } + let mut node = ui.ctx().accesskit_node(parent_id, None); + if let Some(cursor_range) = &cursor_range { - let mut node = ui.ctx().accesskit_node(parent_id, None); let anchor = &cursor_range.secondary.rcursor; let focus = &cursor_range.primary.rcursor; node.text_selection = Some(TextSelection { @@ -734,6 +735,8 @@ impl<'t> TextEdit<'t> { }, }); } + + node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); } TextEditOutput { From b65fd40f8c4cedd0477b2c9302f8140103fae3c5 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 13:04:37 -0600 Subject: [PATCH 07/32] Refactor InputState functions for AccessKit actions --- crates/egui/src/input_state.rs | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index a68583f8931..aa03966fafd 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -395,47 +395,30 @@ impl InputState { } #[cfg(feature = "accesskit")] - pub fn accesskit_action_request( + pub fn accesskit_action_requests( &self, id: crate::Id, action: accesskit::Action, - ) -> Option<&accesskit::ActionRequest> { + ) -> impl Iterator { let accesskit_id = id.accesskit_id(); - for event in &self.events { + self.events.iter().filter_map(move |event| { if let Event::AccessKitActionRequest(request) = event { if request.target == accesskit_id && request.action == action { return Some(request); } } - } - None + None + }) } #[cfg(feature = "accesskit")] pub fn has_accesskit_action_request(&self, id: crate::Id, action: accesskit::Action) -> bool { - self.accesskit_action_request(id, action).is_some() + self.accesskit_action_requests(id, action).next().is_some() } #[cfg(feature = "accesskit")] - pub fn num_accesskit_action_requests( - &self, - id: crate::Id, - desired_action: accesskit::Action, - ) -> usize { - let accesskit_id = id.accesskit_id(); - self.events - .iter() - .filter(|event| { - matches!( - event, - Event::AccessKitActionRequest(accesskit::ActionRequest { - target, - action, - .. - }) if *target == accesskit_id && *action == desired_action - ) - }) - .count() + pub fn num_accesskit_action_requests(&self, id: crate::Id, action: accesskit::Action) -> usize { + self.accesskit_action_requests(id, action).count() } } From 599d147cf6f108273c7fabde7dbf39e86a8804dc Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 13:13:48 -0600 Subject: [PATCH 08/32] Support the AccessKit SetValue for DragValue; this is the only way for a Windows AT to programmatically adjust the value --- crates/egui/src/widgets/drag_value.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index e0227ccba62..fe6f2b36aa6 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -417,6 +417,16 @@ impl<'a> Widget for DragValue<'a> { change }; + #[cfg(feature = "accesskit")] + { + use accesskit::{Action, ActionData}; + for request in ui.input().accesskit_action_requests(id, Action::SetValue) { + if let Some(ActionData::NumericValue(new_value)) = request.data { + value = new_value; + } + } + } + if change != 0.0 { value += speed * change; value = emath::round_to_decimals(value, auto_decimals); @@ -543,6 +553,8 @@ impl<'a> Widget for DragValue<'a> { if clamp_range.end().is_finite() { node.max_numeric_value = Some(*clamp_range.end()); } + node.numeric_value_step = Some(speed); + node.actions |= Action::SetValue; if value < *clamp_range.end() { node.actions |= Action::Increment; } From a31d7dc4f2507314fb1f3096d27661e483d14de0 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 13:43:37 -0600 Subject: [PATCH 09/32] Same for Slider --- crates/egui/src/widgets/slider.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index a80d8163757..d55960c93bd 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -578,6 +578,19 @@ impl<'a> Slider<'a> { self.set_value(new_value); } + #[cfg(feature = "accesskit")] + { + use accesskit::{Action, ActionData}; + for request in ui + .input() + .accesskit_action_requests(response.id, Action::SetValue) + { + if let Some(ActionData::NumericValue(new_value)) = request.data { + self.set_value(new_value); + } + } + } + // Paint it: if ui.is_rect_visible(response.rect) { let value = self.get_value(); @@ -732,6 +745,8 @@ impl<'a> Slider<'a> { let mut node = ui.ctx().accesskit_node(response.id, None); node.min_numeric_value = Some(*self.range.start()); node.max_numeric_value = Some(*self.range.end()); + node.numeric_value_step = self.step; + node.actions |= Action::SetValue; let clamp_range = self.clamp_range(); if value < *clamp_range.end() { node.actions |= Action::Increment; From 9473dbdde16e32d3ecfca604fb4de1608620d805 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 14:03:27 -0600 Subject: [PATCH 10/32] Properly associate the slider label with both the slider and the drag value --- crates/egui/src/widgets/slider.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index d55960c93bd..30a5948a818 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -756,7 +756,9 @@ impl<'a> Slider<'a> { } } - if self.show_value { + let slider_response = response.clone(); + + let value_response = if self.show_value { let position_range = self.position_range(&response.rect); let value_response = self.value_ui(ui, position_range); if value_response.gained_focus() @@ -768,12 +770,23 @@ impl<'a> Slider<'a> { response = value_response.union(response); } else { // Use the slider id as the id for the whole widget - response = response.union(value_response); + response = response.union(value_response.clone()); } - } + Some(value_response) + } else { + None + }; if !self.text.is_empty() { - ui.add(Label::new(self.text.clone()).wrap(false)); + let label_response = ui.add(Label::new(self.text.clone()).wrap(false)); + // The slider already has an accessibility label via widget info, + // but sometimes it's useful for a screen reader to know + // that a piece of text is a label for another widget, + // e.g. so the text itself can be excluded from navigation. + slider_response.labelled_by(label_response.id); + if let Some(value_response) = value_response { + value_response.labelled_by(label_response.id); + } } response From 2114978e9b0e4ea337e323ed718e755cbe7ffd4e Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 16:21:26 -0600 Subject: [PATCH 11/32] Lazily activate egui's AccessKit support --- crates/eframe/src/native/epi_integration.rs | 3 +- crates/egui-winit/src/lib.rs | 19 ++- crates/egui/src/context.rs | 65 ++++++--- crates/egui/src/response.rs | 24 ++-- crates/egui/src/widgets/drag_value.rs | 5 +- crates/egui/src/widgets/slider.rs | 5 +- crates/egui/src/widgets/text_edit/builder.rs | 135 ++++++++++--------- 7 files changed, 152 insertions(+), 104 deletions(-) diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index 13f6f189f41..aec3f4ee0e9 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -272,7 +272,8 @@ impl EpiIntegration { window: &winit::window::Window, event_loop_proxy: winit::event_loop::EventLoopProxy, ) { - self.egui_winit.init_accesskit(window, event_loop_proxy); + self.egui_winit + .init_accesskit(window, event_loop_proxy, self.egui_ctx.clone()); } pub fn warm_up(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) { diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index a33640f7161..a54e2b070ad 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -132,10 +132,21 @@ impl State { &mut self, window: &winit::window::Window, event_loop_proxy: winit::event_loop::EventLoopProxy, + egui_ctx: egui::Context, ) { self.accesskit = Some(accesskit_winit::Adapter::new( window, - Box::new(egui::accesskit_placeholder_tree_update), + Box::new(move || { + // This function is called when an accessibility client + // (e.g. screen reader) makes its first request. If we got here, + // we know that an accessibility tree is actually wanted. + // Tell egui that AccessKit is active, and return a placeholder + // tree for now. `egui::Context::accesskit_activated` + // will request a repaint, and that will provide the first + // real accessibility tree. + egui_ctx.accesskit_activated(); + egui::accesskit_placeholder_tree_update() + }), event_loop_proxy, )); } @@ -645,10 +656,8 @@ impl State { } #[cfg(feature = "accesskit")] - { - if let Some(accesskit) = self.accesskit.as_ref() { - accesskit.update(accesskit_update); - } + if let Some(accesskit) = self.accesskit.as_ref() { + accesskit.update_if_active(|| accesskit_update); } } diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 45e907988ab..88d985ce970 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -67,6 +67,9 @@ struct ContextImpl { layer_rects_this_frame: ahash::HashMap>, /// Read layer_rects_prev_frame: ahash::HashMap>, + + #[cfg(feature = "accesskit")] + was_accesskit_activated: bool, } impl ContextImpl { @@ -107,7 +110,7 @@ impl ContextImpl { ); #[cfg(feature = "accesskit")] - { + if self.was_accesskit_activated { let nodes = &mut self.output.accesskit_update.nodes; assert!(nodes.is_empty()); let id = crate::accesskit_root_id(); @@ -153,7 +156,22 @@ impl ContextImpl { } #[cfg(feature = "accesskit")] - fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { + fn is_accesskit_active_this_frame(&self) -> bool { + // AccessKit is active this frame if a root node was created in + // `ContextImpl::begin_frame_mut`. + !self.output.accesskit_update.nodes.is_empty() + } + + #[cfg(feature = "accesskit")] + fn mutate_accesskit_node( + &mut self, + id: Id, + parent_id: Option, + f: impl FnOnce(&mut accesskit::Node), + ) { + if !self.is_accesskit_active_this_frame() { + return; + } let nodes = &mut self.output.accesskit_update.nodes; let node_map = &mut self.frame_state.accesskit_nodes; let index = node_map.get(&id).copied().unwrap_or_else(|| { @@ -167,7 +185,7 @@ impl ContextImpl { parent.children.push(accesskit_id); index }); - Arc::get_mut(&mut nodes[index].1).unwrap() + f(Arc::get_mut(&mut nodes[index].1).unwrap()); } } @@ -474,14 +492,13 @@ impl Context { self.check_for_id_clash(id, rect, "widget"); #[cfg(feature = "accesskit")] - { - if sense.focusable { - // Make sure anything that can receive focus has an AccessKit node. - // TODO(mwcampbell): For nodes that are filled from widget info, - // some information is written to the node twice. - let mut node = self.accesskit_node(id, None); - response.fill_accesskit_node_common(&mut node); - } + if self.is_accesskit_active() && sense.focusable { + // Make sure anything that can receive focus has an AccessKit node. + // TODO(mwcampbell): For nodes that are filled from widget info, + // some information is written to the node twice. + self.mutate_accesskit_node(id, None, |node| { + response.fill_accesskit_node_common(node); + }); } let clicked_elsewhere = response.clicked_elsewhere(); @@ -1040,7 +1057,9 @@ impl Context { let mut platform_output: PlatformOutput = std::mem::take(&mut self.output()); #[cfg(feature = "accesskit")] - { + // We have to duplicate the logic of `is_accesskit_active_this_frame`, + // because we just took the output. + if !platform_output.accesskit_update.nodes.is_empty() { let has_focus = self.input().raw.has_focus; platform_output.accesskit_update.focus = has_focus.then(|| { let focus_id = self.memory().interaction.focus.id; @@ -1571,12 +1590,28 @@ impl Context { /// ## Accessibility impl Context { #[cfg(feature = "accesskit")] - pub fn accesskit_node( + pub fn mutate_accesskit_node( &self, id: Id, parent_id: Option, - ) -> RwLockWriteGuard<'_, accesskit::Node> { - RwLockWriteGuard::map(self.write(), |c| c.accesskit_node(id, parent_id)) + f: impl FnOnce(&mut accesskit::Node), + ) { + self.write().mutate_accesskit_node(id, parent_id, f) + } + + #[cfg(feature = "accesskit")] + pub fn is_accesskit_active(&self) -> bool { + self.read().is_accesskit_active_this_frame() + } + + #[cfg(feature = "accesskit")] + pub fn accesskit_activated(&self) { + let mut ctx = self.write(); + if !ctx.was_accesskit_activated { + ctx.was_accesskit_activated = true; + drop(ctx); + self.request_repaint(); + } } } diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 500358fcb28..aa404f56f10 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -529,15 +529,17 @@ impl Response { self.output_event(event); } else { #[cfg(feature = "accesskit")] - { - self.fill_accesskit_node_from_widget_info(make_info()); - } + self.ctx.mutate_accesskit_node(self.id, None, |node| { + self.fill_accesskit_node_from_widget_info(node, make_info()); + }); } } pub fn output_event(&self, event: crate::output::OutputEvent) { #[cfg(feature = "accesskit")] - self.fill_accesskit_node_from_widget_info(event.widget_info().clone()); + self.ctx.mutate_accesskit_node(self.id, None, |node| { + self.fill_accesskit_node_from_widget_info(node, event.widget_info().clone()); + }); self.ctx.output().events.push(event); } @@ -558,12 +560,15 @@ impl Response { } #[cfg(feature = "accesskit")] - fn fill_accesskit_node_from_widget_info(&self, info: crate::WidgetInfo) { + fn fill_accesskit_node_from_widget_info( + &self, + node: &mut accesskit::Node, + info: crate::WidgetInfo, + ) { use crate::WidgetType; use accesskit::{CheckedState, Role}; - let mut node = self.ctx.accesskit_node(self.id, None); - self.fill_accesskit_node_common(&mut node); + self.fill_accesskit_node_common(node); node.role = match info.typ { WidgetType::Label => Role::StaticText, WidgetType::Link => Role::Link, @@ -601,10 +606,9 @@ impl Response { /// Associate a label with a control for accessibility. pub fn labelled_by(self, id: Id) -> Self { #[cfg(feature = "accesskit")] - { - let mut node = self.ctx.accesskit_node(self.id, None); + self.ctx.mutate_accesskit_node(self.id, None, |node| { node.labelled_by.push(id.accesskit_id()); - } + }); #[cfg(not(feature = "accesskit"))] { let _ = id; diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index fe6f2b36aa6..d1239726b2b 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -541,9 +541,8 @@ impl<'a> Widget for DragValue<'a> { response.widget_info(|| WidgetInfo::drag_value(value)); #[cfg(feature = "accesskit")] - { + ui.ctx().mutate_accesskit_node(response.id, None, |node| { use accesskit::Action; - let mut node = ui.ctx().accesskit_node(response.id, None); // If either end of the range is unbounded, it's better // to leave the corresponding AccessKit field set to None, // to allow for platform-specific default behavior. @@ -586,7 +585,7 @@ impl<'a> Widget for DragValue<'a> { let value_text = format!("{}{}{}", prefix, value_text, suffix); node.value = Some(value_text.into()); } - } + }); response } diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index 30a5948a818..a2e0b26322c 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -740,9 +740,8 @@ impl<'a> Slider<'a> { response.widget_info(|| WidgetInfo::slider(value, self.text.text())); #[cfg(feature = "accesskit")] - { + ui.ctx().mutate_accesskit_node(response.id, None, |node| { use accesskit::Action; - let mut node = ui.ctx().accesskit_node(response.id, None); node.min_numeric_value = Some(*self.range.start()); node.max_numeric_value = Some(*self.range.end()); node.numeric_value_step = self.step; @@ -754,7 +753,7 @@ impl<'a> Slider<'a> { if value > *clamp_range.start() { node.actions |= Action::Decrement; } - } + }); let slider_response = response.clone(); diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 981c1b6d62c..18d807b2185 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -659,84 +659,85 @@ impl<'t> TextEdit<'t> { } #[cfg(feature = "accesskit")] - { + if ui.ctx().is_accesskit_active() { use accesskit::{Role, TextDirection, TextPosition, TextSelection}; let parent_id = response.id; for (i, row) in galley.rows.iter().enumerate() { let id = parent_id.with(i); - let mut node = ui.ctx().accesskit_node(id, Some(parent_id)); - node.role = Role::InlineTextBox; - let rect = row.rect.translate(text_draw_pos.to_vec2()); - node.bounds = Some(accesskit::kurbo::Rect { - x0: rect.min.x.into(), - y0: rect.min.y.into(), - x1: rect.max.x.into(), - y1: rect.max.y.into(), - }); - node.text_direction = Some(TextDirection::LeftToRight); - // TODO: more info for the whole row - - let glyph_count = row.glyphs.len(); - let mut value = String::new(); - value.reserve(glyph_count); - let mut character_lengths = Vec::::new(); - character_lengths.reserve(glyph_count); - let mut character_positions = Vec::::new(); - character_positions.reserve(glyph_count); - let mut character_widths = Vec::::new(); - character_widths.reserve(glyph_count); - let mut word_lengths = Vec::::new(); - let mut was_at_word_end = false; - let mut last_word_start = 0usize; - - for glyph in &row.glyphs { - let is_word_char = is_word_char(glyph.chr); - if is_word_char && was_at_word_end { - word_lengths.push((character_lengths.len() - last_word_start) as _); - last_word_start = character_lengths.len(); + ui.ctx().mutate_accesskit_node(id, Some(parent_id), |node| { + node.role = Role::InlineTextBox; + let rect = row.rect.translate(text_draw_pos.to_vec2()); + node.bounds = Some(accesskit::kurbo::Rect { + x0: rect.min.x.into(), + y0: rect.min.y.into(), + x1: rect.max.x.into(), + y1: rect.max.y.into(), + }); + node.text_direction = Some(TextDirection::LeftToRight); + // TODO: more info for the whole row + + let glyph_count = row.glyphs.len(); + let mut value = String::new(); + value.reserve(glyph_count); + let mut character_lengths = Vec::::new(); + character_lengths.reserve(glyph_count); + let mut character_positions = Vec::::new(); + character_positions.reserve(glyph_count); + let mut character_widths = Vec::::new(); + character_widths.reserve(glyph_count); + let mut word_lengths = Vec::::new(); + let mut was_at_word_end = false; + let mut last_word_start = 0usize; + + for glyph in &row.glyphs { + let is_word_char = is_word_char(glyph.chr); + if is_word_char && was_at_word_end { + word_lengths.push((character_lengths.len() - last_word_start) as _); + last_word_start = character_lengths.len(); + } + was_at_word_end = !is_word_char; + let old_len = value.len(); + value.push(glyph.chr); + character_lengths.push((value.len() - old_len) as _); + character_positions.push(glyph.pos.x - row.rect.min.x); + character_widths.push(glyph.size.x); } - was_at_word_end = !is_word_char; - let old_len = value.len(); - value.push(glyph.chr); - character_lengths.push((value.len() - old_len) as _); - character_positions.push(glyph.pos.x - row.rect.min.x); - character_widths.push(glyph.size.x); - } - - if row.ends_with_newline { - value.push('\n'); - character_lengths.push(1); - character_positions.push(row.rect.max.x - row.rect.min.x); - character_widths.push(0.0); - } - word_lengths.push((character_lengths.len() - last_word_start) as _); - node.value = Some(value.into()); - node.character_lengths = character_lengths.into(); - node.character_positions = Some(character_positions.into()); - node.character_widths = Some(character_widths.into()); - node.word_lengths = word_lengths.into(); - } + if row.ends_with_newline { + value.push('\n'); + character_lengths.push(1); + character_positions.push(row.rect.max.x - row.rect.min.x); + character_widths.push(0.0); + } + word_lengths.push((character_lengths.len() - last_word_start) as _); - let mut node = ui.ctx().accesskit_node(parent_id, None); - - if let Some(cursor_range) = &cursor_range { - let anchor = &cursor_range.secondary.rcursor; - let focus = &cursor_range.primary.rcursor; - node.text_selection = Some(TextSelection { - anchor: TextPosition { - node: parent_id.with(anchor.row).accesskit_id(), - character_index: anchor.column, - }, - focus: TextPosition { - node: parent_id.with(focus.row).accesskit_id(), - character_index: focus.column, - }, + node.value = Some(value.into()); + node.character_lengths = character_lengths.into(); + node.character_positions = Some(character_positions.into()); + node.character_widths = Some(character_widths.into()); + node.word_lengths = word_lengths.into(); }); } - node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); + ui.ctx().mutate_accesskit_node(parent_id, None, |node| { + if let Some(cursor_range) = &cursor_range { + let anchor = &cursor_range.secondary.rcursor; + let focus = &cursor_range.primary.rcursor; + node.text_selection = Some(TextSelection { + anchor: TextPosition { + node: parent_id.with(anchor.row).accesskit_id(), + character_index: anchor.column, + }, + focus: TextPosition { + node: parent_id.with(focus.row).accesskit_id(), + character_index: focus.column, + }, + }); + } + + node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); + }); } TextEditOutput { From c47c3a1c039f71cb9b063643f62a939a75d8f4f4 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 16:40:16 -0600 Subject: [PATCH 12/32] fix clippy lint --- crates/egui/src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 88d985ce970..13585facd5d 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1596,7 +1596,7 @@ impl Context { parent_id: Option, f: impl FnOnce(&mut accesskit::Node), ) { - self.write().mutate_accesskit_node(id, parent_id, f) + self.write().mutate_accesskit_node(id, parent_id, f); } #[cfg(feature = "accesskit")] From 884001f63327a8cb362bd75b35dc80a0706894de Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 19:47:59 -0600 Subject: [PATCH 13/32] Update AccessKit --- Cargo.lock | 20 +++++++------------- crates/egui-winit/Cargo.toml | 2 +- crates/egui-winit/src/lib.rs | 4 ++-- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 865490abdd1..5e562837bbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -41,9 +41,9 @@ dependencies = [ [[package]] name = "accesskit_macos" -version = "0.1.5" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "64dbd336bb7fc43fa3bfc8b59e5cbab880421983dd043d540ce3d6db3f099e4c" +checksum = "efa5b671f649a554d8c27b238ba69e5eac8be4809713ebc6cfeca78ebb0ee639" dependencies = [ "accesskit", "accesskit_consumer", @@ -54,14 +54,14 @@ dependencies = [ [[package]] name = "accesskit_windows" -version = "0.9.3" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40846a52cc28969f8e9f10fdcc946c2119121e38e9388c42cbdf1a4b6c0a7aef" +checksum = "fa69568d9c5df0b964f1ca1bc49e865732f256832cf21775b08a736975501682" dependencies = [ "accesskit", "accesskit_consumer", "arrayvec 0.7.2", - "lazy-init", + "once_cell", "parking_lot", "paste", "windows 0.42.0", @@ -69,9 +69,9 @@ dependencies = [ [[package]] name = "accesskit_winit" -version = "0.6.6" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5d8630e275f2958ddab357e7dfc11abe41d28b1eba114e0d80cf3df672426fd" +checksum = "ee81f4dce90c61ccbf8579ad2f0c7650022e8a5bb8bd10cd62438cad6330a2ff" dependencies = [ "accesskit", "accesskit_macos", @@ -2236,12 +2236,6 @@ dependencies = [ "serde", ] -[[package]] -name = "lazy-init" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f40963626ac12dcaf92afc15e4c3db624858c92fd9f8ba2125eaada3ac2706f" - [[package]] name = "lazy_static" version = "1.4.0" diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index b1d5556441b..3d24b8f2edb 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -61,7 +61,7 @@ winit = { version = "0.27.2", default-features = false } document-features = { version = "0.2", optional = true } # feature accesskit -accesskit_winit = { version = "0.6.6", optional = true } +accesskit_winit = { version = "0.7.0", optional = true } puffin = { version = "0.14", optional = true } serde = { version = "1.0", optional = true, features = ["derive"] } diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index a54e2b070ad..9c594483a15 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -136,7 +136,7 @@ impl State { ) { self.accesskit = Some(accesskit_winit::Adapter::new( window, - Box::new(move || { + move || { // This function is called when an accessibility client // (e.g. screen reader) makes its first request. If we got here, // we know that an accessibility tree is actually wanted. @@ -146,7 +146,7 @@ impl State { // real accessibility tree. egui_ctx.accesskit_activated(); egui::accesskit_placeholder_tree_update() - }), + }, event_loop_proxy, )); } From 428213398d10af15768b72a0d15e375d8a9b5f1d Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 20:16:03 -0600 Subject: [PATCH 14/32] More documentation, particularly around lazy activation --- crates/egui/src/context.rs | 12 ++++++++++++ crates/egui/src/lib.rs | 2 ++ 2 files changed, 14 insertions(+) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 13585facd5d..66cb6d8acac 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1589,6 +1589,11 @@ impl Context { /// ## Accessibility impl Context { + /// Create an AccessKit node with the specified ID if one doesn't already + /// exist, then call the provided function with a mutable reference + /// to the node. Node that the `parent_id` parameter is ignored if the node + /// already exists. If AccessKit isn't active for this frame, this method + /// does nothing. #[cfg(feature = "accesskit")] pub fn mutate_accesskit_node( &self, @@ -1599,11 +1604,18 @@ impl Context { self.write().mutate_accesskit_node(id, parent_id, f); } + /// Returns whether AccessKit is active for the current frame. #[cfg(feature = "accesskit")] pub fn is_accesskit_active(&self) -> bool { self.read().is_accesskit_active_this_frame() } + /// Indicates that AccessKit has been activated and egui should generate + /// AccessKit tree updates for all subsequent frames. Also requests a repaint + /// so a full AccessKit tree will be available as soon as the repaint + /// is done. As soon as the egui integration knows that accessibility support + /// is desired, it must call this method and provide a placeholder tree + /// to AccessKit through the [`crate::accesskit_placeholder_tree_update`] method. #[cfg(feature = "accesskit")] pub fn accesskit_activated(&self) { let mut ctx = self.write(); diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index 404b5abec5a..814d283a205 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -558,6 +558,8 @@ pub fn accesskit_root_id() -> Id { Id::new("accesskit_root") } +/// Return a tree update that the egui integration should provide to AccessKit +/// before a real tree update is available. See also [`crate::Context::accesskit_activated`]. #[cfg(feature = "accesskit")] pub fn accesskit_placeholder_tree_update() -> accesskit::TreeUpdate { use accesskit::{Node, Role, Tree, TreeUpdate}; From c0668891e6d586c70115db9c68e364dcddab2946 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 20:45:07 -0600 Subject: [PATCH 15/32] Tweak one of the doc comments --- crates/egui/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index 814d283a205..126dc562cbc 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -558,8 +558,9 @@ pub fn accesskit_root_id() -> Id { Id::new("accesskit_root") } -/// Return a tree update that the egui integration should provide to AccessKit -/// before a real tree update is available. See also [`crate::Context::accesskit_activated`]. +/// Return a tree update that the egui integration should provide to the +/// AccessKit adapter before a real tree update is available. See also +/// [`crate::Context::accesskit_activated`]. #[cfg(feature = "accesskit")] pub fn accesskit_placeholder_tree_update() -> accesskit::TreeUpdate { use accesskit::{Node, Role, Tree, TreeUpdate}; From d14eab6cb0cbc89708345f6ecaacfe0c8d4acbda Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 20:45:41 -0600 Subject: [PATCH 16/32] See if I can get AccessKit exempted from the 'missing backticks' lint --- clippy.toml | 1 + 1 file changed, 1 insertion(+) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000000..f09ee2776ef --- /dev/null +++ b/clippy.toml @@ -0,0 +1 @@ +doc-valid-idents = ["AccessKit", ".."] From 3109ee98254fd6d8c2d0d1d8215743f4188ccd3e Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 07:09:03 -0600 Subject: [PATCH 17/32] Make PlatformOutput::accesskit_update an Option --- crates/egui-winit/src/lib.rs | 4 +++- crates/egui/src/context.rs | 43 ++++++++++++++++------------------ crates/egui/src/data/output.rs | 2 +- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 9c594483a15..25ed30a3325 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -657,7 +657,9 @@ impl State { #[cfg(feature = "accesskit")] if let Some(accesskit) = self.accesskit.as_ref() { - accesskit.update_if_active(|| accesskit_update); + if let Some(update) = accesskit_update { + accesskit.update_if_active(|| update); + } } } diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 66cb6d8acac..19918c9ee55 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -111,21 +111,23 @@ impl ContextImpl { #[cfg(feature = "accesskit")] if self.was_accesskit_activated { - let nodes = &mut self.output.accesskit_update.nodes; - assert!(nodes.is_empty()); + assert!(self.output.accesskit_update.is_none()); let id = crate::accesskit_root_id(); let accesskit_id = id.accesskit_id(); - let node = accesskit::Node { + let node = Arc::new(accesskit::Node { role: accesskit::Role::Window, transform: Some( accesskit::kurbo::Affine::scale(self.input.pixels_per_point().into()).into(), ), ..Default::default() - }; - nodes.push((accesskit_id, Arc::new(node))); + }); + let nodes = vec![(accesskit_id, node)]; self.frame_state.accesskit_nodes.insert(id, nodes.len() - 1); - assert!(self.output.accesskit_update.tree.is_none()); - self.output.accesskit_update.tree = Some(accesskit::Tree::new(accesskit_id)); + self.output.accesskit_update = Some(accesskit::TreeUpdate { + nodes, + tree: Some(accesskit::Tree::new(accesskit_id)), + focus: None, + }); } } @@ -155,13 +157,6 @@ impl ContextImpl { } } - #[cfg(feature = "accesskit")] - fn is_accesskit_active_this_frame(&self) -> bool { - // AccessKit is active this frame if a root node was created in - // `ContextImpl::begin_frame_mut`. - !self.output.accesskit_update.nodes.is_empty() - } - #[cfg(feature = "accesskit")] fn mutate_accesskit_node( &mut self, @@ -169,10 +164,14 @@ impl ContextImpl { parent_id: Option, f: impl FnOnce(&mut accesskit::Node), ) { - if !self.is_accesskit_active_this_frame() { - return; - } - let nodes = &mut self.output.accesskit_update.nodes; + let update = match &mut self.output.accesskit_update { + Some(update) => update, + None => { + return; + } + }; + + let nodes = &mut update.nodes; let node_map = &mut self.frame_state.accesskit_nodes; let index = node_map.get(&id).copied().unwrap_or_else(|| { let accesskit_id = id.accesskit_id(); @@ -1057,11 +1056,9 @@ impl Context { let mut platform_output: PlatformOutput = std::mem::take(&mut self.output()); #[cfg(feature = "accesskit")] - // We have to duplicate the logic of `is_accesskit_active_this_frame`, - // because we just took the output. - if !platform_output.accesskit_update.nodes.is_empty() { + if let Some(accesskit_update) = &mut platform_output.accesskit_update { let has_focus = self.input().raw.has_focus; - platform_output.accesskit_update.focus = has_focus.then(|| { + accesskit_update.focus = has_focus.then(|| { let focus_id = self.memory().interaction.focus.id; focus_id.map_or_else( || crate::accesskit_root_id().accesskit_id(), @@ -1607,7 +1604,7 @@ impl Context { /// Returns whether AccessKit is active for the current frame. #[cfg(feature = "accesskit")] pub fn is_accesskit_active(&self) -> bool { - self.read().is_accesskit_active_this_frame() + self.output().accesskit_update.is_some() } /// Indicates that AccessKit has been activated and egui should generate diff --git a/crates/egui/src/data/output.rs b/crates/egui/src/data/output.rs index 57bbae31089..ffc51b086ba 100644 --- a/crates/egui/src/data/output.rs +++ b/crates/egui/src/data/output.rs @@ -87,7 +87,7 @@ pub struct PlatformOutput { pub text_cursor_pos: Option, #[cfg(feature = "accesskit")] - pub accesskit_update: accesskit::TreeUpdate, + pub accesskit_update: Option, } impl PlatformOutput { From 18ccf1fd10655ed744b6853ed42b311f179a94e3 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 08:09:41 -0600 Subject: [PATCH 18/32] Refactor lazy activation --- crates/eframe/src/native/epi_integration.rs | 11 +++++++- crates/egui-winit/src/lib.rs | 14 ++--------- crates/egui/src/context.rs | 28 ++++++++++----------- crates/egui/src/lib.rs | 4 +-- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index aec3f4ee0e9..0cda6fcbbe5 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -272,8 +272,17 @@ impl EpiIntegration { window: &winit::window::Window, event_loop_proxy: winit::event_loop::EventLoopProxy, ) { + let egui_ctx = self.egui_ctx.clone(); self.egui_winit - .init_accesskit(window, event_loop_proxy, self.egui_ctx.clone()); + .init_accesskit(window, event_loop_proxy, move || { + // This function is called when an accessibility client + // (e.g. screen reader) makes its first request. If we got here, + // we know that an accessibility tree is actually wanted. + egui_ctx.enable_accesskit(); + // Enqueue a repaint so we'll receive a full tree update soon. + egui_ctx.request_repaint(); + egui::accesskit_placeholder_tree_update() + }); } pub fn warm_up(&mut self, app: &mut dyn epi::App, window: &winit::window::Window) { diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 25ed30a3325..fe802e1905a 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -132,21 +132,11 @@ impl State { &mut self, window: &winit::window::Window, event_loop_proxy: winit::event_loop::EventLoopProxy, - egui_ctx: egui::Context, + initial_tree_update_factory: impl 'static + FnOnce() -> accesskit::TreeUpdate + Send, ) { self.accesskit = Some(accesskit_winit::Adapter::new( window, - move || { - // This function is called when an accessibility client - // (e.g. screen reader) makes its first request. If we got here, - // we know that an accessibility tree is actually wanted. - // Tell egui that AccessKit is active, and return a placeholder - // tree for now. `egui::Context::accesskit_activated` - // will request a repaint, and that will provide the first - // real accessibility tree. - egui_ctx.accesskit_activated(); - egui::accesskit_placeholder_tree_update() - }, + initial_tree_update_factory, event_loop_proxy, )); } diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 19918c9ee55..e14d12d1617 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -69,7 +69,7 @@ struct ContextImpl { layer_rects_prev_frame: ahash::HashMap>, #[cfg(feature = "accesskit")] - was_accesskit_activated: bool, + is_accesskit_enabled: bool, } impl ContextImpl { @@ -110,7 +110,7 @@ impl ContextImpl { ); #[cfg(feature = "accesskit")] - if self.was_accesskit_activated { + if self.is_accesskit_enabled { assert!(self.output.accesskit_update.is_none()); let id = crate::accesskit_root_id(); let accesskit_id = id.accesskit_id(); @@ -1607,20 +1607,18 @@ impl Context { self.output().accesskit_update.is_some() } - /// Indicates that AccessKit has been activated and egui should generate - /// AccessKit tree updates for all subsequent frames. Also requests a repaint - /// so a full AccessKit tree will be available as soon as the repaint - /// is done. As soon as the egui integration knows that accessibility support - /// is desired, it must call this method and provide a placeholder tree - /// to AccessKit through the [`crate::accesskit_placeholder_tree_update`] method. + /// Enable generation of AccessKit tree updates in all future frames. + /// + /// If it's practical for the egui integration to immediately run the egui + /// application when it is either initializing the AccessKit adapter or + /// being called by the AccessKit adapter to provide the initial tree update, + /// then it should do so, to provide a complete AccessKit tree to the adapter + /// immediately. Otherwise, it should enqueue a repaint and use the + /// placeholder tree update from [`crate::accesskit_placeholder_tree_update`] + /// in the meantime. #[cfg(feature = "accesskit")] - pub fn accesskit_activated(&self) { - let mut ctx = self.write(); - if !ctx.was_accesskit_activated { - ctx.was_accesskit_activated = true; - drop(ctx); - self.request_repaint(); - } + pub fn enable_accesskit(&self) { + self.write().is_accesskit_enabled = true; } } diff --git a/crates/egui/src/lib.rs b/crates/egui/src/lib.rs index 126dc562cbc..67aa46c61a8 100644 --- a/crates/egui/src/lib.rs +++ b/crates/egui/src/lib.rs @@ -559,8 +559,8 @@ pub fn accesskit_root_id() -> Id { } /// Return a tree update that the egui integration should provide to the -/// AccessKit adapter before a real tree update is available. See also -/// [`crate::Context::accesskit_activated`]. +/// AccessKit adapter if it cannot immediately run the egui application +/// to get a full tree update after running [`Context::enable_accesskit`]. #[cfg(feature = "accesskit")] pub fn accesskit_placeholder_tree_update() -> accesskit::TreeUpdate { use accesskit::{Node, Role, Tree, TreeUpdate}; From 49bbcf9b2e12b47360e95591f0c79b4c5b1f337b Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 09:20:12 -0600 Subject: [PATCH 19/32] Refactor node mutation (again) --- crates/egui/src/context.rs | 42 ++++++++------------ crates/egui/src/response.rs | 16 ++++---- crates/egui/src/widgets/drag_value.rs | 4 +- crates/egui/src/widgets/slider.rs | 4 +- crates/egui/src/widgets/text_edit/builder.rs | 8 ++-- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index e14d12d1617..dfccb7e8d2a 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -158,19 +158,8 @@ impl ContextImpl { } #[cfg(feature = "accesskit")] - fn mutate_accesskit_node( - &mut self, - id: Id, - parent_id: Option, - f: impl FnOnce(&mut accesskit::Node), - ) { - let update = match &mut self.output.accesskit_update { - Some(update) => update, - None => { - return; - } - }; - + fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { + let update = self.output.accesskit_update.as_mut().unwrap(); let nodes = &mut update.nodes; let node_map = &mut self.frame_state.accesskit_nodes; let index = node_map.get(&id).copied().unwrap_or_else(|| { @@ -184,7 +173,7 @@ impl ContextImpl { parent.children.push(accesskit_id); index }); - f(Arc::get_mut(&mut nodes[index].1).unwrap()); + Arc::get_mut(&mut nodes[index].1).unwrap() } } @@ -495,9 +484,9 @@ impl Context { // Make sure anything that can receive focus has an AccessKit node. // TODO(mwcampbell): For nodes that are filled from widget info, // some information is written to the node twice. - self.mutate_accesskit_node(id, None, |node| { - response.fill_accesskit_node_common(node); - }); + if let Some(mut node) = self.accesskit_node(id, None) { + response.fill_accesskit_node_common(&mut node); + } } let clicked_elsewhere = response.clicked_elsewhere(); @@ -1586,19 +1575,20 @@ impl Context { /// ## Accessibility impl Context { - /// Create an AccessKit node with the specified ID if one doesn't already - /// exist, then call the provided function with a mutable reference - /// to the node. Node that the `parent_id` parameter is ignored if the node - /// already exists. If AccessKit isn't active for this frame, this method - /// does nothing. + /// If AccessKit support is active for the current frame, get or create + /// a node with the specified ID and return a mutable reference to it. + /// `parent_id is ignored if the node already exists. #[cfg(feature = "accesskit")] - pub fn mutate_accesskit_node( + pub fn accesskit_node( &self, id: Id, parent_id: Option, - f: impl FnOnce(&mut accesskit::Node), - ) { - self.write().mutate_accesskit_node(id, parent_id, f); + ) -> Option> { + let ctx = self.write(); + ctx.output + .accesskit_update + .is_some() + .then(move || RwLockWriteGuard::map(ctx, |c| c.accesskit_node(id, parent_id))) } /// Returns whether AccessKit is active for the current frame. diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index aa404f56f10..79dec2b1aea 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -529,17 +529,17 @@ impl Response { self.output_event(event); } else { #[cfg(feature = "accesskit")] - self.ctx.mutate_accesskit_node(self.id, None, |node| { - self.fill_accesskit_node_from_widget_info(node, make_info()); - }); + if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { + self.fill_accesskit_node_from_widget_info(&mut node, make_info()); + } } } pub fn output_event(&self, event: crate::output::OutputEvent) { #[cfg(feature = "accesskit")] - self.ctx.mutate_accesskit_node(self.id, None, |node| { - self.fill_accesskit_node_from_widget_info(node, event.widget_info().clone()); - }); + if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { + self.fill_accesskit_node_from_widget_info(&mut node, event.widget_info().clone()); + } self.ctx.output().events.push(event); } @@ -606,9 +606,9 @@ impl Response { /// Associate a label with a control for accessibility. pub fn labelled_by(self, id: Id) -> Self { #[cfg(feature = "accesskit")] - self.ctx.mutate_accesskit_node(self.id, None, |node| { + if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { node.labelled_by.push(id.accesskit_id()); - }); + } #[cfg(not(feature = "accesskit"))] { let _ = id; diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index d1239726b2b..0597647eae5 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -541,7 +541,7 @@ impl<'a> Widget for DragValue<'a> { response.widget_info(|| WidgetInfo::drag_value(value)); #[cfg(feature = "accesskit")] - ui.ctx().mutate_accesskit_node(response.id, None, |node| { + if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { use accesskit::Action; // If either end of the range is unbounded, it's better // to leave the corresponding AccessKit field set to None, @@ -585,7 +585,7 @@ impl<'a> Widget for DragValue<'a> { let value_text = format!("{}{}{}", prefix, value_text, suffix); node.value = Some(value_text.into()); } - }); + } response } diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index a2e0b26322c..5c2dbf45c32 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -740,7 +740,7 @@ impl<'a> Slider<'a> { response.widget_info(|| WidgetInfo::slider(value, self.text.text())); #[cfg(feature = "accesskit")] - ui.ctx().mutate_accesskit_node(response.id, None, |node| { + if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { use accesskit::Action; node.min_numeric_value = Some(*self.range.start()); node.max_numeric_value = Some(*self.range.end()); @@ -753,7 +753,7 @@ impl<'a> Slider<'a> { if value > *clamp_range.start() { node.actions |= Action::Decrement; } - }); + } let slider_response = response.clone(); diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 18d807b2185..ce25aeba609 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -665,7 +665,7 @@ impl<'t> TextEdit<'t> { let parent_id = response.id; for (i, row) in galley.rows.iter().enumerate() { let id = parent_id.with(i); - ui.ctx().mutate_accesskit_node(id, Some(parent_id), |node| { + if let Some(mut node) = ui.ctx().accesskit_node(id, Some(parent_id)) { node.role = Role::InlineTextBox; let rect = row.rect.translate(text_draw_pos.to_vec2()); node.bounds = Some(accesskit::kurbo::Rect { @@ -717,10 +717,10 @@ impl<'t> TextEdit<'t> { node.character_positions = Some(character_positions.into()); node.character_widths = Some(character_widths.into()); node.word_lengths = word_lengths.into(); - }); + } } - ui.ctx().mutate_accesskit_node(parent_id, None, |node| { + if let Some(mut node) = ui.ctx().accesskit_node(parent_id, None) { if let Some(cursor_range) = &cursor_range { let anchor = &cursor_range.secondary.rcursor; let focus = &cursor_range.primary.rcursor; @@ -737,7 +737,7 @@ impl<'t> TextEdit<'t> { } node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); - }); + } } TextEditOutput { From 7763ee09dcb6fdf9ad4c444a64940296af1f58c4 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 09:38:02 -0600 Subject: [PATCH 20/32] Eliminate the need for an explicit is_accesskit_active method, at least for now --- crates/egui/src/context.rs | 8 +- crates/egui/src/widgets/text_edit/builder.rs | 136 +++++++++---------- 2 files changed, 69 insertions(+), 75 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index dfccb7e8d2a..866e6ab2de0 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -480,7 +480,7 @@ impl Context { self.check_for_id_clash(id, rect, "widget"); #[cfg(feature = "accesskit")] - if self.is_accesskit_active() && sense.focusable { + if sense.focusable { // Make sure anything that can receive focus has an AccessKit node. // TODO(mwcampbell): For nodes that are filled from widget info, // some information is written to the node twice. @@ -1591,12 +1591,6 @@ impl Context { .then(move || RwLockWriteGuard::map(ctx, |c| c.accesskit_node(id, parent_id))) } - /// Returns whether AccessKit is active for the current frame. - #[cfg(feature = "accesskit")] - pub fn is_accesskit_active(&self) -> bool { - self.output().accesskit_update.is_some() - } - /// Enable generation of AccessKit tree updates in all future frames. /// /// If it's practical for the egui integration to immediately run the egui diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index ce25aeba609..8cc228ae97e 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -659,84 +659,84 @@ impl<'t> TextEdit<'t> { } #[cfg(feature = "accesskit")] - if ui.ctx().is_accesskit_active() { + if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { use accesskit::{Role, TextDirection, TextPosition, TextSelection}; let parent_id = response.id; + + if let Some(cursor_range) = &cursor_range { + let anchor = &cursor_range.secondary.rcursor; + let focus = &cursor_range.primary.rcursor; + node.text_selection = Some(TextSelection { + anchor: TextPosition { + node: parent_id.with(anchor.row).accesskit_id(), + character_index: anchor.column, + }, + focus: TextPosition { + node: parent_id.with(focus.row).accesskit_id(), + character_index: focus.column, + }, + }); + } + + node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); + + drop(node); + for (i, row) in galley.rows.iter().enumerate() { let id = parent_id.with(i); - if let Some(mut node) = ui.ctx().accesskit_node(id, Some(parent_id)) { - node.role = Role::InlineTextBox; - let rect = row.rect.translate(text_draw_pos.to_vec2()); - node.bounds = Some(accesskit::kurbo::Rect { - x0: rect.min.x.into(), - y0: rect.min.y.into(), - x1: rect.max.x.into(), - y1: rect.max.y.into(), - }); - node.text_direction = Some(TextDirection::LeftToRight); - // TODO: more info for the whole row - - let glyph_count = row.glyphs.len(); - let mut value = String::new(); - value.reserve(glyph_count); - let mut character_lengths = Vec::::new(); - character_lengths.reserve(glyph_count); - let mut character_positions = Vec::::new(); - character_positions.reserve(glyph_count); - let mut character_widths = Vec::::new(); - character_widths.reserve(glyph_count); - let mut word_lengths = Vec::::new(); - let mut was_at_word_end = false; - let mut last_word_start = 0usize; - - for glyph in &row.glyphs { - let is_word_char = is_word_char(glyph.chr); - if is_word_char && was_at_word_end { - word_lengths.push((character_lengths.len() - last_word_start) as _); - last_word_start = character_lengths.len(); - } - was_at_word_end = !is_word_char; - let old_len = value.len(); - value.push(glyph.chr); - character_lengths.push((value.len() - old_len) as _); - character_positions.push(glyph.pos.x - row.rect.min.x); - character_widths.push(glyph.size.x); - } - - if row.ends_with_newline { - value.push('\n'); - character_lengths.push(1); - character_positions.push(row.rect.max.x - row.rect.min.x); - character_widths.push(0.0); + let mut node = ui.ctx().accesskit_node(id, Some(parent_id)).unwrap(); + node.role = Role::InlineTextBox; + let rect = row.rect.translate(text_draw_pos.to_vec2()); + node.bounds = Some(accesskit::kurbo::Rect { + x0: rect.min.x.into(), + y0: rect.min.y.into(), + x1: rect.max.x.into(), + y1: rect.max.y.into(), + }); + node.text_direction = Some(TextDirection::LeftToRight); + // TODO: more info for the whole row + + let glyph_count = row.glyphs.len(); + let mut value = String::new(); + value.reserve(glyph_count); + let mut character_lengths = Vec::::new(); + character_lengths.reserve(glyph_count); + let mut character_positions = Vec::::new(); + character_positions.reserve(glyph_count); + let mut character_widths = Vec::::new(); + character_widths.reserve(glyph_count); + let mut word_lengths = Vec::::new(); + let mut was_at_word_end = false; + let mut last_word_start = 0usize; + + for glyph in &row.glyphs { + let is_word_char = is_word_char(glyph.chr); + if is_word_char && was_at_word_end { + word_lengths.push((character_lengths.len() - last_word_start) as _); + last_word_start = character_lengths.len(); } - word_lengths.push((character_lengths.len() - last_word_start) as _); - - node.value = Some(value.into()); - node.character_lengths = character_lengths.into(); - node.character_positions = Some(character_positions.into()); - node.character_widths = Some(character_widths.into()); - node.word_lengths = word_lengths.into(); + was_at_word_end = !is_word_char; + let old_len = value.len(); + value.push(glyph.chr); + character_lengths.push((value.len() - old_len) as _); + character_positions.push(glyph.pos.x - row.rect.min.x); + character_widths.push(glyph.size.x); } - } - if let Some(mut node) = ui.ctx().accesskit_node(parent_id, None) { - if let Some(cursor_range) = &cursor_range { - let anchor = &cursor_range.secondary.rcursor; - let focus = &cursor_range.primary.rcursor; - node.text_selection = Some(TextSelection { - anchor: TextPosition { - node: parent_id.with(anchor.row).accesskit_id(), - character_index: anchor.column, - }, - focus: TextPosition { - node: parent_id.with(focus.row).accesskit_id(), - character_index: focus.column, - }, - }); + if row.ends_with_newline { + value.push('\n'); + character_lengths.push(1); + character_positions.push(row.rect.max.x - row.rect.min.x); + character_widths.push(0.0); } + word_lengths.push((character_lengths.len() - last_word_start) as _); - node.default_action_verb = Some(accesskit::DefaultActionVerb::Focus); + node.value = Some(value.into()); + node.character_lengths = character_lengths.into(); + node.character_positions = Some(character_positions.into()); + node.character_widths = Some(character_widths.into()); + node.word_lengths = word_lengths.into(); } } From 345b3c77d17f9fcbbe91d5670ec287fca2cfc48a Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 09:38:37 -0600 Subject: [PATCH 21/32] Fix doc comment --- crates/egui/src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 866e6ab2de0..935536fee0e 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1577,7 +1577,7 @@ impl Context { impl Context { /// If AccessKit support is active for the current frame, get or create /// a node with the specified ID and return a mutable reference to it. - /// `parent_id is ignored if the node already exists. + /// `parent_id` is ignored if the node already exists. #[cfg(feature = "accesskit")] pub fn accesskit_node( &self, From 4a273c754d9c6d910629662d7f739885eb6abaec Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 13:46:33 -0600 Subject: [PATCH 22/32] More refactoring of tree construction; don't depend on Arc::get_mut --- crates/egui/src/context.rs | 65 ++++++++++++++++------------------ crates/egui/src/frame_state.rs | 6 ++-- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 935536fee0e..3baa5dd7e13 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -111,23 +111,17 @@ impl ContextImpl { #[cfg(feature = "accesskit")] if self.is_accesskit_enabled { - assert!(self.output.accesskit_update.is_none()); let id = crate::accesskit_root_id(); - let accesskit_id = id.accesskit_id(); - let node = Arc::new(accesskit::Node { + let node = Box::new(accesskit::Node { role: accesskit::Role::Window, transform: Some( accesskit::kurbo::Affine::scale(self.input.pixels_per_point().into()).into(), ), ..Default::default() }); - let nodes = vec![(accesskit_id, node)]; - self.frame_state.accesskit_nodes.insert(id, nodes.len() - 1); - self.output.accesskit_update = Some(accesskit::TreeUpdate { - nodes, - tree: Some(accesskit::Tree::new(accesskit_id)), - focus: None, - }); + let mut nodes = IdMap::default(); + nodes.insert(id, node); + self.frame_state.accesskit_nodes = Some(nodes); } } @@ -159,21 +153,14 @@ impl ContextImpl { #[cfg(feature = "accesskit")] fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { - let update = self.output.accesskit_update.as_mut().unwrap(); - let nodes = &mut update.nodes; - let node_map = &mut self.frame_state.accesskit_nodes; - let index = node_map.get(&id).copied().unwrap_or_else(|| { - let accesskit_id = id.accesskit_id(); - nodes.push((accesskit_id, Arc::new(Default::default()))); - let index = nodes.len() - 1; - node_map.insert(id, index); + let nodes = self.frame_state.accesskit_nodes.as_mut().unwrap(); + if !nodes.contains_key(&id) { + nodes.insert(id, Default::default()); let parent_id = parent_id.unwrap_or_else(crate::accesskit_root_id); - let parent_index = node_map.get(&parent_id).unwrap(); - let parent = Arc::get_mut(&mut nodes[*parent_index].1).unwrap(); - parent.children.push(accesskit_id); - index - }); - Arc::get_mut(&mut nodes[index].1).unwrap() + let parent = nodes.get_mut(&parent_id).unwrap(); + parent.children.push(id.accesskit_id()); + } + nodes.get_mut(&id).unwrap() } } @@ -1045,15 +1032,23 @@ impl Context { let mut platform_output: PlatformOutput = std::mem::take(&mut self.output()); #[cfg(feature = "accesskit")] - if let Some(accesskit_update) = &mut platform_output.accesskit_update { - let has_focus = self.input().raw.has_focus; - accesskit_update.focus = has_focus.then(|| { - let focus_id = self.memory().interaction.focus.id; - focus_id.map_or_else( - || crate::accesskit_root_id().accesskit_id(), - |id| id.accesskit_id(), - ) - }); + { + let nodes = self.frame_state().accesskit_nodes.take(); + if let Some(nodes) = nodes { + let has_focus = self.input().raw.has_focus; + let root_id = crate::accesskit_root_id().accesskit_id(); + platform_output.accesskit_update = Some(accesskit::TreeUpdate { + nodes: nodes + .into_iter() + .map(|(id, node)| (id.accesskit_id(), Arc::from(node))) + .collect(), + tree: Some(accesskit::Tree::new(root_id)), + focus: has_focus.then(|| { + let focus_id = self.memory().interaction.focus.id; + focus_id.map_or(root_id, |id| id.accesskit_id()) + }), + }); + } } // if repaint_requests is greater than zero. just set the duration to zero for immediate @@ -1585,8 +1580,8 @@ impl Context { parent_id: Option, ) -> Option> { let ctx = self.write(); - ctx.output - .accesskit_update + ctx.frame_state + .accesskit_nodes .is_some() .then(move || RwLockWriteGuard::map(ctx, |c| c.accesskit_node(id, parent_id))) } diff --git a/crates/egui/src/frame_state.rs b/crates/egui/src/frame_state.rs index 106202d52f1..b0031502778 100644 --- a/crates/egui/src/frame_state.rs +++ b/crates/egui/src/frame_state.rs @@ -43,7 +43,7 @@ pub(crate) struct FrameState { pub(crate) scroll_target: [Option<(RangeInclusive, Option)>; 2], #[cfg(feature = "accesskit")] - pub(crate) accesskit_nodes: IdMap, + pub(crate) accesskit_nodes: Option>>, } impl Default for FrameState { @@ -57,7 +57,7 @@ impl Default for FrameState { scroll_delta: Vec2::ZERO, scroll_target: [None, None], #[cfg(feature = "accesskit")] - accesskit_nodes: Default::default(), + accesskit_nodes: None, } } } @@ -85,7 +85,7 @@ impl FrameState { *scroll_target = [None, None]; #[cfg(feature = "accesskit")] { - accesskit_nodes.clear(); + *accesskit_nodes = None; } } From ae3a982f47d680d813a75855e401b68020bf022c Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 14:04:05 -0600 Subject: [PATCH 23/32] Override a clippy lint; I seem to have no other choice --- crates/egui/src/context.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 3baa5dd7e13..dc22846dc5e 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -154,6 +154,11 @@ impl ContextImpl { #[cfg(feature = "accesskit")] fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { let nodes = self.frame_state.accesskit_nodes.as_mut().unwrap(); + // We have to override clippy's map_entry lint here, because the + // insertion path also modifies another entry, to establish + // the parent/child relationship. Using `HashMap::entry` here + // would require two mutable borrows at once. + #[allow(clippy::map_entry)] if !nodes.contains_key(&id) { nodes.insert(id, Default::default()); let parent_id = parent_id.unwrap_or_else(crate::accesskit_root_id); From 87d3a90718e9ad79666ef5cde6d554fe879dab8e Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 14:42:49 -0600 Subject: [PATCH 24/32] Final planned refactor: a more flexible approach to hierarchy --- crates/egui/src/context.rs | 63 ++++++++--- crates/egui/src/frame_state.rs | 15 ++- crates/egui/src/response.rs | 6 +- crates/egui/src/widgets/drag_value.rs | 2 +- crates/egui/src/widgets/slider.rs | 2 +- crates/egui/src/widgets/text_edit/builder.rs | 108 ++++++++++--------- 6 files changed, 117 insertions(+), 79 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index dc22846dc5e..9284bde3ca5 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -111,6 +111,7 @@ impl ContextImpl { #[cfg(feature = "accesskit")] if self.is_accesskit_enabled { + use crate::frame_state::AccessKitFrameState; let id = crate::accesskit_root_id(); let node = Box::new(accesskit::Node { role: accesskit::Role::Window, @@ -121,7 +122,10 @@ impl ContextImpl { }); let mut nodes = IdMap::default(); nodes.insert(id, node); - self.frame_state.accesskit_nodes = Some(nodes); + self.frame_state.accesskit_state = Some(AccessKitFrameState { + nodes, + parent_stack: vec![id], + }); } } @@ -152,8 +156,9 @@ impl ContextImpl { } #[cfg(feature = "accesskit")] - fn accesskit_node(&mut self, id: Id, parent_id: Option) -> &mut accesskit::Node { - let nodes = self.frame_state.accesskit_nodes.as_mut().unwrap(); + fn accesskit_node(&mut self, id: Id) -> &mut accesskit::Node { + let state = self.frame_state.accesskit_state.as_mut().unwrap(); + let nodes = &mut state.nodes; // We have to override clippy's map_entry lint here, because the // insertion path also modifies another entry, to establish // the parent/child relationship. Using `HashMap::entry` here @@ -161,8 +166,8 @@ impl ContextImpl { #[allow(clippy::map_entry)] if !nodes.contains_key(&id) { nodes.insert(id, Default::default()); - let parent_id = parent_id.unwrap_or_else(crate::accesskit_root_id); - let parent = nodes.get_mut(&parent_id).unwrap(); + let parent_id = state.parent_stack.last().unwrap(); + let parent = nodes.get_mut(parent_id).unwrap(); parent.children.push(id.accesskit_id()); } nodes.get_mut(&id).unwrap() @@ -476,7 +481,7 @@ impl Context { // Make sure anything that can receive focus has an AccessKit node. // TODO(mwcampbell): For nodes that are filled from widget info, // some information is written to the node twice. - if let Some(mut node) = self.accesskit_node(id, None) { + if let Some(mut node) = self.accesskit_node(id) { response.fill_accesskit_node_common(&mut node); } } @@ -1038,12 +1043,13 @@ impl Context { #[cfg(feature = "accesskit")] { - let nodes = self.frame_state().accesskit_nodes.take(); - if let Some(nodes) = nodes { + let state = self.frame_state().accesskit_state.take(); + if let Some(state) = state { let has_focus = self.input().raw.has_focus; let root_id = crate::accesskit_root_id().accesskit_id(); platform_output.accesskit_update = Some(accesskit::TreeUpdate { - nodes: nodes + nodes: state + .nodes .into_iter() .map(|(id, node)| (id.accesskit_id(), Arc::from(node))) .collect(), @@ -1575,20 +1581,43 @@ impl Context { /// ## Accessibility impl Context { + /// Call the provided function with the given ID pushed on the stack of + /// parent IDs for accessibility purposes. If the `accesskit` feature + /// is disabled or if AccessKit support is not active for this frame, + /// the function is still called, but with no other effect. + pub fn with_accessibility_parent(&self, id: Id, f: impl FnOnce()) { + #[cfg(feature = "accesskit")] + { + let mut frame_state = self.frame_state(); + if let Some(state) = frame_state.accesskit_state.as_mut() { + state.parent_stack.push(id); + } + } + #[cfg(not(feature = "accesskit"))] + { + let _ = id; + } + f(); + #[cfg(feature = "accesskit")] + { + let mut frame_state = self.frame_state(); + if let Some(state) = frame_state.accesskit_state.as_mut() { + assert_eq!(state.parent_stack.pop(), Some(id)); + } + } + } + /// If AccessKit support is active for the current frame, get or create /// a node with the specified ID and return a mutable reference to it. - /// `parent_id` is ignored if the node already exists. + /// For newly crated nodes, the parent is the node with the ID at the top + /// of the stack managed by [`Context::with_accessibility_parent`]. #[cfg(feature = "accesskit")] - pub fn accesskit_node( - &self, - id: Id, - parent_id: Option, - ) -> Option> { + pub fn accesskit_node(&self, id: Id) -> Option> { let ctx = self.write(); ctx.frame_state - .accesskit_nodes + .accesskit_state .is_some() - .then(move || RwLockWriteGuard::map(ctx, |c| c.accesskit_node(id, parent_id))) + .then(move || RwLockWriteGuard::map(ctx, |c| c.accesskit_node(id))) } /// Enable generation of AccessKit tree updates in all future frames. diff --git a/crates/egui/src/frame_state.rs b/crates/egui/src/frame_state.rs index b0031502778..73dcfd4c046 100644 --- a/crates/egui/src/frame_state.rs +++ b/crates/egui/src/frame_state.rs @@ -9,6 +9,13 @@ pub(crate) struct TooltipFrameState { pub count: usize, } +#[cfg(feature = "accesskit")] +#[derive(Clone)] +pub(crate) struct AccessKitFrameState { + pub(crate) nodes: IdMap>, + pub(crate) parent_stack: Vec, +} + /// State that is collected during a frame and then cleared. /// Short-term (single frame) memory. #[derive(Clone)] @@ -43,7 +50,7 @@ pub(crate) struct FrameState { pub(crate) scroll_target: [Option<(RangeInclusive, Option)>; 2], #[cfg(feature = "accesskit")] - pub(crate) accesskit_nodes: Option>>, + pub(crate) accesskit_state: Option, } impl Default for FrameState { @@ -57,7 +64,7 @@ impl Default for FrameState { scroll_delta: Vec2::ZERO, scroll_target: [None, None], #[cfg(feature = "accesskit")] - accesskit_nodes: None, + accesskit_state: None, } } } @@ -73,7 +80,7 @@ impl FrameState { scroll_delta, scroll_target, #[cfg(feature = "accesskit")] - accesskit_nodes, + accesskit_state, } = self; used_ids.clear(); @@ -85,7 +92,7 @@ impl FrameState { *scroll_target = [None, None]; #[cfg(feature = "accesskit")] { - *accesskit_nodes = None; + *accesskit_state = None; } } diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 79dec2b1aea..95053e289f2 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -529,7 +529,7 @@ impl Response { self.output_event(event); } else { #[cfg(feature = "accesskit")] - if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { + if let Some(mut node) = self.ctx.accesskit_node(self.id) { self.fill_accesskit_node_from_widget_info(&mut node, make_info()); } } @@ -537,7 +537,7 @@ impl Response { pub fn output_event(&self, event: crate::output::OutputEvent) { #[cfg(feature = "accesskit")] - if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { + if let Some(mut node) = self.ctx.accesskit_node(self.id) { self.fill_accesskit_node_from_widget_info(&mut node, event.widget_info().clone()); } self.ctx.output().events.push(event); @@ -606,7 +606,7 @@ impl Response { /// Associate a label with a control for accessibility. pub fn labelled_by(self, id: Id) -> Self { #[cfg(feature = "accesskit")] - if let Some(mut node) = self.ctx.accesskit_node(self.id, None) { + if let Some(mut node) = self.ctx.accesskit_node(self.id) { node.labelled_by.push(id.accesskit_id()); } #[cfg(not(feature = "accesskit"))] diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 0597647eae5..2b4c7041731 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -541,7 +541,7 @@ impl<'a> Widget for DragValue<'a> { response.widget_info(|| WidgetInfo::drag_value(value)); #[cfg(feature = "accesskit")] - if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { + if let Some(mut node) = ui.ctx().accesskit_node(response.id) { use accesskit::Action; // If either end of the range is unbounded, it's better // to leave the corresponding AccessKit field set to None, diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index 5c2dbf45c32..602f47846d4 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -740,7 +740,7 @@ impl<'a> Slider<'a> { response.widget_info(|| WidgetInfo::slider(value, self.text.text())); #[cfg(feature = "accesskit")] - if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { + if let Some(mut node) = ui.ctx().accesskit_node(response.id) { use accesskit::Action; node.min_numeric_value = Some(*self.range.start()); node.max_numeric_value = Some(*self.range.end()); diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 8cc228ae97e..ea8d16b5eb6 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -659,7 +659,7 @@ impl<'t> TextEdit<'t> { } #[cfg(feature = "accesskit")] - if let Some(mut node) = ui.ctx().accesskit_node(response.id, None) { + if let Some(mut node) = ui.ctx().accesskit_node(response.id) { use accesskit::{Role, TextDirection, TextPosition, TextSelection}; let parent_id = response.id; @@ -683,61 +683,63 @@ impl<'t> TextEdit<'t> { drop(node); - for (i, row) in galley.rows.iter().enumerate() { - let id = parent_id.with(i); - let mut node = ui.ctx().accesskit_node(id, Some(parent_id)).unwrap(); - node.role = Role::InlineTextBox; - let rect = row.rect.translate(text_draw_pos.to_vec2()); - node.bounds = Some(accesskit::kurbo::Rect { - x0: rect.min.x.into(), - y0: rect.min.y.into(), - x1: rect.max.x.into(), - y1: rect.max.y.into(), - }); - node.text_direction = Some(TextDirection::LeftToRight); - // TODO: more info for the whole row - - let glyph_count = row.glyphs.len(); - let mut value = String::new(); - value.reserve(glyph_count); - let mut character_lengths = Vec::::new(); - character_lengths.reserve(glyph_count); - let mut character_positions = Vec::::new(); - character_positions.reserve(glyph_count); - let mut character_widths = Vec::::new(); - character_widths.reserve(glyph_count); - let mut word_lengths = Vec::::new(); - let mut was_at_word_end = false; - let mut last_word_start = 0usize; - - for glyph in &row.glyphs { - let is_word_char = is_word_char(glyph.chr); - if is_word_char && was_at_word_end { - word_lengths.push((character_lengths.len() - last_word_start) as _); - last_word_start = character_lengths.len(); + ui.ctx().with_accessibility_parent(parent_id, || { + for (i, row) in galley.rows.iter().enumerate() { + let id = parent_id.with(i); + let mut node = ui.ctx().accesskit_node(id).unwrap(); + node.role = Role::InlineTextBox; + let rect = row.rect.translate(text_draw_pos.to_vec2()); + node.bounds = Some(accesskit::kurbo::Rect { + x0: rect.min.x.into(), + y0: rect.min.y.into(), + x1: rect.max.x.into(), + y1: rect.max.y.into(), + }); + node.text_direction = Some(TextDirection::LeftToRight); + // TODO: more info for the whole row + + let glyph_count = row.glyphs.len(); + let mut value = String::new(); + value.reserve(glyph_count); + let mut character_lengths = Vec::::new(); + character_lengths.reserve(glyph_count); + let mut character_positions = Vec::::new(); + character_positions.reserve(glyph_count); + let mut character_widths = Vec::::new(); + character_widths.reserve(glyph_count); + let mut word_lengths = Vec::::new(); + let mut was_at_word_end = false; + let mut last_word_start = 0usize; + + for glyph in &row.glyphs { + let is_word_char = is_word_char(glyph.chr); + if is_word_char && was_at_word_end { + word_lengths.push((character_lengths.len() - last_word_start) as _); + last_word_start = character_lengths.len(); + } + was_at_word_end = !is_word_char; + let old_len = value.len(); + value.push(glyph.chr); + character_lengths.push((value.len() - old_len) as _); + character_positions.push(glyph.pos.x - row.rect.min.x); + character_widths.push(glyph.size.x); } - was_at_word_end = !is_word_char; - let old_len = value.len(); - value.push(glyph.chr); - character_lengths.push((value.len() - old_len) as _); - character_positions.push(glyph.pos.x - row.rect.min.x); - character_widths.push(glyph.size.x); - } - if row.ends_with_newline { - value.push('\n'); - character_lengths.push(1); - character_positions.push(row.rect.max.x - row.rect.min.x); - character_widths.push(0.0); - } - word_lengths.push((character_lengths.len() - last_word_start) as _); + if row.ends_with_newline { + value.push('\n'); + character_lengths.push(1); + character_positions.push(row.rect.max.x - row.rect.min.x); + character_widths.push(0.0); + } + word_lengths.push((character_lengths.len() - last_word_start) as _); - node.value = Some(value.into()); - node.character_lengths = character_lengths.into(); - node.character_positions = Some(character_positions.into()); - node.character_widths = Some(character_widths.into()); - node.word_lengths = word_lengths.into(); - } + node.value = Some(value.into()); + node.character_lengths = character_lengths.into(); + node.character_positions = Some(character_positions.into()); + node.character_widths = Some(character_widths.into()); + node.word_lengths = word_lengths.into(); + } + }); } TextEditOutput { From f1908e294383c7837ad937852396deb3a6661f3b Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 09:48:29 -0600 Subject: [PATCH 25/32] Last AccessKit update for this PR; includes an important macOS DPI fix --- Cargo.lock | 20 ++++++++++---------- crates/egui-winit/Cargo.toml | 2 +- crates/egui/Cargo.toml | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5e562837bbd..d8450c1179c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -20,9 +20,9 @@ checksum = "a13739d7177fbd22bb0ed28badfff9f372f8bef46c863db4e1c6248f6b223b6e" [[package]] name = "accesskit" -version = "0.8.0" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2f58dda4ca012077ac19d2062ccc30187fa5588f377961be11674c3ca5f8df1" +checksum = "3083ac5a97521e35388ca80cf365b6be5210962cc59f11ee238cd92ac2fa9524" dependencies = [ "enumset", "kurbo", @@ -31,9 +31,9 @@ dependencies = [ [[package]] name = "accesskit_consumer" -version = "0.9.1" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c22c90fa269beac30826b50ff07c4e0fb1dbc7a6dd80555e1d61f9c599db752" +checksum = "df122220244ca3ab93f6a42da59a5f8b379c8846dbcaedf922d95636d22c4e10" dependencies = [ "accesskit", "parking_lot", @@ -41,9 +41,9 @@ dependencies = [ [[package]] name = "accesskit_macos" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "efa5b671f649a554d8c27b238ba69e5eac8be4809713ebc6cfeca78ebb0ee639" +checksum = "55c97d7b5cbb2409e05b016406a1bd057237d120205cb63220ca86c2ea3790a1" dependencies = [ "accesskit", "accesskit_consumer", @@ -54,9 +54,9 @@ dependencies = [ [[package]] name = "accesskit_windows" -version = "0.10.0" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa69568d9c5df0b964f1ca1bc49e865732f256832cf21775b08a736975501682" +checksum = "4b0cfda25182b83b24e350434a3f63676252a00a295f32760a14d3f55feb8493" dependencies = [ "accesskit", "accesskit_consumer", @@ -69,9 +69,9 @@ dependencies = [ [[package]] name = "accesskit_winit" -version = "0.7.0" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee81f4dce90c61ccbf8579ad2f0c7650022e8a5bb8bd10cd62438cad6330a2ff" +checksum = "cdf20fecd6573e03bebcb4de267f82431e5ea39a293b62aa51a45bdfd69ef39b" dependencies = [ "accesskit", "accesskit_macos", diff --git a/crates/egui-winit/Cargo.toml b/crates/egui-winit/Cargo.toml index 3d24b8f2edb..afd4457a36f 100644 --- a/crates/egui-winit/Cargo.toml +++ b/crates/egui-winit/Cargo.toml @@ -61,7 +61,7 @@ winit = { version = "0.27.2", default-features = false } document-features = { version = "0.2", optional = true } # feature accesskit -accesskit_winit = { version = "0.7.0", optional = true } +accesskit_winit = { version = "0.7.1", optional = true } puffin = { version = "0.14", optional = true } serde = { version = "1.0", optional = true, features = ["derive"] } diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml index 9dec778ca38..a10c047167f 100644 --- a/crates/egui/Cargo.toml +++ b/crates/egui/Cargo.toml @@ -57,7 +57,7 @@ serde = ["dep:serde", "epaint/serde", "accesskit?/serde"] [dependencies] epaint = { version = "0.19.0", path = "../epaint", default-features = false } -accesskit = { version = "0.8.0", optional = true } +accesskit = { version = "0.8.1", optional = true } ahash = { version = "0.8.1", default-features = false, features = [ "no-rng", # we don't need DOS-protection, so we let users opt-in to it instead "std", From 9b71817c6695acbbaa840a3a83f163c2cd6fb435 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 09:57:57 -0600 Subject: [PATCH 26/32] Move and document the optional accesskit dependency --- crates/egui/Cargo.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml index a10c047167f..b930bee92c8 100644 --- a/crates/egui/Cargo.toml +++ b/crates/egui/Cargo.toml @@ -57,7 +57,6 @@ serde = ["dep:serde", "epaint/serde", "accesskit?/serde"] [dependencies] epaint = { version = "0.19.0", path = "../epaint", default-features = false } -accesskit = { version = "0.8.1", optional = true } ahash = { version = "0.8.1", default-features = false, features = [ "no-rng", # we don't need DOS-protection, so we let users opt-in to it instead "std", @@ -65,6 +64,10 @@ ahash = { version = "0.8.1", default-features = false, features = [ nohash-hasher = "0.2" #! ### Optional dependencies +## Exposes detailed accessibility implementation required by platform +## accessibility APIs. Also requires support in the egui integration. +accesskit = { version = "0.8.1", optional = true } + ## Enable this when generating docs. document-features = { version = "0.2", optional = true } From 88840ef0cba49957a04cfd3eb03aa838b893f3e4 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 10:01:13 -0600 Subject: [PATCH 27/32] Fix comment typo Co-authored-by: Emil Ernerfeldt --- crates/egui/src/widgets/drag_value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 2b4c7041731..31e69d95d83 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -449,7 +449,7 @@ impl<'a> Widget for DragValue<'a> { } }; - #[allow(clippy::redundant_clone)] // some clones below are dundant if AccessKit is disabled + #[allow(clippy::redundant_clone)] // some clones below are redundant if AccessKit is disabled let mut response = if is_kb_editing { let button_width = ui.spacing().interact_size.x; let mut value_text = ui From 0a6afeb3e446abe2667ec45650843bb474c04a1c Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 10:11:01 -0600 Subject: [PATCH 28/32] reformat --- crates/egui/src/widgets/drag_value.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 31e69d95d83..59ccbd82de7 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -449,7 +449,8 @@ impl<'a> Widget for DragValue<'a> { } }; - #[allow(clippy::redundant_clone)] // some clones below are redundant if AccessKit is disabled + // some clones below are redundant if AccessKit is disabled + #[allow(clippy::redundant_clone)] let mut response = if is_kb_editing { let button_width = ui.spacing().interact_size.x; let mut value_text = ui From 6d222c683d7d027a6e4f42639b920efa7d42a917 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 10:19:13 -0600 Subject: [PATCH 29/32] More elegant code for conditionally creating a node Co-authored-by: Emil Ernerfeldt --- crates/egui/src/context.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 9284bde3ca5..afe931f5ae3 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -159,13 +159,8 @@ impl ContextImpl { fn accesskit_node(&mut self, id: Id) -> &mut accesskit::Node { let state = self.frame_state.accesskit_state.as_mut().unwrap(); let nodes = &mut state.nodes; - // We have to override clippy's map_entry lint here, because the - // insertion path also modifies another entry, to establish - // the parent/child relationship. Using `HashMap::entry` here - // would require two mutable borrows at once. - #[allow(clippy::map_entry)] - if !nodes.contains_key(&id) { - nodes.insert(id, Default::default()); + if let std::collections::hash_map::Entry::Vacant(entry) = nodes.entry(id) { + entry.insert(Default::default()); let parent_id = state.parent_stack.last().unwrap(); let parent = nodes.get_mut(parent_id).unwrap(); parent.children.push(id.accesskit_id()); From 46e360b5b686e6b84f663084f083e646772324fd Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 10:28:21 -0600 Subject: [PATCH 30/32] Set step to 1.0 for all integer sliders --- crates/egui/src/widgets/slider.rs | 2 +- examples/hello_world/src/main.rs | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index 602f47846d4..bb673afad38 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -450,7 +450,7 @@ impl<'a> Slider<'a> { /// If you use one of the integer constructors (e.g. `Slider::i32`) this is called for you, /// but if you want to have a slider for picking integer values in an `Slider::f64`, use this. pub fn integer(self) -> Self { - self.fixed_decimals(0).smallest_positive(1.0) + self.fixed_decimals(0).smallest_positive(1.0).step_by(1.0) } fn get_value(&mut self) -> f64 { diff --git a/examples/hello_world/src/main.rs b/examples/hello_world/src/main.rs index a96e47d9ada..de4965acce4 100644 --- a/examples/hello_world/src/main.rs +++ b/examples/hello_world/src/main.rs @@ -37,11 +37,7 @@ impl eframe::App for MyApp { ui.text_edit_singleline(&mut self.name) .labelled_by(name_label.id); }); - ui.add( - egui::Slider::new(&mut self.age, 0..=120) - .step_by(1.0) - .text("age"), - ); + ui.add(egui::Slider::new(&mut self.age, 0..=120).text("age")); if ui.button("Click each year").clicked() { self.age += 1; } From 30771775e95e185638f4ffe7042f5761788f7704 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 10:44:50 -0600 Subject: [PATCH 31/32] Add doc example for Response::labelled_by --- crates/egui/src/response.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 95053e289f2..7e108f3759e 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -604,6 +604,18 @@ impl Response { } /// Associate a label with a control for accessibility. + /// + /// # Example + /// + /// ``` + /// # egui::__run_test_ui(|ui| { + /// # let mut text = "Arthur".to_string(); + /// ui.horizontal(|ui| { + /// let label = ui.label("Your name: "); + /// ui.text_edit_singleline(&mut text).labelled_by(label.id); + /// }); + /// # }); + /// ``` pub fn labelled_by(self, id: Id) -> Self { #[cfg(feature = "accesskit")] if let Some(mut node) = self.ctx.accesskit_node(self.id) { From e522a9bb088a660e3e333e6f3b16600223cda2ca Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Sun, 4 Dec 2022 11:44:54 -0600 Subject: [PATCH 32/32] Clarify a TODO comment I left for myself --- crates/egui/src/widgets/text_edit/builder.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index ea8d16b5eb6..bc4f768a908 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -696,7 +696,8 @@ impl<'t> TextEdit<'t> { y1: rect.max.y.into(), }); node.text_direction = Some(TextDirection::LeftToRight); - // TODO: more info for the whole row + // TODO(mwcampbell): Set more node fields for the row + // once AccessKit adapters expose text formatting info. let glyph_count = row.glyphs.len(); let mut value = String::new();