diff --git a/internal/compiler/generator/cpp.rs b/internal/compiler/generator/cpp.rs index 8d920a94476..1e2180f76f9 100644 --- a/internal/compiler/generator/cpp.rs +++ b/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(¤t_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) diff --git a/internal/compiler/generator/rust.rs b/internal/compiler/generator/rust.rs index 08a3a15c80c..f1623f589a7 100644 --- a/internal/compiler/generator/rust.rs +++ b/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( ¤t_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 ); diff --git a/internal/compiler/object_tree.rs b/internal/compiler/object_tree.rs index f227f09c6b3..0ca0fad8f8c 100644 --- a/internal/compiler/object_tree.rs +++ b/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, diff --git a/internal/compiler/passes.rs b/internal/compiler/passes.rs index 08168beda4a..3d7fa389bf0 100644 --- a/internal/compiler/passes.rs +++ b/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); diff --git a/internal/compiler/passes/ensure_window.rs b/internal/compiler/passes/ensure_window.rs index 5608c09aba7..f5cf6e4718f 100644 --- a/internal/compiler/passes/ensure_window.rs +++ b/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(), diff --git a/internal/compiler/passes/inlining.rs b/internal/compiler/passes/inlining.rs index a9f0dcc5181..02d0dc817a9 100644 --- a/internal/compiler/passes/inlining.rs +++ b/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()); diff --git a/internal/compiler/passes/lower_popups.rs b/internal/compiler/passes/lower_popups.rs index 5a402dcca9e..14c357945ca 100644 --- a/internal/compiler/passes/lower_popups.rs +++ b/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(); diff --git a/internal/compiler/passes/optimize_useless_rectangles.rs b/internal/compiler/passes/optimize_useless_rectangles.rs index 5544de44714..09241d77f64 100644 --- a/internal/compiler/passes/optimize_useless_rectangles.rs +++ b/internal/compiler/passes/optimize_useless_rectangles.rs @@ -36,7 +36,7 @@ pub fn optimize_useless_rectangles(root_component: &Rc) { /// 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; }; diff --git a/internal/compiler/passes/repeater_component.rs b/internal/compiler/passes/repeater_component.rs index db3ac99c15c..331c9ac31b8 100644 --- a/internal/compiler/passes/repeater_component.rs +++ b/internal/compiler/passes/repeater_component.rs @@ -14,7 +14,6 @@ use std::rc::Rc; pub fn process_repeater_components(component: &Rc) { create_repeater_components(component); adjust_references(component); - adjust_popups(component); } fn create_repeater_components(component: &Rc) { @@ -43,6 +42,7 @@ fn create_repeater_components(component: &Rc) { 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) { }) }); } - -fn adjust_popups(component: &Rc) { - 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 - }); -} diff --git a/tests/cases/crashes/issue1113_popup_in_repeater.slint b/tests/cases/crashes/issue1113_popup_in_repeater.slint index ee19bf7ff90..9535deeca45 100644 --- a/tests/cases/crashes/issue1113_popup_in_repeater.slint +++ b/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 { } + } + }