Component container subcomponent (#4355)

This changes the component containers away from using a "MAGIC" index in the
placeholder dynamic item tree node it creates. These are hard to
integrate across sub-components.

Use index numbers right after the index numbers used by repeaters and
"extend" the repeater offset by the number of component containers in
the sub-component. This way we can piggy-back on the forwarding of
repeaters.

This has one annoying side-effect: We do have indices in our item tree
that are out of range for a repeater. But I think that is acceptable
considering that we never materialize that array anyway.

internal/compiler/generator.rs

@@ -115,6 +115,7 @@ pub trait ItemTreeBuilder {
     fn push_component_placeholder_item(
         &mut self,
         item: &crate::object_tree::ElementRc,
+        container_count: u32, // Must start at repeater.len()!
         parent_index: u32,
         component_state: &Self::SubComponentState,
     );
@@ -163,7 +164,17 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
         build_item_tree::<T>(sub_component, &sub_compo_state, builder);
     } else {
         let mut repeater_count = 0;
-        visit_item(initial_state, &root_component.root_element, 1, &mut repeater_count, 0, builder);
+        let mut container_count =
+            repeater_count_in_sub_component(&root_component.root_element) as u32;
+        visit_item(
+            initial_state,
+            &root_component.root_element,
+            1,
+            &mut repeater_count,
+            &mut container_count,
+            0,
+            builder,
+        );
 
         visit_children(
             initial_state,
@@ -175,6 +186,7 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
             1,
             1,
             &mut repeater_count,
+            &mut container_count,
             builder,
         );
     }
@@ -193,6 +205,15 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
         count
     }
 
+    // Number of repeaters in this sub component
+    fn repeater_count_in_sub_component(e: &ElementRc) -> usize {
+        let mut count = if e.borrow().repeated.is_some() { 0 } else { 1 };
+        for i in &e.borrow().children {
+            count += repeater_count_in_sub_component(i);
+        }
+        count
+    }
+
     fn visit_children<T: ItemTreeBuilder>(
         state: &T::SubComponentState,
         children: &[ElementRc],
@@ -203,6 +224,7 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
         children_offset: u32,
         relative_children_offset: u32,
         repeater_count: &mut u32,
+        container_count: &mut u32,
         builder: &mut T,
     ) {
         debug_assert_eq!(
@@ -241,6 +263,7 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
                     children_offset,
                     relative_children_offset,
                     repeater_count,
+                    container_count,
                     builder,
                 );
                 return;
@@ -260,12 +283,21 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
                     &sub_component.root_element,
                     offset,
                     repeater_count,
+                    container_count,
                     parent_index,
                     builder,
                 );
                 sub_component_states.push_back(sub_component_state);
             } else {
-                visit_item(state, child, offset, repeater_count, parent_index, builder);
+                visit_item(
+                    state,
+                    child,
+                    offset,
+                    repeater_count,
+                    container_count,
+                    parent_index,
+                    builder,
+                );
             }
             offset += item_sub_tree_size(child) as u32;
         }
@@ -289,6 +321,7 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
                     offset,
                     1,
                     repeater_count,
+                    container_count,
                     builder,
                 );
             } else {
@@ -302,6 +335,7 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
                     offset,
                     relative_offset,
                     repeater_count,
+                    container_count,
                     builder,
                 );
             }
@@ -319,11 +353,17 @@ pub fn build_item_tree<T: ItemTreeBuilder>(
         item: &ElementRc,
         children_offset: u32,
         repeater_count: &mut u32,
+        container_count: &mut u32,
         parent_index: u32,
         builder: &mut T,
     ) {
         if item.borrow().is_component_placeholder {
-            builder.push_component_placeholder_item(item, parent_index, component_state);
+            builder.push_component_placeholder_item(
+                item,
+                *container_count,
+                parent_index,
+                component_state,
+            );
         } else if item.borrow().repeated.is_some() {
             builder.push_repeated_item(item, *repeater_count, parent_index, component_state);
             *repeater_count += 1;

internal/compiler/generator/rust.rs

@@ -743,9 +743,10 @@ fn generate_sub_component(
         repeated_element_components.push(rep_inner_component_id);
     }
 
-    for container in component.component_containers.iter() {
+    // Use ids following the real repeaters to piggyback on their forwarding through sub-components!
+    for (idx, container) in component.component_containers.iter().enumerate() {
+        let idx = (component.repeated.len() + idx) as u32;
         let items_index = container.component_container_items_index;
-        let repeater_index = container.component_container_item_tree_index.as_repeater_index();
 
         let embed_item = access_member(
             &llr::PropertyReference::InNativeItem {
@@ -763,19 +764,19 @@ fn generate_sub_component(
         };
 
         repeated_visit_branch.push(quote!(
-            #repeater_index => {
+            #idx => {
                 #ensure_updated
                 #embed_item.visit_children_item(-1, order, visitor)
             }
         ));
         repeated_subtree_ranges.push(quote!(
-            #repeater_index => {
+            #idx => {
                 #ensure_updated
-               #embed_item.subtree_range()
+                #embed_item.subtree_range()
             }
         ));
         repeated_subtree_components.push(quote!(
-            #repeater_index => {
+            #idx => {
                 #ensure_updated
                 if subtree_index == 0 {
                     *result = #embed_item.subtree_component()
@@ -1350,7 +1351,7 @@ fn generate_item_tree(
     sub_tree.tree.visit_in_array(&mut |node, children_offset, parent_index| {
         let parent_index = parent_index as u32;
         let (path, component) = follow_sub_component_path(&sub_tree.root, &node.sub_component_path);
-        if node.repeated {
+        if node.repeated || node.component_container {
             assert_eq!(node.children.len(), 0);
             let mut repeater_index = node.item_index;
             let mut sub_component = &sub_tree.root;

internal/compiler/llr/item_tree.rs

@@ -142,47 +142,10 @@ pub struct RepeatedElement {
     pub listview: Option<ListViewInfo>,
 }
 
-#[derive(Clone, Debug)]
-pub struct ComponentContainerIndex(u32);
-
-impl From<u32> for ComponentContainerIndex {
-    fn from(value: u32) -> Self {
-        assert!(value < ComponentContainerIndex::MAGIC);
-        ComponentContainerIndex(value + ComponentContainerIndex::MAGIC)
-    }
-}
-
-impl ComponentContainerIndex {
-    // Choose a MAGIC value that is big enough so we can have lots of repeaters
-    // (repeater_index must be < MAGIC), but small enough to leave room for
-    // lots of embeddings (which will use item_index + MAGIC as its
-    // repeater_index).
-    // Also pick a MAGIC that works on 32bit as well as 64bit systems.
-    const MAGIC: u32 = (u32::MAX / 2) + 1;
-
-    pub fn as_item_tree_index(&self) -> u32 {
-        assert!(self.0 >= ComponentContainerIndex::MAGIC);
-        self.0 - ComponentContainerIndex::MAGIC
-    }
-
-    pub fn as_repeater_index(&self) -> u32 {
-        assert!(self.0 >= ComponentContainerIndex::MAGIC);
-        self.0
-    }
-
-    pub fn try_from_repeater_index(index: u32) -> Option<Self> {
-        if index >= ComponentContainerIndex::MAGIC {
-            Some(ComponentContainerIndex(index))
-        } else {
-            None
-        }
-    }
-}
-
 #[derive(Debug)]
 pub struct ComponentContainerElement {
-    /// The item tree index of the `ComponentContainer` item node, controlling this Placeholder
+    /// The index of the `ComponentContainer` in the enclosing components `item_tree` array
-    pub component_container_item_tree_index: ComponentContainerIndex,
+    pub component_container_item_tree_index: u32,
     /// The index of the `ComponentContainer` item in the enclosing components `items` array
     pub component_container_items_index: u32,
     /// The index to a dynamic tree node where the component is supposed to be embedded at
@@ -209,9 +172,10 @@ impl std::fmt::Debug for Item {
 #[derive(Debug)]
 pub struct TreeNode {
     pub sub_component_path: Vec<usize>,
-    /// Either an index in the items or repeater, depending on repeated
+    /// Either an index in the items or repeater, depending on (repeated || component_container)
     pub item_index: u32,
     pub repeated: bool,
+    pub component_container: bool,
     pub children: Vec<TreeNode>,
     pub is_accessible: bool,
 }
@@ -301,7 +265,7 @@ pub struct PropAnalysis {
 impl SubComponent {
     /// total count of repeater, including in sub components
     pub fn repeater_count(&self) -> u32 {
-        let mut count = self.repeated.len() as u32;
+        let mut count = (self.repeated.len() + self.component_containers.len()) as u32;
         for x in self.sub_components.iter() {
             count += x.ty.repeater_count();
         }

internal/compiler/llr/lower_to_item_tree.rs

@@ -62,13 +62,15 @@ pub enum LoweredElement {
     SubComponent { sub_component_index: usize },
     NativeItem { item_index: u32 },
     Repeated { repeated_index: u32 },
-    ComponentPlaceholder { component_container_index: ComponentContainerIndex },
+    ComponentPlaceholder { repeated_index: u32 },
 }
 
 #[derive(Default, Debug, Clone)]
 pub struct LoweredSubComponentMapping {
     pub element_mapping: HashMap<ByAddress<ElementRc>, LoweredElement>,
     pub property_mapping: HashMap<NamedReference, PropertyReference>,
+    pub repeater_count: u32,
+    pub container_count: u32,
 }
 
 impl LoweredSubComponentMapping {
@@ -259,11 +261,13 @@ fn lower_sub_component(
             });
         }
         if elem.is_component_placeholder {
-            let index = parent.as_ref().unwrap().borrow().item_index.get().copied().unwrap();
             mapping.element_mapping.insert(
                 element.clone().into(),
-                LoweredElement::ComponentPlaceholder { component_container_index: index.into() },
+                LoweredElement::ComponentPlaceholder {
+                    repeated_index: component_container_data.len() as u32,
+                },
             );
+            mapping.container_count += 1;
             component_container_data.push(parent.as_ref().unwrap().clone());
             return None;
         }
@@ -273,6 +277,7 @@ fn lower_sub_component(
                 LoweredElement::Repeated { repeated_index: repeated.len() as u32 },
             );
             repeated.push(element.clone());
+            mapping.repeater_count += 1;
             return None;
         }
         match &elem.base_type {
@@ -391,7 +396,8 @@ fn lower_sub_component(
     sub_component.repeated =
         repeated.into_iter().map(|elem| lower_repeated_component(&elem, &ctx)).collect();
     for s in &mut sub_component.sub_components {
-        s.repeater_offset += sub_component.repeated.len() as u32;
+        s.repeater_offset +=
+            (sub_component.repeated.len() + sub_component.component_containers.len()) as u32;
     }
     sub_component.component_containers = component_container_data
         .into_iter()
@@ -563,10 +569,12 @@ fn lower_component_container(
 ) -> ComponentContainerElement {
     let c = container.borrow();
 
-    let component_container_index: ComponentContainerIndex = (*c.item_index.get().unwrap()).into();
+    let component_container_index = *c.item_index.get().unwrap();
-    let ti = component_container_index.as_item_tree_index();
+    let component_container_items_index = sub_component
-    let component_container_items_index =
+        .items
-        sub_component.items.iter().position(|i| i.index_in_tree == ti).unwrap() as u32;
+        .iter()
+        .position(|i| i.index_in_tree == component_container_index)
+        .unwrap() as u32;
 
     ComponentContainerElement {
         component_container_item_tree_index: component_container_index,
@@ -738,6 +746,7 @@ fn make_tree(
 ) -> TreeNode {
     let e = element.borrow();
     let children = e.children.iter().map(|c| make_tree(state, c, component, sub_component_path));
+    let repeater_count = component.mapping.repeater_count;
     match component.mapping.element_mapping.get(&ByAddress(element.clone())).unwrap() {
         LoweredElement::SubComponent { sub_component_index } => {
             let sub_component = e.sub_component().unwrap();
@@ -762,6 +771,7 @@ fn make_tree(
             item_index: *item_index,
             children: children.collect(),
             repeated: false,
+            component_container: false,
         },
         LoweredElement::Repeated { repeated_index } => TreeNode {
             is_accessible: false,
@@ -769,13 +779,15 @@ fn make_tree(
             item_index: *repeated_index,
             children: vec![],
             repeated: true,
+            component_container: false,
         },
-        LoweredElement::ComponentPlaceholder { component_container_index } => TreeNode {
+        LoweredElement::ComponentPlaceholder { repeated_index } => TreeNode {
             is_accessible: false,
             sub_component_path: sub_component_path.into(),
-            item_index: component_container_index.as_repeater_index(),
+            item_index: *repeated_index + repeater_count,
             children: vec![],
-            repeated: true,
+            repeated: false,
+            component_container: true,
         },
     }
 }

internal/compiler/passes/generate_item_indices.rs

@@ -69,6 +69,7 @@ impl crate::generator::ItemTreeBuilder for Helper {
     fn push_component_placeholder_item(
         &mut self,
         item: &crate::object_tree::ElementRc,
+        _container_count: u32,
         _parent_index: u32,
         component_state: &Self::SubComponentState,
     ) {

internal/interpreter/dynamic_item_tree.rs

@@ -9,7 +9,6 @@ use dynamic_type::{Instance, InstanceBox};
 use i_slint_compiler::diagnostics::SourceFileVersion;
 use i_slint_compiler::expression_tree::{Expression, NamedReference};
 use i_slint_compiler::langtype::{ElementType, Type};
-use i_slint_compiler::llr::ComponentContainerIndex;
 use i_slint_compiler::object_tree::ElementRc;
 use i_slint_compiler::*;
 use i_slint_compiler::{diagnostics::BuildDiagnostics, object_tree::PropertyDeclaration};
@@ -673,8 +672,8 @@ extern "C" fn visit_children_item(
         order,
         v,
         |_, order, visitor, index| {
-            if let Some(_) = ComponentContainerIndex::try_from_repeater_index(index) {
+            if index as usize >= instance_ref.description.repeater.len() {
-                // Do nothing: Our parent already did all the work!
+                // Do nothing: We are ComponentContainer and Our parent already did all the work!
                 VisitChildrenResult::CONTINUE
             } else {
                 // `ensure_updated` needs a 'static lifetime so we must call get_untagged.
@@ -895,14 +894,12 @@ pub(crate) fn generate_item_tree<'id>(
         fn push_component_placeholder_item(
             &mut self,
             item: &i_slint_compiler::object_tree::ElementRc,
+            container_count: u32,
             parent_index: u32,
             _component_state: &Self::SubComponentState,
         ) {
-            let component_index = ComponentContainerIndex::from(parent_index);
+            self.tree_array
-            self.tree_array.push(ItemTreeNode::DynamicTree {
+                .push(ItemTreeNode::DynamicTree { index: container_count, parent_index });
-                index: component_index.as_repeater_index(),
-                parent_index,
-            });
             self.original_elements.push(item.clone());
         }
 
@@ -1647,8 +1644,16 @@ unsafe extern "C" fn get_item_ref(component: ItemTreeRefPin, index: u32) -> Pin<
 extern "C" fn get_subtree_range(component: ItemTreeRefPin, index: u32) -> IndexRange {
     generativity::make_guard!(guard);
     let instance_ref = unsafe { InstanceRef::from_pin_ref(component, guard) };
-    if let Some(container_index) = ComponentContainerIndex::try_from_repeater_index(index) {
+    if index as usize >= instance_ref.description.repeater.len() {
-        let container = component.as_ref().get_item_ref(container_index.as_item_tree_index());
+        let container_index = {
+            let tree_node = &component.as_ref().get_item_tree()[index as usize];
+            if let ItemTreeNode::DynamicTree { parent_index, .. } = tree_node {
+                *parent_index
+            } else {
+                u32::MAX
+            }
+        };
+        let container = component.as_ref().get_item_ref(container_index);
         let container = i_slint_core::items::ItemRef::downcast_pin::<
             i_slint_core::items::ComponentContainer,
         >(container)
@@ -1673,8 +1678,16 @@ extern "C" fn get_subtree(
 ) {
     generativity::make_guard!(guard);
     let instance_ref = unsafe { InstanceRef::from_pin_ref(component, guard) };
-    if let Some(container_index) = ComponentContainerIndex::try_from_repeater_index(index) {
+    if index as usize >= instance_ref.description.repeater.len() {
-        let container = component.as_ref().get_item_ref(container_index.as_item_tree_index());
+        let container_index = {
+            let tree_node = &component.as_ref().get_item_tree()[index as usize];
+            if let ItemTreeNode::DynamicTree { parent_index, .. } = tree_node {
+                *parent_index
+            } else {
+                u32::MAX
+            }
+        };
+        let container = component.as_ref().get_item_ref(container_index);
         let container = i_slint_core::items::ItemRef::downcast_pin::<
             i_slint_core::items::ComponentContainer,
         >(container)

tests/cases/elements/component_container_component.slint

@@ -0,0 +1,81 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial
+
+// FIXME: Skip embedding test on C++ and NodeJS since ComponentFactory is not
+// implemented there!
+//ignore: cpp,js
+
+import { Button } from "std-widgets.slint";
+
+component Container {
+    in property<component-factory> c1 <=> cont1.component-factory;
+
+    cont1 := ComponentContainer { }
+}
+
+export component TestCase inherits Rectangle {
+
+    in property<component-factory> c1;
+    out property<bool> outside-focus <=> outside.has-focus;
+
+    outside := Button { text: "Outside button"; }
+
+    Container {
+        c1 <=> root.c1;
+    }
+}
+
+/*
+```cpp
+// ComponentFactory not supported yet!
+```
+
+```rust
+let factory = slint::ComponentFactory::new(|ctx| {
+
+    let mut compiler = slint_interpreter::ComponentCompiler::new();
+    let e = spin_on::spin_on(compiler.build_from_source(
+        r#"import { Button } from "std-widgets.slint";
+
+export component E1 inherits Rectangle {
+    background: Colors.red;
+    forward-focus: b;
+    b := Button {
+        text: "red";
+    }
+}"#.into(),
+        std::path::PathBuf::from("embedded.slint"),
+     )).unwrap();
+     e.create_embedded(ctx).ok()
+});
+
+let instance = TestCase::new().unwrap();
+assert!(!instance.get_outside_focus()); // Nothing has focus be default
+slint_testing::send_keyboard_string_sequence(&instance, "\t");
+assert!(instance.get_outside_focus()); // The outside button is the only thing
+                                       // accepting focus at this point.
+
+instance.set_c1(factory);
+
+assert!(instance.get_outside_focus()); // embedding does not move the focus
+
+// focus the two embedded buttons:
+slint_testing::send_keyboard_string_sequence(&instance, "\t");
+assert!(!instance.get_outside_focus());
+
+// Go back to outside button
+slint_testing::send_keyboard_string_sequence(&instance, "\t");
+assert!(instance.get_outside_focus());
+
+// Focus backwards over the embedded buttons
+slint_testing::send_keyboard_string_sequence(&instance, "\u{0019}");
+assert!(!instance.get_outside_focus());
+
+slint_testing::send_keyboard_string_sequence(&instance, "\u{0019}");
+assert!(instance.get_outside_focus());
+```
+
+```js
+var _instance = new slint.TestCase();
+```
+*/