Skip to content

Commit

Permalink
Simplify accesskit NodeId encoding
Browse files Browse the repository at this point in the history 
Since we rebuild the accesskit node tree on any Slint item tree changes,
we might as well just use the index in our vector of property trackers
as node id.
  • Loading branch information
@tronical
tronical committed Jun 13, 2023
1 parent 17139fa commit 0f0b275
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 94 deletions.
17 changes: 0 additions & 17 deletions helper_crates/vtable/src/vrc.rs
View file
Expand Up @@ -207,23 +207,6 @@ impl<VTable: VTableMetaDropInPlace + 'static> VRc<VTable, Dyn> {
) -> VRcMapped<VTable, MappedType> {
VRcMapped { parent_strong: this.clone(), object: map_fn(Self::borrow_pin(&this)).get_ref() }
}

/// Returns a raw pointer to the inner data structure that holds the reference counted object.
/// This pointer is only valid as long as a `VRc` remains alive.
pub fn as_raw_ptr(&self) -> *const core::ffi::c_void {
self.inner.as_ptr() as _
}

/// Transmutes a raw pointer to VRc's inner data structure back into a VRc. Use in combination
/// with `as_raw_ptr()`.
///
/// Safety: This is highly unsafe. Don't use this unless you're absolutely certain that somebody
/// else is keeping the object alive via a VRc.
pub unsafe fn from_raw_ptr(raw_dyn: *const core::ffi::c_void) -> Self {
let inner: NonNull<VRcInner<'static, VTable, Dyn>> = core::mem::transmute(raw_dyn);
inner.as_ref().strong_ref.fetch_add(1, Ordering::SeqCst);
Self { inner }
}
}
impl<VTable: VTableMetaDropInPlace, X> VRc<VTable, X> {
/// Create a Pinned reference to the inner.
Expand Down
181 changes: 104 additions & 77 deletions internal/backends/winit/accesskit.rs
View file
Expand Up @@ -10,9 +10,10 @@ use accesskit::{
Action, ActionRequest, CheckedState, Node, NodeBuilder, NodeId, Role, Tree, TreeUpdate,
};
use i_slint_core::accessibility::AccessibleStringProperty;
use i_slint_core::item_tree::ItemWeak;
use i_slint_core::items::{ItemRc, WindowItem};
use i_slint_core::window::WindowInner;
use i_slint_core::{component::ComponentRef, lengths::ScaleFactor};
use i_slint_core::{component::ComponentVTable, window::WindowInner};
use i_slint_core::{properties::PropertyTracker, window::WindowAdapter};

use super::WinitWindowAdapter;
Expand All @@ -21,7 +22,8 @@ use super::WinitWindowAdapter;
///
/// The entire item tree is mapped to accesskit's node tree. Any changes to an individual accessible item results
/// in an access kit tree update with just changed nodes. Any changes in the tree structure result in a complete
/// tree rebuild. This could be implemented more efficiently.
/// tree rebuild. This could be implemented more efficiently, but it requires encoding the item index, raw vrc pointer,
/// and tree generation into the nodeid.
///
/// For unix it's necessary to inform accesskit about any changes to the position or size of the window, hence
/// the `on_event` function that needs calling.
Expand All @@ -38,8 +40,8 @@ pub struct AccessKitAdapter {
window_adapter_weak: Weak<WinitWindowAdapter>,

node_classes: RefCell<accesskit::NodeClassSet>,
tree_generation: Cell<u16>,
all_nodes: RefCell<Vec<CachedNode>>,
tree_generation: Cell<usize>,
all_nodes: RefCell<Vec<MappedNode>>,
global_property_tracker: Pin<Box<PropertyTracker<AccessibilitiesPropertyTracker>>>,
}

Expand All @@ -58,7 +60,7 @@ impl AccessKitAdapter {
),
window_adapter_weak: window_adapter_weak.clone(),
node_classes: RefCell::new(accesskit::NodeClassSet::new()),
tree_generation: Cell::new(0),
tree_generation: Cell::new(1),
all_nodes: Default::default(),
global_property_tracker: Box::pin(PropertyTracker::new_with_dirty_handler(
AccessibilitiesPropertyTracker { window_adapter_weak: window_adapter_weak.clone() },
Expand All @@ -78,16 +80,15 @@ impl AccessKitAdapter {
self.inner.update_if_active(|| TreeUpdate {
nodes: vec![],
tree: None,
focus: new.map(|item| node_id_for_item_rc(item, self.tree_generation.get())),
focus: new.map(|item| self.find_node_id_by_item_rc(item).unwrap()),
})
}

fn handle_request(&self, request: ActionRequest) {
let Some(window_adapter) = self.window_adapter_weak.upgrade() else { return };
match request.action {
Action::Focus => {
if let Some(item) = item_rc_for_node_id(request.target, self.tree_generation.get())
{
if let Some(item) = self.item_rc_for_node_id(request.target) {
WindowInner::from_pub(window_adapter.window()).set_focus_item(&item);
}
}
Expand All @@ -109,6 +110,50 @@ impl AccessKitAdapter {
});
}

fn add_node(&self, node: MappedNode) -> NodeId {
let index: usize = self.all_nodes.borrow().len();
let id = NodeId(
std::num::NonZeroU128::new(
(index as u128) << usize::BITS
| (self.tree_generation.get() as u128 & usize::MAX as u128),
)
.unwrap(),
);
self.all_nodes.borrow_mut().push(node);
id
}

fn nodes_iter(&self) -> NodeIter<'_> {
NodeIter {
nodes: Some(std::cell::Ref::map(self.all_nodes.borrow(), |vec| &vec[..])),
index: 0,
tree_generation: self.tree_generation.get(),
}
}

fn item_rc_for_node_id(&self, id: NodeId) -> Option<ItemRc> {
let index: usize = (id.0.get() >> usize::BITS) as _;
let generation: usize = (id.0.get() & usize::MAX as u128) as _;
if generation != self.tree_generation.get() {
return None;
}
self.all_nodes.borrow().get(index).and_then(|cached_node| cached_node.item.upgrade())
}

fn find_node_id_by_item_rc(&self, mut item: ItemRc) -> Option<NodeId> {
while !item.is_accessible() {
if let Some(parent) = item.parent_item() {
item = parent;
} else {
break;
}
}

self.nodes_iter().find_map(|(id, cached_node)| {
cached_node.item.upgrade().and_then(|cached_item| (cached_item.eq(&item)).then(|| id))
})
}

fn rebuild_tree_of_dirty_nodes(&self) {
if !self.global_property_tracker.is_dirty() {
return;
Expand All @@ -127,28 +172,18 @@ impl AccessKitAdapter {
self.global_property_tracker.as_ref().evaluate_as_dependency_root(|| {
let scale_factor = ScaleFactor::new(window.scale_factor());

let all_nodes = self.all_nodes.borrow();
let nodes = self.nodes_iter().filter_map(|(id, cached_node)| {
cached_node.tracker.as_ref().evaluate_if_dirty(|| {
let item = cached_node.item.upgrade()?;

let nodes = all_nodes.iter().filter_map(|cached_node| {
if !cached_node.tracker.is_dirty() {
return None;
}
cached_node
.tracker
.as_ref()
.evaluate_if_dirty(|| {
let item =
item_rc_for_node_id(cached_node.id, self.tree_generation.get())?;
let mut builder = self.build_node_without_children(&item, scale_factor);

let mut builder = self.build_node_without_children(&item, scale_factor);
builder.set_children(cached_node.children.clone());

builder.set_children(cached_node.children.clone());
let node = builder.build(&mut self.node_classes.borrow_mut());

let node = builder.build(&mut self.node_classes.borrow_mut());

Some((cached_node.id, node))
})
.unwrap() // Must be Some() because `is_dirty` returned true.
Some((id, node))
})?
});

let update = TreeUpdate {
Expand All @@ -158,7 +193,7 @@ impl AccessKitAdapter {
.focus_item
.borrow()
.upgrade()
.map(|item| node_id_for_item_rc(item, self.tree_generation.get())),
.and_then(|item| self.find_node_id_by_item_rc(item)),
};

update
Expand All @@ -174,25 +209,20 @@ impl AccessKitAdapter {
) -> NodeId {
let tracker = Box::pin(PropertyTracker::default());

let (id, children) = tracker.as_ref().evaluate(|| {
let mut builder = self.build_node_without_children(&item, scale_factor);

let children = i_slint_core::accessibility::accessible_descendents(&item)
.map(|child| self.build_node_for_item_recursively(child, nodes, scale_factor))
.collect::<Vec<NodeId>>();
let mut builder =
tracker.as_ref().evaluate(|| self.build_node_without_children(&item, scale_factor));

builder.set_children(children.clone());
let children = i_slint_core::accessibility::accessible_descendents(&item)
.map(|child| self.build_node_for_item_recursively(child, nodes, scale_factor))
.collect::<Vec<NodeId>>();

let id = node_id_for_item_rc(item, self.tree_generation.get());
builder.set_children(children.clone());

let node = builder.build(&mut self.node_classes.borrow_mut());
let id = self.add_node(MappedNode { item: item.downgrade(), children, tracker });
let node = builder.build(&mut self.node_classes.borrow_mut());

nodes.push((id, node));

(id, children)
});
nodes.push((id, node));

self.all_nodes.borrow_mut().push(CachedNode { id, children, tracker });
id
}

Expand Down Expand Up @@ -222,7 +252,7 @@ impl AccessKitAdapter {
.focus_item
.borrow()
.upgrade()
.map(|item| node_id_for_item_rc(item, tree_generation)),
.and_then(|item| self.find_node_id_by_item_rc(item)),
};

update
Expand Down Expand Up @@ -349,45 +379,42 @@ impl AccessKitAdapter {
}
}

// Encode a u128 accesskit node id for an ItemRc by assuming that the ItemRc remains
// alive and storing the item index, raw VRc inner pointer, and a tree generation index
// in the u128.
fn node_id_for_item_rc(mut item: ItemRc, tree_generation: u16) -> NodeId {
while !item.is_accessible() {
if let Some(parent) = item.parent_item() {
item = parent;
} else {
break;
}
}
struct NodeIter<'a> {
nodes: Option<std::cell::Ref<'a, [MappedNode]>>,
index: usize,
tree_generation: usize,
}

let component_ptr = item.component().as_raw_ptr();
let component_usize: u128 = component_ptr as usize as u128;
let item_index: u128 = item.index() as u32 as u128;
impl<'a> Iterator for NodeIter<'a> {
type Item = (NodeId, std::cell::Ref<'a, MappedNode>);

let encoded_id = std::num::NonZeroU128::new(
(component_usize << 64) | ((tree_generation as u128) << 48) | item_index,
)
.unwrap();
NodeId(encoded_id)
}
fn next(&mut self) -> Option<Self::Item> {
if self.nodes.is_none() {
return None;
}

fn item_rc_for_node_id(id: NodeId, expected_generation: u16) -> Option<ItemRc> {
let encoded_component = id.0.get() >> 64;
let encoded_generation = ((id.0.get() >> 48) & 0xffff) as u16;
let encoded_item_index = (id.0.get() & 0xffffffff) as u32;
let Some(remaining_slice) = self.nodes.take() else { return None; };

if encoded_generation != expected_generation {
return None;
}
if remaining_slice.is_empty() {
return None;
}

// Safety: When the tree structure changes, the tree generation value is increased, causing a mismatch
// caught earlier. Therefore, at this point the pointer should be safe to convert back into a VRc.
let component_rc = unsafe {
vtable::VRc::<ComponentVTable, vtable::Dyn>::from_raw_ptr(encoded_component as *const _)
};
let (head, tail) =
std::cell::Ref::map_split(remaining_slice, |slice| (&slice[0], &slice[1..]));
self.nodes.replace(tail);

Some(ItemRc::new(component_rc, encoded_item_index as usize))
let index = self.index;
self.index += 1;
let id = NodeId(
std::num::NonZeroU128::new(
(index as u128) << usize::BITS
| (self.tree_generation as u128 & usize::MAX as u128),
)
.unwrap(),
);

Some((id, head))
}
}

struct AccessibilitiesPropertyTracker {
Expand All @@ -405,8 +432,8 @@ impl i_slint_core::properties::PropertyDirtyHandler for AccessibilitiesPropertyT
}
}

struct CachedNode {
id: NodeId,
struct MappedNode {
item: ItemWeak,
children: Vec<NodeId>,
tracker: Pin<Box<PropertyTracker>>,
}
Expand Down

0 comments on commit 0f0b275

Please sign in to comment.