Compiler optimizations

internal/compiler/expression_tree.rs

@@ -1042,7 +1042,8 @@ impl Expression {
             Expression::ElementReference(_) => false,
             Expression::RepeaterIndexReference { .. } => false,
             Expression::RepeaterModelReference { .. } => false,
-            Expression::FunctionParameterReference { .. } => false,
+            // Allow functions to be marked as const
+            Expression::FunctionParameterReference { .. } => true,
             Expression::StructFieldAccess { base, .. } => base.is_constant(),
             Expression::ArrayIndex { array, index } => array.is_constant() && index.is_constant(),
             Expression::Cast { from, .. } => from.is_constant(),
@@ -1074,9 +1075,9 @@ impl Expression {
                 Path::Events(_, _) => true,
                 Path::Commands(_) => false,
             },
-            Expression::StoreLocalVariable { .. } => false,
-            // we should somehow find out if this is constant or not
-            Expression::ReadLocalVariable { .. } => false,
+            Expression::StoreLocalVariable { value, .. } => value.is_constant(),
+            // We only load what we store, and stores are alredy checked
+            Expression::ReadLocalVariable { .. } => true,
             Expression::EasingCurve(_) => true,
             Expression::LinearGradient { angle, stops } => {
                 angle.is_constant() && stops.iter().all(|(c, s)| c.is_constant() && s.is_constant())

internal/compiler/llr/pretty_print.rs

@@ -211,6 +211,9 @@ impl<T> Display for DisplayPropertyRef<'_, T> {
             }
             PropertyReference::InParent { level, parent_reference } => {
                 for _ in 0..level.get() {
+                    if ctx.parent.is_none() {
+                        return write!(f, "<invalid parent reference>");
+                    }
                     ctx = ctx.parent.unwrap().ctx;
                 }
                 write!(f, "{}", Self(parent_reference, ctx))

internal/compiler/passes.rs

@@ -198,7 +198,6 @@ pub async fn run_passes(
     unique_id::assign_unique_id(doc);
 
     doc.visit_all_used_components(|component| {
-        deduplicate_property_read::deduplicate_property_read(component);
         // Don't perform the empty rectangle removal when debug info is requested, because the resulting
         // item tree ends up with a hierarchy where certain items have children that aren't child elements
         // but siblings or sibling children. We need a new data structure to perform a correct element tree
@@ -217,6 +216,7 @@ pub async fn run_passes(
             // binding loop causes panics in const_propagation
             const_propagation::const_propagation(component);
         }
+        deduplicate_property_read::deduplicate_property_read(component);
         if !component.is_global() {
             resolve_native_classes::resolve_native_classes(component);
         }

internal/compiler/passes/const_propagation.rs

@@ -105,6 +105,19 @@ fn simplify_expression(expr: &mut Expression) -> bool {
             }
             can_inline
         }
+        Expression::UnaryOp { sub, op } => {
+            let can_inline = simplify_expression(sub);
+            let new = match (*op, &mut **sub) {
+                ('!', Expression::BoolLiteral(b)) => Some(Expression::BoolLiteral(!*b)),
+                ('-', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(-*n, *u)),
+                ('+', Expression::NumberLiteral(n, u)) => Some(Expression::NumberLiteral(*n, *u)),
+                _ => None,
+            };
+            if let Some(new) = new {
+                *expr = new;
+            }
+            can_inline
+        }
         Expression::StructFieldAccess { base, name } => {
             let r = simplify_expression(base);
             if let Expression::Struct { values, .. } = &mut **base {
@@ -148,6 +161,38 @@ fn simplify_expression(expr: &mut Expression) -> bool {
             }
             can_inline
         }
+        Expression::Condition { condition, true_expr, false_expr } => {
+            let mut can_inline = simplify_expression(condition);
+            can_inline &= match &**condition {
+                Expression::BoolLiteral(true) => {
+                    *expr = *true_expr.clone();
+                    simplify_expression(expr)
+                }
+                Expression::BoolLiteral(false) => {
+                    *expr = *false_expr.clone();
+                    simplify_expression(expr)
+                }
+                _ => simplify_expression(true_expr) && simplify_expression(false_expr),
+            };
+            can_inline
+        }
+        Expression::CodeBlock(stmts) if stmts.len() == 1 => {
+            *expr = stmts[0].clone();
+            simplify_expression(expr)
+        }
+        Expression::FunctionCall { function, arguments, .. } => {
+            let mut args_can_inline = true;
+            for arg in arguments.iter_mut() {
+                args_can_inline &= simplify_expression(arg);
+            }
+            if args_can_inline {
+                if let Some(inlined) = try_inline_function(function, arguments) {
+                    *expr = inlined;
+                    true;
+                }
+            }
+            false
+        }
         Expression::ElementReference { .. } => false,
         Expression::LayoutCacheAccess { .. } => false,
         Expression::SolveLayout { .. } => false,
@@ -199,6 +244,30 @@ fn extract_constant_property_reference(nr: &NamedReference) -> Option<Expression
     Some(expression)
 }
 
+fn try_inline_function(function: &Callable, arguments: &[Expression]) -> Option<Expression> {
+    let Callable::Function(function) = function else {
+        return None;
+    };
+    if !function.is_constant() {
+        return None;
+    }
+    let mut body = extract_constant_property_reference(function)?;
+
+    body.visit_recursive_mut(&mut |e| {
+        if let Expression::FunctionParameterReference { index, ty } = e {
+            let Some(e_new) = arguments.get(*index).cloned() else { return };
+            if e_new.ty() == *ty {
+                *e = e_new;
+            }
+        }
+    });
+    if simplify_expression(&mut body) {
+        Some(body)
+    } else {
+        None
+    }
+}
+
 #[test]
 fn test() {
     let mut compiler_config =

internal/compiler/passes/inlining.rs

@@ -23,14 +23,15 @@ pub fn inline(doc: &Document, inline_selection: InlineSelection, diag: &mut Buil
     fn inline_components_recursively(
         component: &Rc<Component>,
         roots: &HashSet<ByAddress<Rc<Component>>>,
+        used_once: &HashSet<ByAddress<Rc<Component>>>,
         inline_selection: InlineSelection,
         diag: &mut BuildDiagnostics,
     ) {
         recurse_elem_no_borrow(&component.root_element, &(), &mut |elem, _| {
             let base = elem.borrow().base_type.clone();
             if let ElementType::Component(c) = base {
                 // First, make sure that the component itself is properly inlined
-                inline_components_recursively(&c, roots, inline_selection, diag);
+                inline_components_recursively(&c, roots, used_once, inline_selection, diag);
 
                 if c.parent_element.upgrade().is_some() {
                     // We should not inline a repeated element
@@ -43,28 +44,32 @@ pub fn inline(doc: &Document, inline_selection: InlineSelection, diag: &mut Buil
                     InlineSelection::InlineOnlyRequiredComponents => {
                         component_requires_inlining(&c)
                             || element_require_inlining(elem)
-                            // We always inline the root in case the element that instantiate this component needs full inlining
-                            || Rc::ptr_eq(elem, &component.root_element)
+                            // We always inline the root in case the element that instantiate this component needs full inlining,
+                            // except when the root is a repeater component, which are never inlined.
+                            || component.parent_element.upgrade().is_none() && Rc::ptr_eq(elem, &component.root_element)
                             // We always inline other roots as a component can't be both a sub component and a root
                             || roots.contains(&ByAddress(c.clone()))
+                            || used_once.contains(&ByAddress(c.clone()))
                     }
                 } {
                     inline_element(elem, &c, component, diag);
                 }
             }
         });
         component.popup_windows.borrow().iter().for_each(|p| {
-            inline_components_recursively(&p.component, roots, inline_selection, diag)
+            inline_components_recursively(&p.component, roots, used_once, inline_selection, diag)
         })
     }
     let mut roots = HashSet::new();
+    let mut used_once = HashSet::new();
     if inline_selection == InlineSelection::InlineOnlyRequiredComponents {
         for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
             roots.insert(ByAddress(component.clone()));
         }
+        used_once = collect_subcomponents_used_once(doc);
     }
     for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
-        inline_components_recursively(&component, &roots, inline_selection, diag);
+        inline_components_recursively(&component, &roots, &used_once, inline_selection, diag);
         let mut init_code = component.init_code.borrow_mut();
         let inlined_init_code = core::mem::take(&mut init_code.inlined_init_code);
         init_code.constructor_code.splice(0..0, inlined_init_code.into_values());
@@ -629,6 +634,12 @@ fn element_require_inlining(elem: &ElementRc) -> bool {
         // the generators assume that the children list is complete, which sub-components may break
         return true;
     }
+
+    // Popup windows need to be inlined for root.close() to work properly.
+    if super::lower_popups::is_popup_window(elem) {
+        return true;
+    }
+
     for (prop, binding) in &elem.borrow().bindings {
         if prop == "clip" {
             // otherwise the children of the clipped items won't get moved as child of the Clip element
@@ -644,3 +655,35 @@ fn element_require_inlining(elem: &ElementRc) -> bool {
 
     false
 }
+
+fn collect_subcomponents_used_once(doc: &Document) -> HashSet<ByAddress<Rc<Component>>> {
+    fn recursive(component: &Rc<Component>, hash: &mut HashMap<ByAddress<Rc<Component>>, bool>) {
+        match hash.entry(ByAddress(component.clone())) {
+            std::collections::hash_map::Entry::Occupied(mut o) => {
+                if !*o.get() {
+                    // We have already set this element (and its children to "set multiple times"
+                    return;
+                }
+                // Element used multiple times, visit all children to set them to multiple times too.
+                *o.get_mut() = false;
+            }
+            std::collections::hash_map::Entry::Vacant(v) => {
+                v.insert(true);
+            }
+        }
+
+        recurse_elem(&component.root_element, &(), &mut |elem: &ElementRc, &()| {
+            let ElementType::Component(base_comp) = &elem.borrow().base_type else { return };
+            recursive(base_comp, hash);
+        });
+        for popup in component.popup_windows.borrow().iter() {
+            recursive(&popup.component, hash);
+        }
+    }
+    // Stores true for elements that are used once, false for elements that are used multiple times;
+    let mut result = HashMap::new();
+    for component in doc.exported_roots().chain(doc.popup_menu_impl.iter().cloned()) {
+        recursive(&component, &mut result);
+    }
+    result.into_iter().filter(|(_, v)| *v).map(|(k, _)| k).collect()
+}

internal/compiler/passes/lower_popups.rs

@@ -25,20 +25,22 @@ pub fn lower_popups(
         component,
         &None,
         &mut |elem, parent_element: &Option<ElementRc>| {
-            let is_popup = match &elem.borrow().base_type {
-                ElementType::Builtin(base_type) => base_type.name == "PopupWindow",
-                ElementType::Component(base_type) => base_type.inherits_popup_window.get(),
-                _ => false,
-            };
-
-            if is_popup {
+            if is_popup_window(elem) {
                 lower_popup_window(elem, parent_element.as_ref(), &window_type, diag);
             }
             Some(elem.clone())
         },
     )
 }
 
+pub fn is_popup_window(element: &ElementRc) -> bool {
+    match &element.borrow().base_type {
+        ElementType::Builtin(base_type) => base_type.name == "PopupWindow",
+        ElementType::Component(base_type) => base_type.inherits_popup_window.get(),
+        _ => false,
+    }
+}
+
 fn lower_popup_window(
     popup_window_element: &ElementRc,
     parent_element: Option<&ElementRc>,

internal/compiler/passes/materialize_fake_properties.rs

@@ -149,17 +149,33 @@ pub fn initialize(elem: &ElementRc, name: &str) -> Option<Expression> {
         };
     }
 
-    let expr = match name {
-        "min-height" => layout_constraint_prop(elem, "min", Orientation::Vertical),
-        "min-width" => layout_constraint_prop(elem, "min", Orientation::Horizontal),
-        "max-height" => layout_constraint_prop(elem, "max", Orientation::Vertical),
-        "max-width" => layout_constraint_prop(elem, "max", Orientation::Horizontal),
-        "preferred-height" => layout_constraint_prop(elem, "preferred", Orientation::Vertical),
-        "preferred-width" => layout_constraint_prop(elem, "preferred", Orientation::Horizontal),
-        "horizontal-stretch" => layout_constraint_prop(elem, "stretch", Orientation::Horizontal),
-        "vertical-stretch" => layout_constraint_prop(elem, "stretch", Orientation::Vertical),
-        "opacity" => Expression::NumberLiteral(1., Unit::None),
-        "visible" => Expression::BoolLiteral(true),
+    let builtin_name = match elem.borrow().builtin_type() {
+        Some(b) => b.name.clone(),
+        None => SmolStr::default(),
+    };
+
+    let expr = match (name, builtin_name.as_str()) {
+        ("min-height", "Image") => Expression::NumberLiteral(0., Unit::Px),
+        ("min-width", "Image") => Expression::NumberLiteral(0., Unit::Px),
+        ("max-height", "Image") => Expression::NumberLiteral(f32::MAX as _, Unit::Px),
+        ("max-width", "Image") => Expression::NumberLiteral(f32::MAX as _, Unit::Px),
+        ("horizontal-stretch", "Image") => Expression::NumberLiteral(0., Unit::None),
+        ("vertical-stretch", "Image") => Expression::NumberLiteral(0., Unit::None),
+
+        ("min-height", _) => layout_constraint_prop(elem, "min", Orientation::Vertical),
+        ("min-width", _) => layout_constraint_prop(elem, "min", Orientation::Horizontal),
+        ("max-height", _) => layout_constraint_prop(elem, "max", Orientation::Vertical),
+        ("max-width", _) => layout_constraint_prop(elem, "max", Orientation::Horizontal),
+        ("horizontal-stretch", _) => {
+            layout_constraint_prop(elem, "stretch", Orientation::Horizontal)
+        }
+        ("vertical-stretch", _) => layout_constraint_prop(elem, "stretch", Orientation::Vertical),
+        ("preferred-height", _) => layout_constraint_prop(elem, "preferred", Orientation::Vertical),
+        ("preferred-width", _) => {
+            layout_constraint_prop(elem, "preferred", Orientation::Horizontal)
+        }
+        ("opacity", _) => Expression::NumberLiteral(1., Unit::None),
+        ("visible", _) => Expression::BoolLiteral(true),
         _ => return None,
     };
     Some(expr)