Fix read-access through C++ interpreter to models declared as arrays

Arrays declared in .60 are mapped to a SharedVectorModel<Value> in order to permit for write access. They can also be turned intoValue::Array / SharedVector<Value> very cheaply, when reading from C++.
slint-ui · Jan 26, 2022 · b6492b0 · b6492b0
1 parent 99644a7
commit b6492b0
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 14 deletions.
diff --git a/api/sixtyfps-cpp/include/sixtyfps_interpreter.h b/api/sixtyfps-cpp/include/sixtyfps_interpreter.h
@@ -419,8 +419,10 @@ inline Value::Value(const sixtyfps::SharedVector<Value> &array)
 
 inline std::optional<sixtyfps::SharedVector<Value>> Value::to_array() const
 {
-    if (auto *array = cbindgen_private::sixtyfps_interpreter_value_to_array(&inner)) {
-        return *reinterpret_cast<const sixtyfps::SharedVector<Value> *>(array);
+    sixtyfps::SharedVector<Value> array;
+    if (cbindgen_private::sixtyfps_interpreter_value_to_array(
+                &inner, &reinterpret_cast<sixtyfps::SharedVector<ValueOpaque> &>(array))) {
+        return array;
     } else {
         return {};
     }

diff --git a/sixtyfps_runtime/corelib/model.rs b/sixtyfps_runtime/corelib/model.rs
@@ -11,7 +11,7 @@ use crate::item_tree::TraversalOrder;
 use crate::items::ItemRef;
 use crate::layout::Orientation;
 use crate::properties::dependency_tracker::DependencyNode;
-use crate::Property;
+use crate::{Property, SharedVector};
 use alloc::boxed::Box;
 use alloc::vec::Vec;
 use core::cell::{Cell, RefCell};
@@ -369,6 +369,59 @@ impl<T: Clone + 'static> Model for VecModel<T> {
     }
 }
 
+/// A model backed by a `SharedVector<T>`
+#[derive(Default)]
+pub struct SharedVectorModel<T> {
+    array: RefCell<SharedVector<T>>,
+    notify: ModelNotify,
+}
+
+impl<T: Clone + 'static> SharedVectorModel<T> {
+    /// Add a row at the end of the model
+    pub fn push(&self, value: T) {
+        self.array.borrow_mut().push(value);
+        self.notify.row_added(self.array.borrow().len() - 1, 1)
+    }
+}
+
+impl<T> SharedVectorModel<T> {
+    /// Returns a clone of the model's backing shared vector.
+    pub fn shared_vector(&self) -> SharedVector<T> {
+        self.array.borrow_mut().clone()
+    }
+}
+
+impl<T> From<SharedVector<T>> for SharedVectorModel<T> {
+    fn from(array: SharedVector<T>) -> Self {
+        SharedVectorModel { array: RefCell::new(array), notify: Default::default() }
+    }
+}
+
+impl<T: Clone + 'static> Model for SharedVectorModel<T> {
+    type Data = T;
+
+    fn row_count(&self) -> usize {
+        self.array.borrow().len()
+    }
+
+    fn row_data(&self, row: usize) -> Option<Self::Data> {
+        self.array.borrow().get(row).cloned()
+    }
+
+    fn set_row_data(&self, row: usize, data: Self::Data) {
+        self.array.borrow_mut().make_mut_slice()[row] = data;
+        self.notify.row_changed(row);
+    }
+
+    fn model_tracker(&self) -> &dyn ModelTracker {
+        &self.notify
+    }
+
+    fn as_any(&self) -> &dyn core::any::Any {
+        self
+    }
+}
+
 impl Model for usize {
     type Data = i32;
 

diff --git a/sixtyfps_runtime/interpreter/api.rs b/sixtyfps_runtime/interpreter/api.rs
@@ -4,6 +4,7 @@
 use core::convert::TryInto;
 use sixtyfps_compilerlib::langtype::Type as LangType;
 use sixtyfps_corelib::graphics::Image;
+use sixtyfps_corelib::model::SharedVectorModel;
 use sixtyfps_corelib::{Brush, PathData, SharedString, SharedVector};
 use std::borrow::Cow;
 use std::collections::HashMap;
@@ -123,7 +124,17 @@ impl Value {
             Value::String(_) => ValueType::String,
             Value::Bool(_) => ValueType::Bool,
             Value::Array(_) => ValueType::Array,
-            Value::Model(_) => ValueType::Model,
+            Value::Model(model) => {
+                if model
+                    .as_any()
+                    .downcast_ref::<sixtyfps_corelib::model::SharedVectorModel<Value>>()
+                    .is_some()
+                {
+                    ValueType::Array
+                } else {
+                    ValueType::Model
+                }
+            }
             Value::Struct(_) => ValueType::Struct,
             Value::Brush(_) => ValueType::Brush,
             Value::Image(_) => ValueType::Image,
@@ -146,8 +157,34 @@ impl PartialEq for Value {
             Value::String(lhs) => matches!(other, Value::String(rhs) if lhs == rhs),
             Value::Bool(lhs) => matches!(other, Value::Bool(rhs) if lhs == rhs),
             Value::Image(lhs) => matches!(other, Value::Image(rhs) if lhs == rhs),
-            Value::Array(lhs) => matches!(other, Value::Array(rhs) if lhs == rhs),
-            Value::Model(lhs) => matches!(other, Value::Model(rhs) if Rc::ptr_eq(lhs, rhs)),
+            Value::Array(lhs) => {
+                match other {
+                    Value::Array(rhs) => return lhs == rhs,
+                    Value::Model(model) => {
+                        if let Some(rhs_array) =
+                            model.as_any().downcast_ref::<SharedVectorModel<Value>>()
+                        {
+                            return lhs == &rhs_array.shared_vector();
+                        }
+                    }
+                    _ => {}
+                }
+                false
+            }
+            Value::Model(lhs) => {
+                match other {
+                    Value::Model(rhs) => return Rc::ptr_eq(lhs, rhs),
+                    Value::Array(rhs_array) => {
+                        if let Some(lhs_array) =
+                            lhs.as_any().downcast_ref::<SharedVectorModel<Value>>()
+                        {
+                            return &lhs_array.shared_vector() == rhs_array;
+                        }
+                    }
+                    _ => {}
+                }
+                false
+            }
             Value::Struct(lhs) => matches!(other, Value::Struct(rhs) if lhs == rhs),
             Value::Brush(lhs) => matches!(other, Value::Brush(rhs) if lhs == rhs),
             Value::PathData(lhs) => matches!(other, Value::PathData(rhs) if lhs == rhs),

diff --git a/sixtyfps_runtime/interpreter/eval.rs b/sixtyfps_runtime/interpreter/eval.rs
@@ -573,8 +573,8 @@ pub fn eval_expression(expression: &Expression, local_context: &mut EvalLocalCon
             }
         }
         Expression::Array { values, .. } => Value::Model(
-            Rc::new(corelib::model::VecModel::from(
-                values.iter().map(|e| eval_expression(e, local_context)).collect::<Vec<_>>()
+            Rc::new(corelib::model::SharedVectorModel::from(
+                values.iter().map(|e| eval_expression(e, local_context)).collect::<SharedVector<_>>()
             )) as Rc<dyn corelib::model::Model<Data = Value>>
         ),
         Expression::Struct { values, .. } => Value::Struct(

diff --git a/sixtyfps_runtime/interpreter/ffi.rs b/sixtyfps_runtime/interpreter/ffi.rs
@@ -4,7 +4,7 @@
 use crate::dynamic_component::ErasedComponentBox;
 
 use super::*;
-use sixtyfps_corelib::model::{Model, ModelNotify};
+use sixtyfps_corelib::model::{Model, ModelNotify, SharedVectorModel};
 use sixtyfps_corelib::slice::Slice;
 use sixtyfps_corelib::window::{WindowHandleAccess, WindowRc};
 use std::ffi::c_void;
@@ -155,16 +155,42 @@ pub extern "C" fn sixtyfps_interpreter_value_to_bool(val: &ValueOpaque) -> Optio
     }
 }
 
+/// Extracts a SharedVector<ValueOpaque> out of the given value `val`, writes that into the
+/// `out` parameter and returns true; returns false if the value does not hold an extractable
+/// array.
 #[no_mangle]
 pub extern "C" fn sixtyfps_interpreter_value_to_array(
     val: &ValueOpaque,
-) -> Option<&SharedVector<ValueOpaque>> {
+    out: *mut SharedVector<ValueOpaque>,
+) -> bool {
     match val.as_value() {
-        Value::Array(v) => Some(unsafe {
+        Value::Array(v) => unsafe {
             // Safety: We assert that Value and ValueOpaque have the same size and alignment
-            std::mem::transmute::<&SharedVector<Value>, &SharedVector<ValueOpaque>>(v)
-        }),
-        _ => None,
+            std::ptr::write(
+                out,
+                std::mem::transmute::<&SharedVector<Value>, &SharedVector<ValueOpaque>>(v).clone(),
+            );
+            true
+        },
+        Value::Model(m) => {
+            if let Some(model) = m.as_any().downcast_ref::<SharedVectorModel<Value>>() {
+                let vec = model.shared_vector();
+                // Safety: We assert that Value and ValueOpaque have the same size and alignment
+                unsafe {
+                    std::ptr::write(
+                        out,
+                        std::mem::transmute::<&SharedVector<Value>, &SharedVector<ValueOpaque>>(
+                            &vec,
+                        )
+                        .clone(),
+                    );
+                }
+                true
+            } else {
+                false
+            }
+        }
+        _ => false,
     }
 }