Add support for using own properties in PopupWindow's x and y properties

This allows positioning popup windows in a way that takes their width/height into account.
task-jp · Jul 10, 2024 · 419042f · 419042f
1 parent ec87f55
commit 419042f
Show file tree

Hide file tree

Showing 12 changed files with 134 additions and 82 deletions.
diff --git a/api/cpp/include/slint_window.h b/api/cpp/include/slint_window.h
@@ -105,12 +105,14 @@ class WindowAdapterRc
         slint_windowrc_set_component(&inner, &item_tree_rc);
     }
 
-    template<typename Component, typename Parent>
-    void show_popup(const Parent *parent_component, cbindgen_private::Point p, bool close_on_click,
-                    cbindgen_private::ItemRc parent_item) const
-    {
-        auto popup = Component::create(parent_component).into_dyn();
-        cbindgen_private::slint_windowrc_show_popup(&inner, &popup, p, close_on_click,
+    template<typename Component, typename Parent, typename XGetter, typename YGetter>
+    void show_popup(const Parent *parent_component, XGetter x_getter, YGetter y_getter,
+                    bool close_on_click, cbindgen_private::ItemRc parent_item) const
+    {
+        auto popup = Component::create(parent_component);
+        cbindgen_private::Point p { x_getter(popup), y_getter(popup) };
+        auto popup_dyn = popup.into_dyn();
+        cbindgen_private::slint_windowrc_show_popup(&inner, &popup_dyn, p, close_on_click,
                                                     &parent_item);
     }
 

diff --git a/internal/compiler/generator/cpp.rs b/internal/compiler/generator/cpp.rs
@@ -1082,7 +1082,7 @@ fn generate_public_component(
             add_friends(friends, &repeater.sub_tree.root, false)
         }
         for popup in &sc.popup_windows {
-            add_friends(friends, &popup.root, false)
+            add_friends(friends, &popup.item_tree.root, false)
         }
     }
 
@@ -1650,12 +1650,12 @@ fn generate_sub_component(
         parent_ctx,
     );
 
-    component.popup_windows.iter().for_each(|c| {
-        let component_id = ident(&c.root.name);
+    component.popup_windows.iter().for_each(|popup| {
+        let component_id = ident(&popup.item_tree.root.name);
         let mut popup_struct = Struct { name: component_id.clone(), ..Default::default() };
         generate_item_tree(
             &mut popup_struct,
-            c,
+            &popup.item_tree,
             root,
             Some(ParentCtx::new(&ctx, None)),
             component_id,
@@ -3304,7 +3304,7 @@ fn compile_builtin_function_call(
             format!("{}.text_input_focused()", access_window_field(ctx))
         }
         BuiltinFunction::ShowPopupWindow => {
-            if let [llr::Expression::NumberLiteral(popup_index), x, y, close_on_click, llr::Expression::PropertyReference(parent_ref)] =
+            if let [llr::Expression::NumberLiteral(popup_index), close_on_click, llr::Expression::PropertyReference(parent_ref)] =
                 arguments
             {
                 let mut parent_ctx = ctx;
@@ -3319,14 +3319,24 @@ fn compile_builtin_function_call(
 
                 let window = access_window_field(ctx);
                 let current_sub_component = parent_ctx.current_sub_component.unwrap();
+                let popup = &current_sub_component.popup_windows[*popup_index as usize];
                 let popup_window_id =
-                    ident(&current_sub_component.popup_windows[*popup_index as usize].root.name);
+                    ident(&popup.item_tree.root.name);
                 let parent_component = access_item_rc(parent_ref, ctx);
-                let x = compile_expression(x, ctx);
-                let y = compile_expression(y, ctx);
+
+                let popup_ctx = EvaluationContext::new_sub_component(
+                    ctx.compilation_unit,
+                    &popup.item_tree.root,
+                    CppGeneratorContext { global_access: "self->globals".into(), conditional_includes: ctx.generator_state.conditional_includes },
+                    Some(ParentCtx::new(&ctx, None)),
+                );
+
+                let x = access_member(&popup.x_prop, &popup_ctx);
+                let y = access_member(&popup.y_prop, &popup_ctx);
+
                 let close_on_click = compile_expression(close_on_click, ctx);
                 format!(
-                    "{window}.show_popup<{popup_window_id}>({component_access}, {{ static_cast<float>({x}), static_cast<float>({y}) }}, {close_on_click}, {{ {parent_component} }})"
+                    "{window}.show_popup<{popup_window_id}>({component_access}, [=](auto self) {{ return {x}.get(); }}, [=](auto self) {{ return {y}.get(); }}, {close_on_click}, {{ {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
@@ -641,7 +641,9 @@ fn generate_sub_component(
     let mut extra_components = component
         .popup_windows
         .iter()
-        .map(|c| generate_item_tree(c, root, Some(ParentCtx::new(&ctx, None)), None))
+        .map(|popup| {
+            generate_item_tree(&popup.item_tree, root, Some(ParentCtx::new(&ctx, None)), None)
+        })
         .collect::<Vec<_>>();
 
     let mut declared_property_vars = vec![];
@@ -2450,7 +2452,7 @@ fn compile_builtin_function_call(
             }
         }
         BuiltinFunction::ShowPopupWindow => {
-            if let [Expression::NumberLiteral(popup_index), x, y, close_on_click, Expression::PropertyReference(parent_ref)] =
+            if let [Expression::NumberLiteral(popup_index), close_on_click, Expression::PropertyReference(parent_ref)] =
                 arguments
             {
                 let mut parent_ctx = ctx;
@@ -2463,26 +2465,42 @@ fn compile_builtin_function_call(
                     }
                 }
                 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,
-                );
+                let popup = &current_sub_component.popup_windows[*popup_index as usize];
+                let popup_window_id = inner_component_id(&popup.item_tree.root);
                 let parent_component = access_item_rc(parent_ref, ctx);
-                let x = compile_expression(x, ctx);
-                let y = compile_expression(y, ctx);
+
+                let popup_ctx = EvaluationContext::new_sub_component(
+                    ctx.compilation_unit,
+                    &popup.item_tree.root,
+                    RustGeneratorContext { global_access: quote!(_self.globals.get().unwrap()) },
+                    Some(ParentCtx::new(&ctx, None)),
+                );
+                let x = primitive_property_value(
+                    &Type::LogicalLength,
+                    access_member(&popup.x_prop, &popup_ctx),
+                );
+                let y = primitive_property_value(
+                    &Type::LogicalLength,
+                    access_member(&popup.y_prop, &popup_ctx),
+                );
+
                 let close_on_click = compile_expression(close_on_click, ctx);
                 let window_adapter_tokens = access_window_adapter_field(ctx);
-                quote!(
+                quote!({
+                    let popup_instance = #popup_window_id::new(#component_access_tokens.self_weak.get().unwrap().clone()).unwrap();
+                    let popup_instance_vrc = sp::VRc::map(popup_instance.clone(), |x| x);
+                    #popup_window_id::user_init(popup_instance_vrc.clone());
+                    let x = { let _self = popup_instance_vrc.as_pin_ref(); #x };
+                    let y = { let _self = popup_instance_vrc.as_pin_ref(); #y };
                     sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_popup(
                         &sp::VRc::into_dyn({
-                            let instance = #popup_window_id::new(#component_access_tokens.self_weak.get().unwrap().clone()).unwrap();
-                            #popup_window_id::user_init(sp::VRc::map(instance.clone(), |x| x));
-                            instance.into()
+                            popup_instance.into()
                         }),
-                        sp::Point::new(#x as sp::Coord, #y as sp::Coord),
+                        sp::Point::new(x as sp::Coord, y as sp::Coord),
                         #close_on_click,
                         #parent_component
                     )
-                )
+                })
             } else {
                 panic!("internal error: invalid args to ShowPopupWindow {:?}", arguments)
             }

diff --git a/internal/compiler/llr/item_tree.rs b/internal/compiler/llr/item_tree.rs
@@ -240,7 +240,7 @@ pub struct SubComponent {
     pub items: Vec<Item>,
     pub repeated: Vec<RepeatedElement>,
     pub component_containers: Vec<ComponentContainerElement>,
-    pub popup_windows: Vec<ItemTree>,
+    pub popup_windows: Vec<PopupWindow>,
     pub sub_components: Vec<SubComponentInstance>,
     /// The initial value or binding for properties.
     /// This is ordered in the order they must be set.
@@ -268,6 +268,13 @@ pub struct SubComponent {
     pub prop_analysis: HashMap<PropertyReference, PropAnalysis>,
 }
 
+#[derive(Debug)]
+pub struct PopupWindow {
+    pub item_tree: ItemTree,
+    pub x_prop: PropertyReference,
+    pub y_prop: PropertyReference,
+}
+
 #[derive(Debug, Clone)]
 pub struct PropAnalysis {
     /// Index in SubComponent::property_init for this property
@@ -363,8 +370,13 @@ impl CompilationUnit {
                     Some(ParentCtx::new(&ctx, Some(idx as u32))),
                 );
             }
-            for x in &c.popup_windows {
-                visit_component(root, &x.root, visitor, Some(ParentCtx::new(&ctx, None)));
+            for popup in &c.popup_windows {
+                visit_component(
+                    root,
+                    &popup.item_tree.root,
+                    visitor,
+                    Some(ParentCtx::new(&ctx, None)),
+                );
             }
         }
         for c in &self.sub_components {

diff --git a/internal/compiler/llr/lower_expression.rs b/internal/compiler/llr/lower_expression.rs
@@ -370,8 +370,6 @@ fn lower_show_popup(args: &[tree_Expression], ctx: &ExpressionContext) -> llr_Ex
             .enumerate()
             .find(|(_, p)| Rc::ptr_eq(&p.component, &pop_comp))
             .unwrap();
-        let x = llr_Expression::PropertyReference(ctx.map_property_reference(&popup.x));
-        let y = llr_Expression::PropertyReference(ctx.map_property_reference(&popup.y));
         let item_ref = lower_expression(
             &tree_Expression::ElementReference(Rc::downgrade(&popup.parent_element)),
             ctx,
@@ -380,8 +378,6 @@ fn lower_show_popup(args: &[tree_Expression], ctx: &ExpressionContext) -> llr_Ex
             function: BuiltinFunction::ShowPopupWindow,
             arguments: vec![
                 llr_Expression::NumberLiteral(popup_index as _),
-                x,
-                y,
                 llr_Expression::BoolLiteral(popup.close_on_click),
                 item_ref,
             ],

diff --git a/internal/compiler/llr/lower_to_item_tree.rs b/internal/compiler/llr/lower_to_item_tree.rs
@@ -4,11 +4,12 @@
 use by_address::ByAddress;
 
 use super::lower_expression::ExpressionContext;
+use super::PopupWindow as llr_PopupWindow;
 use crate::expression_tree::Expression as tree_Expression;
 use crate::langtype::{ElementType, Type};
 use crate::llr::item_tree::*;
 use crate::namedreference::NamedReference;
-use crate::object_tree::{Component, ElementRc, PropertyAnalysis, PropertyVisibility};
+use crate::object_tree::{Component, ElementRc, PopupWindow, PropertyAnalysis, PropertyVisibility};
 use crate::CompilerConfiguration;
 use std::collections::{BTreeMap, HashMap};
 use std::rc::Rc;
@@ -443,7 +444,7 @@ fn lower_sub_component(
         .popup_windows
         .borrow()
         .iter()
-        .map(|popup| lower_popup_component(&popup.component, &ctx, &compiler_config))
+        .map(|popup| lower_popup_component(&popup, &ctx, &compiler_config))
         .collect();
 
     crate::generator::for_each_const_properties(component, |elem, n| {
@@ -644,16 +645,17 @@ fn lower_component_container(
 }
 
 fn lower_popup_component(
-    component: &Rc<Component>,
+    popup: &PopupWindow,
     ctx: &ExpressionContext,
     compiler_config: &CompilerConfiguration,
-) -> ItemTree {
-    let sc = lower_sub_component(component, ctx.state, Some(ctx), compiler_config);
-    ItemTree {
-        tree: make_tree(ctx.state, &component.root_element, &sc, &[]),
+) -> llr_PopupWindow {
+    let sc = lower_sub_component(&popup.component, ctx.state, Some(ctx), compiler_config);
+    let item_tree = ItemTree {
+        tree: make_tree(ctx.state, &popup.component.root_element, &sc, &[]),
         root: Rc::try_unwrap(sc.sub_component).unwrap(),
         parent_context: Some(
-            component
+            popup
+                .component
                 .parent_element
                 .upgrade()
                 .unwrap()
@@ -664,6 +666,12 @@ fn lower_popup_component(
                 .id
                 .clone(),
         ),
+    };
+
+    llr_PopupWindow {
+        item_tree,
+        x_prop: sc.mapping.map_property_reference(&popup.x, ctx.state),
+        y_prop: sc.mapping.map_property_reference(&popup.y, ctx.state),
     }
 }
 

diff --git a/internal/compiler/llr/optim_passes/count_property_use.rs b/internal/compiler/llr/optim_passes/count_property_use.rs
@@ -118,6 +118,18 @@ pub fn count_property_use(root: &CompilationUnit) {
             visit_property(p, ctx);
             e.visit_recursive(&mut |e| visit_expression(e, ctx));
         }
+
+        // 10. popup x/y coordinates
+        for popup in &sc.popup_windows {
+            let popup_ctx = EvaluationContext::new_sub_component(
+                root,
+                &popup.item_tree.root,
+                (),
+                Some(ParentCtx::new(&ctx, None)),
+            );
+            visit_property(&popup.x_prop, &popup_ctx);
+            visit_property(&popup.y_prop, &popup_ctx);
+        }
     });
 
     // TODO: only visit used function

diff --git a/internal/compiler/llr/pretty_print.rs b/internal/compiler/llr/pretty_print.rs
@@ -109,7 +109,7 @@ impl<'a> PrettyPrinter<'a> {
         }
         for w in &sc.popup_windows {
             self.indent()?;
-            self.print_component(root, &w.root, Some(ParentCtx::new(&ctx, None)))?
+            self.print_component(root, &w.item_tree.root, Some(ParentCtx::new(&ctx, None)))?
         }
         self.indentation -= 1;
         self.indent()?;

diff --git a/internal/compiler/passes/lower_popups.rs b/internal/compiler/passes/lower_popups.rs
@@ -122,8 +122,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_element, "x");
-    let coord_y = create_coordinate(&popup_comp, parent_element, "y");
+    let coord_x = create_coordinate(&popup_comp.root_element, "x");
+    let coord_y = create_coordinate(&popup_comp.root_element, "y");
 
     // Throw error when accessing the popup from outside
     // FIXME:
@@ -150,26 +150,16 @@ fn lower_popup_window(
     });
 }
 
-fn create_coordinate(
-    popup_comp: &Rc<Component>,
-    parent_element: &ElementRc,
-    coord: &str,
-) -> NamedReference {
-    let expression = popup_comp
-        .root_element
-        .borrow_mut()
+fn create_coordinate(popup_root_element: &ElementRc, coord: &str) -> NamedReference {
+    let mut elem = popup_root_element.borrow_mut();
+    let expression = elem
         .bindings
         .remove(coord)
         .map(|e| e.into_inner().expression)
         .unwrap_or(Expression::NumberLiteral(0., crate::expression_tree::Unit::Phx));
-    let property_name = format!("{}-popup-{}", popup_comp.root_element.borrow().id, coord);
-    parent_element
-        .borrow_mut()
-        .property_declarations
-        .insert(property_name.clone(), Type::LogicalLength.into());
-    parent_element
-        .borrow_mut()
-        .bindings
-        .insert(property_name.clone(), RefCell::new(expression.into()));
-    NamedReference::new(parent_element, &property_name)
+    let property_name = format!("{}-popup-{}", elem.id, coord);
+    elem.property_declarations.insert(property_name.clone(), Type::LogicalLength.into());
+    elem.bindings.insert(property_name.clone(), RefCell::new(expression.into()));
+    drop(elem);
+    NamedReference::new(popup_root_element, &property_name)
 }
diff --git a/internal/interpreter/dynamic_item_tree.rs b/internal/interpreter/dynamic_item_tree.rs
@@ -2234,7 +2234,7 @@ impl<'a, 'id> InstanceRef<'a, 'id> {
 /// Show the popup at the given location
 pub fn show_popup(
     popup: &object_tree::PopupWindow,
-    pos: i_slint_core::graphics::Point,
+    pos_getter: impl FnOnce(InstanceRef<'_, '_>) -> i_slint_core::graphics::Point,
     close_on_click: bool,
     parent_comp: ErasedItemTreeBoxWeak,
     parent_window_adapter: WindowAdapterRc,
@@ -2251,6 +2251,13 @@ pub fn show_popup(
         Default::default(),
     );
     inst.run_setup_code();
+
+    let pos = {
+        generativity::make_guard!(guard);
+        let compo_box = inst.unerase(guard);
+        let instance_ref = compo_box.borrow_instance();
+        pos_getter(instance_ref)
+    };
     WindowInner::from_pub(parent_window_adapter.window()).show_popup(
         &vtable::VRc::into_dyn(inst),
         pos,