Add support for tracking the length of a model in Rust

This is done by exposing the ModelNotify to the caller via the new ModelTracker trait, which has a function that allows "hooking" into the dirty tracking of the size. By extension, this also works in JavaScript. cc #98
slint-ui · Oct 20, 2021 · 63bf1af · 63bf1af
1 parent e3d9abb
commit 63bf1af
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ All notable changes to this project will be documented in this file.
 
  - The TouchArea will grab the mouse for every button instead of just the left button
  - The ScrollView's default viewport size is no longer hardcoded to 1000px. Its size now depends on the contents
+ - In Rust, the `sixtyfps::Model` trait deprecates the `attach_peer` function in favor of `model_tracker`, where all
+   you need to do is return a reference to your `sixtyfps::ModelNotify` field.
 
 ### Added
 

diff --git a/api/sixtyfps-node/native/js_model.rs b/api/sixtyfps-node/native/js_model.rs
@@ -89,8 +89,8 @@ impl Model for JsModel {
         r.into_inner()
     }
 
-    fn attach_peer(&self, peer: sixtyfps_corelib::model::ModelPeer) {
-        self.notify.attach(peer)
+    fn model_tracker(&self) -> &dyn sixtyfps_corelib::model::ModelTracker {
+        &self.notify
     }
 
     fn set_row_data(&self, row: usize, data: Self::Data) {

diff --git a/api/sixtyfps-rs/lib.rs b/api/sixtyfps-rs/lib.rs
@@ -219,7 +219,7 @@ pub use sixtyfps_corelib::graphics::{
     Brush, Color, Image, LoadImageError, Rgb8Pixel, Rgba8Pixel, RgbaColor, SharedPixelBuffer,
 };
 pub use sixtyfps_corelib::model::{
-    Model, ModelHandle, ModelNotify, ModelPeer, StandardListViewItem, VecModel,
+    Model, ModelHandle, ModelNotify, ModelPeer, ModelTracker, StandardListViewItem, VecModel,
 };
 pub use sixtyfps_corelib::sharedvector::SharedVector;
 pub use sixtyfps_corelib::string::SharedString;

diff --git a/sixtyfps_compiler/generator/rust.rs b/sixtyfps_compiler/generator/rust.rs
@@ -1239,7 +1239,7 @@ fn compile_expression(expr: &Expression, component: &Rc<Component>) -> TokenStre
                 quote!((|x: Image| -> Size { x.size() }))
             }
             BuiltinFunction::ArrayLength => {
-                quote!((|x: ModelHandle<_>| -> i32 { x.row_count() as i32 }))
+                quote!((|x: ModelHandle<_>| -> i32 { x.model_tracker().track_row_count_changes(); x.row_count() as i32 }))
             }
 
             BuiltinFunction::Rgb => {

diff --git a/sixtyfps_runtime/corelib/model.rs b/sixtyfps_runtime/corelib/model.rs
@@ -31,11 +31,29 @@ pub struct ModelPeer {
     inner: Weak<RefCell<ModelPeerInner>>,
 }
 
+/// This trait defines the interface that users of a model can use to track changes
+/// to a model. It is supplied via [`Model::model_tracker`] and implementation usually
+/// return a reference to its field of [`ModelNotify`].
+pub trait ModelTracker {
+    /// Attach one peer. The peer will be notified when the model changes
+    fn attach_peer(&self, peer: ModelPeer);
+    /// Register the model as a dependency to the current binding being evaluated, so
+    /// that it will be notified when the model changes its size.
+    fn track_row_count_changes(&self);
+}
+
+impl ModelTracker for () {
+    fn attach_peer(&self, _peer: ModelPeer) {}
+
+    fn track_row_count_changes(&self) {}
+}
+
 /// Dispatch notifications from a [`Model`] to one or several [`ModelPeer`].
 /// Typically, you would want to put this in the implementation of the Model
 #[derive(Default)]
 pub struct ModelNotify {
     inner: RefCell<weak_table::PtrWeakHashSet<Weak<RefCell<ModelPeerInner>>>>,
+    model_dirty_property: RefCell<Option<Pin<Rc<Property<()>>>>>,
 }
 
 impl ModelNotify {
@@ -47,20 +65,44 @@ impl ModelNotify {
     }
     /// Notify the peers that rows were added
     pub fn row_added(&self, index: usize, count: usize) {
+        if let Some(model_dirty_tracker) = self.model_dirty_property.borrow().as_ref() {
+            model_dirty_tracker.set_dirty()
+        }
         for peer in self.inner.borrow().iter() {
             peer.borrow_mut().row_added(index, count)
         }
     }
     /// Notify the peers that rows were removed
     pub fn row_removed(&self, index: usize, count: usize) {
+        if let Some(model_dirty_tracker) = self.model_dirty_property.borrow().as_ref() {
+            model_dirty_tracker.set_dirty()
+        }
         for peer in self.inner.borrow().iter() {
             peer.borrow_mut().row_removed(index, count)
         }
     }
     /// Attach one peer. The peer will be notified when the model changes
+    #[deprecated(
+        note = "In your model, re-implement `model_tracker` of the `Model` trait instead of calling this function from our `attach_peer` implementation"
+    )]
     pub fn attach(&self, peer: ModelPeer) {
+        self.attach_peer(peer)
+    }
+}
+
+impl ModelTracker for ModelNotify {
+    /// Attach one peer. The peer will be notified when the model changes
+    fn attach_peer(&self, peer: ModelPeer) {
         peer.inner.upgrade().map(|rc| self.inner.borrow_mut().insert(rc));
     }
+
+    fn track_row_count_changes(&self) {
+        self.model_dirty_property
+            .borrow_mut()
+            .get_or_insert_with(|| Rc::pin(Default::default()))
+            .as_ref()
+            .get();
+    }
 }
 
 /// A Model is providing Data for the Repeater or ListView elements of the `.60` language
@@ -150,7 +192,15 @@ pub trait Model {
         );
     }
     /// The implementation should forward to [`ModelNotify::attach`]
-    fn attach_peer(&self, peer: ModelPeer);
+    #[deprecated(note = "Re-implement model_tracker instead of this function")]
+    fn attach_peer(&self, peer: ModelPeer) {
+        self.model_tracker().attach_peer(peer)
+    }
+
+    /// The implementation should return a reference to its [`ModelNotify`] field.
+    fn model_tracker(&self) -> &dyn ModelTracker {
+        &()
+    }
 
     /// Returns an iterator visiting all elements of the model.
     fn iter(&self) -> ModelIterator<Self::Data>
@@ -264,8 +314,8 @@ impl<T: Clone + 'static> Model for VecModel<T> {
         self.notify.row_changed(row);
     }
 
-    fn attach_peer(&self, peer: ModelPeer) {
-        self.notify.attach(peer);
+    fn model_tracker(&self) -> &dyn ModelTracker {
+        &self.notify
     }
 
     fn as_any(&self) -> &dyn core::any::Any {
@@ -284,10 +334,6 @@ impl Model for usize {
         row as i32
     }
 
-    fn attach_peer(&self, _peer: ModelPeer) {
-        // The model is read_only: nothing to do
-    }
-
     fn as_any(&self) -> &dyn core::any::Any {
         self
     }
@@ -306,10 +352,6 @@ impl Model for bool {
 
     fn row_data(&self, _row: usize) -> Self::Data {}
 
-    fn attach_peer(&self, _peer: ModelPeer) {
-        // The model is read_only: nothing to do
-    }
-
     fn as_any(&self) -> &dyn core::any::Any {
         self
     }
@@ -379,10 +421,8 @@ impl<T> Model for ModelHandle<T> {
         self.0.as_ref().unwrap().set_row_data(row, data)
     }
 
-    fn attach_peer(&self, peer: ModelPeer) {
-        if let Some(model) = self.0.as_ref() {
-            model.attach_peer(peer);
-        }
+    fn model_tracker(&self) -> &dyn ModelTracker {
+        self.0.as_ref().map_or(&(), |model| model.model_tracker())
     }
 
     fn as_any(&self) -> &dyn core::any::Any {
@@ -544,6 +584,7 @@ impl<C: RepeatedComponent + 'static> Repeater<C> {
             (*Rc::make_mut(&mut self.inner.borrow_mut()).get_mut()) = RepeaterInner::default();
             if let ModelHandle(Some(m)) = model.get() {
                 let peer: Rc<RefCell<dyn ViewAbstraction>> = self.inner.borrow().clone();
+                #[allow(deprecated)]
                 m.attach_peer(ModelPeer { inner: Rc::downgrade(&peer) });
             }
         }
@@ -829,3 +870,40 @@ pub struct StandardListViewItem {
     /// The text content of the item
     pub text: crate::SharedString,
 }
+
+#[test]
+fn test_tracking_model_handle() {
+    let model: Rc<VecModel<u8>> = Rc::new(Default::default());
+    let handle = ModelHandle::new(model.clone());
+    let tracker = Box::pin(crate::properties::PropertyTracker::default());
+    assert_eq!(
+        tracker.as_ref().evaluate(|| {
+            handle.model_tracker().track_row_count_changes();
+            handle.row_count()
+        }),
+        0
+    );
+    assert!(!tracker.is_dirty());
+    model.push(42);
+    model.push(100);
+    assert!(tracker.is_dirty());
+    assert_eq!(
+        tracker.as_ref().evaluate(|| {
+            handle.model_tracker().track_row_count_changes();
+            handle.row_count()
+        }),
+        2
+    );
+    assert!(!tracker.is_dirty());
+    model.set_row_data(0, 41);
+    assert!(!tracker.is_dirty());
+    model.remove(0);
+    assert!(tracker.is_dirty());
+    assert_eq!(
+        tracker.as_ref().evaluate(|| {
+            handle.model_tracker().track_row_count_changes();
+            handle.row_count()
+        }),
+        1
+    );
+}
diff --git a/sixtyfps_runtime/corelib/properties.rs b/sixtyfps_runtime/corelib/properties.rs
@@ -675,6 +675,12 @@ impl<T: Clone> Property<T> {
     pub fn is_dirty(&self) -> bool {
         self.handle.access(|binding| binding.map_or(false, |b| b.dirty.get()))
     }
+
+    /// Internal function to mark the property as dirty and notify dependencies, regardless of
+    /// whether the property value has actually changed or not.
+    pub(crate) fn set_dirty(&self) {
+        self.handle.mark_dirty()
+    }
 }
 
 impl<T: Clone + InterpolatedPropertyValue + 'static> Property<T> {

diff --git a/sixtyfps_runtime/interpreter/ffi.rs b/sixtyfps_runtime/interpreter/ffi.rs
@@ -11,7 +11,7 @@ LICENSE END */
 use crate::dynamic_component::ErasedComponentBox;
 
 use super::*;
-use sixtyfps_corelib::model::{Model, ModelNotify, ModelPeer};
+use sixtyfps_corelib::model::{Model, ModelNotify};
 use sixtyfps_corelib::slice::Slice;
 use sixtyfps_corelib::window::{WindowHandleAccess, WindowRc};
 use std::ffi::c_void;
@@ -597,8 +597,8 @@ impl Model for ModelAdaptorWrapper {
         }
     }
 
-    fn attach_peer(&self, peer: ModelPeer) {
-        self.0.get_notify().as_model_notify().attach(peer);
+    fn model_tracker(&self) -> &dyn sixtyfps_corelib::model::ModelTracker {
+        self.0.get_notify().as_model_notify()
     }
 
     fn set_row_data(&self, row: usize, data: Value) {
@@ -609,10 +609,10 @@ impl Model for ModelAdaptorWrapper {
 
 #[repr(C)]
 #[cfg(target_pointer_width = "64")]
-pub struct ModelNotifyOpaque([usize; 6]);
+pub struct ModelNotifyOpaque([usize; 8]);
 #[repr(C)]
 #[cfg(target_pointer_width = "32")]
-pub struct ModelNotifyOpaque([usize; 10]);
+pub struct ModelNotifyOpaque([usize; 12]);
 /// Asserts that ModelNotifyOpaque is at least as large as ModelNotify, otherwise this would overflow
 const _: usize = std::mem::size_of::<ModelNotifyOpaque>() - std::mem::size_of::<ModelNotify>();
 

diff --git a/sixtyfps_runtime/interpreter/value_model.rs b/sixtyfps_runtime/interpreter/value_model.rs
@@ -9,7 +9,7 @@
 LICENSE END */
 
 use crate::api::Value;
-use sixtyfps_corelib::model::Model;
+use sixtyfps_corelib::model::{Model, ModelTracker};
 use std::cell::RefCell;
 
 pub struct ValueModel {
@@ -23,6 +23,19 @@ impl ValueModel {
     }
 }
 
+impl ModelTracker for ValueModel {
+    fn attach_peer(&self, peer: sixtyfps_corelib::model::ModelPeer) {
+        if let Value::Model(ref model_ptr) = *self.value.borrow() {
+            model_ptr.model_tracker().attach_peer(peer.clone())
+        }
+        self.notify.attach_peer(peer)
+    }
+
+    fn track_row_count_changes(&self) {
+        self.notify.track_row_count_changes()
+    }
+}
+
 impl Model for ValueModel {
     type Data = Value;
 
@@ -53,11 +66,8 @@ impl Model for ValueModel {
         }
     }
 
-    fn attach_peer(&self, peer: sixtyfps_corelib::model::ModelPeer) {
-        if let Value::Model(ref model_ptr) = *self.value.borrow() {
-            model_ptr.attach_peer(peer.clone())
-        }
-        self.notify.attach(peer)
+    fn model_tracker(&self) -> &dyn ModelTracker {
+        self
     }
 
     fn set_row_data(&self, row: usize, data: Self::Data) {

diff --git a/tests/cases/models/array.60 b/tests/cases/models/array.60
@@ -30,16 +30,22 @@ let instance = TestCase::new();
 
 assert_eq!(instance.get_num_ints(), 5);
 
-instance.set_ints(sixtyfps::VecModel::from_slice(&[1, 2, 3, 4, 5, 6, 7]));
+let model: std::rc::Rc<sixtyfps::VecModel<i32>> = std::rc::Rc::new(vec![1, 2, 3, 4, 5, 6, 7].into());
+instance.set_ints(sixtyfps::ModelHandle::new(model.clone()));
 assert_eq!(instance.get_num_ints(), 7);
+model.push(8);
+assert_eq!(instance.get_num_ints(), 8);
 ```
 
 ```js
 var instance = new sixtyfps.TestCase();
 
 assert.equal(instance.num_ints, 5);
 
-instance.ints = [1, 2, 3, 4, 5, 6, 7];
+let model = new sixtyfpslib.ArrayModel([1, 2, 3, 4, 5, 6, 7]);
+instance.ints = model;
+assert.equal(instance.num_ints, 7);
+model.push(8);
 assert.equal(instance.num_ints, 7);
 ```
 */