Fix a bunch more issue with PopupWindow

 * Make sure that the compiler don't panic if the parent of a PopupWindow
   is optimized (by not optiizing such element)

 * Ensure that we can call popup.show() from within a deeper repeater

 * Ensure that the parent element of the popup is the right one in case of
   repeater (and not the node in the parent component)

This partially revert ad5991f and
6c7a7ae because we must do the lower_popup
adter the repeater pass, because otherwise the parent element of the
created component for the PopupWindow might be wrong and it is not easy to
adjust (we would have to make Component::parent_element a RefCell or duplicate
it again.

Fixes #1132

internal/compiler/generator/cpp.rs

@@ -2320,16 +2320,26 @@ fn compile_builtin_function_call(
             if let [llr::Expression::NumberLiteral(popup_index), x, y, llr::Expression::PropertyReference(parent_ref)] =
                 arguments
             {
+                let mut parent_ctx = ctx;
+                let mut component_access = "self".into();
+
+                if let llr::PropertyReference::InParent { level, .. } = parent_ref {
+                    for _ in 0..level.get() {
+                        component_access = format!("{}->parent", component_access);
+                        parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
+                    }
+                };
+
                 let window = access_window_field(ctx);
-                let current_sub_component = ctx.current_sub_component.unwrap();
+                let current_sub_component = parent_ctx.current_sub_component.unwrap();
                 let popup_window_id =
                     ident(&current_sub_component.popup_windows[*popup_index as usize].root.name);
                 let parent_component = access_item_rc(parent_ref, ctx);
                 let x = compile_expression(x, ctx);
                 let y = compile_expression(y, ctx);
                 format!(
-                    "{}.show_popup<{}>(self, {{ {}, {} }}, {{ {} }})",
-                    window, popup_window_id, x, y, parent_component,
+                    "{}.show_popup<{}>({}, {{ {}, {} }}, {{ {} }})",
+                    window, popup_window_id, component_access, x, y, parent_component,
                 )
             } else {
                 panic!("internal error: invalid args to ShowPopupWindow {:?}", arguments)

internal/compiler/generator/rust.rs

@@ -1886,7 +1886,16 @@ fn compile_builtin_function_call(
             if let [Expression::NumberLiteral(popup_index), x, y, Expression::PropertyReference(parent_ref)] =
                 arguments
             {
-                let current_sub_component = ctx.current_sub_component.unwrap();
+                let mut parent_ctx = ctx;
+                let mut component_access_tokens = quote!(_self);
+                if let llr::PropertyReference::InParent { level, .. } = parent_ref {
+                    for _ in 0..level.get() {
+                        component_access_tokens =
+                            quote!(#component_access_tokens.parent.upgrade().unwrap().as_pin_ref());
+                        parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
+                    }
+                }
+                let current_sub_component = parent_ctx.current_sub_component.unwrap();
                 let popup_window_id = inner_component_id(
                     &current_sub_component.popup_windows[*popup_index as usize].root,
                 );
@@ -1896,7 +1905,7 @@ fn compile_builtin_function_call(
                 let window_tokens = access_window_field(ctx);
                 quote!(
                     #window_tokens.show_popup(
-                        &VRc::into_dyn(#popup_window_id::new(_self.self_weak.get().unwrap().clone()).into()),
+                        &VRc::into_dyn(#popup_window_id::new(#component_access_tokens.self_weak.get().unwrap().clone()).into()),
                         Point::new(#x as f32, #y as f32),
                         #parent_component
                     );

internal/compiler/object_tree.rs

@@ -458,6 +458,10 @@ pub struct Element {
     /// true if this Element is the fake Flickable viewport
     pub is_flickable_viewport: bool,
 
+    /// true if this Element may have a popup as child meaning it cannot be optimized
+    /// because the popup references it.
+    pub has_popup_child: bool,
+
     /// This is the component-local index of this item in the item tree array.
     /// It is generated after the last pass and before the generators run.
     pub item_index: once_cell::unsync::OnceCell<usize>,

internal/compiler/passes.rs

@@ -116,8 +116,8 @@ pub async fn run_passes(
         .chain(std::iter::once(root_component))
     {
         flickable::handle_flickable(component, &global_type_registry.borrow());
-        lower_popups::lower_popups(component, &doc.local_registry, diag);
         repeater_component::process_repeater_components(component);
+        lower_popups::lower_popups(component, &doc.local_registry, diag);
         lower_layout::lower_layouts(component, type_loader, diag).await;
         default_geometry::default_geometry(component, diag);
         z_order::reorder_by_z_order(component, diag);

internal/compiler/passes/ensure_window.rs

@@ -41,6 +41,7 @@ pub fn ensure_window(
         states: Default::default(),
         transitions: Default::default(),
         child_of_layout: false,
+        has_popup_child: false,
         layout_info_prop: Default::default(),
         is_flickable_viewport: false,
         item_index: Default::default(),

internal/compiler/passes/inlining.rs

@@ -218,6 +218,7 @@ fn duplicate_element_with_mapping(
         item_index: Default::default(), // Not determined yet
         item_index_of_first_children: Default::default(),
         is_flickable_viewport: elem.is_flickable_viewport,
+        has_popup_child: elem.has_popup_child,
         inline_depth: elem.inline_depth + 1,
     }));
     mapping.insert(element_key(element.clone()), new.clone());

internal/compiler/passes/lower_popups.rs

@@ -37,14 +37,6 @@ fn lower_popup_window(
     window_type: &Type,
     diag: &mut BuildDiagnostics,
 ) {
-    if popup_window_element.borrow().repeated.is_some() {
-        diag.push_error(
-            "PopupWindow cannot be directly repeated or conditional".into(),
-            &*popup_window_element.borrow(),
-        );
-        return;
-    }
-
     let parent_element = match parent_element {
         None => {
             diag.push_error(
@@ -57,6 +49,13 @@ fn lower_popup_window(
     };
 
     let parent_component = popup_window_element.borrow().enclosing_component.upgrade().unwrap();
+    if Rc::ptr_eq(&parent_component.root_element, popup_window_element) {
+        diag.push_error(
+            "PopupWindow cannot be directly repeated or conditional".into(),
+            &*popup_window_element.borrow(),
+        );
+        return;
+    }
 
     // Remove the popup_window_element from its parent
     let old_size = parent_element.borrow().children.len();
@@ -66,6 +65,7 @@ fn lower_popup_window(
         old_size,
         "Exactly one child must be removed (the popup itself)"
     );
+    parent_element.borrow_mut().has_popup_child = true;
 
     popup_window_element.borrow_mut().base_type = window_type.clone();
 

internal/compiler/passes/optimize_useless_rectangles.rs

@@ -36,7 +36,7 @@ pub fn optimize_useless_rectangles(root_component: &Rc<Component>) {
 /// Check that this is a element we can optimize
 fn can_optimize(elem: &ElementRc) -> bool {
     let e = elem.borrow();
-    if e.is_flickable_viewport {
+    if e.is_flickable_viewport || e.has_popup_child {
         return false;
     };
 

internal/compiler/passes/repeater_component.rs

@@ -14,7 +14,6 @@ use std::rc::Rc;
 pub fn process_repeater_components(component: &Rc<Component>) {
     create_repeater_components(component);
     adjust_references(component);
-    adjust_popups(component);
 }
 
 fn create_repeater_components(component: &Rc<Component>) {
@@ -43,6 +42,7 @@ fn create_repeater_components(component: &Rc<Component>) {
                 child_of_layout: elem.child_of_layout || is_listview.is_some(),
                 layout_info_prop: elem.layout_info_prop.take(),
                 is_flickable_viewport: elem.is_flickable_viewport,
+                has_popup_child: elem.has_popup_child,
                 item_index: Default::default(), // Not determined yet
                 item_index_of_first_children: Default::default(),
                 inline_depth: 0,
@@ -114,22 +114,3 @@ fn adjust_references(comp: &Rc<Component>) {
         })
     });
 }
-
-fn adjust_popups(component: &Rc<Component>) {
-    component.popup_windows.borrow_mut().retain(|popup| {
-        let parent = popup
-            .component
-            .parent_element
-            .upgrade()
-            .unwrap()
-            .borrow()
-            .enclosing_component
-            .upgrade()
-            .unwrap();
-        if Rc::ptr_eq(&parent, component) {
-            return true;
-        }
-        parent.popup_windows.borrow_mut().push(popup.clone());
-        false
-    });
-}

tests/cases/crashes/issue1113_popup_in_repeater.slint

@@ -12,5 +12,26 @@ App := Window {
             }
        }
     }
+
+    if true : Rectangle {
+        Rectangle {
+            TouchArea {
+                clicked => {
+                    popup2.show();
+                }
+            }
+            popup2 := PopupWindow { }
+        }
+    }
+
+    // Issue #1132
+    for menu-item in [0] : TouchArea {
+        clicked => { popup3.show(); }
+        if true: TouchArea {
+            clicked => { popup3.show(); }
+        }
+        popup3 := PopupWindow { }
+    }
+
 }