From 0cb827a9018305eb4b51a15a26c09d8f417493a8 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 24 Sep 2020 14:10:52 +0200 Subject: [PATCH] Two ways binding with optimized public property in the interpreter --- sixtyfps_runtime/corelib/items.rs | 2 +- .../interpreter/dynamic_component.rs | 3 + sixtyfps_runtime/interpreter/eval.rs | 69 ++++++++++++++----- sixtyfps_runtime/interpreter/lib.rs | 41 +++++++---- tests/cases/two_way_simple.60 | 23 +++++++ 5 files changed, 104 insertions(+), 34 deletions(-) diff --git a/sixtyfps_runtime/corelib/items.rs b/sixtyfps_runtime/corelib/items.rs index 8311dc232..b6dfeef2e 100644 --- a/sixtyfps_runtime/corelib/items.rs +++ b/sixtyfps_runtime/corelib/items.rs @@ -746,7 +746,7 @@ pub unsafe extern "C" fn sixtyfps_flickable_data_free(data: *mut FlickableDataBo /// The implementation of the `PropertyAnimation` element #[repr(C)] -#[derive(FieldOffsets, Default, BuiltinItem, Clone)] +#[derive(FieldOffsets, Default, BuiltinItem, Clone, Debug)] #[pin] pub struct PropertyAnimation { #[rtti_field] diff --git a/sixtyfps_runtime/interpreter/dynamic_component.rs b/sixtyfps_runtime/interpreter/dynamic_component.rs index 98e68ca61..b00ff2996 100644 --- a/sixtyfps_runtime/interpreter/dynamic_component.rs +++ b/sixtyfps_runtime/interpreter/dynamic_component.rs @@ -512,6 +512,9 @@ fn generate_component<'id>( } for (name, decl) in &root_component.root_element.borrow().property_declarations { + if decl.is_alias.is_some() { + continue; + } let (prop, type_info) = match decl.property_type { Type::Float32 => animated_property_info::(), Type::Int32 => animated_property_info::(), diff --git a/sixtyfps_runtime/interpreter/eval.rs b/sixtyfps_runtime/interpreter/eval.rs index 25ec58686..7ab32efba 100644 --- a/sixtyfps_runtime/interpreter/eval.rs +++ b/sixtyfps_runtime/interpreter/eval.rs @@ -194,18 +194,20 @@ pub fn eval_expression( "naked builtin function reference not allowed, should be handled by function call" ), Expression::PropertyReference(NamedReference { element, name }) => { - load_property(component, &element.upgrade().unwrap(), name.as_ref()) + load_property(component, &element.upgrade().unwrap(), name.as_ref()).unwrap() } Expression::RepeaterIndexReference { element } => load_property( component, &element.upgrade().unwrap().borrow().base_type.as_component().root_element, "index", - ), + ) + .unwrap(), Expression::RepeaterModelReference { element } => load_property( component, &element.upgrade().unwrap().borrow().base_type.as_component().root_element, "model_data", - ), + ) + .unwrap(), Expression::FunctionParameterReference { index, .. } => { local_context.function_arguments[*index].clone() } @@ -275,18 +277,19 @@ pub fn eval_expression( } Expression::SelfAssignment { lhs, rhs, op } => match &**lhs { Expression::PropertyReference(NamedReference { element, name }) => { - let mut eval = |lhs| { - let rhs = eval_expression(&**rhs, component, local_context); - match (lhs, rhs, op) { - (_, rhs, '=') => rhs, - (Value::Number(a), Value::Number(b), '+') => Value::Number(a + b), - (Value::Number(a), Value::Number(b), '-') => Value::Number(a - b), - (Value::Number(a), Value::Number(b), '/') => Value::Number(a / b), - (Value::Number(a), Value::Number(b), '*') => Value::Number(a * b), - (lhs, rhs, op) => panic!("unsupported {:?} {} {:?}", lhs, op, rhs), - } + let rhs = eval_expression(&**rhs, component, local_context); + if *op == '=' { + store_property(component, &element.upgrade().unwrap(), name.as_ref(), rhs) + .unwrap(); + return Value::Void; + } + let eval = |lhs| match (lhs, rhs, op) { + (Value::Number(a), Value::Number(b), '+') => Value::Number(a + b), + (Value::Number(a), Value::Number(b), '-') => Value::Number(a - b), + (Value::Number(a), Value::Number(b), '/') => Value::Number(a / b), + (Value::Number(a), Value::Number(b), '*') => Value::Number(a * b), + (lhs, rhs, op) => panic!("unsupported {:?} {} {:?}", lhs, op, rhs), }; - let element = element.upgrade().unwrap(); generativity::make_guard!(guard); let enclosing_component = @@ -386,16 +389,14 @@ pub fn eval_expression( } } -fn load_property(component: InstanceRef, element: &ElementRc, name: &str) -> Value { +pub fn load_property(component: InstanceRef, element: &ElementRc, name: &str) -> Result { generativity::make_guard!(guard); let enclosing_component = enclosing_component_for_element(&element, component, guard); let element = element.borrow(); if element.id == element.enclosing_component.upgrade().unwrap().root_element.borrow().id { if let Some(x) = enclosing_component.component_type.custom_properties.get(name) { return unsafe { - x.prop - .get(Pin::new_unchecked(&*enclosing_component.as_ptr().add(x.offset))) - .unwrap() + x.prop.get(Pin::new_unchecked(&*enclosing_component.as_ptr().add(x.offset))) }; } }; @@ -406,7 +407,37 @@ fn load_property(component: InstanceRef, element: &ElementRc, name: &str) -> Val .unwrap_or_else(|| panic!("Unkown element for {}.{}", element.id, name)); core::mem::drop(element); let item = unsafe { item_info.item_from_component(enclosing_component.as_ptr()) }; - item_info.rtti.properties[name].get(item) + Ok(item_info.rtti.properties.get(name).ok_or(())?.get(item)) +} + +pub fn store_property( + component_instance: InstanceRef, + element: &ElementRc, + name: &str, + value: Value, +) -> Result<(), ()> { + generativity::make_guard!(guard); + let enclosing_component = enclosing_component_for_element(&element, component_instance, guard); + let maybe_animation = crate::dynamic_component::animation_for_property( + enclosing_component, + &element.borrow().property_animations, + name, + ); + + let component = element.borrow().enclosing_component.upgrade().unwrap(); + if element.borrow().id == component.root_element.borrow().id { + if let Some(x) = enclosing_component.component_type.custom_properties.get(name) { + unsafe { + let p = Pin::new_unchecked(&*enclosing_component.as_ptr().add(x.offset)); + return x.prop.set(p, value, maybe_animation); + } + } + }; + let item_info = &enclosing_component.component_type.items[element.borrow().id.as_str()]; + let item = unsafe { item_info.item_from_component(enclosing_component.as_ptr()) }; + let p = &item_info.rtti.properties.get(name).ok_or(())?; + p.set(item, value, maybe_animation); + Ok(()) } pub fn window_ref(component: InstanceRef) -> Option { diff --git a/sixtyfps_runtime/interpreter/lib.rs b/sixtyfps_runtime/interpreter/lib.rs index 6f1acd272..b3f8f145d 100644 --- a/sixtyfps_runtime/interpreter/lib.rs +++ b/sixtyfps_runtime/interpreter/lib.rs @@ -68,19 +68,19 @@ impl<'id> dynamic_component::ComponentDescription<'id> { if !core::ptr::eq((&self.ct) as *const _, component.get_vtable() as *const _) { return Err(()); } - let x = self.custom_properties.get(name).ok_or(())?; generativity::make_guard!(guard); - let maybe_animation = dynamic_component::animation_for_property( - unsafe { InstanceRef::from_pin_ref(component, guard) }, - &self.original.root_element.borrow().property_animations, - name, - ); - unsafe { - x.prop.set( - Pin::new_unchecked(&*component.as_ptr().add(x.offset)), - value, - maybe_animation, - ) + let c = unsafe { InstanceRef::from_pin_ref(component, guard) }; + if let Some(alias) = self + .original + .root_element + .borrow() + .property_declarations + .get(name) + .and_then(|d| d.is_alias.as_ref()) + { + eval::store_property(c, &alias.element.upgrade().unwrap(), &alias.name, value) + } else { + eval::store_property(c, &self.original.root_element, name, value) } } @@ -114,8 +114,21 @@ impl<'id> dynamic_component::ComponentDescription<'id> { if !core::ptr::eq((&self.ct) as *const _, component.get_vtable() as *const _) { return Err(()); } - let x = self.custom_properties.get(name).ok_or(())?; - unsafe { x.prop.get(Pin::new_unchecked(&*component.as_ptr().add(x.offset))) } + if let Some(alias) = self + .original + .root_element + .borrow() + .property_declarations + .get(name) + .and_then(|d| d.is_alias.as_ref()) + { + generativity::make_guard!(guard); + let c = unsafe { InstanceRef::from_pin_ref(component, guard) }; + eval::load_property(c, &alias.element.upgrade().unwrap(), &alias.name) + } else { + let x = self.custom_properties.get(name).ok_or(())?; + unsafe { x.prop.get(Pin::new_unchecked(&*component.as_ptr().add(x.offset))) } + } } /// Sets an handler for a signal diff --git a/tests/cases/two_way_simple.60 b/tests/cases/two_way_simple.60 index 831e44590..cd9463515 100644 --- a/tests/cases/two_way_simple.60 +++ b/tests/cases/two_way_simple.60 @@ -76,5 +76,28 @@ assert_eq(instance.get_sub_foo1(), 15); assert_eq(instance.get_sub_foo2(), 15); ``` +```js +var instance = new sixtyfps.TestCase({}); +assert.equal(instance.sub_width1, 80.); +assert.equal(instance.sub_width2, 80.); +instance.sub_width1 = (99.); +assert.equal(instance.sub_width1, 99.); +assert.equal(instance.sub_width2, 99.); + +// breaks the binding +instance.sub_width2 = (23.); +assert.equal(instance.sub_width1, 99.); +assert.equal(instance.sub_width2, 23.); +instance.sub_width1 = (88.); +assert.equal(instance.sub_width1, 88.); +assert.equal(instance.sub_width2, 23.); + + +assert.equal(instance.sub_foo1, 44); +assert.equal(instance.sub_foo2, 44); +instance.sub_foo1 = (15); +assert.equal(instance.sub_foo1, 15); +assert.equal(instance.sub_foo2, 15); +``` */