Fix PopupWindow position when all elements are not inlined

Pass a reference to the parent item in the show_popup function
so we can compute the exact location at runtime.

api/sixtyfps-cpp/include/sixtyfps.h

@@ -139,10 +139,11 @@ public:
     }
 
     template<typename Component, typename Parent>
-    void show_popup(const Parent *parent_component, cbindgen_private::Point p) const
+    void show_popup(const Parent *parent_component, cbindgen_private::Point p,
+                    cbindgen_private::ItemRc parent_item) const
     {
         auto popup = Component::create(parent_component).into_dyn();
-        cbindgen_private::sixtyfps_windowrc_show_popup(&inner, &popup, p);
+        cbindgen_private::sixtyfps_windowrc_show_popup(&inner, &popup, p, &parent_item);
     }
 
 private:

sixtyfps_compiler/generator/cpp.rs

@@ -1753,7 +1753,12 @@ fn compile_expression(
                     let popup = popup_list.iter().find(|p| Rc::ptr_eq(&p.component, &pop_comp)).unwrap();
                     let x = access_named_reference(&popup.x, component, "self");
                     let y = access_named_reference(&popup.y, component, "self");
-                    format!("self->m_window.window_handle().show_popup<{}>(self, {{ {}.get(), {}.get() }} );", popup_window_rcid, x, y)
+                    let parent_component_ref = access_element_component(&popup.parent_element, component, "self");
+                    format!(
+                        "self->m_window.window_handle().show_popup<{}>(self, {{ {}.get(), {}.get() }}, {{ {}->self_weak.lock()->into_dyn(), {} }} );",
+                        popup_window_rcid, x, y,
+                        parent_component_ref, popup.parent_element.borrow().item_index.get().unwrap(),
+                    )
                 } else {
                     panic!("internal error: argument to SetFocusItem must be an element")
                 }

sixtyfps_compiler/generator/rust.rs

@@ -1342,10 +1342,13 @@ fn compile_expression(expr: &Expression, component: &Rc<Component>) -> TokenStre
                         let popup = popup_list.iter().find(|p| Rc::ptr_eq(&p.component, &pop_comp)).unwrap();
                         let x = access_named_reference(&popup.x, component, quote!(_self));
                         let y = access_named_reference(&popup.y, component, quote!(_self));
+                        let parent_component_ref = access_element_component(&popup.parent_element, component, quote!(_self));
+                        let parent_index = *popup.parent_element.borrow().item_index.get().unwrap();
                         quote!(
                             _self.window.window_handle().show_popup(
                                 &VRc::into_dyn(#popup_window_id::new(_self.self_weak.get().unwrap().clone(), &_self.window.window_handle()).into()),
-                                Point::new(#x.get(), #y.get())
+                                Point::new(#x.get(), #y.get()),
+                                &ItemRc::new(VRc::into_dyn(#parent_component_ref.self_weak.get().unwrap().upgrade().unwrap()), #parent_index)
                             );
                         )
                     } else {

sixtyfps_compiler/object_tree.rs

@@ -171,6 +171,7 @@ pub struct PopupWindow {
     pub component: Rc<Component>,
     pub x: NamedReference,
     pub y: NamedReference,
+    pub parent_element: ElementRc,
 }
 
 type ChildrenInsertionPoint = (ElementRc, syntax_nodes::ChildrenPlaceholder);

sixtyfps_compiler/passes/inlining.rs

@@ -298,6 +298,10 @@ fn duplicate_popup(
         x: p.x.clone(),
         y: p.y.clone(),
         component: duplicate_sub_component(&p.component, &parent, mapping),
+        parent_element: mapping
+            .get(&element_key(p.parent_element.clone()))
+            .expect("Parent element must be in the mapping")
+            .clone(),
     }
 }
 

sixtyfps_compiler/passes/lower_popups.rs

@@ -25,27 +25,24 @@ pub fn lower_popups(
 
     recurse_elem_including_sub_components_no_borrow(
         component,
-        &Vec::new(),
-        &mut |elem, parent_stack: &Vec<ElementRc>| {
+        &None,
+        &mut |elem, parent_element: &Option<ElementRc>| {
             let is_popup = elem.borrow().base_type.to_string() == "PopupWindow";
             if is_popup {
-                lower_popup_window(elem, parent_stack, &window_type, diag);
+                lower_popup_window(elem, parent_element.as_ref(), &window_type, diag);
             }
-            // this could be implemented in a better way with less cloning of the state
-            let mut parent_stack = parent_stack.clone();
-            parent_stack.push(elem.clone());
-            parent_stack
+            Some(elem.clone())
         },
     )
 }
 
 fn lower_popup_window(
     popup_window_element: &ElementRc,
-    parent_stack: &[ElementRc],
+    parent_element: Option<&ElementRc>,
     window_type: &Type,
     diag: &mut BuildDiagnostics,
 ) {
-    let parent_element = match parent_stack.last() {
+    let parent_element = match parent_element {
         None => {
             diag.push_error(
                 "PopupWindow cannot be the top level".into(),
@@ -76,8 +73,8 @@ fn lower_popup_window(
 
     // Generate a x and y property, relative to the window coordinate
     // FIXME: this is a hack that doesn't always work, perhaps should we store an item ref or something
-    let coord_x = create_coordinate(&popup_comp, parent_stack, "x");
-    let coord_y = create_coordinate(&popup_comp, parent_stack, "y");
+    let coord_x = create_coordinate(&popup_comp, parent_element, "x");
+    let coord_y = create_coordinate(&popup_comp, parent_element, "y");
 
     // Throw error when accessing the popup from outside
     // FIXME:
@@ -99,30 +96,22 @@ fn lower_popup_window(
         component: popup_comp,
         x: coord_x,
         y: coord_y,
+        parent_element: parent_element.clone(),
     });
 }
 
 fn create_coordinate(
     popup_comp: &Rc<Component>,
-    parent_stack: &[ElementRc],
+    parent_element: &ElementRc,
     coord: &str,
 ) -> NamedReference {
-    let mut expression = popup_comp
+    let expression = popup_comp
         .root_element
         .borrow()
         .bindings
         .get(coord)
         .map(|e| e.expression.clone())
         .unwrap_or(Expression::NumberLiteral(0., crate::expression_tree::Unit::Phx));
-
-    for parent in parent_stack {
-        expression = Expression::BinaryExpression {
-            lhs: Box::new(expression),
-            rhs: Box::new(Expression::PropertyReference(NamedReference::new(parent, coord))),
-            op: '+',
-        };
-    }
-    let parent_element = parent_stack.last().unwrap();
     let property_name = format!("{}-popup-{}", popup_comp.root_element.borrow().id, coord);
     parent_element
         .borrow_mut()

sixtyfps_runtime/corelib/window.rs

@@ -472,6 +472,19 @@ impl Window {
         size
     }
 
+    /// Show a popup at the given position relative to the item
+    pub fn show_popup(&self, popup: &ComponentRc, mut position: Point, parent_item: &ItemRc) {
+        let mut parent_item = parent_item.clone();
+        loop {
+            position += parent_item.borrow().as_ref().geometry().origin.to_vector();
+            parent_item = match parent_item.parent_item().upgrade() {
+                None => break,
+                Some(pi) => pi,
+            }
+        }
+        self.platform_window.get().unwrap().show_popup(popup, position)
+    }
+
     /// Removes any active popup.
     pub fn close_popup(&self) {
         if let Some(current_popup) = self.active_popup.replace(None) {
@@ -671,9 +684,10 @@ pub mod ffi {
         handle: *const WindowRcOpaque,
         popup: &ComponentRc,
         position: crate::graphics::Point,
+        parent_item: &ItemRc,
     ) {
         let window = &*(handle as *const WindowRc);
-        window.show_popup(popup, position);
+        window.show_popup(popup, position, parent_item);
     }
     /// Close the current popup
     pub unsafe extern "C" fn sixtyfps_windowrc_close_popup(handle: *const WindowRcOpaque) {

sixtyfps_runtime/interpreter/dynamic_component.rs

@@ -1592,16 +1592,15 @@ impl<'a, 'id> InstanceRef<'a, 'id> {
 /// Show the popup at the given location
 pub fn show_popup(
     popup: &object_tree::PopupWindow,
-    x: f32,
-    y: f32,
+    pos: sixtyfps_corelib::graphics::Point,
     parent_comp: ComponentRefPin,
     parent_window: &WindowRc,
+    parent_item: &ItemRc,
 ) {
     generativity::make_guard!(guard);
     // FIXME: we should compile once and keep the cached compiled component
     let compiled = generate_component(&popup.component, guard);
     let inst = instantiate(compiled, Some(parent_comp), Some(parent_window));
     inst.run_setup_code();
-    parent_window
-        .show_popup(&vtable::VRc::into_dyn(inst), sixtyfps_corelib::graphics::Point::new(x, y));
+    parent_window.show_popup(&vtable::VRc::into_dyn(inst), pos, parent_item);
 }

sixtyfps_runtime/interpreter/eval.rs

@@ -305,7 +305,20 @@ pub fn eval_expression(expression: &Expression, local_context: &mut EvalLocalCon
                     let popup = popup_list.iter().find(|p| Rc::ptr_eq(&p.component, &pop_comp)).unwrap();
                     let x = load_property_helper(local_context.component_instance, &popup.x.element(), popup.x.name()).unwrap();
                     let y = load_property_helper(local_context.component_instance, &popup.y.element(), popup.y.name()).unwrap();
-                    crate::dynamic_component::show_popup(popup, x.try_into().unwrap(), y.try_into().unwrap(), component.borrow(), window_ref(component).unwrap());
+
+                    generativity::make_guard!(guard);
+                    let enclosing_component =
+                        enclosing_component_for_element(&popup.parent_element, component, guard);
+                    let parent_item_info = &enclosing_component.component_type.items[popup.parent_element.borrow().id.as_str()];
+                    let parent_item_comp = enclosing_component.self_weak().get().unwrap().upgrade().unwrap();
+                    let parent_item = corelib::items::ItemRc::new(vtable::VRc::into_dyn(parent_item_comp), parent_item_info.item_index());
+
+                    crate::dynamic_component::show_popup(
+                        popup,
+                        sixtyfps_corelib::graphics::Point::new(x.try_into().unwrap(), y.try_into().unwrap()),
+                        component.borrow(),
+                        window_ref(component).unwrap(),
+                        &parent_item);
                     Value::Void
                 } else {
                     panic!("internal error: argument to SetFocusItem must be an element")