diff --git a/CHANGELOG.md b/CHANGELOG.md index 980374009..2cca3d125 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,6 @@ All notable changes to this project will be documented in this file. has changed. This was undocumented, but if one was handling this in the `FocusScope` event, these keys will now be ignored. Use the `Keys.LeftArrow` and other code exposed in the `Keys` namespace instead -- **Breaking:** Changed the base class of `ModelHandle` to be a `SharedModel` which wraps the - `Rc` that was used before. ### Added diff --git a/api/sixtyfps-node/native/lib.rs b/api/sixtyfps-node/native/lib.rs index e6cffcb44..1ed682b27 100644 --- a/api/sixtyfps-node/native/lib.rs +++ b/api/sixtyfps-node/native/lib.rs @@ -188,7 +188,7 @@ fn to_eval_value<'cx>( obj.get(cx, "rowCount")?.downcast_or_throw::(cx)?; obj.get(cx, "rowData")?.downcast_or_throw::(cx)?; let m = js_model::JsModel::new(obj, *a, cx, persistent_context)?; - Ok(Value::Model(sixtyfps_corelib::model::SharedModel(m))) + Ok(Value::Model(m)) } }, Type::Image => { diff --git a/sixtyfps_runtime/corelib/model.rs b/sixtyfps_runtime/corelib/model.rs index 6e01920a2..1e42e1d06 100644 --- a/sixtyfps_runtime/corelib/model.rs +++ b/sixtyfps_runtime/corelib/model.rs @@ -291,7 +291,7 @@ impl VecModel { where T: Clone, { - ModelHandle::new(Rc::::new(slice.to_vec().into())) + ModelHandle(Some(Rc::::new(slice.to_vec().into()))) } /// Add a row at the end of the model @@ -372,49 +372,14 @@ impl Model for bool { } } -/// A shared model -#[repr(transparent)] -#[derive(derive_more::Deref, derive_more::DerefMut)] -pub struct SharedModel(pub Rc>); - -impl std::fmt::Debug for SharedModel { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "SharedModel(dyn Model)") - } -} - -impl Clone for SharedModel { - fn clone(&self) -> Self { - Self(self.0.clone()) - } -} - -impl core::cmp::PartialEq for SharedModel { - #[allow(clippy::vtable_address_comparisons)] - // We think vtable_address_comparisons are safe here! - // Rc::ptr_eq will compare a fat pointer consisting of a `vtable` and a `self` pointer. - // - // We can get: - // * false positives: Two different `Model`s get considered the same, even though they are not. - // For this the `self` pointer need to point to the same place. That can not happen here - // since `Rc` will allocate for the refcounts, even if `T` is `()`. - // * false negatives: The same `Model` gets considered to be two different `Model`s. - // In this case the `vtable` pointer must differ, e.g. because two crates created separate - // instances of the vtable for the `Model` trait. - // This will trigger some unnecessary rendering, but is benign otherwise. - fn eq(&self, other: &Self) -> bool { - Rc::ptr_eq(&self.0, &other.0) - } -} - /// Properties of type array in the .60 language are represented as -/// an [`Option`] of an [`SharedModel`] +/// an [`Option`] of an [`Rc`] of something implemented the [`Model`] trait #[derive(derive_more::Deref, derive_more::DerefMut, derive_more::From, derive_more::Into)] -pub struct ModelHandle(pub Option>); +pub struct ModelHandle(pub Option>>); impl core::fmt::Debug for ModelHandle { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "ModelHandle({:?})", self.0.as_ref()) + write!(f, "ModelHandle({:?})", self.0.as_ref().map(|_| "dyn Model")) } } @@ -433,22 +398,26 @@ impl core::cmp::PartialEq for ModelHandle { fn eq(&self, other: &Self) -> bool { match (&self.0, &other.0) { (None, None) => true, - (Some(a), Some(b)) => a == b, - _ => false, + (None, Some(_)) => false, + (Some(_), None) => false, + (Some(a), Some(b)) => core::ptr::eq( + (&**a) as *const dyn Model as *const u8, + (&**b) as *const dyn Model as *const u8, + ), } } } impl From>> for ModelHandle { fn from(x: Rc>) -> Self { - Self(Some(SharedModel(x))) + Self(Some(x)) } } impl ModelHandle { /// Create a new handle wrapping the given model pub fn new(model: Rc>) -> Self { - Self(Some(SharedModel(model))) + Self(Some(model)) } } diff --git a/sixtyfps_runtime/interpreter/api.rs b/sixtyfps_runtime/interpreter/api.rs index d65fbe5a3..8d2568526 100644 --- a/sixtyfps_runtime/interpreter/api.rs +++ b/sixtyfps_runtime/interpreter/api.rs @@ -97,7 +97,7 @@ pub enum Value { /// An Array in the .60 language. Array(SharedVector), /// A more complex model which is not created by the interpreter itself (Value::Array can also be used for models) - Model(sixtyfps_corelib::model::SharedModel), + Model(Rc>), /// An object Struct(Struct), /// Correspond to `brush` or `color` type in .60. For color, this is then a [`Brush::SolidColor`] @@ -149,7 +149,7 @@ impl PartialEq for Value { 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 lhs == rhs), + Value::Model(lhs) => matches!(other, Value::Model(rhs) if Rc::ptr_eq(lhs, rhs)), Value::Struct(lhs) => matches!(other, Value::Struct(rhs) if lhs == rhs), Value::Brush(lhs) => matches!(other, Value::Brush(rhs) if lhs == rhs), Value::PathElements(lhs) => matches!(other, Value::PathElements(rhs) if lhs == rhs), diff --git a/sixtyfps_runtime/interpreter/dynamic_component.rs b/sixtyfps_runtime/interpreter/dynamic_component.rs index b9ad35565..2ef18027a 100644 --- a/sixtyfps_runtime/interpreter/dynamic_component.rs +++ b/sixtyfps_runtime/interpreter/dynamic_component.rs @@ -1365,8 +1365,8 @@ pub fn instantiate( InstanceRef::from_pin_ref(c, guard) }), ); - sixtyfps_corelib::model::ModelHandle::new(Rc::new(crate::value_model::ValueModel::new( - m, + sixtyfps_corelib::model::ModelHandle(Some(Rc::new( + crate::value_model::ValueModel::new(m), ))) }); } diff --git a/sixtyfps_runtime/interpreter/ffi.rs b/sixtyfps_runtime/interpreter/ffi.rs index 949c24588..94cd44d23 100644 --- a/sixtyfps_runtime/interpreter/ffi.rs +++ b/sixtyfps_runtime/interpreter/ffi.rs @@ -123,10 +123,7 @@ pub unsafe extern "C" fn sixtyfps_interpreter_value_new_model( model: vtable::VBox, val: *mut ValueOpaque, ) { - std::ptr::write( - val as *mut Value, - Value::Model(sixtyfps_corelib::model::SharedModel(Rc::new(ModelAdaptorWrapper(model)))), - ) + std::ptr::write(val as *mut Value, Value::Model(Rc::new(ModelAdaptorWrapper(model)))) } #[no_mangle]