From f5aeb9ba6094def76cd9ce3c38b9feca9f7de4bb Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 4 Sep 2020 15:35:49 +0200 Subject: [PATCH] Only the computation of the model needs to be done in the evaluation scope for it Otherwise any change in any of the properties of the delegate will cause the model to be reset. --- .../include/sixtyfps_properties.h | 9 +++- sixtyfps_compiler/generator/cpp.rs | 16 ++++--- sixtyfps_compiler/generator/rust.rs | 19 ++++----- .../interpreter/dynamic_component.rs | 42 +++++++++---------- 4 files changed, 44 insertions(+), 42 deletions(-) diff --git a/api/sixtyfps-cpp/include/sixtyfps_properties.h b/api/sixtyfps-cpp/include/sixtyfps_properties.h index 53a4c7a26..dbedf15ed 100644 --- a/api/sixtyfps-cpp/include/sixtyfps_properties.h +++ b/api/sixtyfps-cpp/include/sixtyfps_properties.h @@ -124,7 +124,7 @@ struct PropertyTracker } template - void evaluate(const F &f) const { + auto evaluate(const F &f) const -> std::enable_if_t> { cbindgen_private::sixtyfps_property_tracker_evaluate( &inner, [](void *f){ (*reinterpret_cast(f))(); }, @@ -132,6 +132,13 @@ struct PropertyTracker ); } + template + auto evaluate(const F &f) const -> std::enable_if_t, decltype(f())> { + decltype(f()) result; + this->evaluate([&] { result = f(); } ); + return result; + } + private: cbindgen_private::PropertyTrackerOpaque inner; }; diff --git a/sixtyfps_compiler/generator/cpp.rs b/sixtyfps_compiler/generator/cpp.rs index 9b07a9d1f..09eba1c28 100644 --- a/sixtyfps_compiler/generator/cpp.rs +++ b/sixtyfps_compiler/generator/cpp.rs @@ -373,13 +373,11 @@ fn handle_repeater( let repeater_id = format!("repeater_{}", base_component.parent_element.upgrade().unwrap().borrow().id); - let model = compile_expression(&repeated.model, parent_component); - let model = if !repeated.is_conditional_element { - format!("{}.get()", model) - } else { + let mut model = compile_expression(&repeated.model, parent_component); + if repeated.is_conditional_element { // bool converts to int // FIXME: don't do a heap allocation here - format!("std::make_shared({}).get()", model) + model = format!("std::make_shared({})", model) }; if repeated.model.is_constant() { @@ -389,7 +387,7 @@ fn handle_repeater( i = repeater_count )); init.push(format!( - "self->{repeater_id}.update_model({model}, self);", + "self->{repeater_id}.update_model({model}.get(), self);", repeater_id = repeater_id, model = model, )); @@ -406,9 +404,9 @@ fn handle_repeater( children_visitor_cases.push(format!( "\n case {i}: {{ if (self->model_{i}.is_dirty()) {{ - self->model_{i}.evaluate([&] {{ - self->{id}.update_model({model}, self); - }}); + self->{id}.update_model(self->model_{i}.evaluate([&] {{ + return {model}; + }}).get(), self); }} return self->{id}.visit(order, visitor); }}", diff --git a/sixtyfps_compiler/generator/rust.rs b/sixtyfps_compiler/generator/rust.rs index 100ab48ea..3b604291a 100644 --- a/sixtyfps_compiler/generator/rust.rs +++ b/sixtyfps_compiler/generator/rust.rs @@ -233,12 +233,12 @@ fn generate_component( let mut model = compile_expression(&repeated.model, component); if repeated.is_conditional_element { - model = quote!((if #model {Some(())} else {None}).iter().cloned()) + model = quote!((if #model {Some(())} else {None})) } if repeated.model.is_constant() { init.push(quote! { - self_pinned.#repeater_id.update_model(#model, || { + self_pinned.#repeater_id.update_model((#model).into_iter(), || { #rep_component_id::new(self_pinned.self_weak.get().unwrap().clone()) }); }); @@ -250,12 +250,13 @@ fn generate_component( repeated_visit_branch.push(quote!( #repeater_index => { if self_pinned.#model_name.is_dirty() { - #component_id::FIELD_OFFSETS.#model_name.apply_pin(self_pinned).evaluate(|| { - let _self = self_pinned.clone(); - self_pinned.#repeater_id.update_model(#model, || { - #rep_component_id::new(self_pinned.self_weak.get().unwrap().clone()) - }); - }); + self_pinned.#repeater_id.update_model( + #component_id::FIELD_OFFSETS.#model_name.apply_pin(self_pinned).evaluate(|| { + let _self = self_pinned.clone(); + #model + }).into_iter(), + || { #rep_component_id::new(self_pinned.self_weak.get().unwrap().clone()) } + ); } self_pinned.#repeater_id.visit(order, visitor) } @@ -683,7 +684,6 @@ fn compile_expression(e: &Expression, component: &Rc) -> TokenStream quote!(sixtyfps::re_exports::SharedString::from(format!("{}", #f).as_str())) } (Type::Float32, Type::Model) | (Type::Int32, Type::Model) => quote!((0..#f as i32)), - (Type::Array(_), Type::Model) => quote!(#f.iter().cloned()), (Type::Float32, Type::Color) => { quote!(sixtyfps::re_exports::Color::from_argb_encoded(#f as u32)) } @@ -705,7 +705,6 @@ fn compile_expression(e: &Expression, component: &Rc) -> TokenStream let window_ref = window_ref_expression(component); quote!(#window_ref.scale_factor()) } - BuiltinFunction::Debug => { quote!(println!("FIXME: the debug statement in rust should print the argument");) } diff --git a/sixtyfps_runtime/interpreter/dynamic_component.rs b/sixtyfps_runtime/interpreter/dynamic_component.rs index a4083818a..87df14849 100644 --- a/sixtyfps_runtime/interpreter/dynamic_component.rs +++ b/sixtyfps_runtime/interpreter/dynamic_component.rs @@ -221,32 +221,30 @@ extern "C" fn visit_children_item( if let Some(listener_offset) = rep_in_comp.property_tracker { let listener = listener_offset.apply_pin(instance); if listener.is_dirty() { - listener.evaluate(|| { - match eval::eval_expression( + match listener.evaluate(|| { + eval::eval_expression( &rep_in_comp.model, InstanceRef { instance, component_type }, &mut Default::default(), - ) { - crate::Value::Number(count) => populate_model( - &mut *vec, - rep_in_comp, - component, - (0..count as i32) - .into_iter() - .map(|v| crate::Value::Number(v as f64)), - ), - crate::Value::Array(a) => { - populate_model(&mut *vec, rep_in_comp, component, a.into_iter()) - } - crate::Value::Bool(b) => populate_model( - &mut *vec, - rep_in_comp, - component, - (if b { Some(crate::Value::Void) } else { None }).into_iter(), - ), - _ => panic!("Unsupported model"), + ) + }) { + crate::Value::Number(count) => populate_model( + &mut *vec, + rep_in_comp, + component, + (0..count as i32).into_iter().map(|v| crate::Value::Number(v as f64)), + ), + crate::Value::Array(a) => { + populate_model(&mut *vec, rep_in_comp, component, a.into_iter()) } - }); + crate::Value::Bool(b) => populate_model( + &mut *vec, + rep_in_comp, + component, + (if b { Some(crate::Value::Void) } else { None }).into_iter(), + ), + _ => panic!("Unsupported model"), + } } } match order {