Skip to content

Commit

Permalink
Add support for optimized rectangles in ElementHandle
Browse files Browse the repository at this point in the history
Keep merging elements, but remember the boundaries in the debug info, separated by a slash.

Also fixed tests that rely on accessible-label being set only once. For example

```
Button { text: "foo"; }
```

will certainly have "foo" as accessible-label on `Button`, but its internal `Text` element has
an implicit "accessible-label" set to the same
value.

So don't rely on that for now but search by id instead.
  • Loading branch information
tronical committed May 21, 2024
1 parent 86f8dd8 commit 7fb1ef8
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 31 deletions.
52 changes: 50 additions & 2 deletions internal/backends/testing/search_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ pub(crate) fn search_item(
#[repr(C)]
pub struct ElementHandle {
item: ItemWeak,
element_index: usize, // When multiple elements get optimized into a single ItemRc, this index separates.
}

impl ElementHandle {
fn collect_elements(item: ItemRc) -> impl Iterator<Item = ElementHandle> {
core::iter::once(ElementHandle { item: item.downgrade() })
(0..item.element_count())
.map(move |element_index| ElementHandle { item: item.downgrade(), element_index })
}

/// Returns true if the element still exists in the in UI and is valid to access; false otherwise.
Expand Down Expand Up @@ -133,7 +135,9 @@ impl ElementHandle {
pub fn element_type_names_and_ids(
&self,
) -> Option<impl Iterator<Item = (SharedString, SharedString)>> {
self.item.upgrade().map(|item| item.element_type_names_and_ids().into_iter())
self.item
.upgrade()
.map(|item| item.element_type_names_and_ids(self.element_index).into_iter())
}

/// Invokes the default accessible action on the element. For example a `MyButton` element might declare
Expand Down Expand Up @@ -269,3 +273,47 @@ impl ElementHandle {
}
}
}

#[test]
fn test_optimized() {
crate::init_no_event_loop();

slint::slint! {
#[debug_info]
export component App inherits Window {
first := Rectangle {
second := Rectangle {
third := Rectangle {}
}
}
}
}

let app = App::new().unwrap();
let mut it = ElementHandle::find_by_element_id(&app, "App::first");
let first = it.next().unwrap();
assert!(it.next().is_none());

assert_eq!(
first.element_type_names_and_ids().unwrap().collect::<Vec<_>>(),
vec![("Rectangle".into(), "App::first".into())]
);

it = ElementHandle::find_by_element_id(&app, "App::second");
let second = it.next().unwrap();
assert!(it.next().is_none());

assert_eq!(
second.element_type_names_and_ids().unwrap().collect::<Vec<_>>(),
vec![("Rectangle".into(), "App::second".into())]
);

it = ElementHandle::find_by_element_id(&app, "App::third");
let third = it.next().unwrap();
assert!(it.next().is_none());

assert_eq!(
third.element_type_names_and_ids().unwrap().collect::<Vec<_>>(),
vec![("Rectangle".into(), "App::third".into())]
);
}
2 changes: 1 addition & 1 deletion internal/compiler/generator/cpp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2038,7 +2038,7 @@ fn generate_sub_component(
component
.element_infos
.iter()
.map(|(index, ids)| format!(" case {index}: return \"{}\";", ids.join(";"))),
.map(|(index, ids)| format!(" case {index}: return \"{}\";", ids)),
);
element_infos_cases.push("}".into());

Expand Down
5 changes: 1 addition & 4 deletions internal/compiler/generator/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,10 +837,7 @@ fn generate_sub_component(
let mut item_element_infos_branch = component
.element_infos
.iter()
.map(|(item_index, ids)| {
let ids = ids.join(";");
quote!(#item_index => { return #ids.into(); })
})
.map(|(item_index, ids)| quote!(#item_index => { return #ids.into(); }))
.collect::<Vec<_>>();

let mut user_init_code: Vec<TokenStream> = Vec::new();
Expand Down
2 changes: 1 addition & 1 deletion internal/compiler/llr/item_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ pub struct SubComponent {
pub accessible_prop: BTreeMap<(u32, String), MutExpression>,

/// Maps item index to a list of encoded element infos of the element (type name, qualified ids).
pub element_infos: BTreeMap<u32, Vec<String>>,
pub element_infos: BTreeMap<u32, String>,

pub prop_analysis: HashMap<PropertyReference, PropAnalysis>,
}
Expand Down
29 changes: 22 additions & 7 deletions internal/compiler/object_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,10 @@ pub struct ElementDebugInfo {
// Field to indicate wether this element was a layout that had
// been lowered into a rectangle in the lower_layouts pass.
pub layout: Option<crate::layout::Layout>,
/// Set to true if the ElementDebugInfo following this one in the debug vector
/// in Element::debug is the last one and the next entry belongs to an other element.
/// This can happen as a result of rectangle optimization, for example.
pub element_boundary: bool,
}

impl ElementDebugInfo {
Expand Down Expand Up @@ -952,6 +956,7 @@ impl Element {
type_name,
node: node.clone(),
layout: None,
element_boundary: false,
}],
is_legacy_syntax,
..Default::default()
Expand Down Expand Up @@ -1779,18 +1784,28 @@ impl Element {
}
}

pub fn element_infos(&self) -> Vec<String> {
let mut infos = self
.debug
.iter()
.map(|debug_info| debug_info.encoded_element_info())
.collect::<Vec<_>>();
pub fn element_infos(&self) -> String {
let mut debug_infos = self.debug.clone();
let mut base = self.base_type.clone();
while let ElementType::Component(b) = base {
let elem = b.root_element.borrow();
base = elem.base_type.clone();
infos.extend(elem.debug.iter().map(|debug_info| debug_info.encoded_element_info()));
debug_infos.extend(elem.debug.iter().cloned());
}

let (infos, _, _) = debug_infos.into_iter().fold(
(String::new(), false, true),
|(mut infos, elem_boundary, first), debug_info| {
if elem_boundary {
infos.push('/');
} else if !first {
infos.push(';');
}

infos.push_str(&debug_info.encoded_element_info());
(infos, debug_info.element_boundary, false)
},
);
infos
}
}
Expand Down
3 changes: 3 additions & 0 deletions internal/compiler/passes/optimize_useless_rectangles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ pub fn optimize_useless_rectangles(root_component: &Rc<Component>) {
}

parent.children.extend(std::mem::take(&mut elem.borrow_mut().children));
if let Some(last) = parent.debug.last_mut() {
last.element_boundary = true;
}
parent.debug.extend(std::mem::take(&mut elem.borrow_mut().debug));

let enclosing = parent.enclosing_component.upgrade().unwrap();
Expand Down
15 changes: 14 additions & 1 deletion internal/core/item_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,25 @@ impl ItemRc {
comp_ref_pin.as_ref().supported_accessibility_actions(self.index)
}

pub fn element_type_names_and_ids(&self) -> Vec<(SharedString, SharedString)> {
pub fn element_count(&self) -> usize {
let comp_ref_pin = vtable::VRc::borrow_pin(&self.item_tree);
let mut result = SharedString::new();
comp_ref_pin.as_ref().item_element_infos(self.index, &mut result);
result.as_str().split("/").count()
}

pub fn element_type_names_and_ids(
&self,
element_index: usize,
) -> Vec<(SharedString, SharedString)> {
let comp_ref_pin = vtable::VRc::borrow_pin(&self.item_tree);
let mut result = SharedString::new();
comp_ref_pin.as_ref().item_element_infos(self.index, &mut result);
result
.as_str()
.split("/")
.nth(element_index)
.unwrap()
.split(";")
.map(|encoded_elem_info| {
let mut decoder = encoded_elem_info.split(',');
Expand Down
1 change: 0 additions & 1 deletion internal/interpreter/dynamic_item_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2024,7 +2024,6 @@ extern "C" fn item_element_infos(
*result = instance_ref.description.original_elements[item_index as usize]
.borrow()
.element_infos()
.join(";")
.into();
}

Expand Down
10 changes: 5 additions & 5 deletions tests/cases/widgets/button.slint
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export component TestCase inherits Window {
clicked => {clicked += "a"; }
}

Button {
b := Button {
text: "Bbb";
accessible-description: "Normal Button";
clicked => {clicked += "b"; }
Expand All @@ -46,7 +46,7 @@ slint_testing::send_keyboard_string_sequence(&instance, &SharedString::from(Key:
assert_eq!(instance.get_clicked(), SharedString::from(""));
assert_eq!(instance.get_a_checked(), false);
let mut result = slint_testing::ElementHandle::find_by_accessible_label(&instance, "Aaa").collect::<Vec<_>>();
let mut result = slint_testing::ElementHandle::find_by_element_id(&instance, "TestCase::a").collect::<Vec<_>>();
assert_eq!(result.len(), 1);
let aaa = result.pop().unwrap();
assert_eq!(aaa.accessible_label().unwrap(), "Aaa");
Expand Down Expand Up @@ -79,7 +79,7 @@ assert_eq!(instance.get_a_checked(), true, "button aaa was not toggled on enter"
assert_eq!(aaa.accessible_checked(), Some(true));
assert_eq!(instance.get_a_focused(), true);
let mut result = slint_testing::ElementHandle::find_by_accessible_label(&instance, "Bbb").collect::<Vec<_>>();
let mut result = slint_testing::ElementHandle::find_by_element_id(&instance, "TestCase::b").collect::<Vec<_>>();
assert_eq!(result.len(), 1);
let bbb = result.pop().unwrap();
assert_eq!(bbb.accessible_label().unwrap(), "Bbb");
Expand All @@ -103,7 +103,7 @@ assert_eq!(instance.get_a_focused(), true);
auto handle = TestCase::create();
const TestCase &instance = *handle;
auto label_search = slint::testing::ElementHandle::find_by_accessible_label(handle, "Aaa");
auto label_search = slint::testing::ElementHandle::find_by_element_id(handle, "TestCase::a");
assert(label_search.size() == 1);
auto aaa = label_search[0];
assert_eq(aaa.accessible_label().value(), "Aaa");
Expand Down Expand Up @@ -131,7 +131,7 @@ assert_eq(instance.get_clicked(), "aa");
assert_eq(aaa.accessible_checked().value(), true);
assert_eq(instance.get_a_focused(), true);
label_search = slint::testing::ElementHandle::find_by_accessible_label(handle, "Bbb");
label_search = slint::testing::ElementHandle::find_by_element_id(handle, "TestCase::b");
assert(label_search.size() == 1);
auto bbb = label_search[0];
assert_eq(bbb.accessible_label().value(), "Bbb");
Expand Down
18 changes: 9 additions & 9 deletions tests/cases/widgets/spinbox_default_value.slint
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@ import { SpinBox } from "std-widgets.slint";
export component TestCase inherits Window {
width: 100px;
height: 100px;

spin_min_pos := SpinBox {
accessible-label: "spinbox";
minimum: 10;
accessible-label: "spinbox";
minimum: 10;
}

out property <int> pos-val <=> spin-min-pos.value;

spin_min_default := SpinBox {}
out property <int> default-min <=> spin-min-default.minimum;
spin_min_default := SpinBox { }

out property <int> default-min <=> spin-min-default.minimum;
out property <int> default-val <=> spin-min-default.value;

spin_min_neg := SpinBox {
minimum: -10;
}

out property <int> neg-val <=> spin-min-neg.value;
}


/*
```rust
Expand All @@ -34,7 +34,7 @@ assert_eq!(instance.get_default_min(), 0);
assert_eq!(instance.get_default_val(), 0);
assert_eq!(instance.get_neg_val(), -10);
let mut label_search = slint_testing::ElementHandle::find_by_accessible_label(&instance, "spinbox").collect::<Vec<_>>();
let mut label_search = slint_testing::ElementHandle::find_by_element_id(&instance, "TestCase::spin-min-pos").collect::<Vec<_>>();
assert_eq!(label_search.len(), 1);
let spinbox = label_search.pop().unwrap();
assert_eq!(spinbox.accessible_value_maximum(), Some(100f32));
Expand All @@ -53,7 +53,7 @@ assert_eq!(spinbox.accessible_value().unwrap(), "10");
```cpp
auto handle = TestCase::create();
auto label_search = slint::testing::ElementHandle::find_by_accessible_label(handle, "spinbox");
auto label_search = slint::testing::ElementHandle::find_by_element_id(handle, "TestCase::spin-min-pos");
assert(label_search.size() == 1);
auto spinbox = label_search[0];
assert_eq(spinbox.accessible_value_maximum().value(), 100);
Expand Down

0 comments on commit 7fb1ef8

Please sign in to comment.