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,10 +1146,14 @@ class Repeater
     void ensure_updated(const Parent *parent) const
     {
         if (model.is_dirty()) {
-            inner = std::make_shared<RepeaterInner>();
-            if (auto m = model.get()) {
-                inner->model = m;
-                m->attach_peer(inner);
+            auto old_model = model.get_internal();
+            auto m = model.get();
+            if (!inner || old_model != m) {
+                inner = std::make_shared<RepeaterInner>();
+                if (m) {
+                    inner->model = m;
+                    m->attach_peer(inner);
+                }
             }
         }
 

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() {
-            *self.data().inner.borrow_mut() = RepeaterInner::default();
-            self.data().is_dirty.set(true);
+            let old_model = model.get_internal();
             let m = model.get();
-            let peer = self.project_ref().0.model_peer();
-            m.model_tracker().attach_peer(peer);
+            if old_model != m {
+                *self.data().inner.borrow_mut() = RepeaterInner::default();
+                self.data().is_dirty.set(true);
+                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() }

tests/cases/models/dirty_model.slint

@@ -0,0 +1,92 @@
+// Copyright © SixtyFPS GmbH <[email protected]>
+// 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, "");
+```
+*/
+