From 71c1e36d1e436631af212b593048a184d2122885 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Fri, 22 Sep 2023 13:35:30 -0400 Subject: [PATCH] Put `Theme` behind an `Arc` (#3017) This PR puts the `Theme` returned from the `theme` function behind an `Arc`. ### Motivation While working on wiring up window focus events for the `TitleBar` component we ran into issues where `theme` was holding an immutable borrow to the `ViewContext` for the entirety of the `render` scope, which prevented having mutable borrows in the same scope. ### Explanation To avoid this, we can make `theme` return an `Arc` to allow for cheap clones and avoiding the issues with the borrow checker. Release Notes: - N/A Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> --- crates/gpui/src/app/window.rs | 14 +++++++++----- crates/storybook/src/collab_panel.rs | 14 +++++++------- crates/ui/src/templates/collab_panel.rs | 14 +++++++------- crates/ui/src/templates/status_bar.rs | 4 ++-- crates/ui/src/theme.rs | 3 ++- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 626a969bd8..4eca6f3a30 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -33,6 +33,7 @@ use std::{ any::{type_name, Any, TypeId}, mem, ops::{Deref, DerefMut, Range, Sub}, + sync::Arc, }; use taffy::{ tree::{Measurable, MeasureFunc}, @@ -56,7 +57,7 @@ pub struct Window { pub(crate) rendered_views: HashMap>, scene: SceneBuilder, pub(crate) text_style_stack: Vec, - pub(crate) theme_stack: Vec>, + pub(crate) theme_stack: Vec>, pub(crate) new_parents: HashMap, pub(crate) views_to_notify_if_ancestors_change: HashMap>, titlebar_height: f32, @@ -1336,18 +1337,21 @@ impl<'a> WindowContext<'a> { self.window.text_style_stack.pop(); } - pub fn theme(&self) -> &T { + pub fn theme(&self) -> Arc { self.window .theme_stack .iter() .rev() - .find_map(|theme| theme.downcast_ref()) + .find_map(|theme| { + let entry = Arc::clone(theme); + entry.downcast::().ok() + }) .ok_or_else(|| anyhow!("no theme provided of type {}", type_name::())) .unwrap() } - pub fn push_theme(&mut self, theme: T) { - self.window.theme_stack.push(Box::new(theme)); + pub fn push_theme(&mut self, theme: T) { + self.window.theme_stack.push(Arc::new(theme)); } pub fn pop_theme(&mut self) { diff --git a/crates/storybook/src/collab_panel.rs b/crates/storybook/src/collab_panel.rs index 30d15a5b04..a6be248d6a 100644 --- a/crates/storybook/src/collab_panel.rs +++ b/crates/storybook/src/collab_panel.rs @@ -52,12 +52,12 @@ impl CollabPanelElement { //:: https://tailwindcss.com/docs/hover-focus-and-other-states#styling-based-on-parent-state // .group() // List Section Header - .child(self.list_section_header("#CRDB", true, theme)) + .child(self.list_section_header("#CRDB", true, &theme)) // List Item Large .child(self.list_item( "http://github.com/maxbrunsfeld.png?s=50", "maxbrunsfeld", - theme, + &theme, )), ) .child( @@ -65,31 +65,31 @@ impl CollabPanelElement { .py_2() .flex() .flex_col() - .child(self.list_section_header("CHANNELS", true, theme)), + .child(self.list_section_header("CHANNELS", true, &theme)), ) .child( div() .py_2() .flex() .flex_col() - .child(self.list_section_header("CONTACTS", true, theme)) + .child(self.list_section_header("CONTACTS", true, &theme)) .children( std::iter::repeat_with(|| { vec![ self.list_item( "http://github.com/as-cii.png?s=50", "as-cii", - theme, + &theme, ), self.list_item( "http://github.com/nathansobo.png?s=50", "nathansobo", - theme, + &theme, ), self.list_item( "http://github.com/maxbrunsfeld.png?s=50", "maxbrunsfeld", - theme, + &theme, ), ] }) diff --git a/crates/ui/src/templates/collab_panel.rs b/crates/ui/src/templates/collab_panel.rs index 7e76cb6835..3d6efe54f4 100644 --- a/crates/ui/src/templates/collab_panel.rs +++ b/crates/ui/src/templates/collab_panel.rs @@ -52,12 +52,12 @@ impl CollabPanelElement { //:: https://tailwindcss.com/docs/hover-focus-and-other-states#styling-based-on-parent-state // .group() // List Section Header - .child(self.list_section_header("#CRDB", true, theme)) + .child(self.list_section_header("#CRDB", true, &theme)) // List Item Large .child(self.list_item( "http://github.com/maxbrunsfeld.png?s=50", "maxbrunsfeld", - theme, + &theme, )), ) .child( @@ -65,31 +65,31 @@ impl CollabPanelElement { .py_2() .flex() .flex_col() - .child(self.list_section_header("CHANNELS", true, theme)), + .child(self.list_section_header("CHANNELS", true, &theme)), ) .child( div() .py_2() .flex() .flex_col() - .child(self.list_section_header("CONTACTS", true, theme)) + .child(self.list_section_header("CONTACTS", true, &theme)) .children( std::iter::repeat_with(|| { vec![ self.list_item( "http://github.com/as-cii.png?s=50", "as-cii", - theme, + &theme, ), self.list_item( "http://github.com/nathansobo.png?s=50", "nathansobo", - theme, + &theme, ), self.list_item( "http://github.com/maxbrunsfeld.png?s=50", "maxbrunsfeld", - theme, + &theme, ), ] }) diff --git a/crates/ui/src/templates/status_bar.rs b/crates/ui/src/templates/status_bar.rs index 79265a7572..22a99fdbf8 100644 --- a/crates/ui/src/templates/status_bar.rs +++ b/crates/ui/src/templates/status_bar.rs @@ -96,8 +96,8 @@ impl StatusBar { .justify_between() .w_full() .fill(theme.lowest.base.default.background) - .child(self.left_tools(theme)) - .child(self.right_tools(theme)) + .child(self.left_tools(&theme)) + .child(self.right_tools(&theme)) } fn left_tools(&self, theme: &Theme) -> impl Element { diff --git a/crates/ui/src/theme.rs b/crates/ui/src/theme.rs index 71235625d6..d4c8c26273 100644 --- a/crates/ui/src/theme.rs +++ b/crates/ui/src/theme.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::fmt; use std::marker::PhantomData; +use std::sync::Arc; use gpui2::color::Hsla; use gpui2::element::Element; @@ -190,6 +191,6 @@ fn preferred_theme(cx: &AppContext) -> Theme { .clone() } -pub fn theme<'a>(cx: &'a WindowContext) -> &'a Theme { +pub fn theme(cx: &WindowContext) -> Arc { cx.theme::() }