Only re-create elements if the model actually changed

Being dirty is not enough

Fixes #7245

ChangeLog: Elements of a `for` now only get re-created if the model is
changed, not if it is only dirty

api/cpp/include/slint.h

@@ -1146,12 +1146,16 @@ public:
     void ensure_updated(const Parent *parent) const
     {
         if (model.is_dirty()) {
+            auto old_model = model.get_internal();
+            auto m = model.get();
+            if (!inner || old_model != m) {
                 inner = std::make_shared<RepeaterInner>();
-            if (auto m = model.get()) {
+                if (m) {
                     inner->model = m;
                     m->attach_peer(inner);
                 }
             }
+        }
 
         if (inner && inner->is_dirty.get()) {
             inner->is_dirty.set(false);

api/cpp/include/slint_properties.h

@@ -184,6 +184,8 @@ struct Property
     {
     }
 
+    const T &get_internal() const { return value; }
+
 private:
     cbindgen_private::PropertyHandleOpaque inner;
     mutable T value {};

internal/core/model.rs

@@ -895,15 +895,17 @@ impl<C: RepeatedItemTree + 'static> Repeater<C> {
     }
 
     fn model(self: Pin<&Self>) -> ModelRc<C::Data> {
-        // Safety: Repeater does not implement drop and never allows access to model as mutable
         let model = self.data().project_ref().model;
 
         if model.is_dirty() {
+            let old_model = model.get_internal();
+            let m = model.get();
+            if old_model != m {
                 *self.data().inner.borrow_mut() = RepeaterInner::default();
                 self.data().is_dirty.set(true);
-            let m = model.get();
                 let peer = self.project_ref().0.model_peer();
                 m.model_tracker().attach_peer(peer);
+            }
             m
         } else {
             model.get()

internal/core/properties.rs

@@ -861,8 +861,8 @@ impl<T: Clone> Property<T> {
         self.get_internal()
     }
 
-    /// Get the value without registering any dependencies or executing any binding
-    fn get_internal(&self) -> T {
+    /// Get the cached value without registering any dependencies or executing any binding
+    pub fn get_internal(&self) -> T {
         self.handle.access(|_| {
             // Safety: PropertyHandle::access ensure that the value is locked
             unsafe { (*self.value.get()).clone() }

internal/interpreter/dynamic_item_tree.rs

@@ -1725,7 +1725,11 @@ pub fn instantiate(
         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 {
                 i_slint_core::model::ModelRc::new(crate::value_model::ValueModel::new(m))
+            }
         });
     }
 

tests/cases/models/dirty_model.slint

@@ -0,0 +1,92 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+// Test that having the model being dirty doesn't re-create items
+// Only actually changing the model does.
+
+export component TestCase inherits Window {
+    property <{xx: int, model :[string]}> model: { model: ["AA", "BB"] };
+    in-out property <string> result ;
+    public function mark_dirty() {
+        model.xx += 1;
+    }
+    public function change() {
+        model.model = ["CC"];
+    }
+
+    HorizontalLayout {
+        Rectangle {}
+        for m in model.model : Rectangle {
+            init => {
+                result += "Init '" + m + "' (" + model.xx + ")\n";
+            }
+        }
+    }
+
+}
+
+/*
+```rust
+let instance = TestCase::new().unwrap();
+
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq!(instance.get_result(), "Init 'AA' (0)\nInit 'BB' (0)\n");
+instance.set_result("".into());
+slint_testing::send_mouse_click(&instance, 15., 5.);
+instance.invoke_mark_dirty();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq!(instance.get_result(), "");
+slint_testing::send_mouse_click(&instance, 15., 5.);
+instance.invoke_change();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq!(instance.get_result(), "Init 'CC' (1)\n");
+instance.set_result("".into());
+instance.invoke_mark_dirty();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq!(instance.get_result(), "");
+
+```
+
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq(instance.get_result(), "Init 'AA' (0)\nInit 'BB' (0)\n");
+instance.set_result("");
+slint_testing::send_mouse_click(&instance, 15., 5.);
+instance.invoke_mark_dirty();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq(instance.get_result(), "");
+slint_testing::send_mouse_click(&instance, 15., 5.);
+instance.invoke_change();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq(instance.get_result(), "Init 'CC' (1)\n");
+instance.set_result("");
+instance.invoke_mark_dirty();
+slint_testing::send_mouse_click(&instance, 15., 5.);
+assert_eq(instance.get_result(), "");
+
+```
+
+
+```js
+var instance = new slint.TestCase({});
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+assert.equal(instance.result, "Init 'AA' (0)\nInit 'BB' (0)\n");
+instance.result = "";
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+instance.mark_dirty();
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+assert.equal(instance.result, "");
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+instance.change();
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+assert.equal(instance.result, "Init 'CC' (1)\n");
+instance.result = "";
+instance.mark_dirty();
+slintlib.private_api.send_mouse_click(instance, 15., 5.);
+assert.equal(instance.result, "");
+```
+*/
+