From 87d3a90718e9ad79666ef5cde6d554fe879dab8e Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 14:42:49 -0600 Subject: [PATCH] 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 | 104 ++++++++++--------- 6 files changed, 115 insertions(+), 77 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index dc22846d..9284bde3 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 b0031502..73dcfd4c 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 79dec2b1..95053e28 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 0597647e..2b4c7041 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 5c2dbf45..602f4784 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 8cc228ae..ea8d16b5 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 + 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; + 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(); + 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 {