Two ways binding works also work as three ways binding and more

A side effect is that the order of calling set_binding and link_two_ways is no longer relevant

api/sixtyfps-cpp/include/sixtyfps_properties.h

@@ -60,7 +60,7 @@ struct Property
                     *reinterpret_cast<T *>(value) = (*reinterpret_cast<F *>(user_data))();
                 },
                 new F(binding), [](void *user_data) { delete reinterpret_cast<F *>(user_data); },
-                nullptr);
+                nullptr, nullptr);
     }
 
     inline void set_animated_value(const T &value,
@@ -91,10 +91,16 @@ struct Property
                 *reinterpret_cast<const T *>(value));
             return true;
         };
+        auto intercept_binding_fn = [] (void *user_data, void *value) {
+            cbindgen_private::sixtyfps_property_set_binding_internal(
+                &reinterpret_cast<TwoWayBinding *>(user_data)->common_property->inner,
+                value);
+            return true;
+        };
         cbindgen_private::sixtyfps_property_set_binding(&p1->inner, call_fn,
-            new TwoWayBinding{common_property}, del_fn, intercept_fn);
+            new TwoWayBinding{common_property}, del_fn, intercept_fn, intercept_binding_fn);
         cbindgen_private::sixtyfps_property_set_binding(&p2->inner, call_fn,
-            new TwoWayBinding{common_property}, del_fn, intercept_fn);
+            new TwoWayBinding{common_property}, del_fn, intercept_fn, intercept_binding_fn);
     }
 
     /// Internal (private) constructor used by link_two_way

sixtyfps_compiler/generator/cpp.rs

@@ -306,12 +306,7 @@ fn property_set_binding_code(
     }
 }
 
-fn handle_item(
+fn handle_item(elem: &ElementRc, main_struct: &mut Struct, init: &mut Vec<String>) {
-    elem: &ElementRc,
-    main_struct: &mut Struct,
-    init_begin: &mut Vec<String>,
-    init_end: &mut Vec<String>,
-) {
     let item = elem.borrow();
     main_struct.members.push((
         Access::Private,
@@ -325,7 +320,7 @@ fn handle_item(
     let component = item.enclosing_component.upgrade().unwrap();
 
     let id = &item.id;
-    for (prop_name, binding_expression) in &item.bindings {
+    init.extend(item.bindings.iter().map(|(prop_name, binding_expression)| {
         let prop_ty = item.lookup_property(prop_name.as_str());
         if let Type::Signal { args } = &prop_ty {
             let signal_accessor_prefix = if item.property_declarations.contains_key(prop_name) {
@@ -337,7 +332,7 @@ fn handle_item(
                 format!("[[maybe_unused]] {} arg_{}", ty.cpp_type().unwrap_or_default(), i)
             });
 
-            init_begin.push(format!(
+            format!(
                 "{signal_accessor_prefix}{prop}.set_handler(
                     [this]({params}) {{
                         [[maybe_unused]] auto self = this;
@@ -347,14 +342,14 @@ fn handle_item(
                 prop = prop_name,
                 params = params.join(", "),
                 code = compile_expression(binding_expression, &component)
-            ));
+            )
         } else if let Expression::TwoWayBinding(nr) = &binding_expression.expression {
-            init_end.push(format!(
+            format!(
                 "sixtyfps::Property<{ty}>::link_two_way(&{p1}, &{p2});",
                 ty = prop_ty.cpp_type().unwrap_or_default(),
                 p1 = access_member(elem, prop_name, &component, "this"),
                 p2 = access_member(&nr.element.upgrade().unwrap(), &nr.name, &component, "this")
-            ));
+            )
         } else {
             let accessor_prefix = if item.property_declarations.contains_key(prop_name) {
                 String::new()
@@ -377,25 +372,23 @@ fn handle_item(
                 );
                 property_set_binding_code(component, &item, prop_name, binding_code)
             };
-            init_begin.push(
+            if let Some(vp) = super::as_flickable_viewport_property(elem, prop_name) {
-                if let Some(vp) = super::as_flickable_viewport_property(elem, prop_name) {
+                format!(
-                    format!(
+                    "{accessor_prefix}viewport.{cpp_prop}.{setter};",
-                        "{accessor_prefix}viewport.{cpp_prop}.{setter};",
+                    accessor_prefix = accessor_prefix,
-                        accessor_prefix = accessor_prefix,
+                    cpp_prop = vp,
-                        cpp_prop = vp,
+                    setter = setter,
-                        setter = setter,
+                )
-                    )
+            } else {
-                } else {
+                format!(
-                    format!(
+                    "{accessor_prefix}{cpp_prop}.{setter};",
-                        "{accessor_prefix}{cpp_prop}.{setter};",
+                    accessor_prefix = accessor_prefix,
-                        accessor_prefix = accessor_prefix,
+                    cpp_prop = prop_name,
-                        cpp_prop = prop_name,
+                    setter = setter,
-                        setter = setter,
+                )
-                    )
+            }
-                },
-            );
         }
-    }
+    }));
 }
 
 fn handle_repeater(
@@ -537,7 +530,6 @@ fn generate_component(
 
     let is_root = component.parent_element.upgrade().is_none();
     let mut init = vec!["[[maybe_unused]] auto self = this;".into()];
-    let mut init_last = vec![];
 
     for (cpp_name, property_decl) in component.root_element.borrow().property_declarations.iter() {
         let ty = if let Type::Signal { args } = &property_decl.property_type {
@@ -777,12 +769,10 @@ fn generate_component(
                 if super::is_flickable(item_rc) { 1 } else { item.children.len() },
                 children_offset,
             ));
-            handle_item(item_rc, &mut component_struct, &mut init, &mut init_last);
+            handle_item(item_rc, &mut component_struct, &mut init);
         }
     });
 
-    init.append(&mut init_last);
-
     component_struct.members.push((
         Access::Public,
         Declaration::Function(Function {

sixtyfps_compiler/generator/rust.rs

@@ -251,7 +251,6 @@ fn generate_component(
     let mut repeated_key_event_branch = Vec::new();
     let mut repeated_focus_branch = Vec::new();
     let mut init = Vec::new();
-    let mut init_last = Vec::new();
     let mut maybe_window_field_decl = None;
     let mut maybe_window_field_init = None;
     super::build_array_helper(component, |item_rc, children_index, is_flickable_rect| {
@@ -386,7 +385,7 @@ fn generate_component(
                         quote!(self_pinned.as_ref()),
                         false,
                     );
-                    init_last.push(quote!(
+                    init.push(quote!(
                         Property::link_two_way(#rust_property, #p2);
                     ));
                 } else {
@@ -676,7 +675,6 @@ fn generate_component(
                 self_pinned.self_weak.set(PinWeak::downgrade(self_pinned.clone())).map_err(|_|())
                     .expect("Can only be pinned once");
                 #(#init)*
-                #(#init_last)*
                 self_pinned
             }
             #(#property_and_signal_accessors)*

sixtyfps_compiler/passes/remove_aliases.rs

@@ -117,11 +117,16 @@ fn process_alias<'a>(
             // Cannot remove if this is not a declaration
             return;
         }
-        let x = aliases_to_invert.insert(
+        let k = NamedReference { element: Rc::downgrade(&from.0), name: from.1.to_string() };
-            NamedReference { element: Rc::downgrade(&from.0), name: from.1.to_string() },
+        match aliases_to_invert.entry(k) {
-            NamedReference { element: Rc::downgrade(&to.0), name: to.1.to_string() },
+            Entry::Occupied(_) => {
-        );
+                // TODO: maybe there are still way to optimize (three way bindings)
-        assert!(x.is_none())
+                return;
+            }
+            Entry::Vacant(e) => {
+                e.insert(NamedReference { element: Rc::downgrade(&to.0), name: to.1.to_string() });
+            }
+        }
     } else if !is_declaration(&from) {
         // Cannot remove if this is not a declaration
         return;

sixtyfps_runtime/corelib/properties.rs

@@ -97,6 +97,8 @@ struct BindingVTable {
     evaluate: unsafe fn(_self: *mut BindingHolder, value: *mut ()) -> BindingResult,
     mark_dirty: unsafe fn(_self: *const BindingHolder),
     intercept_set: unsafe fn(_self: *const BindingHolder, value: *const ()) -> bool,
+    intercept_set_binding:
+        unsafe fn(_self: *const BindingHolder, new_binding: *mut BindingHolder) -> bool,
 }
 
 /// A binding trait object can be used to dynamically produces values for a property.
@@ -109,7 +111,7 @@ trait BindingCallable {
     /// and therefore this binding may evaluate to a different value, too.
     fn mark_dirty(self: Pin<&Self>) {}
 
-    /// Allow the binding to t what happens when the value is set.
+    /// Allow the binding to intercept what happens when the value is set.
     /// The default implementation returns false, meaning the binding will simply be removed and
     /// the property will get the new value.
     /// When returning true, the call was intercepted and the binding will not be removed,
@@ -117,6 +119,13 @@ trait BindingCallable {
     unsafe fn intercept_set(self: Pin<&Self>, _value: *const ()) -> bool {
         false
     }
+
+    /// Allow the binding to intercept what happens when the value is set.
+    /// The default implementation returns false, meaning the binding will simply be removed.
+    /// When returning true, the call was intercepted and the binding will not be removed.
+    unsafe fn intercept_set_binding(self: Pin<&Self>, _new_binding: *mut BindingHolder) -> bool {
+        false
+    }
 }
 
 impl<F: Fn(*mut ()) -> BindingResult> BindingCallable for F {
@@ -172,6 +181,14 @@ fn alloc_binding_holder<B: BindingCallable + 'static>(binding: B) -> *mut Bindin
         Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding)).intercept_set(value)
     }
 
+    unsafe fn intercept_set_binding<B: BindingCallable>(
+        _self: *const BindingHolder,
+        new_binding: *mut BindingHolder,
+    ) -> bool {
+        Pin::new_unchecked(&((*(_self as *const BindingHolder<B>)).binding))
+            .intercept_set_binding(new_binding)
+    }
+
     trait HasBindingVTable {
         const VT: &'static BindingVTable;
     }
@@ -181,6 +198,7 @@ fn alloc_binding_holder<B: BindingCallable + 'static>(binding: B) -> *mut Bindin
             evaluate: evaluate::<B>,
             mark_dirty: mark_dirty::<B>,
             intercept_set: intercept_set::<B>,
+            intercept_set_binding: intercept_set_binding::<B>,
         };
     }
 
@@ -347,8 +365,22 @@ impl PropertyHandle {
     }
 
     fn set_binding<B: BindingCallable + 'static>(&self, binding: B) {
-        self.remove_binding();
         let binding = alloc_binding_holder::<B>(binding);
+        self.set_binding_impl(binding);
+    }
+
+    /// Implementation of Self::set_binding.
+    fn set_binding_impl(&self, binding: *mut BindingHolder) {
+        if self.access(|b| {
+            b.map_or(false, |b| unsafe {
+                // Safety: b is a BindingHolder<T>
+                (b.vtable.intercept_set_binding)(&*b as *const BindingHolder, binding)
+            })
+        }) {
+            return;
+        }
+
+        self.remove_binding();
         debug_assert!((binding as usize) & 0b11 == 0);
         debug_assert!(self.handle.get() & 0b11 == 0);
         unsafe {
@@ -1110,6 +1142,7 @@ impl Default for PropertyTracker {
             evaluate: |_, _| BindingResult::KeepBinding,
             mark_dirty: |_| (),
             intercept_set: |_, _| false,
+            intercept_set_binding: |_, _| false,
         };
 
         let holder = BindingHolder {
@@ -1219,6 +1252,9 @@ pub(crate) mod ffi {
         intercept_set: Option<
             extern "C" fn(user_data: *mut c_void, pointer_to_value: *const c_void) -> bool,
         >,
+        intercept_set_binding: Option<
+            extern "C" fn(user_data: *mut c_void, new_binding: *mut c_void) -> bool,
+        >,
     ) -> impl BindingCallable {
         struct CFunctionBinding<T> {
             binding_function: extern "C" fn(*mut c_void, *mut T),
@@ -1227,6 +1263,8 @@ pub(crate) mod ffi {
             intercept_set: Option<
                 extern "C" fn(user_data: *mut c_void, pointer_to_value: *const c_void) -> bool,
             >,
+            intercept_set_binding:
+                Option<extern "C" fn(user_data: *mut c_void, new_binding: *mut c_void) -> bool>,
         }
 
         impl<T> Drop for CFunctionBinding<T> {
@@ -1248,9 +1286,24 @@ pub(crate) mod ffi {
                     Some(intercept_set) => intercept_set(self.user_data, value),
                 }
             }
+            unsafe fn intercept_set_binding(
+                self: Pin<&Self>,
+                new_binding: *mut BindingHolder,
+            ) -> bool {
+                match self.intercept_set_binding {
+                    None => false,
+                    Some(intercept_set_b) => intercept_set_b(self.user_data, new_binding.cast()),
+                }
+            }
         }
 
-        CFunctionBinding { binding_function: binding, user_data, drop_user_data, intercept_set }
+        CFunctionBinding {
+            binding_function: binding,
+            user_data,
+            drop_user_data,
+            intercept_set,
+            intercept_set_binding,
+        }
     }
 
     /// Set a binding
@@ -1267,13 +1320,33 @@ pub(crate) mod ffi {
         user_data: *mut c_void,
         drop_user_data: Option<extern "C" fn(*mut c_void)>,
         intercept_set: Option<
-            extern "C" fn(user_data: *mut c_void, pointer_to_value: *const c_void) -> bool,
+            extern "C" fn(user_data: *mut c_void, pointer_to_Value: *const c_void) -> bool,
+        >,
+        intercept_set_binding: Option<
+            extern "C" fn(user_data: *mut c_void, new_binding: *mut c_void) -> bool,
         >,
     ) {
-        let binding = make_c_function_binding(binding, user_data, drop_user_data, intercept_set);
+        let binding = make_c_function_binding(
+            binding,
+            user_data,
+            drop_user_data,
+            intercept_set,
+            intercept_set_binding,
+        );
         handle.0.set_binding(binding);
     }
 
+    /// Set a binding using an already allocated building holder
+    ///
+    //// (take ownershipo of the binding)
+    #[no_mangle]
+    pub unsafe extern "C" fn sixtyfps_property_set_binding_internal(
+        handle: &PropertyHandleOpaque,
+        binding: *mut c_void,
+    ) {
+        handle.0.set_binding_impl(binding.cast());
+    }
+
     /// Returns whether the property behind this handle is marked as dirty
     #[no_mangle]
     pub extern "C" fn sixtyfps_property_is_dirty(handle: &PropertyHandleOpaque) -> bool {
@@ -1360,6 +1433,7 @@ pub(crate) mod ffi {
                         user_data,
                         drop_user_data,
                         None,
+                        None,
                     )) as usize)
                         | 0b10,
                 ),

sixtyfps_runtime/interpreter/dynamic_component.rs

@@ -683,8 +683,6 @@ pub fn instantiate<'id>(
         ));
     }
 
-    let mut execute_last = Vec::<Box<dyn FnOnce()>>::new();
-
     for item_within_component in component_type.items.values() {
         unsafe {
             let item = item_within_component.item_from_component(mem);
@@ -736,15 +734,8 @@ pub fn instantiate<'id>(
                         let maybe_animation =
                             animation_for_element_property(instance_ref, &elem, prop);
                         if let Expression::TwoWayBinding(nr) = &expr.expression {
-                            execute_last.push(Box::new({
+                            // Safety: The compiler must have ensured that the properties exist and are of the same type
-                                let prop_rtti = prop_rtti.clone();
+                            prop_rtti.link_two_ways(item, get_property_ptr(&nr, instance_ref));
-                                let nr = nr.clone();
-                                move || {
-                                    // Safety: The compiler must have ensured that the properties exist and are of the same type
-                                    prop_rtti
-                                        .link_two_ways(item, get_property_ptr(&nr, instance_ref));
-                                }
-                            }));
                         } else if expr.is_constant() {
                             prop_rtti.set(
                                 item,
@@ -786,15 +777,7 @@ pub fn instantiate<'id>(
 
                         if let Expression::TwoWayBinding(nr) = &expr.expression {
                             // Safety: The compiler must have ensured that the properties exist and are of the same type
-                            execute_last.push(Box::new({
+                            prop_info.link_two_ways(item, get_property_ptr(&nr, instance_ref));
-                                let prop_info = prop_info.clone();
-                                let nr = nr.clone();
-                                move || {
-                                    // Safety: The compiler must have ensured that the properties exist and are of the same type
-                                    prop_info
-                                        .link_two_ways(item, get_property_ptr(&nr, instance_ref));
-                                }
-                            }));
                         } else if expr.is_constant() {
                             let v =
                                 eval::eval_expression(expr, instance_ref, &mut Default::default());
@@ -830,10 +813,6 @@ pub fn instantiate<'id>(
         }
     }
 
-    for x in execute_last {
-        x();
-    }
-
     for rep_in_comp in &component_type.repeater {
         generativity::make_guard!(guard);
         let rep_in_comp = rep_in_comp.unerase(guard);

tests/cases/two_way_binding.60

@@ -28,6 +28,10 @@ TestCase := Rectangle {
         text: text2;
     }
 
+    Text {
+        text <=> text1;
+    }
+
     signal set_ti1(string);
     set_ti1(a) => { ti1.text = a; }
     signal set_ti2(string);