Reinstating weak_clone and non-weak callbacks

Somehow I missed that my changes for weak callbacks broke the theme
editor. I thought I had it working with the try_get changes, but I
discovered several flaws in this approach.

In the end, ownership has been transferred to the CallbackHandle, and a
CallbackHandle can relinquish its reference to create weak graphs. This
is how weak_clone now works.
This commit is contained in:
Jonathan Johnson 2024-01-04 13:54:30 -08:00
parent 36b80e8f34
commit eb20133116
No known key found for this signature in database
GPG key ID: A66D6A34D6620579
3 changed files with 85 additions and 27 deletions

View file

@ -213,28 +213,26 @@ fn optional_editor(label: &str, color: &Dynamic<ColorSource>) -> (Dynamic<bool>,
}
fn color_editor(color: &Dynamic<ColorSource>) -> impl MakeWidget {
let hue = color.map_each(|color| color.hue.into_positive_degrees());
let hue = color.map_each_cloned(|color| color.hue.into_positive_degrees());
hue.for_each_cloned({
let color = color.clone();
move |hue| {
if let Ok(mut source) = color.try_get() {
source.hue = OklabHue::new(hue);
color.set(source);
}
let mut source = color.get();
source.hue = OklabHue::new(hue);
color.set(source);
}
})
.persist();
let hue_text = hue.linked_string();
let saturation = color.map_each(|color| color.saturation);
let saturation = color.map_each_cloned(|color| color.saturation);
saturation
.for_each_cloned({
let color = color.clone();
move |saturation| {
if let Ok(mut source) = color.try_get() {
source.saturation = saturation;
color.set(source);
}
let mut source = color.get();
source.saturation = saturation;
color.set(source);
}
})
.persist();

View file

@ -53,7 +53,7 @@ impl DebugContext {
section.values.lock().push(Box::new(RegisteredValue {
label: label.into(),
value: reader.clone(),
widget: make_observer(value.clone()).make_widget(),
widget: make_observer(value.weak_clone()).make_widget(),
}))
});
let this = self.clone();

View file

@ -290,6 +290,30 @@ pub trait Source<T> {
mapped
}
/// Returns a new [`Dynamic`] that contains a clone of each value from
/// `self`.
///
/// The returned dynamic does not hold a strong reference to `self`,
/// ensuring that `self` can be cleaned up even if the returned dynamic
/// still exists.
fn weak_clone(&self) -> Dynamic<T>
where
T: Clone + Send + 'static,
{
let mapped = Dynamic::new(self.get());
let mapped_weak = mapped.downgrade();
mapped.set_source(
self.for_each_cloned_try(move |value| {
let mapped = mapped_weak.upgrade().ok_or(CallbackDisconnected)?;
*mapped.lock() = value.clone();
Ok(())
})
.weak(),
);
mapped
}
/// Returns a new dynamic that is updated using `U::from(T.clone())` each
/// time `self` is updated.
#[must_use]
@ -510,7 +534,7 @@ impl<T> Source<T> for Arc<DynamicData<T>> {
+ 'static,
{
let this = WeakDynamic(Arc::downgrade(self));
DynamicData::for_each(self, move || {
dynamic_for_each(self, move || {
let this = this.upgrade().ok_or(CallbackDisconnected)?;
this.map_generational(&mut for_each)?;
Ok(())
@ -523,7 +547,7 @@ impl<T> Source<T> for Arc<DynamicData<T>> {
F: FnMut(GenerationalValue<T>) -> Result<(), CallbackDisconnected> + Send + 'static,
{
let this = WeakDynamic(Arc::downgrade(self));
DynamicData::for_each(self, move || {
dynamic_for_each(self, move || {
let this = this.upgrade().ok_or(CallbackDisconnected)?;
if let Ok(value) = this.try_map_generational(GenerationalValue::clone) {
@ -726,6 +750,7 @@ impl<T> Source<T> for Owned<T> {
let mut callbacks = self.callbacks.active.lock().ignore_poison();
CallbackHandle(CallbackHandleInner::Single(CallbackHandleData {
id: Some(callbacks.push(Box::new(for_each))),
owner: None,
callbacks: self.callbacks.clone(),
}))
}
@ -944,9 +969,10 @@ impl<T> Dynamic<T> {
Ok(())
}));
let t_weak = self.downgrade();
// The linked dynamic holds a reference to the original, since it's
// being created from the original.
let t = self.clone();
self.set_source(r.for_each_try(move |r| {
let t = t_weak.upgrade().ok_or(CallbackDisconnected)?;
if let Some(update) = r_into_t(r).into() {
let _result = t.replace(update);
}
@ -1388,18 +1414,20 @@ impl<T> DynamicData<T> {
Ok(old)
}
}
pub fn for_each<F>(&self, map: F) -> CallbackHandle
where
F: for<'a> FnMut() -> Result<(), CallbackDisconnected> + Send + 'static,
{
let state = self.state().expect("deadlocked");
let mut data = state.callbacks.callbacks.lock().ignore_poison();
CallbackHandle(CallbackHandleInner::Single(CallbackHandleData {
id: Some(data.callbacks.push(Box::new(map))),
callbacks: state.callbacks.clone(),
}))
}
fn dynamic_for_each<T, F>(this: &Arc<DynamicData<T>>, map: F) -> CallbackHandle
where
F: for<'a> FnMut() -> Result<(), CallbackDisconnected> + Send + 'static,
T: Send + 'static,
{
let state = this.state().expect("deadlocked");
let mut data = state.callbacks.callbacks.lock().ignore_poison();
CallbackHandle(CallbackHandleInner::Single(CallbackHandleData {
id: Some(data.callbacks.push(Box::new(map))),
owner: Some(this.clone()),
callbacks: state.callbacks.clone(),
}))
}
/// A callback function is no longer connected to its source.
@ -1467,8 +1495,12 @@ enum CallbackHandleInner {
Multi(Vec<CallbackHandleData>),
}
trait ReferencedDynamic: Sync + Send + 'static {}
impl<T> ReferencedDynamic for T where T: Sync + Send + 'static {}
struct CallbackHandleData {
id: Option<LotId>,
owner: Option<Arc<dyn ReferencedDynamic>>,
callbacks: Arc<dyn CallbackCollection>,
}
@ -1508,6 +1540,33 @@ impl CallbackHandle {
}
}
}
/// Drops any references to owning [`Dynamic`]s associated with this
/// callback.
///
/// This enables creating weak connections between callback graphs.
pub fn forget_owners(&mut self) {
match &mut self.0 {
CallbackHandleInner::None => {}
CallbackHandleInner::Single(handle) => {
handle.owner = None;
}
CallbackHandleInner::Multi(handles) => {
for handle in handles {
handle.owner = None;
}
}
}
}
/// Drops any references to owning [`Dynamic`]s associated with this
/// callback, and returns self.
///
/// This uses [`Self::forget_owners()`].
pub fn weak(mut self) -> Self {
self.forget_owners();
self
}
}
impl Eq for CallbackHandle {}
@ -1539,6 +1598,7 @@ impl Drop for CallbackHandleData {
}
}
}
impl PartialEq for CallbackHandleData {
fn eq(&self, other: &Self) -> bool {
self.id == other.id && Arc::ptr_eq(&self.callbacks, &other.callbacks)
@ -2932,7 +2992,7 @@ macro_rules! impl_tuple_for_each_cloned {
}
};
($self:ident $for_each:ident $handles:ident [] [$type:ident $field:tt $var:ident]) => {
$handles += $self.$field.for_each(move |field: &$type| $for_each((field.clone(),)));
$handles += $self.$field.for_each_cloned(move |field| $for_each((field,)));
};
($self:ident $for_each:ident $handles:ident [] [$($type:ident $field:tt $var:ident),+]) => {
let $for_each = Arc::new(Mutex::new($for_each));