Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-11-22 12:43:34 +00:00
Code Issues Projects Releases Packages Wiki Activity

Refactor function update_accessibility_nodes (#10911)

Browse source 
# Objective

`update_accessibility_nodes` is one of the most nested functions in the
entire Bevy repository, with a maximum of 9 levels of indentations. This
PR refactors it down to 3 levels of indentations, while improving
readability on other fronts. The result is a function that is actually
understandable at a first glance.

- This is a proof of concept to demonstrate that it is possible to
gradually lower the nesting limit proposed by #10896.

PS: I read AccessKit's documentation, but I don't have experience with
it. Therefore, naming of variables and subroutines may be a bit off.

PS2: I don't know if the test suite covers the functionality of this
system, but since I've spent quite some time on it and the changes were
simple, I'm pretty confident the refactor is functionally equivalent to
the original.

## Solution

I documented each change with a commit, but as a summary I did the
following to reduce nesting:

- I moved from `if-let` blocks to `let-else` statements where
appropriate to reduce rightward shift
- I extracted the closure body to a new function `update_adapter`
- I factored out parts of `update_adapter` into new functions
`queue_node_for_update` and `add_children_nodes`

**Note for reviewers:** GitHub's diff viewer is not the greatest in
showing horizontal code shifts, therefore you may want to use a
different tool like VSCode to review some commits, especially the second
one (anyway, that commit is very straightforward, despite changing many
lines).
This commit is contained in:
Federico Rinaldi 2023-12-10 01:52:16 +01:00 committed by GitHub
parent 4a46f273a1
commit 47fa620034
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 87 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

136
crates/bevy_winit/src/accessibility.rs
View file

@ -86,55 +86,93 @@ fn update_accessibility_nodes(
)>,
node_entities: Query<Entity, With<AccessibilityNode>>,
) {
if let Ok((primary_window_id, primary_window)) = primary_window.get_single() {
if let Some(adapter) = adapters.get(&primary_window_id) {
let should_run = focus.is_changed() || !nodes.is_empty();
if should_run {
adapter.update_if_active(|| {
let mut to_update = vec![];
let mut name = None;
if primary_window.focused {
let title = primary_window.title.clone();
name = Some(title.into_boxed_str());
}
let focus_id = (*focus).unwrap_or_else(|| primary_window_id).to_bits();
let mut root_children = vec![];
for (entity, node, children, parent) in &nodes {
let mut node = (**node).clone();
if let Some(parent) = parent {
if !node_entities.contains(**parent) {
root_children.push(NodeId(entity.to_bits()));
}
} else {
root_children.push(NodeId(entity.to_bits()));
}
if let Some(children) = children {
for child in children {
if node_entities.contains(*child) {
node.push_child(NodeId(child.to_bits()));
}
}
}
to_update.push((
NodeId(entity.to_bits()),
node.build(&mut NodeClassSet::lock_global()),
));
}
let mut root = NodeBuilder::new(Role::Window);
if let Some(name) = name {
root.set_name(name);
}
root.set_children(root_children);
let root = root.build(&mut NodeClassSet::lock_global());
let window_update = (NodeId(primary_window_id.to_bits()), root);
to_update.insert(0, window_update);
TreeUpdate {
nodes: to_update,
tree: None,
focus: NodeId(focus_id),
}
});
}
let Ok((primary_window_id, primary_window)) = primary_window.get_single() else {
return;
};
let Some(adapter) = adapters.get(&primary_window_id) else {
return;
};
if focus.is_changed() || !nodes.is_empty() {
adapter.update_if_active(|| {
update_adapter(
nodes,
node_entities,
primary_window,
primary_window_id,
focus,
)
});
}
}
fn update_adapter(
nodes: Query<(
Entity,
&AccessibilityNode,
Option<&Children>,
Option<&Parent>,
)>,
node_entities: Query<Entity, With<AccessibilityNode>>,
primary_window: &Window,
primary_window_id: Entity,
focus: Res<Focus>,
) -> TreeUpdate {
let mut to_update = vec![];
let mut window_children = vec![];
for (entity, node, children, parent) in &nodes {
let mut node = (**node).clone();
queue_node_for_update(entity, parent, &node_entities, &mut window_children);
add_children_nodes(children, &node_entities, &mut node);
let node_id = NodeId(entity.to_bits());
let node = node.build(&mut NodeClassSet::lock_global());
to_update.push((node_id, node));
}
let mut window_node = NodeBuilder::new(Role::Window);
if primary_window.focused {
let title = primary_window.title.clone();
window_node.set_name(title.into_boxed_str());
}
window_node.set_children(window_children);
let window_node = window_node.build(&mut NodeClassSet::lock_global());
let node_id = NodeId(primary_window_id.to_bits());
let window_update = (node_id, window_node);
to_update.insert(0, window_update);
TreeUpdate {
nodes: to_update,
tree: None,
focus: NodeId(focus.unwrap_or(primary_window_id).to_bits()),
}
}
#[inline]
fn queue_node_for_update(
node_entity: Entity,
parent: Option<&Parent>,
node_entities: &Query<Entity, With<AccessibilityNode>>,
window_children: &mut Vec<NodeId>,
) {
let should_push = if let Some(parent) = parent {
node_entities.contains(parent.get())
} else {
true
};
if should_push {
window_children.push(NodeId(node_entity.to_bits()));
}
}
#[inline]
fn add_children_nodes(
children: Option<&Children>,
node_entities: &Query<Entity, With<AccessibilityNode>>,
node: &mut NodeBuilder,
) {
let Some(children) = children else {
return;
};
for child in children {
if node_entities.contains(*child) {
node.push_child(NodeId(child.to_bits()));
}
}
}