From a318df522b65dd1f6d8bb52c1e83bca71d17f59a Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 12 Jul 2021 16:46:59 +0200 Subject: [PATCH] Prepare for optional inlining Even if we make inlining optional, there are *some* situations where we just need to do the inlining anyway to avoid needless complexity. --- sixtyfps_compiler/object_tree.rs | 3 +- sixtyfps_compiler/passes.rs | 75 +++++++++++-------- .../passes/collect_subcomponents.rs | 4 +- sixtyfps_compiler/passes/default_geometry.rs | 28 +++++++ sixtyfps_compiler/passes/flickable.rs | 6 +- sixtyfps_compiler/passes/focus_item.rs | 15 +++- sixtyfps_compiler/passes/inlining.rs | 52 +++++++++++-- sixtyfps_compiler/passes/lower_layout.rs | 4 + 8 files changed, 144 insertions(+), 43 deletions(-) diff --git a/sixtyfps_compiler/object_tree.rs b/sixtyfps_compiler/object_tree.rs index 3d1a3cf79..2088fc0c2 100644 --- a/sixtyfps_compiler/object_tree.rs +++ b/sixtyfps_compiler/object_tree.rs @@ -23,8 +23,8 @@ use crate::parser; use crate::parser::{syntax_nodes, SyntaxKind, SyntaxNode}; use crate::typeloader::ImportedTypes; use crate::typeregister::TypeRegister; -use std::cell::RefCell; use std::collections::btree_map::Entry; +use std::cell::{Cell, RefCell}; use std::collections::{BTreeMap, HashMap}; use std::iter::FromIterator; use std::rc::{Rc, Weak}; @@ -225,6 +225,7 @@ pub struct Component { /// The names under which this component should be accessible /// if it is a global singleton and exported. pub exported_global_names: RefCell>, + pub requires_inlining: Cell, } impl Component { diff --git a/sixtyfps_compiler/passes.rs b/sixtyfps_compiler/passes.rs index 9c3f2454d..bc5368409 100644 --- a/sixtyfps_compiler/passes.rs +++ b/sixtyfps_compiler/passes.rs @@ -67,55 +67,64 @@ pub async fn run_passes( check_public_api::check_public_api(doc, diag); collect_subcomponents::collect_subcomponents(root_component); - for component in root_component + + let components = root_component .used_types .borrow() .sub_components .iter() - .chain(std::iter::once(root_component)) - { + .cloned() + .chain(std::iter::once(root_component.clone())) + .collect::>(); + + for component in &components { compile_paths::compile_paths(component, &doc.local_registry, diag); lower_tabwidget::lower_tabwidget(component, &mut type_loader, diag).await; } embed_images::embed_images(root_component, compiler_config.embed_resources, diag); - inlining::inline(doc); - focus_item::resolve_element_reference_in_set_focus_calls(root_component, diag); - focus_item::determine_initial_focus_item(root_component, diag); - focus_item::erase_forward_focus_properties(root_component); - flickable::handle_flickable(root_component, &global_type_registry.borrow()); - lower_states::lower_states(root_component, &doc.local_registry, diag); - repeater_component::process_repeater_components(root_component); - lower_popups::lower_popups(root_component, &doc.local_registry, diag); - lower_layout::lower_layouts(root_component, &mut type_loader, diag).await; - z_order::reorder_by_z_order(root_component, diag); - lower_shadows::lower_shadow_properties(root_component, &doc.local_registry, diag); - clip::handle_clip(root_component, &global_type_registry.borrow(), diag); - transform_and_opacity::handle_transform_and_opacity( - root_component, - &global_type_registry.borrow(), - diag, - ); - default_geometry::default_geometry(root_component, diag); - visible::handle_visible(root_component, &global_type_registry.borrow()); - materialize_fake_properties::materialize_fake_properties(root_component); - ensure_window::ensure_window(root_component, &doc.local_registry); - apply_default_properties_from_style::apply_default_properties_from_style( - root_component, - &mut type_loader, - diag, - ) - .await; - collect_globals::collect_globals(doc, diag); - unique_id::assign_unique_id(root_component); - binding_analysis::binding_analysis(root_component, diag); + inlining::inline(doc, inlining::InlineSelection::InlineOnlyRequiredComponents); + + for component in &components { + focus_item::resolve_element_reference_in_set_focus_calls(component, diag); + focus_item::determine_initial_focus_item(component, diag); + focus_item::erase_forward_focus_properties(component); + flickable::handle_flickable(component, &global_type_registry.borrow()); + lower_states::lower_states(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, &mut type_loader, diag).await; + z_order::reorder_by_z_order(component, diag); + lower_shadows::lower_shadow_properties(component, &doc.local_registry, diag); + clip::handle_clip(component, &global_type_registry.borrow(), diag); + transform_and_opacity::handle_transform_and_opacity( + component, + &global_type_registry.borrow(), + diag, + ); + default_geometry::default_geometry(component, diag); + materialize_fake_properties::materialize_fake_properties(component); + apply_default_properties_from_style::apply_default_properties_from_style( + component, + &mut type_loader, + diag, + ) + .await; + ensure_window::ensure_window(component, &doc.local_registry); + unique_id::assign_unique_id(component); + collect_globals::collect_globals(&doc, diag); + + binding_analysis::binding_analysis(component, diag); + } + deduplicate_property_read::deduplicate_property_read(root_component); optimize_useless_rectangles::optimize_useless_rectangles(root_component); move_declarations::move_declarations(root_component, diag); remove_aliases::remove_aliases(root_component, diag); resolve_native_classes::resolve_native_classes(root_component); remove_unused_properties::remove_unused_properties(root_component); + collect_structs::collect_structs(root_component, diag); generate_item_indices::generate_item_indices(root_component); collect_custom_fonts::collect_custom_fonts( diff --git a/sixtyfps_compiler/passes/collect_subcomponents.rs b/sixtyfps_compiler/passes/collect_subcomponents.rs index ee10adba0..92c6d2451 100644 --- a/sixtyfps_compiler/passes/collect_subcomponents.rs +++ b/sixtyfps_compiler/passes/collect_subcomponents.rs @@ -44,6 +44,8 @@ fn collect_subcomponents_recursive( _ => return, }; collect_subcomponents_recursive(&base_comp, result, hash); - result.push(base_comp); + if !base_comp.requires_inlining.get() { + result.push(base_comp); + } }); } diff --git a/sixtyfps_compiler/passes/default_geometry.rs b/sixtyfps_compiler/passes/default_geometry.rs index 2e050f390..801856e85 100644 --- a/sixtyfps_compiler/passes/default_geometry.rs +++ b/sixtyfps_compiler/passes/default_geometry.rs @@ -275,3 +275,31 @@ fn make_default_aspect_ratio_preserving_binding( elem.borrow_mut().bindings.insert(missing_size_property.to_string(), binding.into()); } + +fn implicit_layout_info_call(elem: &ElementRc, orientation: Orientation) -> Expression { + Expression::FunctionCall { + function: Box::new(Expression::BuiltinFunctionReference( + BuiltinFunction::ImplicitLayoutInfo(orientation), + None, + )), + arguments: vec![Expression::ElementReference(Rc::downgrade(elem))], + source_location: None, + } +} + +pub fn element_requires_parent_for_geometry(element: &ElementRc) -> bool { + for property in ["width", "height"] { + if !element.borrow().bindings.get(property).map_or(false, |b| b.ty() == Type::Percent) { + return true; + } + } + if let Type::Builtin(builtin_type) = &element.borrow().base_type { + if matches!(builtin_type.default_size_binding, DefaultSizeBinding::ExpandsToParentGeometry) + { + // FIXME: this does not apply to children of layouts + return true; + } + } + + false +} diff --git a/sixtyfps_compiler/passes/flickable.rs b/sixtyfps_compiler/passes/flickable.rs index 10bf4689d..2b24c937c 100644 --- a/sixtyfps_compiler/passes/flickable.rs +++ b/sixtyfps_compiler/passes/flickable.rs @@ -27,6 +27,10 @@ use crate::langtype::{NativeClass, Type}; use crate::object_tree::{Component, Element, ElementRc}; use crate::typeregister::TypeRegister; +pub fn is_flickable_element(element: &ElementRc) -> bool { + matches!(&element.borrow().base_type, Type::Builtin(n) if n.name == "Flickable") +} + pub fn handle_flickable(root_component: &Rc, tr: &TypeRegister) { let mut native_rect = tr.lookup("Rectangle").as_builtin().native_class.clone(); while let Some(p) = native_rect.parent.clone() { @@ -37,7 +41,7 @@ pub fn handle_flickable(root_component: &Rc, tr: &TypeRegister) { root_component, &(), &mut |elem: &ElementRc, _| { - if !matches!(&elem.borrow().base_type, Type::Builtin(n) if n.name == "Flickable") { + if !is_flickable_element(elem) { return; } diff --git a/sixtyfps_compiler/passes/focus_item.rs b/sixtyfps_compiler/passes/focus_item.rs index fb3c88749..a10d976e4 100644 --- a/sixtyfps_compiler/passes/focus_item.rs +++ b/sixtyfps_compiler/passes/focus_item.rs @@ -13,7 +13,7 @@ LICENSE END */ use std::rc::Rc; use crate::diagnostics::{BuildDiagnostics, SourceLocation, Spanned}; -use crate::expression_tree::{BuiltinFunction, Expression}; +use crate::expression_tree::{BindingExpression, BuiltinFunction, Expression}; use crate::langtype::Type; use crate::object_tree::*; @@ -23,8 +23,19 @@ enum FocusCheckResult { ElementIsNotFocusable, } +pub fn get_explicit_forward_focus( + element: &ElementRc, +) -> Option> { + let element = element.borrow(); + if element.bindings.contains_key("forward_focus") { + Some(std::cell::Ref::map(element, |elem| &elem.bindings["forward_focus"])) + } else { + None + } +} + fn element_focus_check(element: &ElementRc) -> FocusCheckResult { - if let Some(forwarded_focus_binding) = element.borrow().bindings.get("forward-focus") { + if let Some(forwarded_focus_binding) = get_explicit_forward_focus(element) { if let Expression::ElementReference(target) = &forwarded_focus_binding.expression { return FocusCheckResult::FocusForwarded( target.upgrade().unwrap(), diff --git a/sixtyfps_compiler/passes/inlining.rs b/sixtyfps_compiler/passes/inlining.rs index 927a0c5f5..b4445fb91 100644 --- a/sixtyfps_compiler/passes/inlining.rs +++ b/sixtyfps_compiler/passes/inlining.rs @@ -17,19 +17,37 @@ use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; -pub fn inline(doc: &Document) { - fn inline_components_recursively(component: &Rc) { +#[derive(Copy, Clone)] +pub enum InlineSelection { + InlineAllComponents, + #[allow(dead_code)] // allow until it's an option globally used in the compiler + InlineOnlyRequiredComponents, +} + +pub fn inline(doc: &Document, inline_selection: InlineSelection) { + fn inline_components_recursively(component: &Rc, inline_selection: InlineSelection) { recurse_elem(&component.root_element, &(), &mut |elem, _| { let base = elem.borrow().base_type.clone(); if let Type::Component(c) = base { // First, make sure that the component itself is properly inlined - inline_components_recursively(&c); + inline_components_recursively(&c, inline_selection); // Inline this component. - inline_element(elem, &c, component); + if match inline_selection { + InlineSelection::InlineAllComponents => true, + InlineSelection::InlineOnlyRequiredComponents + if component_requires_inlining(&c) => + { + c.requires_inlining.set(true); + true + } + _ => false, + } { + inline_element(elem, &c, component); + } } }) } - inline_components_recursively(&doc.root_component) + inline_components_recursively(&doc.root_component, inline_selection) } fn clone_tuple((u, v): (&U, &V)) -> (U, V) { @@ -261,3 +279,27 @@ fn duplicate_transition( node: t.node.clone(), } } + +// Some components need to be inlined to avoid increased complexity in handling them +// in the code generators and subsequent passes. +fn component_requires_inlining(component: &Rc) -> bool { + if component.child_insertion_point.borrow().is_some() { + return true; + } + + let root_element = &component.root_element; + if super::flickable::is_flickable_element(root_element) + || super::focus_item::get_explicit_forward_focus(root_element).is_some() + || super::lower_layout::is_layout_element(root_element) + { + return true; + } + + // For now any implicit bindings to the parent geometry require inlining, but this should + // be changed to instead create the bindings on the use-site instead. + if super::default_geometry::element_requires_parent_for_geometry(root_element) { + return true; + } + + false +} diff --git a/sixtyfps_compiler/passes/lower_layout.rs b/sixtyfps_compiler/passes/lower_layout.rs index 2a59b95c2..9aa82d7da 100644 --- a/sixtyfps_compiler/passes/lower_layout.rs +++ b/sixtyfps_compiler/passes/lower_layout.rs @@ -94,6 +94,10 @@ fn lower_element_layout( } } +pub fn is_layout_element(element: &ElementRc) -> bool { + matches!(&element.borrow().base_type, Type::Builtin(n) if n.name == "GridLayout" || n.name == "HorizontalLayout" || n.name == "VerticalLayout" || n.name == "PathLayout") +} + fn lower_grid_layout( component: &Rc, grid_layout_element: &ElementRc,