From 1f94463ce2a23f0cf128fd62fbad5492da5430aa Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 21 Jan 2024 15:35:47 -0800 Subject: [PATCH 1/7] Switch Arc> to Rc>, a relic of the GPUI2 port. Make gpui pass clippy --- crates/gpui/src/app.rs | 17 +++++++++-------- crates/gpui/src/app/entity_map.rs | 5 ++--- crates/gpui/src/color.rs | 14 +++++--------- crates/gpui/src/element.rs | 4 +--- crates/gpui/src/elements/div.rs | 7 +++---- crates/gpui/src/elements/overlay.rs | 2 +- crates/gpui/src/elements/uniform_list.rs | 2 +- crates/gpui/src/geometry.rs | 4 ++-- crates/gpui/src/gpui.rs | 3 +++ crates/gpui/src/interactive.rs | 2 +- crates/gpui/src/key_dispatch.rs | 15 ++++++--------- crates/gpui/src/{keymap => }/keymap.rs | 10 +++++++++- crates/gpui/src/keymap/context.rs | 4 ++-- crates/gpui/src/keymap/matcher.rs | 12 ++++++------ crates/gpui/src/keymap/mod.rs | 9 --------- crates/gpui/src/subscription.rs | 1 - crates/gpui/src/taffy.rs | 14 ++++---------- crates/gpui/src/text_system.rs | 2 +- crates/gpui/src/text_system/line.rs | 2 ++ crates/gpui/src/text_system/line_layout.rs | 1 + crates/gpui/src/window.rs | 2 +- crates/media/src/media.rs | 15 +++++++++++++++ 22 files changed, 75 insertions(+), 72 deletions(-) rename crates/gpui/src/{keymap => }/keymap.rs (97%) delete mode 100644 crates/gpui/src/keymap/mod.rs diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 7b349d92568e72b9b4929655e93dd1683636964e..dfecffb80a26461cc16d40489213988494229672 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -25,13 +25,12 @@ use crate::{ use anyhow::{anyhow, Result}; use collections::{FxHashMap, FxHashSet, VecDeque}; use futures::{channel::oneshot, future::LocalBoxFuture, Future}; -use parking_lot::Mutex; + use slotmap::SlotMap; use std::{ any::{type_name, TypeId}, cell::{Ref, RefCell, RefMut}, marker::PhantomData, - mem, ops::{Deref, DerefMut}, path::{Path, PathBuf}, rc::{Rc, Weak}, @@ -109,6 +108,7 @@ pub struct App(Rc); /// configured, you'll start the app with `App::run`. impl App { /// Builds an app with the given asset source. + #[allow(clippy::new_without_default)] pub fn new() -> Self { Self(AppContext::new( current_platform(), @@ -224,7 +224,7 @@ pub struct AppContext { pub(crate) entities: EntityMap, pub(crate) new_view_observers: SubscriberSet, pub(crate) windows: SlotMap>, - pub(crate) keymap: Arc>, + pub(crate) keymap: Rc>, pub(crate) global_action_listeners: FxHashMap>>, pending_effects: VecDeque, @@ -242,6 +242,7 @@ pub struct AppContext { } impl AppContext { + #[allow(clippy::new_ret_no_self)] pub(crate) fn new( platform: Rc, asset_source: Arc, @@ -285,7 +286,7 @@ impl AppContext { entities, new_view_observers: SubscriberSet::new(), windows: SlotMap::with_key(), - keymap: Arc::new(Mutex::new(Keymap::default())), + keymap: Rc::new(RefCell::new(Keymap::default())), global_action_listeners: FxHashMap::default(), pending_effects: VecDeque::new(), pending_notifications: FxHashSet::default(), @@ -763,7 +764,7 @@ impl AppContext { /// so it can be held across `await` points. pub fn to_async(&self) -> AsyncAppContext { AsyncAppContext { - app: unsafe { mem::transmute(self.this.clone()) }, + app: self.this.clone(), background_executor: self.background_executor.clone(), foreground_executor: self.foreground_executor.clone(), } @@ -996,13 +997,13 @@ impl AppContext { /// Register key bindings. pub fn bind_keys(&mut self, bindings: impl IntoIterator) { - self.keymap.lock().add_bindings(bindings); + self.keymap.borrow_mut().add_bindings(bindings); self.pending_effects.push_back(Effect::Refresh); } /// Clear all key bindings in the app. pub fn clear_key_bindings(&mut self) { - self.keymap.lock().clear(); + self.keymap.borrow_mut().clear(); self.pending_effects.push_back(Effect::Refresh); } @@ -1106,7 +1107,7 @@ impl AppContext { /// Sets the menu bar for this application. This will replace any existing menu bar. pub fn set_menus(&mut self, menus: Vec) { - self.platform.set_menus(menus, &self.keymap.lock()); + self.platform.set_menus(menus, &self.keymap.borrow()); } /// Dispatch an action to the currently active window or global action handler diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index db5d844d17c173bbc0541d49aaf5f67e70f139a0..6383cdad7b5efba1859f34fc4ce16c11520d4576 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -242,7 +242,7 @@ impl Clone for AnyModel { assert_ne!(prev_count, 0, "Detected over-release of a model."); } - let this = Self { + Self { entity_id: self.entity_id, entity_type: self.entity_type, entity_map: self.entity_map.clone(), @@ -254,8 +254,7 @@ impl Clone for AnyModel { .write() .leak_detector .handle_created(self.entity_id), - }; - this + } } } diff --git a/crates/gpui/src/color.rs b/crates/gpui/src/color.rs index e76c31d6f15db5a7690aab3bafd0b950f79e2824..dadc3d5ad301ba3cd2145215bb7909701fd9ed27 100644 --- a/crates/gpui/src/color.rs +++ b/crates/gpui/src/color.rs @@ -203,20 +203,16 @@ impl PartialEq for Hsla { impl PartialOrd for Hsla { fn partial_cmp(&self, other: &Self) -> Option { - // SAFETY: The total ordering relies on this always being Some() - Some( - self.h - .total_cmp(&other.h) - .then(self.s.total_cmp(&other.s)) - .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))), - ) + Some(self.cmp(other)) } } impl Ord for Hsla { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - // SAFETY: The partial comparison is a total comparison - unsafe { self.partial_cmp(other).unwrap_unchecked() } + self.h + .total_cmp(&other.h) + .then(self.s.total_cmp(&other.s)) + .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))) } } diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index cd5e6ea9dcd2af407e1de2cfa2f6f25e3e6c392c..4dbc7be652d3acf73bc90e3c709039fb79223074 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -133,9 +133,7 @@ pub trait Render: 'static + Sized { } impl Render for () { - fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { - () - } + fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement {} } /// You can derive [`IntoElement`] on any type that implements this trait. diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index b1d936546b91f0387bfacf907d309a78d79d4a44..7ccdf8c2bf04891d9084b2170dc926874143a563 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1008,10 +1008,9 @@ pub(crate) type ActionListener = Box Div { #[cfg(debug_assertions)] - let interactivity = { - let mut interactivity = Interactivity::default(); - interactivity.location = Some(*core::panic::Location::caller()); - interactivity + let interactivity = Interactivity { + location: Some(*core::panic::Location::caller()), + ..Default::default() }; #[cfg(not(debug_assertions))] diff --git a/crates/gpui/src/elements/overlay.rs b/crates/gpui/src/elements/overlay.rs index 61c34bd9385f6580feb3930dd3b3ea4b1494e919..9db75b75ba0d8138d75fd4a9aa90071e15689a10 100644 --- a/crates/gpui/src/elements/overlay.rs +++ b/crates/gpui/src/elements/overlay.rs @@ -222,7 +222,7 @@ impl OverlayPositionMode { ) -> (Point, Bounds) { match self { OverlayPositionMode::Window => { - let anchor_position = anchor_position.unwrap_or_else(|| bounds.origin); + let anchor_position = anchor_position.unwrap_or(bounds.origin); let bounds = anchor_corner.get_bounds(anchor_position, size); (anchor_position, bounds) } diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 108e669f7550a2e6387641aaa4c1dae6e59f6313..ce32b993a5592732f763712b84fa33f24e2738f4 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -181,7 +181,7 @@ impl Element for UniformList { let shared_scroll_offset = element_state .interactive .scroll_offset - .get_or_insert_with(|| Rc::default()) + .get_or_insert_with(Rc::default) .clone(); let item_height = self.measure_item(Some(padded_bounds.size.width), cx).height; diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index d986f26be4475a72b0da8b75de8b3847e3fce588..0a22611fac8a2bdc38087710e4509cf57feb9e57 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -2020,13 +2020,13 @@ impl Eq for Pixels {} impl PartialOrd for Pixels { fn partial_cmp(&self, other: &Self) -> Option { - self.0.partial_cmp(&other.0) + Some(self.cmp(other)) } } impl Ord for Pixels { fn cmp(&self, other: &Self) -> cmp::Ordering { - self.partial_cmp(other).unwrap() + self.0.total_cmp(&other.0) } } diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 929e9ebfb4f262088e90e7ab5b99477a9933d998..a64d043ab87e84d1fdf2f9812db205616503ead5 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -26,6 +26,9 @@ //! TODO!(docs): Wrap up with a conclusion and links to other places? Zed / GPUI website? //! Discord for chatting about it? Other tutorials or references? +// #![deny(missing_docs)] +#![allow(clippy::type_complexity)] + #[macro_use] mod action; mod app; diff --git a/crates/gpui/src/interactive.rs b/crates/gpui/src/interactive.rs index 4faa92ce043ae32e6e795b296daeb9ad281475f1..1bc32717c4b7702ca37ee413092d13e12fc6114f 100644 --- a/crates/gpui/src/interactive.rs +++ b/crates/gpui/src/interactive.rs @@ -343,7 +343,7 @@ impl ExternalPaths { impl Render for ExternalPaths { fn render(&mut self, _: &mut ViewContext) -> impl IntoElement { - () // Intentionally left empty because the platform will render icons for the dragged files + // Intentionally left empty because the platform will render icons for the dragged files } } diff --git a/crates/gpui/src/key_dispatch.rs b/crates/gpui/src/key_dispatch.rs index dd5a7ab84e25e267012de5e9dfb216404b8dfa7d..b4de09e36c58e6b32bacac0e0b90d160f3de0195 100644 --- a/crates/gpui/src/key_dispatch.rs +++ b/crates/gpui/src/key_dispatch.rs @@ -54,13 +54,12 @@ use crate::{ KeyContext, Keymap, KeymatchResult, Keystroke, KeystrokeMatcher, WindowContext, }; use collections::FxHashMap; -use parking_lot::Mutex; use smallvec::{smallvec, SmallVec}; use std::{ any::{Any, TypeId}, + cell::RefCell, mem, rc::Rc, - sync::Arc, }; #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] @@ -73,7 +72,7 @@ pub(crate) struct DispatchTree { focusable_node_ids: FxHashMap, view_node_ids: FxHashMap, keystroke_matchers: FxHashMap, KeystrokeMatcher>, - keymap: Arc>, + keymap: Rc>, action_registry: Rc, } @@ -96,7 +95,7 @@ pub(crate) struct DispatchActionListener { } impl DispatchTree { - pub fn new(keymap: Arc>, action_registry: Rc) -> Self { + pub fn new(keymap: Rc>, action_registry: Rc) -> Self { Self { node_stack: Vec::new(), context_stack: Vec::new(), @@ -307,7 +306,7 @@ impl DispatchTree { action: &dyn Action, context_stack: &Vec, ) -> Vec { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); keymap .bindings_for_action(action) .filter(|binding| { @@ -440,9 +439,7 @@ impl DispatchTree { #[cfg(test)] mod tests { - use std::{rc::Rc, sync::Arc}; - - use parking_lot::Mutex; + use std::{cell::RefCell, rc::Rc}; use crate::{Action, ActionRegistry, DispatchTree, KeyBinding, KeyContext, Keymap}; @@ -496,7 +493,7 @@ mod tests { registry.load_action::(); - let keymap = Arc::new(Mutex::new(keymap)); + let keymap = Rc::new(RefCell::new(keymap)); let tree = DispatchTree::new(keymap, Rc::new(registry)); diff --git a/crates/gpui/src/keymap/keymap.rs b/crates/gpui/src/keymap.rs similarity index 97% rename from crates/gpui/src/keymap/keymap.rs rename to crates/gpui/src/keymap.rs index 2eefbb841ee5be0b499e7b32aaf28aa7da36a072..45e0ebbe951fe4a260cf869787e24322a47b1bd0 100644 --- a/crates/gpui/src/keymap/keymap.rs +++ b/crates/gpui/src/keymap.rs @@ -1,4 +1,12 @@ -use crate::{Action, KeyBinding, KeyBindingContextPredicate, KeyContext, Keystroke, NoAction}; +mod binding; +mod context; +mod matcher; + +pub use binding::*; +pub use context::*; +pub(crate) use matcher::*; + +use crate::{Action, Keystroke, NoAction}; use collections::HashSet; use smallvec::SmallVec; use std::{ diff --git a/crates/gpui/src/keymap/context.rs b/crates/gpui/src/keymap/context.rs index 05ef67ba2bc9d260215fc0df89a8a6e3039450f5..6c22fa9fd69bfacf650e549d786ebfc0cb83de21 100644 --- a/crates/gpui/src/keymap/context.rs +++ b/crates/gpui/src/keymap/context.rs @@ -267,8 +267,8 @@ impl KeyBindingContextPredicate { '(' => { source = skip_whitespace(&source[1..]); let (predicate, rest) = Self::parse_expr(source, 0)?; - if rest.starts_with(')') { - source = skip_whitespace(&rest[1..]); + if let Some(stripped) = rest.strip_prefix(')') { + source = skip_whitespace(stripped); Ok((predicate, source)) } else { Err(anyhow!("expected a ')'")) diff --git a/crates/gpui/src/keymap/matcher.rs b/crates/gpui/src/keymap/matcher.rs index 09ba281a0d7ebc1e032100eb9463f32767afa403..88a8826e653dc6418313df723f2748baf588bb66 100644 --- a/crates/gpui/src/keymap/matcher.rs +++ b/crates/gpui/src/keymap/matcher.rs @@ -1,11 +1,10 @@ use crate::{KeyBinding, KeyContext, Keymap, KeymapVersion, Keystroke}; -use parking_lot::Mutex; use smallvec::SmallVec; -use std::sync::Arc; +use std::{cell::RefCell, rc::Rc}; pub(crate) struct KeystrokeMatcher { pending_keystrokes: Vec, - keymap: Arc>, + keymap: Rc>, keymap_version: KeymapVersion, } @@ -15,8 +14,8 @@ pub struct KeymatchResult { } impl KeystrokeMatcher { - pub fn new(keymap: Arc>) -> Self { - let keymap_version = keymap.lock().version(); + pub fn new(keymap: Rc>) -> Self { + let keymap_version = keymap.borrow().version(); Self { pending_keystrokes: Vec::new(), keymap_version, @@ -42,7 +41,8 @@ impl KeystrokeMatcher { keystroke: &Keystroke, context_stack: &[KeyContext], ) -> KeymatchResult { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); + // Clear pending keystrokes if the keymap has changed since the last matched keystroke. if keymap.version() != self.keymap_version { self.keymap_version = keymap.version(); diff --git a/crates/gpui/src/keymap/mod.rs b/crates/gpui/src/keymap/mod.rs deleted file mode 100644 index 6f1a018322b1a7f55349af70729860b9d85bde60..0000000000000000000000000000000000000000 --- a/crates/gpui/src/keymap/mod.rs +++ /dev/null @@ -1,9 +0,0 @@ -mod binding; -mod context; -mod keymap; -mod matcher; - -pub use binding::*; -pub use context::*; -pub use keymap::*; -pub(crate) use matcher::*; diff --git a/crates/gpui/src/subscription.rs b/crates/gpui/src/subscription.rs index 9dca2e3a4841cc6c4ccd85c16700d08201218bd2..c75d10ee6289ed7c7e02436ca4aed54b78675bda 100644 --- a/crates/gpui/src/subscription.rs +++ b/crates/gpui/src/subscription.rs @@ -41,7 +41,6 @@ where /// are inert, meaning that they won't be listed when calling `[SubscriberSet::remove]` or `[SubscriberSet::retain]`. /// This method returns a tuple of a [`Subscription`] and an `impl FnOnce`, and you can use the latter /// to activate the [`Subscription`]. - #[must_use] pub fn insert( &self, emitter_key: EmitterKey, diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index ea7a4575cc3a22ae820e37f7a26e1f4f1a78a2d7..0797c8f3b497b561897ee68fd3216499824b59db 100644 --- a/crates/gpui/src/taffy.rs +++ b/crates/gpui/src/taffy.rs @@ -12,22 +12,16 @@ use taffy::{ Taffy, }; +type NodeMeasureFn = + Box>, Size, &mut WindowContext) -> Size>; + pub struct TaffyLayoutEngine { taffy: Taffy, styles: FxHashMap, children_to_parents: FxHashMap, absolute_layout_bounds: FxHashMap>, computed_layouts: FxHashSet, - nodes_to_measure: FxHashMap< - LayoutId, - Box< - dyn FnMut( - Size>, - Size, - &mut WindowContext, - ) -> Size, - >, - >, + nodes_to_measure: FxHashMap, } static EXPECT_MESSAGE: &str = "we should avoid taffy layout errors by construction if possible"; diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index cadc000f9a203f879a83e529c325061f7db0eeba..1d097ca1eed23dd316476d293b0c0944b346567a 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -71,7 +71,7 @@ impl TextSystem { .all_font_names() .into_iter() .collect(); - names.extend(self.platform_text_system.all_font_families().into_iter()); + names.extend(self.platform_text_system.all_font_families()); names.extend( self.fallback_font_stack .iter() diff --git a/crates/gpui/src/text_system/line.rs b/crates/gpui/src/text_system/line.rs index 8013f5a1e05e3e61276d89db4d3cb76c26b7ba80..d4f23c9539f16840ab224b924f96348141ada902 100644 --- a/crates/gpui/src/text_system/line.rs +++ b/crates/gpui/src/text_system/line.rs @@ -25,6 +25,7 @@ pub struct ShapedLine { impl ShapedLine { /// The length of the line in utf-8 bytes. + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len } @@ -58,6 +59,7 @@ pub struct WrappedLine { } impl WrappedLine { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len() } diff --git a/crates/gpui/src/text_system/line_layout.rs b/crates/gpui/src/text_system/line_layout.rs index 6c466f9680e8ec8ff75812bcdf15a172c9ff6258..166fefd26a55822bd6ff4475a7f839c20d849d2c 100644 --- a/crates/gpui/src/text_system/line_layout.rs +++ b/crates/gpui/src/text_system/line_layout.rs @@ -162,6 +162,7 @@ pub struct WrapBoundary { } impl WrappedLineLayout { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.unwrapped_layout.len } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 5796d139f91f0ca822dec66efb6b048fbb1c6af4..5ffe753dc1d179843dfd7a14ddf234d13d366861 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2560,7 +2560,7 @@ impl Display for ElementId { ElementId::View(entity_id) => write!(f, "view-{}", entity_id)?, ElementId::Integer(ix) => write!(f, "{}", ix)?, ElementId::Name(name) => write!(f, "{}", name)?, - ElementId::FocusHandle(__) => write!(f, "FocusHandle")?, + ElementId::FocusHandle(_) => write!(f, "FocusHandle")?, ElementId::NamedInteger(s, i) => write!(f, "{}-{}", s, i)?, } diff --git a/crates/media/src/media.rs b/crates/media/src/media.rs index 650af06c37dbbc325bc825c06e375fef8adfe6a5..269244dbf9d15f4de4438fe180f4f1f8cb3cd0c5 100644 --- a/crates/media/src/media.rs +++ b/crates/media/src/media.rs @@ -108,6 +108,9 @@ pub mod core_video { impl_CFTypeDescription!(CVMetalTextureCache); impl CVMetalTextureCache { + /// # Safety + /// + /// metal_device must be valid according to CVMetalTextureCacheCreate pub unsafe fn new(metal_device: *mut MTLDevice) -> Result { let mut this = ptr::null(); let result = CVMetalTextureCacheCreate( @@ -124,6 +127,9 @@ pub mod core_video { } } + /// # Safety + /// + /// The arguments to this function must be valid according to CVMetalTextureCacheCreateTextureFromImage pub unsafe fn create_texture_from_image( &self, source: CVImageBufferRef, @@ -434,6 +440,12 @@ pub mod video_toolbox { impl_CFTypeDescription!(VTCompressionSession); impl VTCompressionSession { + /// Create a new compression session. + /// + /// # Safety + /// + /// The callback must be a valid function pointer. and the callback_data must be valid + /// in whatever terms that callback expects. pub unsafe fn new( width: usize, height: usize, @@ -465,6 +477,9 @@ pub mod video_toolbox { } } + /// # Safety + /// + /// The arguments to this function must be valid according to VTCompressionSessionEncodeFrame pub unsafe fn encode_frame( &self, buffer: CVImageBufferRef, From 1902df931608eb351b34d48175437c557bc16a03 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 21 Jan 2024 15:51:33 -0800 Subject: [PATCH 2/7] WIP: Start geometry crate --- crates/gpui/src/geometry.rs | 8 ++++++++ crates/gpui/src/gpui.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index 0a22611fac8a2bdc38087710e4509cf57feb9e57..ef71f94ef664d381fef8fa54ba72718fdcb5cf99 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -1,3 +1,7 @@ +//! The GPUI geometry module is a collection of types and traits that +//! can be used to describe common units, concepts, and the relationships +//! between them. + use core::fmt::Debug; use derive_more::{Add, AddAssign, Div, DivAssign, Mul, Neg, Sub, SubAssign}; use refineable::Refineable; @@ -8,13 +12,17 @@ use std::{ ops::{Add, Div, Mul, MulAssign, Sub}, }; +/// An axis along which a measurement can be made. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum Axis { + /// The y axis, or up and down Vertical, + /// The x axis, or left and right Horizontal, } impl Axis { + /// Swap this axis to the opposite axis. pub fn invert(&self) -> Self { match self { Axis::Vertical => Axis::Horizontal, diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index a64d043ab87e84d1fdf2f9812db205616503ead5..8ecfd8ad6d834ac8c6577e2f724e22719e15e36e 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -26,7 +26,7 @@ //! TODO!(docs): Wrap up with a conclusion and links to other places? Zed / GPUI website? //! Discord for chatting about it? Other tutorials or references? -// #![deny(missing_docs)] +#![deny(missing_docs)] #![allow(clippy::type_complexity)] #[macro_use] From 0c3fb449f044d550b871abfa3f568343ddf38cc5 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 08:11:21 -0800 Subject: [PATCH 3/7] Document geometry --- crates/gpui/src/geometry.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index ef71f94ef664d381fef8fa54ba72718fdcb5cf99..dd826b68f541c98c88d364156dafc641bdbc8afa 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -31,11 +31,15 @@ impl Axis { } } +/// A trait for accessing the given unit along a certain axis. pub trait Along { + /// The unit associated with this type type Unit; + /// Returns the unit along the given axis. fn along(&self, axis: Axis) -> Self::Unit; + /// Applies the given function to the unit along the given axis and returns a new value. fn apply_along(&self, axis: Axis, f: impl FnOnce(Self::Unit) -> Self::Unit) -> Self; } @@ -55,7 +59,9 @@ pub trait Along { #[refineable(Debug)] #[repr(C)] pub struct Point { + /// The x coordinate of the point. pub x: T, + /// The y coordinate of the point. pub y: T, } @@ -342,7 +348,9 @@ impl Clone for Point { #[refineable(Debug)] #[repr(C)] pub struct Size { + /// The width component of the size. pub width: T, + /// The height component of the size. pub height: T, } @@ -648,7 +656,9 @@ impl Size { #[refineable(Debug)] #[repr(C)] pub struct Bounds { + /// The origin point of this area. pub origin: Point, + /// The size of the rectangle. pub size: Size, } @@ -1200,9 +1210,13 @@ impl Copy for Bounds {} #[refineable(Debug)] #[repr(C)] pub struct Edges { + /// The size of the top edge. pub top: T, + /// The size of the right edge. pub right: T, + /// The size of the bottom edge. pub bottom: T, + /// The size of the left edge. pub left: T, } @@ -1608,9 +1622,13 @@ impl From for Edges { #[refineable(Debug)] #[repr(C)] pub struct Corners { + /// The value associated with the top left corner. pub top_left: T, + /// The value associated with the top right corner. pub top_right: T, + /// The value associated with the bottom right corner. pub bottom_right: T, + /// The value associated with the bottom left corner. pub bottom_left: T, } From a99d5b87e8ed0900b433f8b0c28e092c5c62bb0c Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 09:35:01 -0800 Subject: [PATCH 4/7] WIP: text_system --- crates/gpui/src/platform/mac/text_system.rs | 14 +-- crates/gpui/src/text_system.rs | 90 ++++++++++++++----- crates/gpui/src/text_system/font_features.rs | 7 +- crates/gpui/src/window/element_cx.rs | 24 ++++- .../src/derive_refineable.rs | 7 +- 5 files changed, 106 insertions(+), 36 deletions(-) diff --git a/crates/gpui/src/platform/mac/text_system.rs b/crates/gpui/src/platform/mac/text_system.rs index 21fe3c70a8a8787056fd3409c837af76a70427cf..138a465775f83e4297417ca3ae6f365dc93873bc 100644 --- a/crates/gpui/src/platform/mac/text_system.rs +++ b/crates/gpui/src/platform/mac/text_system.rs @@ -143,7 +143,7 @@ impl PlatformTextSystem for MacTextSystem { fn typographic_bounds(&self, font_id: FontId, glyph_id: GlyphId) -> Result> { Ok(self.0.read().fonts[font_id.0] - .typographic_bounds(glyph_id.into())? + .typographic_bounds(glyph_id.0)? .into()) } @@ -221,11 +221,13 @@ impl MacTextSystemState { } fn advance(&self, font_id: FontId, glyph_id: GlyphId) -> Result> { - Ok(self.fonts[font_id.0].advance(glyph_id.into())?.into()) + Ok(self.fonts[font_id.0].advance(glyph_id.0)?.into()) } fn glyph_for_char(&self, font_id: FontId, ch: char) -> Option { - self.fonts[font_id.0].glyph_for_char(ch).map(Into::into) + self.fonts[font_id.0] + .glyph_for_char(ch) + .map(|glyph_id| GlyphId(glyph_id)) } fn id_for_native_font(&mut self, requested_font: CTFont) -> FontId { @@ -259,7 +261,7 @@ impl MacTextSystemState { let scale = Transform2F::from_scale(params.scale_factor); Ok(font .raster_bounds( - params.glyph_id.into(), + params.glyph_id.0, params.font_size.into(), scale, HintingOptions::None, @@ -334,7 +336,7 @@ impl MacTextSystemState { .native_font() .clone_with_font_size(f32::from(params.font_size) as CGFloat) .draw_glyphs( - &[u32::from(params.glyph_id) as CGGlyph], + &[params.glyph_id.0 as CGGlyph], &[CGPoint::new( (subpixel_shift.x / params.scale_factor) as CGFloat, (subpixel_shift.y / params.scale_factor) as CGFloat, @@ -419,7 +421,7 @@ impl MacTextSystemState { let glyph_utf16_ix = usize::try_from(*glyph_utf16_ix).unwrap(); ix_converter.advance_to_utf16_ix(glyph_utf16_ix); glyphs.push(ShapedGlyph { - id: (*glyph_id).into(), + id: GlyphId(*glyph_id as u32), position: point(position.x as f32, position.y as f32).map(px), index: ix_converter.utf8_ix, is_emoji: self.is_emoji(font_id), diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 1d097ca1eed23dd316476d293b0c0944b346567a..9662247e6f248bc329a195c5abb44373d8151fdd 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -26,15 +26,18 @@ use std::{ sync::Arc, }; +/// An opaque identifier for a specific font. #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] #[repr(C)] pub struct FontId(pub usize); +/// An opaque identifier for a specific font family. #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] pub struct FontFamilyId(pub usize); pub(crate) const SUBPIXEL_VARIANTS: u8 = 4; +/// The GPUI text layout and rendering sub system. pub struct TextSystem { line_layout_cache: Arc, platform_text_system: Arc, @@ -65,6 +68,7 @@ impl TextSystem { } } + /// Get a list of all available font names from the operating system. pub fn all_font_names(&self) -> Vec { let mut names: BTreeSet<_> = self .platform_text_system @@ -79,10 +83,13 @@ impl TextSystem { ); names.into_iter().collect() } + + /// Add a font's data to the text system. pub fn add_fonts(&self, fonts: &[Arc>]) -> Result<()> { self.platform_text_system.add_fonts(fonts) } + /// Get the FontId for the configure font family and style. pub fn font_id(&self, font: &Font) -> Result { let font_id = self.font_ids_by_font.read().get(font).copied(); if let Some(font_id) = font_id { @@ -120,10 +127,14 @@ impl TextSystem { ); } + /// Get the bounding box for the given font and font size. + /// A font's bounding box is the smallest rectangle that could enclose all glyphs + /// in the font. superimposed over one another. pub fn bounding_box(&self, font_id: FontId, font_size: Pixels) -> Bounds { self.read_metrics(font_id, |metrics| metrics.bounding_box(font_size)) } + /// Get the typographic bounds for the given character, in the given font and size. pub fn typographic_bounds( &self, font_id: FontId, @@ -142,6 +153,7 @@ impl TextSystem { })) } + /// Get the advance width for the given character, in the given font and size. pub fn advance(&self, font_id: FontId, font_size: Pixels, ch: char) -> Result> { let glyph_id = self .platform_text_system @@ -153,26 +165,35 @@ impl TextSystem { Ok(result * font_size) } + /// Get the number of font size units per 'em square', + /// Per MDN: "an abstract square whose height is the intended distance between + /// lines of type in the same type size" pub fn units_per_em(&self, font_id: FontId) -> u32 { self.read_metrics(font_id, |metrics| metrics.units_per_em) } + /// Get the height of a captial letter in the given font and size. pub fn cap_height(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.cap_height(font_size)) } + /// Get the height of the x character in the given font and size. pub fn x_height(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.x_height(font_size)) } + /// Get the recommended distance from the baseline for the given font pub fn ascent(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.ascent(font_size)) } + /// Get the recommended distance below the baseline for the given font, + /// in single spaced text. pub fn descent(&self, font_id: FontId, font_size: Pixels) -> Pixels { self.read_metrics(font_id, |metrics| metrics.descent(font_size)) } + /// Get the recommended baseline offset for the given font and line height. pub fn baseline_offset( &self, font_id: FontId, @@ -199,10 +220,14 @@ impl TextSystem { } } - pub fn with_view(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R { + pub(crate) fn with_view(&self, view_id: EntityId, f: impl FnOnce() -> R) -> R { self.line_layout_cache.with_view(view_id, f) } + /// Layout the given line of text, at the given font_size. + /// Subsets of the line can be styled independently with the `runs` parameter. + /// Generally, you should prefer to use `TextLayout::shape_line` instead, which + /// can be painted directly. pub fn layout_line( &self, text: &str, @@ -234,6 +259,12 @@ impl TextSystem { Ok(layout) } + /// Shape the given line, at the given font_size, for painting to the screen. + /// Subsets of the line can be styled independently with the `runs` parameter. + /// + /// Note that this method can only shape a single line of text. It will panic + /// if the text contains newlines. If you need to shape multiple lines of text, + /// use `TextLayout::shape_text` instead. pub fn shape_line( &self, text: SharedString, @@ -273,6 +304,9 @@ impl TextSystem { }) } + /// Shape a multi line string of text, at the given font_size, for painting to the screen. + /// Subsets of the text can be styled independently with the `runs` parameter. + /// If `wrap_width` is provided, the line breaks will be adjusted to fit within the given width. pub fn shape_text( &self, text: SharedString, @@ -381,6 +415,7 @@ impl TextSystem { self.line_layout_cache.finish_frame(reused_views) } + /// Returns a handle to a line wrapper, for the given font and font size. pub fn line_wrapper(self: &Arc, font: Font, font_size: Pixels) -> LineWrapperHandle { let lock = &mut self.wrapper_pool.lock(); let font_id = self.resolve_font(&font); @@ -397,7 +432,8 @@ impl TextSystem { } } - pub fn raster_bounds(&self, params: &RenderGlyphParams) -> Result> { + /// Get the rasterized size and location of a specific, rendered glyph. + pub(crate) fn raster_bounds(&self, params: &RenderGlyphParams) -> Result> { let raster_bounds = self.raster_bounds.upgradable_read(); if let Some(bounds) = raster_bounds.get(params) { Ok(*bounds) @@ -409,7 +445,7 @@ impl TextSystem { } } - pub fn rasterize_glyph( + pub(crate) fn rasterize_glyph( &self, params: &RenderGlyphParams, ) -> Result<(Size, Vec)> { @@ -425,6 +461,7 @@ struct FontIdWithSize { font_size: Pixels, } +/// A handle into the text system, which can be used to compute the wrapped layout of text pub struct LineWrapperHandle { wrapper: Option, text_system: Arc, @@ -517,40 +554,28 @@ impl Display for FontStyle { } } +/// A styled run of text, for use in [`TextLayout`]. #[derive(Clone, Debug, PartialEq, Eq)] pub struct TextRun { - // number of utf8 bytes + /// A number of utf8 bytes pub len: usize, + /// The font to use for this run. pub font: Font, + /// The color pub color: Hsla, + /// The background color (if any) pub background_color: Option, + /// The underline style (if any) pub underline: Option, } +/// An identifier for a specific glyph, as returned by [`TextSystem::layout_line`]. #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] #[repr(C)] -pub struct GlyphId(u32); - -impl From for u32 { - fn from(value: GlyphId) -> Self { - value.0 - } -} - -impl From for GlyphId { - fn from(num: u16) -> Self { - GlyphId(num as u32) - } -} - -impl From for GlyphId { - fn from(num: u32) -> Self { - GlyphId(num) - } -} +pub struct GlyphId(pub(crate) u32); #[derive(Clone, Debug, PartialEq)] -pub struct RenderGlyphParams { +pub(crate) struct RenderGlyphParams { pub(crate) font_id: FontId, pub(crate) glyph_id: GlyphId, pub(crate) font_size: Pixels, @@ -571,6 +596,7 @@ impl Hash for RenderGlyphParams { } } +/// The parameters for rendering an emoji glyph. #[derive(Clone, Debug, PartialEq)] pub struct RenderEmojiParams { pub(crate) font_id: FontId, @@ -590,14 +616,23 @@ impl Hash for RenderEmojiParams { } } +/// The configuration details for identifying a specific font. #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct Font { + /// The font family name. pub family: SharedString, + + /// The font features to use. pub features: FontFeatures, + + /// The font weight. pub weight: FontWeight, + + /// The font style. pub style: FontStyle, } +/// Get a [`Font`] for a given name. pub fn font(family: impl Into) -> Font { Font { family: family.into(), @@ -608,10 +643,17 @@ pub fn font(family: impl Into) -> Font { } impl Font { + /// Set this Font to be bold pub fn bold(mut self) -> Self { self.weight = FontWeight::BOLD; self } + + /// Set this Font to be italic + pub fn italic(mut self) -> Self { + self.style = FontStyle::Italic; + self + } } /// A struct for storing font metrics. diff --git a/crates/gpui/src/text_system/font_features.rs b/crates/gpui/src/text_system/font_features.rs index ce93fd5bc93fe59c5d7c113c29cc9bf91f69d42e..1824932328a43eca32e5fadfd89556d4799b3915 100644 --- a/crates/gpui/src/text_system/font_features.rs +++ b/crates/gpui/src/text_system/font_features.rs @@ -4,7 +4,9 @@ use schemars::{ }; macro_rules! create_definitions { - ($($(#[$meta:meta])* ($name:ident, $idx:expr)),* $(,)?) => { + ($($(#[$meta:meta])* ($name:ident, $idx:expr), ($comment:tt)),* $(,)?) => { + + /// The font features that can be configured for a given font. #[derive(Default, Copy, Clone, Eq, PartialEq, Hash)] pub struct FontFeatures { enabled: u64, @@ -13,6 +15,7 @@ macro_rules! create_definitions { impl FontFeatures { $( + /// $comment pub fn $name(&self) -> Option { if (self.enabled & (1 << $idx)) != 0 { Some(true) @@ -125,7 +128,7 @@ macro_rules! create_definitions { } create_definitions!( - (calt, 0), + (calt, 0, CALT), (case, 1), (cpsp, 2), (frac, 3), diff --git a/crates/gpui/src/window/element_cx.rs b/crates/gpui/src/window/element_cx.rs index 132cc72a5c7d344cf3dedbbc693ff2306b5bb8b9..c48374529ea90f7186c35f6f61e450ed16958bf0 100644 --- a/crates/gpui/src/window/element_cx.rs +++ b/crates/gpui/src/window/element_cx.rs @@ -1,3 +1,17 @@ +//! The element context is the main interface for interacting with the frame during a paint. +//! +//! Elements are hierarchical and with a few exceptions the context accumulates state in a stack +//! as it processes all of the elements in the frame. The methods that interact with this stack +//! are generally marked with `with_*`, and take a callback to denote the region of code that +//! should be executed with that state. +//! +//! The other main interface is the `paint_*` family of methods, which push basic drawing commands +//! to the GPU. Everything in a GPUI app is drawn with these methods. +//! +//! There are also several internal methds that GPUI uses, such as [`ElementContext::with_element_state`] +//! to call the paint and layout methods on elements. These have been included as they're often useful +//! for taking manual control of the layouting or painting of specialized elements. + use std::{ any::{Any, TypeId}, borrow::{Borrow, BorrowMut, Cow}, @@ -153,6 +167,9 @@ pub struct ElementContext<'a> { } impl<'a> WindowContext<'a> { + /// Convert this window context into an ElementContext in this callback. + /// If you need to use this method, you're probably intermixing the imperative + /// and declarative APIs, which is not recommended. pub fn with_element_context(&mut self, f: impl FnOnce(&mut ElementContext) -> R) -> R { f(&mut ElementContext { cx: WindowContext::new(self.app, self.window), @@ -338,6 +355,8 @@ impl<'a> ElementContext<'a> { self.window.next_frame.next_stacking_order_id = next_stacking_order_id; } + /// Push a text style onto the stack, and call a function with that style active. + /// Use [`AppContext::text_style`] to get the current, combined text style. pub fn with_text_style(&mut self, style: Option, f: F) -> R where F: FnOnce(&mut Self) -> R, @@ -979,9 +998,8 @@ impl<'a> ElementContext<'a> { self.window.layout_engine = Some(layout_engine); } - /// Obtain the bounds computed for the given LayoutId relative to the window. This method should not - /// be invoked until the paint phase begins, and will usually be invoked by GPUI itself automatically - /// in order to pass your element its `Bounds` automatically. + /// Obtain the bounds computed for the given LayoutId relative to the window. This method will usually be invoked by + /// GPUI itself automatically in order to pass your element its `Bounds` automatically. pub fn layout_bounds(&mut self, layout_id: LayoutId) -> Bounds { let mut bounds = self .window diff --git a/crates/refineable/derive_refineable/src/derive_refineable.rs b/crates/refineable/derive_refineable/src/derive_refineable.rs index bc906daece556106978f9788ff1502f004812180..294738b20eb93f3a8f6ab8886d57dbbca816e226 100644 --- a/crates/refineable/derive_refineable/src/derive_refineable.rs +++ b/crates/refineable/derive_refineable/src/derive_refineable.rs @@ -245,10 +245,14 @@ pub fn derive_refineable(input: TokenStream) -> TokenStream { } let gen = quote! { + /// A refinable version of [`#ident`], see that documentation for details. #[derive(Clone)] #derive_stream pub struct #refinement_ident #impl_generics { - #( #field_visibilities #field_names: #wrapped_types ),* + #( + #[allow(missing_docs)] + #field_visibilities #field_names: #wrapped_types + ),* } impl #impl_generics Refineable for #ident #ty_generics @@ -304,6 +308,7 @@ pub fn derive_refineable(input: TokenStream) -> TokenStream { impl #impl_generics #refinement_ident #ty_generics #where_clause { + /// Returns `true` if all fields are `Some` pub fn is_some(&self) -> bool { #( if self.#field_names.is_some() { From eab2e2112636c52e3b8217a9e2a7bd07ab0623db Mon Sep 17 00:00:00 2001 From: Mikayla Date: Mon, 22 Jan 2024 19:00:16 -0800 Subject: [PATCH 5/7] Document more styling functions --- crates/gpui/src/style.rs | 76 ++++++++++++++++++-- crates/gpui/src/styled.rs | 38 +++++++++- crates/gpui/src/text_system/font_features.rs | 8 +-- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index e1d82adea8751a8f0baf60c40dac4efbec993b1b..a12bb6df12836390855c6dfd6da078481425fabc 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -7,7 +7,7 @@ use crate::{ SizeRefinement, Styled, TextRun, }; use collections::HashSet; -use refineable::{Cascade, Refineable}; +use refineable::Refineable; use smallvec::SmallVec; pub use taffy::style::{ AlignContent, AlignItems, AlignSelf, Display, FlexDirection, FlexWrap, JustifyContent, @@ -15,10 +15,12 @@ pub use taffy::style::{ }; #[cfg(debug_assertions)] +/// Use this struct for interfacing with the 'debug_below' styling from your own elements. +/// If a parent element has this style set on it, then this struct will be set as a global in +/// GPUI. pub struct DebugBelow; -pub type StyleCascade = Cascade