From 73682daf38c023646b1fec6cffad9412661f059c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 21 Dec 2023 09:41:48 +0100 Subject: [PATCH] Provide mut access to allocated arena struct via non-cloneable ArenaBox This commit also allows turning an `ArenaBox` into an `ArenaRef` when non-mutable access is required, which makes `ArenaRef: Clone`. This fixes a bug that prevented the command palette from reading all the available actions while the `command_palette::Toggle` action was being dispatched. --- crates/gpui2/src/arena.rs | 42 ++++++++++++--- crates/gpui2/src/element.rs | 4 +- crates/gpui2/src/key_dispatch.rs | 5 +- crates/gpui2/src/window.rs | 92 ++++++++++++-------------------- 4 files changed, 70 insertions(+), 73 deletions(-) diff --git a/crates/gpui2/src/arena.rs b/crates/gpui2/src/arena.rs index ef66188a0ebcf7b6b4c4175bec2c7a571478f650..bb493a6d06487a5b07f7a2afbedc36d85fa9f715 100644 --- a/crates/gpui2/src/arena.rs +++ b/crates/gpui2/src/arena.rs @@ -52,7 +52,7 @@ impl Arena { } #[inline(always)] - pub fn alloc(&mut self, f: impl FnOnce() -> T) -> ArenaRef { + pub fn alloc(&mut self, f: impl FnOnce() -> T) -> ArenaBox { #[inline(always)] unsafe fn inner_writer(ptr: *mut T, f: F) where @@ -70,7 +70,7 @@ impl Arena { let next_offset = self.offset.add(layout.size()); assert!(next_offset <= self.end); - let result = ArenaRef { + let result = ArenaBox { ptr: self.offset.cast(), valid: self.valid.clone(), }; @@ -93,15 +93,15 @@ impl Drop for Arena { } } -pub struct ArenaRef { +pub struct ArenaBox { ptr: *mut T, valid: Rc>, } -impl ArenaRef { +impl ArenaBox { #[inline(always)] - pub fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaRef { - ArenaRef { + pub fn map(mut self, f: impl FnOnce(&mut T) -> &mut U) -> ArenaBox { + ArenaBox { ptr: f(&mut self), valid: self.valid, } @@ -115,7 +115,7 @@ impl ArenaRef { } } -impl Deref for ArenaRef { +impl Deref for ArenaBox { type Target = T; #[inline(always)] @@ -125,7 +125,7 @@ impl Deref for ArenaRef { } } -impl DerefMut for ArenaRef { +impl DerefMut for ArenaBox { #[inline(always)] fn deref_mut(&mut self) -> &mut Self::Target { self.validate(); @@ -133,6 +133,32 @@ impl DerefMut for ArenaRef { } } +pub struct ArenaRef(ArenaBox); + +impl From> for ArenaRef { + fn from(value: ArenaBox) -> Self { + ArenaRef(value) + } +} + +impl Clone for ArenaRef { + fn clone(&self) -> Self { + Self(ArenaBox { + ptr: self.0.ptr, + valid: self.0.valid.clone(), + }) + } +} + +impl Deref for ArenaRef { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + #[cfg(test)] mod tests { use std::{cell::Cell, rc::Rc}; diff --git a/crates/gpui2/src/element.rs b/crates/gpui2/src/element.rs index d0ed50a3d54780130a0894b77c35493b0278e344..4201123a10110f540258b8afc462aeab63b3fb1a 100644 --- a/crates/gpui2/src/element.rs +++ b/crates/gpui2/src/element.rs @@ -1,5 +1,5 @@ use crate::{ - ArenaRef, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, Size, + ArenaBox, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, Size, ViewContext, WindowContext, ELEMENT_ARENA, }; use derive_more::{Deref, DerefMut}; @@ -405,7 +405,7 @@ where } } -pub struct AnyElement(ArenaRef); +pub struct AnyElement(ArenaBox); impl AnyElement { pub fn new(element: E) -> Self diff --git a/crates/gpui2/src/key_dispatch.rs b/crates/gpui2/src/key_dispatch.rs index 3201e00bc6b6ebd6bbbbf9ff28dc01ff14246447..cc6376e68d4c47e4476dd9ef2ae08a300f43727c 100644 --- a/crates/gpui2/src/key_dispatch.rs +++ b/crates/gpui2/src/key_dispatch.rs @@ -35,6 +35,7 @@ pub(crate) struct DispatchNode { type KeyListener = ArenaRef; +#[derive(Clone)] pub(crate) struct DispatchActionListener { pub(crate) action_type: TypeId, pub(crate) listener: ArenaRef, @@ -268,10 +269,6 @@ impl DispatchTree { &self.nodes[node_id.0] } - pub fn node_mut(&mut self, node_id: DispatchNodeId) -> &mut DispatchNode { - &mut self.nodes[node_id.0] - } - fn active_node(&mut self) -> &mut DispatchNode { let active_node_id = self.active_node_id(); &mut self.nodes[active_node_id.0] diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 49fb60efe406341ead1edbc18a45ed696ce3dba2..e57984fa0d0fe2ee577a162608159cc72ea0f848 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -1,17 +1,15 @@ use crate::{ - arena::{Arena, ArenaRef}, - key_dispatch::DispatchActionListener, - px, size, transparent_black, Action, AnyDrag, AnyView, AppContext, AsyncWindowContext, - AvailableSpace, Bounds, BoxShadow, Context, Corners, CursorStyle, DevicePixels, DispatchNodeId, - DispatchTree, DisplayId, Edges, Effect, Entity, EntityId, EventEmitter, FileDropEvent, Flatten, - FontId, GlobalElementId, GlyphId, Hsla, ImageData, InputEvent, IsZero, KeyBinding, KeyContext, - KeyDownEvent, KeystrokeEvent, LayoutId, Model, ModelContext, Modifiers, MonochromeSprite, - MouseButton, MouseMoveEvent, MouseUpEvent, Path, Pixels, PlatformAtlas, PlatformDisplay, - PlatformInputHandler, PlatformWindow, Point, PolychromeSprite, PromptLevel, Quad, Render, - RenderGlyphParams, RenderImageParams, RenderSvgParams, ScaledPixels, Scene, SceneBuilder, - Shadow, SharedString, Size, Style, SubscriberSet, Subscription, Surface, TaffyLayoutEngine, - Task, Underline, UnderlineStyle, View, VisualContext, WeakView, WindowBounds, WindowOptions, - SUBPIXEL_VARIANTS, + px, size, transparent_black, Action, AnyDrag, AnyView, AppContext, Arena, ArenaBox, ArenaRef, + AsyncWindowContext, AvailableSpace, Bounds, BoxShadow, Context, Corners, CursorStyle, + DevicePixels, DispatchActionListener, DispatchNodeId, DispatchTree, DisplayId, Edges, Effect, + Entity, EntityId, EventEmitter, FileDropEvent, Flatten, FontId, GlobalElementId, GlyphId, Hsla, + ImageData, InputEvent, IsZero, KeyBinding, KeyContext, KeyDownEvent, KeystrokeEvent, LayoutId, + Model, ModelContext, Modifiers, MonochromeSprite, MouseButton, MouseMoveEvent, MouseUpEvent, + Path, Pixels, PlatformAtlas, PlatformDisplay, PlatformInputHandler, PlatformWindow, Point, + PolychromeSprite, PromptLevel, Quad, Render, RenderGlyphParams, RenderImageParams, + RenderSvgParams, ScaledPixels, Scene, SceneBuilder, Shadow, SharedString, Size, Style, + SubscriberSet, Subscription, Surface, TaffyLayoutEngine, Task, Underline, UnderlineStyle, View, + VisualContext, WeakView, WindowBounds, WindowOptions, SUBPIXEL_VARIANTS, }; use anyhow::{anyhow, Context as _, Result}; use collections::FxHashMap; @@ -96,7 +94,7 @@ impl DispatchPhase { } type AnyObserver = Box bool + 'static>; -type AnyMouseListener = ArenaRef; +type AnyMouseListener = ArenaBox; type AnyWindowFocusListener = Box bool + 'static>; struct FocusEvent { @@ -906,7 +904,10 @@ impl<'a> WindowContext<'a> { } }) .map(|handler| handler as _); - self.window.next_frame.dispatch_tree.on_key_event(listener); + self.window + .next_frame + .dispatch_tree + .on_key_event(ArenaRef::from(listener)); } /// Register an action listener on the window for the next frame. The type of action @@ -928,7 +929,7 @@ impl<'a> WindowContext<'a> { self.window .next_frame .dispatch_tree - .on_action(action_type, listener); + .on_action(action_type, ArenaRef::from(listener)); } pub fn is_action_available(&self, action: &dyn Action) -> bool { @@ -1339,7 +1340,7 @@ impl<'a> WindowContext<'a> { cx.window .next_frame .dispatch_tree - .on_action(*action_type, listener) + .on_action(*action_type, ArenaRef::from(listener)) } } @@ -1583,43 +1584,30 @@ impl<'a> WindowContext<'a> { self.propagate_event = true; for node_id in &dispatch_path { - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); + let node = self.window.rendered_frame.dispatch_tree.node(*node_id); + if let Some(context) = node.context.clone() { context_stack.push(context); } - let key_listeners = mem::take(&mut node.key_listeners); - for key_listener in &key_listeners { + for key_listener in node.key_listeners.clone() { key_listener(event, DispatchPhase::Capture, self); if !self.propagate_event { - break; + return; } } - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - node.key_listeners = key_listeners; - - if !self.propagate_event { - return; - } } // Bubble phase for node_id in dispatch_path.iter().rev() { // Handle low level key events - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - let key_listeners = mem::take(&mut node.key_listeners); - for key_listener in &key_listeners { + let node = self.window.rendered_frame.dispatch_tree.node(*node_id); + for key_listener in node.key_listeners.clone() { key_listener(event, DispatchPhase::Bubble, self); if !self.propagate_event { - break; + return; } } - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - node.key_listeners = key_listeners; - - if !self.propagate_event { - return; - } // Match keystrokes let node = self.window.rendered_frame.dispatch_tree.node(*node_id); @@ -1667,52 +1655,38 @@ impl<'a> WindowContext<'a> { // Capture phase for node_id in &dispatch_path { - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - let action_listeners = mem::take(&mut node.action_listeners); + let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, listener, - } in &action_listeners + } in node.action_listeners.clone() { let any_action = action.as_any(); - if *action_type == any_action.type_id() { + if action_type == any_action.type_id() { listener(any_action, DispatchPhase::Capture, self); if !self.propagate_event { - break; + return; } } } - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - node.action_listeners = action_listeners; - - if !self.propagate_event { - return; - } } // Bubble phase for node_id in dispatch_path.iter().rev() { - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - let action_listeners = mem::take(&mut node.action_listeners); + let node = self.window.rendered_frame.dispatch_tree.node(*node_id); for DispatchActionListener { action_type, listener, - } in &action_listeners + } in node.action_listeners.clone() { let any_action = action.as_any(); - if *action_type == any_action.type_id() { + if action_type == any_action.type_id() { self.propagate_event = false; // Actions stop propagation by default during the bubble phase listener(any_action, DispatchPhase::Bubble, self); if !self.propagate_event { - break; + return; } } } - - let node = self.window.rendered_frame.dispatch_tree.node_mut(*node_id); - node.action_listeners = action_listeners; - if !self.propagate_event { - return; - } } }