From 49bbcf9b2e12b47360e95591f0c79b4c5b1f337b Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 30 Nov 2022 09:20:12 -0600 Subject: [PATCH] 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 e14d12d1..dfccb7e8 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 aa404f56..79dec2b1 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 d1239726..0597647e 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 a2e0b263..5c2dbf45 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 18d807b2..ce25aeba 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 {