diff --git a/api/cpp/include/slint_models.h b/api/cpp/include/slint_models.h index 8fdd502ad..f5b678163 100644 --- a/api/cpp/include/slint_models.h +++ b/api/cpp/include/slint_models.h @@ -816,8 +816,6 @@ namespace private_api { template class Repeater { - private_api::Property>> model; - struct RepeaterInner : ModelChangeListener { enum class State { Clean, Dirty }; @@ -866,10 +864,16 @@ class Repeater } }; -public: - // FIXME: should be private, but layouting code uses it. + private_api::Property>> model; mutable std::shared_ptr inner; + vtable::VRef item_at(int i) const + { + const auto &x = inner->data.at(i); + return { &C::static_vtable, const_cast(&(**x.ptr)) }; + } + +public: template void set_model_binding(F &&binding) const { @@ -947,12 +951,6 @@ public: return std::numeric_limits::max(); } - vtable::VRef item_at(int i) const - { - const auto &x = inner->data.at(i); - return { &C::static_vtable, const_cast(&(**x.ptr)) }; - } - vtable::VWeak instance_at(std::size_t i) const { if (i >= inner->data.size()) { @@ -967,6 +965,8 @@ public: return private_api::IndexRange { 0, inner->data.size() }; } + std::size_t len() const { return inner ? inner->data.size() : 0; } + float compute_layout_listview(const private_api::Property *viewport_width, float listview_width, float viewport_y) const { @@ -992,6 +992,72 @@ public: } } } + + void for_each(auto &&f) const + { + if (inner) { + for (auto &&x : inner->data) { + f(*x.ptr); + } + } + } +}; + +template +class Conditional +{ + private_api::Property model; + mutable std::optional> instance; + +public: + template + void set_model_binding(F &&binding) const + { + model.set_binding(std::forward(binding)); + } + + template + void ensure_updated(const Parent *parent) const + { + if (!model.get()) { + instance = std::nullopt; + } else if (!instance) { + instance = C::create(parent); + (*instance)->init(); + } + } + + uint64_t visit(TraversalOrder order, private_api::ItemVisitorRefMut visitor) const + { + if (instance) { + vtable::VRef ref { &C::static_vtable, + const_cast(&(**instance)) }; + if (ref.vtable->visit_children_item(ref, -1, order, visitor) + != std::numeric_limits::max()) { + return 0; + } + } + return std::numeric_limits::max(); + } + + vtable::VWeak instance_at(std::size_t i) const + { + if (i != 0 || !instance) { + return {}; + } + return vtable::VWeak { instance->into_dyn() }; + } + + private_api::IndexRange index_range() const { return private_api::IndexRange { 0, len() }; } + + std::size_t len() const { return instance ? 1 : 0; } + + void for_each(auto &&f) const + { + if (instance) { + f(*instance); + } + } }; } // namespace private_api diff --git a/internal/compiler/generator/cpp.rs b/internal/compiler/generator/cpp.rs index 044791f43..8cce902d7 100644 --- a/internal/compiler/generator/cpp.rs +++ b/internal/compiler/generator/cpp.rs @@ -2079,17 +2079,13 @@ fn generate_sub_component( for (idx, repeated) in component.repeated.iter_enumerated() { let sc = &root.sub_components[repeated.sub_tree.root]; - let data_type = if let Some(data_prop) = repeated.data_prop { - sc.properties[data_prop].ty.clone() - } else { - Type::Int32 - }; + let data_type = repeated.data_prop.map(|data_prop| sc.properties[data_prop].ty.clone()); generate_repeated_component( repeated, root, ParentCtx::new(&ctx, Some(idx)), - &data_type, + data_type.as_ref(), file, conditional_includes, ); @@ -2097,12 +2093,7 @@ fn generate_sub_component( let idx = usize::from(idx); let repeater_id = format_smolstr!("repeater_{}", idx); - let mut model = compile_expression(&repeated.model.borrow(), &ctx); - if repeated.model.ty(&ctx) == Type::Bool { - // bool converts to int - // FIXME: don't do a heap allocation here - model = format!("std::make_shared({model})") - } + let model = compile_expression(&repeated.model.borrow(), &ctx); // FIXME: optimize if repeated.model.is_constant() properties_init_code.push(format!( @@ -2143,17 +2134,19 @@ fn generate_sub_component( }}", )); - target_struct.members.push(( - field_access, - Declaration::Var(Var { - ty: format_smolstr!( + let rep_type = match data_type { + Some(data_type) => { + format_smolstr!( "slint::private_api::Repeater", ident(&sc.name), - data_type.cpp_type().unwrap(), - ), - name: repeater_id, - ..Default::default() - }), + data_type.cpp_type().unwrap() + ) + } + None => format_smolstr!("slint::private_api::Conditional", ident(&sc.name)), + }; + target_struct.members.push(( + field_access, + Declaration::Var(Var { ty: rep_type, name: repeater_id, ..Default::default() }), )); } @@ -2438,7 +2431,7 @@ fn generate_repeated_component( repeated: &llr::RepeatedElement, root: &llr::CompilationUnit, parent_ctx: ParentCtx, - model_data_type: &Type, + model_data_type: Option<&Type>, file: &mut File, conditional_includes: &ConditionalIncludes, ) { @@ -2474,22 +2467,24 @@ fn generate_repeated_component( let index_prop = repeated.index_prop.iter().map(access_prop); let data_prop = repeated.data_prop.iter().map(access_prop); - let mut update_statements = vec!["[[maybe_unused]] auto self = this;".into()]; - update_statements.extend(index_prop.map(|prop| format!("{prop}.set(i);"))); - update_statements.extend(data_prop.map(|prop| format!("{prop}.set(data);"))); + if let Some(model_data_type) = model_data_type { + let mut update_statements = vec!["[[maybe_unused]] auto self = this;".into()]; + update_statements.extend(index_prop.map(|prop| format!("{prop}.set(i);"))); + update_statements.extend(data_prop.map(|prop| format!("{prop}.set(data);"))); - repeater_struct.members.push(( - Access::Public, // Because Repeater accesses it - Declaration::Function(Function { - name: "update_data".into(), - signature: format!( - "([[maybe_unused]] int i, [[maybe_unused]] const {} &data) const -> void", - model_data_type.cpp_type().unwrap() - ), - statements: Some(update_statements), - ..Function::default() - }), - )); + repeater_struct.members.push(( + Access::Public, // Because Repeater accesses it + Declaration::Function(Function { + name: "update_data".into(), + signature: format!( + "([[maybe_unused]] int i, [[maybe_unused]] const {} &data) const -> void", + model_data_type.cpp_type().unwrap() + ), + statements: Some(update_statements), + ..Function::default() + }), + )); + } repeater_struct.members.push(( Access::Public, // Because Repeater accesses it @@ -3972,19 +3967,19 @@ fn box_layout_function( if let Some(ri) = &repeated_indices { write!(push_code, "{}_array[{}] = cells_vector.size();", ri, repeater_idx * 2) .unwrap(); - write!(push_code, - "{ri}_array[{c}] = self->repeater_{id}.inner ? self->repeater_{id}.inner->data.size() : 0;", + write!( + push_code, + "{ri}_array[{c}] = self->repeater_{id}.len();", ri = ri, c = repeater_idx * 2 + 1, id = repeater, - ).unwrap(); + ) + .unwrap(); } repeater_idx += 1; write!( push_code, - "if (self->repeater_{id}.inner) \ - for (auto &&sub_comp : self->repeater_{id}.inner->data) \ - cells_vector.push_back((*sub_comp.ptr)->box_layout_data({o}));", + "self->repeater_{id}.for_each([&](const auto &sub_comp){{ cells_vector.push_back(sub_comp->box_layout_data({o})); }});", id = repeater, o = to_cpp_orientation(orientation), ) diff --git a/internal/compiler/generator/rust.rs b/internal/compiler/generator/rust.rs index 77e55a5d9..2720d9345 100644 --- a/internal/compiler/generator/rust.rs +++ b/internal/compiler/generator/rust.rs @@ -766,9 +766,8 @@ fn generate_sub_component( } } - let mut repeated_element_names: Vec = vec![]; let mut repeated_visit_branch: Vec = vec![]; - let mut repeated_element_components: Vec = vec![]; + let mut repeated_element_components: Vec = vec![]; let mut repeated_subtree_ranges: Vec = vec![]; let mut repeated_subtree_components: Vec = vec![]; @@ -783,11 +782,7 @@ fn generate_sub_component( let rep_inner_component_id = self::inner_component_id(&root.sub_components[repeated.sub_tree.root]); - let mut model = compile_expression(&repeated.model.borrow(), &ctx); - if repeated.model.ty(&ctx) == Type::Bool { - model = quote!(sp::ModelRc::new(#model as bool)) - } - + let model = compile_expression(&repeated.model.borrow(), &ctx); init.push(quote! { _self.#repeater_id.set_model_binding({ let self_weak = sp::VRcMapped::downgrade(&self_rc); @@ -838,8 +833,11 @@ fn generate_sub_component( } } )); - repeated_element_names.push(repeater_id); - repeated_element_components.push(rep_inner_component_id); + repeated_element_components.push(if repeated.index_prop.is_some() { + quote!(#repeater_id: sp::Repeater<#rep_inner_component_id>) + } else { + quote!(#repeater_id: sp::Conditional<#rep_inner_component_id>) + }); } // Use ids following the real repeaters to piggyback on their forwarding through sub-components! @@ -1149,7 +1147,7 @@ fn generate_sub_component( #(#popup_id_names : ::core::cell::Cell>,)* #(#declared_property_vars : sp::Property<#declared_property_types>,)* #(#declared_callbacks : sp::Callback<(#(#declared_callbacks_types,)*), #declared_callbacks_ret>,)* - #(#repeated_element_names : sp::Repeater<#repeated_element_components>,)* + #(#repeated_element_components,)* #(#change_tracker_names : sp::ChangeTracker,)* #(#timer_names : sp::Timer,)* self_weak : sp::OnceCell>, diff --git a/internal/compiler/llr/item_tree.rs b/internal/compiler/llr/item_tree.rs index e619c1cc1..269aa0b7a 100644 --- a/internal/compiler/llr/item_tree.rs +++ b/internal/compiler/llr/item_tree.rs @@ -157,9 +157,9 @@ pub struct ListViewInfo { #[derive(Debug)] pub struct RepeatedElement { pub model: MutExpression, - /// Within the sub_tree's root component + /// Within the sub_tree's root component. None for `if` pub index_prop: Option, - /// Within the sub_tree's root component + /// Within the sub_tree's root component. None for `if` pub data_prop: Option, pub sub_tree: ItemTree, /// The index of the item node in the parent tree diff --git a/internal/core/model.rs b/internal/core/model.rs index 341a3321c..d434e43b8 100644 --- a/internal/core/model.rs +++ b/internal/core/model.rs @@ -1274,6 +1274,91 @@ impl Repeater { } } +#[pin_project] +pub struct Conditional { + #[pin] + model: Property, + instance: RefCell>>, +} + +impl Default for Conditional { + fn default() -> Self { + Self { + model: Property::new_named(false, "i_slint_core::Conditional::model"), + instance: RefCell::new(None), + } + } +} + +impl Conditional { + /// Call this function to make sure that the model is updated. + /// The init function is the function to create a ItemTree + pub fn ensure_updated(self: Pin<&Self>, init: impl Fn() -> ItemTreeRc) { + let model = self.project_ref().model.get(); + + if !model { + drop(self.instance.replace(None)); + } else if self.instance.borrow().is_none() { + let i = init(); + self.instance.replace(Some(i.clone())); + i.init(); + } + } + + /// Set the model binding + pub fn set_model_binding(&self, binding: impl Fn() -> bool + 'static) { + self.model.set_binding(binding); + } + + /// Call the visitor for the root of each instance + pub fn visit( + &self, + order: TraversalOrder, + mut visitor: crate::item_tree::ItemVisitorRefMut, + ) -> crate::item_tree::VisitChildrenResult { + // We can't keep self.inner borrowed because the event might modify the model + let instance = self.instance.borrow().clone(); + if let Some(c) = instance { + if c.as_pin_ref().visit_children_item(-1, order, visitor.borrow_mut()).has_aborted() { + return crate::item_tree::VisitChildrenResult::abort(0, 0); + } + } + + crate::item_tree::VisitChildrenResult::CONTINUE + } + + /// Return the amount of instances (1 if the conditional is active, 0 otherwise) + pub fn len(&self) -> usize { + self.instance.borrow().is_some() as usize + } + + /// Return the range of indices used by this Conditional. + /// + /// Similar to Repeater::range, but the range is always [0, 1] if the Conditional is active. + pub fn range(&self) -> core::ops::Range { + 0..self.len() + } + + /// Return the instance for the given model index. + /// The index should be within [`Self::range()`] + pub fn instance_at(&self, index: usize) -> Option> { + if index != 0 { + return None; + } + self.instance.borrow().clone() + } + + /// Return true if the Repeater as empty + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Returns a vector containing all instances + pub fn instances_vec(&self) -> Vec> { + self.instance.borrow().clone().into_iter().collect() + } +} + impl From for StandardListViewItem { fn from(value: SharedString) -> Self { StandardListViewItem { text: value } diff --git a/internal/interpreter/dynamic_item_tree.rs b/internal/interpreter/dynamic_item_tree.rs index 98169d623..3b04d6f45 100644 --- a/internal/interpreter/dynamic_item_tree.rs +++ b/internal/interpreter/dynamic_item_tree.rs @@ -118,6 +118,8 @@ pub(crate) struct RepeaterWithinItemTree<'par_id, 'sub_id> { pub(crate) model: Expression, /// Offset of the `Repeater` offset: FieldOffset, Repeater>, + /// Whether this is a `if` or a `for` + is_conditional: bool, } impl RepeatedItemTree for ErasedItemTreeBox { @@ -1052,6 +1054,7 @@ pub(crate) fn generate_item_tree<'id>( let base_component = item.base_type.as_component(); self.repeater_names.insert(item.id.clone(), self.repeater.len()); generativity::make_guard!(guard); + let repeated_element_info = item.repeated.as_ref().unwrap(); self.repeater.push( RepeaterWithinItemTree { item_tree_to_repeat: generate_item_tree( @@ -1062,7 +1065,8 @@ pub(crate) fn generate_item_tree<'id>( guard, ), offset: self.type_builder.add_field_type::>(), - model: item.repeated.as_ref().unwrap().model.clone(), + model: repeated_element_info.model.clone(), + is_conditional: repeated_element_info.is_conditional_element, } .into(), ); @@ -1701,14 +1705,23 @@ pub fn instantiate( let repeater = rep_in_comp.offset.apply_pin(instance_ref.instance); let expr = rep_in_comp.model.clone(); let model_binding_closure = make_binding_eval_closure(expr, &self_weak); - repeater.set_model_binding(move || { - let m = model_binding_closure(); - if let Value::Model(m) = m { - m.clone() - } else { - ModelRc::new(crate::value_model::ValueModel::new(m)) - } - }); + if rep_in_comp.is_conditional { + let bool_model = Rc::new(crate::value_model::BoolModel::default()); + repeater.set_model_binding(move || { + let v = model_binding_closure(); + bool_model.set_value(v.try_into().expect("condition model is bool")); + ModelRc::from(bool_model.clone()) + }); + } else { + repeater.set_model_binding(move || { + let m = model_binding_closure(); + if let Value::Model(m) = m { + m.clone() + } else { + ModelRc::new(crate::value_model::ValueModel::new(m)) + } + }); + } } self_rc } diff --git a/internal/interpreter/value_model.rs b/internal/interpreter/value_model.rs index ac991bf6b..10f8974d2 100644 --- a/internal/interpreter/value_model.rs +++ b/internal/interpreter/value_model.rs @@ -2,7 +2,8 @@ // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 use crate::api::Value; -use i_slint_core::model::{Model, ModelTracker}; +use core::cell::Cell; +use i_slint_core::model::{Model, ModelNotify, ModelTracker}; pub struct ValueModel { value: Value, @@ -81,3 +82,36 @@ impl Model for ValueModel { self } } + +/// A model for conditional elements +#[derive(Default)] +pub(crate) struct BoolModel { + value: Cell, + notify: ModelNotify, +} + +impl Model for BoolModel { + type Data = Value; + fn row_count(&self) -> usize { + if self.value.get() { + 1 + } else { + 0 + } + } + fn row_data(&self, row: usize) -> Option { + (row == 0 && self.value.get()).then_some(Value::Void) + } + fn model_tracker(&self) -> &dyn ModelTracker { + &self.notify + } +} + +impl BoolModel { + pub fn set_value(&self, val: bool) { + let old = self.value.replace(val); + if old != val { + self.notify.reset(); + } + } +} diff --git a/tests/cases/models/if_dirty.slint b/tests/cases/models/if_dirty.slint new file mode 100644 index 000000000..711c2b04a --- /dev/null +++ b/tests/cases/models/if_dirty.slint @@ -0,0 +1,146 @@ +// Copyright © SixtyFPS GmbH +// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0 + +// Test for 3953 + +export component TestCase inherits Window { + width: 100phx; + height: 100phx; + + in property value: 42; + in-out property r; + + if value > 0 : TouchArea { + property cL; + width: 50%; + x: 0px; + clicked => { + cL += 1; + r += "clickL(" + cL + ")."; + } + init => { + r += "initL(" + cL + ")."; + } + + } + + if value.mod(2) == 0 : TouchArea { + property cR; + width: 50%; + x: parent.width - self.width; + clicked => { + cR += 1; + r += "clickR(" + cR + ")."; + } + init => { + r += "initR(" + cR + ")."; + } + } + +} + + + + +/* +```cpp +auto handle = TestCase::create(); +const TestCase &instance = *handle; + +// condition is false +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq(instance.get_r(), "initR(0).initL(0).clickL(1)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq(instance.get_r(), "initR(0).initL(0).clickL(1).clickR(1)."); +instance.set_r(""); + +instance.set_value(-42); +assert_eq(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq(instance.get_r(), "clickR(2)."); +instance.set_r(""); + +instance.set_value(48); +assert_eq(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq(instance.get_r(), "initL(0).clickL(1)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq(instance.get_r(), "initL(0).clickL(1).clickR(3)."); +instance.set_r(""); + +instance.set_value(49); +assert_eq(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq(instance.get_r(), "clickL(2)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq(instance.get_r(), "clickL(2)."); +``` + + +```rust +let instance = TestCase::new().unwrap(); + +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq!(instance.get_r(), "initR(0).initL(0).clickL(1)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq!(instance.get_r(), "initR(0).initL(0).clickL(1).clickR(1)."); +instance.set_r("".into()); + +instance.set_value(-42); +assert_eq!(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq!(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq!(instance.get_r(), "clickR(2)."); +instance.set_r("".into()); + +instance.set_value(48); +assert_eq!(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq!(instance.get_r(), "initL(0).clickL(1)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq!(instance.get_r(), "initL(0).clickL(1).clickR(3)."); +instance.set_r("".into()); + +instance.set_value(49); +assert_eq!(instance.get_r(), ""); +slint_testing::send_mouse_click(&instance, 5., 5.); +assert_eq!(instance.get_r(), "clickL(2)."); +slint_testing::send_mouse_click(&instance, 95., 5.); +assert_eq!(instance.get_r(), "clickL(2)."); +``` + +```js +var instance = new slint.TestCase(); +slintlib.private_api.send_mouse_click(instance, 5., 5.); +assert.equal(instance.r, "initR(0).initL(0).clickL(1)."); +slintlib.private_api.send_mouse_click(instance, 95., 5.); +assert.equal(instance.r, "initR(0).initL(0).clickL(1).clickR(1)."); +instance.r = ""; + +instance.value = -42; +assert.equal(instance.r, ""); +slintlib.private_api.send_mouse_click(instance, 5., 5.); +assert.equal(instance.r, ""); +slintlib.private_api.send_mouse_click(instance, 95., 5.); +assert.equal(instance.r, "clickR(2)."); +instance.r = ""; + +instance.value = 48; +assert.equal(instance.r, ""); +slintlib.private_api.send_mouse_click(instance, 5., 5.); +assert.equal(instance.r, "initL(0).clickL(1)."); +slintlib.private_api.send_mouse_click(instance, 95., 5.); +assert.equal(instance.r, "initL(0).clickL(1).clickR(3)."); +instance.r = ""; + +instance.value = 49; +assert.equal(instance.r, ""); +slintlib.private_api.send_mouse_click(instance, 5., 5.); +assert.equal(instance.r, "clickL(2)."); +slintlib.private_api.send_mouse_click(instance, 95., 5.); +assert.equal(instance.r, "clickL(2)."); +``` +*/