Merge pull request #1809 from zed-industries/contacts-popover-z-index

Antonio Scandurra created

Prevent expanded dock from hiding contacts popover

Change summary

crates/collab_ui/src/collab_titlebar_item.rs       |  1 
crates/editor/src/element.rs                       | 12 +-
crates/gpui/src/elements/overlay.rs                |  9 +
crates/gpui/src/elements/resizable.rs              |  2 
crates/gpui/src/gpui.rs                            |  2 
crates/gpui/src/presenter.rs                       | 47 +++++----
crates/gpui/src/scene.rs                           | 74 ++++++++++-----
crates/workspace/src/pane/dragged_item_receiver.rs |  2 
8 files changed, 92 insertions(+), 57 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -31,7 +31,7 @@ use gpui::{
     text_layout::{self, Line, RunStyle, TextLayoutCache},
     AppContext, Axis, Border, CursorRegion, Element, ElementBox, EventContext, LayoutContext,
     Modifiers, MouseButton, MouseButtonEvent, MouseMovedEvent, MouseRegion, MutableAppContext,
-    PaintContext, Quad, Scene, SizeConstraint, ViewContext, WeakViewHandle,
+    PaintContext, Quad, SceneBuilder, SizeConstraint, ViewContext, WeakViewHandle,
 };
 use json::json;
 use language::{Bias, CursorShape, DiagnosticSeverity, OffsetUtf16, Point, Selection};
@@ -791,7 +791,7 @@ impl EditorElement {
         cx.scene.pop_layer();
 
         if let Some((position, context_menu)) = layout.context_menu.as_mut() {
-            cx.scene.push_stacking_context(None);
+            cx.scene.push_stacking_context(None, None);
             let cursor_row_layout =
                 &layout.position_map.line_layouts[(position.row() - start_row) as usize];
             let x = cursor_row_layout.x_for_index(position.column() as usize) - scroll_left;
@@ -820,7 +820,7 @@ impl EditorElement {
         }
 
         if let Some((position, hover_popovers)) = layout.hover_popovers.as_mut() {
-            cx.scene.push_stacking_context(None);
+            cx.scene.push_stacking_context(None, None);
 
             // This is safe because we check on layout whether the required row is available
             let hovered_row_layout =
@@ -2189,7 +2189,7 @@ pub struct HighlightedRangeLine {
 }
 
 impl HighlightedRange {
-    pub fn paint(&self, bounds: RectF, scene: &mut Scene) {
+    pub fn paint(&self, bounds: RectF, scene: &mut SceneBuilder) {
         if self.lines.len() >= 2 && self.lines[0].start_x > self.lines[1].end_x {
             self.paint_lines(self.start_y, &self.lines[0..1], bounds, scene);
             self.paint_lines(
@@ -2208,7 +2208,7 @@ impl HighlightedRange {
         start_y: f32,
         lines: &[HighlightedRangeLine],
         bounds: RectF,
-        scene: &mut Scene,
+        scene: &mut SceneBuilder,
     ) {
         if lines.is_empty() {
             return;
@@ -2375,7 +2375,7 @@ mod tests {
 
         let mut element = EditorElement::new(editor.downgrade(), editor.read(cx).style(cx));
 
-        let mut scene = Scene::new(1.0);
+        let mut scene = SceneBuilder::new(1.0);
         let mut presenter = cx.build_presenter(window_id, 30., Default::default());
         let mut layout_cx = presenter.build_layout_context(Vector2F::zero(), false, cx);
         let (size, mut state) = element.layout(

crates/gpui/src/elements/overlay.rs 🔗

@@ -16,6 +16,7 @@ pub struct Overlay {
     fit_mode: OverlayFitMode,
     position_mode: OverlayPositionMode,
     hoverable: bool,
+    z_index: Option<usize>,
 }
 
 #[derive(Copy, Clone)]
@@ -82,6 +83,7 @@ impl Overlay {
             fit_mode: OverlayFitMode::None,
             position_mode: OverlayPositionMode::Window,
             hoverable: false,
+            z_index: None,
         }
     }
 
@@ -109,6 +111,11 @@ impl Overlay {
         self.hoverable = hoverable;
         self
     }
+
+    pub fn with_z_index(mut self, z_index: usize) -> Self {
+        self.z_index = Some(z_index);
+        self
+    }
 }
 
 impl Element for Overlay {
@@ -204,7 +211,7 @@ impl Element for Overlay {
             OverlayFitMode::None => {}
         }
 
-        cx.paint_stacking_context(None, |cx| {
+        cx.paint_stacking_context(None, self.z_index, |cx| {
             if self.hoverable {
                 enum OverlayHoverCapture {}
                 // Block hovers in lower stacking contexts

crates/gpui/src/elements/resizable.rs 🔗

@@ -149,7 +149,7 @@ impl Element for Resizable {
         _child_size: &mut Self::LayoutState,
         cx: &mut crate::PaintContext,
     ) -> Self::PaintState {
-        cx.scene.push_stacking_context(None);
+        cx.scene.push_stacking_context(None, None);
 
         let handle_region = self.side.of_rect(bounds, self.handle_size);
 

crates/gpui/src/gpui.rs 🔗

@@ -16,7 +16,7 @@ pub mod fonts;
 pub mod geometry;
 mod presenter;
 pub mod scene;
-pub use scene::{Border, CursorRegion, MouseRegion, MouseRegionId, Quad, Scene};
+pub use scene::{Border, CursorRegion, MouseRegion, MouseRegionId, Quad, Scene, SceneBuilder};
 pub mod text_layout;
 pub use text_layout::TextLayoutCache;
 mod util;

crates/gpui/src/presenter.rs 🔗

@@ -8,13 +8,14 @@ use crate::{
     platform::{CursorStyle, Event},
     scene::{
         CursorRegion, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseEvent, MouseHover,
-        MouseMove, MouseScrollWheel, MouseUp, MouseUpOut,
+        MouseMove, MouseScrollWheel, MouseUp, MouseUpOut, Scene,
     },
     text_layout::TextLayoutCache,
     Action, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, AnyWeakViewHandle, Appearance,
     AssetCache, ElementBox, Entity, FontSystem, ModelHandle, MouseButton, MouseMovedEvent,
-    MouseRegion, MouseRegionId, ParentId, ReadModel, ReadView, RenderContext, RenderParams, Scene,
-    UpgradeModelHandle, UpgradeViewHandle, View, ViewHandle, WeakModelHandle, WeakViewHandle,
+    MouseRegion, MouseRegionId, ParentId, ReadModel, ReadView, RenderContext, RenderParams,
+    SceneBuilder, UpgradeModelHandle, UpgradeViewHandle, View, ViewHandle, WeakModelHandle,
+    WeakViewHandle,
 };
 use collections::{HashMap, HashSet};
 use pathfinder_geometry::vector::{vec2f, Vector2F};
@@ -135,17 +136,18 @@ impl Presenter {
         refreshing: bool,
         cx: &mut MutableAppContext,
     ) -> Scene {
-        let mut scene = Scene::new(scale_factor);
+        let mut scene_builder = SceneBuilder::new(scale_factor);
 
         if let Some(root_view_id) = cx.root_view_id(self.window_id) {
             self.layout(window_size, refreshing, cx);
-            let mut paint_cx = self.build_paint_context(&mut scene, window_size, cx);
+            let mut paint_cx = self.build_paint_context(&mut scene_builder, window_size, cx);
             paint_cx.paint(
                 root_view_id,
                 Vector2F::zero(),
                 RectF::new(Vector2F::zero(), window_size),
             );
             self.text_layout_cache.finish_frame();
+            let scene = scene_builder.build();
             self.cursor_regions = scene.cursor_regions();
             self.mouse_regions = scene.mouse_regions();
 
@@ -154,11 +156,12 @@ impl Presenter {
                     self.dispatch_event(event, true, cx);
                 }
             }
+
+            scene
         } else {
             log::error!("could not find root_view_id for window {}", self.window_id);
+            scene_builder.build()
         }
-
-        scene
     }
 
     fn layout(&mut self, window_size: Vector2F, refreshing: bool, cx: &mut MutableAppContext) {
@@ -196,7 +199,7 @@ impl Presenter {
 
     pub fn build_paint_context<'a>(
         &'a mut self,
-        scene: &'a mut Scene,
+        scene: &'a mut SceneBuilder,
         window_size: Vector2F,
         cx: &'a mut MutableAppContext,
     ) -> PaintContext {
@@ -357,14 +360,14 @@ impl Presenter {
         for mut mouse_event in mouse_events {
             let mut valid_regions = Vec::new();
 
-            // GPUI elements are arranged by depth but sibling elements can register overlapping
+            // GPUI elements are arranged by z_index but sibling elements can register overlapping
             // mouse regions. As such, hover events are only fired on overlapping elements which
-            // are at the same depth as the topmost element which overlaps with the mouse.
+            // are at the same z-index as the topmost element which overlaps with the mouse.
             match &mouse_event {
                 MouseEvent::Hover(_) => {
-                    let mut top_most_depth = None;
+                    let mut highest_z_index = None;
                     let mouse_position = self.mouse_position.clone();
-                    for (region, depth) in self.mouse_regions.iter().rev() {
+                    for (region, z_index) in self.mouse_regions.iter().rev() {
                         // Allow mouse regions to appear transparent to hovers
                         if !region.hoverable {
                             continue;
@@ -372,15 +375,15 @@ impl Presenter {
 
                         let contains_mouse = region.bounds.contains_point(mouse_position);
 
-                        if contains_mouse && top_most_depth.is_none() {
-                            top_most_depth = Some(depth);
+                        if contains_mouse && highest_z_index.is_none() {
+                            highest_z_index = Some(z_index);
                         }
 
                         // This unwrap relies on short circuiting boolean expressions
                         // The right side of the && is only executed when contains_mouse
                         // is true, and we know above that when contains_mouse is true
-                        // top_most_depth is set
-                        if contains_mouse && depth == top_most_depth.unwrap() {
+                        // highest_z_index is set.
+                        if contains_mouse && z_index == highest_z_index.unwrap() {
                             //Ensure that hover entrance events aren't sent twice
                             if self.hovered_region_ids.insert(region.id()) {
                                 valid_regions.push(region.clone());
@@ -689,7 +692,7 @@ pub struct PaintContext<'a> {
     rendered_views: &'a mut HashMap<usize, ElementBox>,
     view_stack: Vec<usize>,
     pub window_size: Vector2F,
-    pub scene: &'a mut Scene,
+    pub scene: &'a mut SceneBuilder,
     pub font_cache: &'a FontCache,
     pub text_layout_cache: &'a TextLayoutCache,
     pub app: &'a AppContext,
@@ -706,11 +709,15 @@ impl<'a> PaintContext<'a> {
     }
 
     #[inline]
-    pub fn paint_stacking_context<F>(&mut self, clip_bounds: Option<RectF>, f: F)
-    where
+    pub fn paint_stacking_context<F>(
+        &mut self,
+        clip_bounds: Option<RectF>,
+        z_index: Option<usize>,
+        f: F,
+    ) where
         F: FnOnce(&mut Self),
     {
-        self.scene.push_stacking_context(clip_bounds);
+        self.scene.push_stacking_context(clip_bounds, z_index);
         f(self);
         self.scene.pop_stacking_context();
     }

crates/gpui/src/scene.rs 🔗

@@ -18,7 +18,7 @@ use crate::{
 pub use mouse_event::*;
 pub use mouse_region::*;
 
-pub struct Scene {
+pub struct SceneBuilder {
     scale_factor: f32,
     stacking_contexts: Vec<StackingContext>,
     active_stacking_context_stack: Vec<usize>,
@@ -26,10 +26,15 @@ pub struct Scene {
     mouse_region_ids: HashSet<MouseRegionId>,
 }
 
+pub struct Scene {
+    scale_factor: f32,
+    stacking_contexts: Vec<StackingContext>,
+}
+
 struct StackingContext {
     layers: Vec<Layer>,
     active_layer_stack: Vec<usize>,
-    depth: usize,
+    z_index: usize,
 }
 
 #[derive(Default)]
@@ -177,17 +182,6 @@ pub struct Image {
 }
 
 impl Scene {
-    pub fn new(scale_factor: f32) -> Self {
-        let stacking_context = StackingContext::new(0, None);
-        Scene {
-            scale_factor,
-            stacking_contexts: vec![stacking_context],
-            active_stacking_context_stack: vec![0],
-            #[cfg(debug_assertions)]
-            mouse_region_ids: Default::default(),
-        }
-    }
-
     pub fn scale_factor(&self) -> f32 {
         self.scale_factor
     }
@@ -204,24 +198,50 @@ impl Scene {
     }
 
     pub fn mouse_regions(&self) -> Vec<(MouseRegion, usize)> {
-        let mut regions = Vec::new();
-        for stacking_context in self.stacking_contexts.iter() {
-            for layer in &stacking_context.layers {
-                for mouse_region in &layer.mouse_regions {
-                    regions.push((mouse_region.clone(), stacking_context.depth));
-                }
-            }
+        self.stacking_contexts
+            .iter()
+            .flat_map(|context| {
+                context
+                    .layers
+                    .iter()
+                    .flat_map(|layer| &layer.mouse_regions)
+                    .map(|region| (region.clone(), context.z_index))
+            })
+            .collect()
+    }
+}
+
+impl SceneBuilder {
+    pub fn new(scale_factor: f32) -> Self {
+        let stacking_context = StackingContext::new(None, 0);
+        SceneBuilder {
+            scale_factor,
+            stacking_contexts: vec![stacking_context],
+            active_stacking_context_stack: vec![0],
+            #[cfg(debug_assertions)]
+            mouse_region_ids: Default::default(),
         }
-        regions.sort_by_key(|(_, depth)| *depth);
-        regions
     }
 
-    pub fn push_stacking_context(&mut self, clip_bounds: Option<RectF>) {
-        let depth = self.active_stacking_context().depth + 1;
+    pub fn build(mut self) -> Scene {
+        self.stacking_contexts
+            .sort_by_key(|context| context.z_index);
+        Scene {
+            scale_factor: self.scale_factor,
+            stacking_contexts: self.stacking_contexts,
+        }
+    }
+
+    pub fn scale_factor(&self) -> f32 {
+        self.scale_factor
+    }
+
+    pub fn push_stacking_context(&mut self, clip_bounds: Option<RectF>, z_index: Option<usize>) {
+        let z_index = z_index.unwrap_or_else(|| self.active_stacking_context().z_index + 1);
         self.active_stacking_context_stack
             .push(self.stacking_contexts.len());
         self.stacking_contexts
-            .push(StackingContext::new(depth, clip_bounds))
+            .push(StackingContext::new(clip_bounds, z_index))
     }
 
     pub fn pop_stacking_context(&mut self) {
@@ -313,11 +333,11 @@ impl Scene {
 }
 
 impl StackingContext {
-    fn new(depth: usize, clip_bounds: Option<RectF>) -> Self {
+    fn new(clip_bounds: Option<RectF>, z_index: usize) -> Self {
         Self {
             layers: vec![Layer::new(clip_bounds)],
             active_layer_stack: vec![0],
-            depth,
+            z_index,
         }
     }
 

crates/workspace/src/pane/dragged_item_receiver.rs 🔗

@@ -48,7 +48,7 @@ where
                             .map(|(dir, margin)| dir.along_edge(bounds, margin))
                             .unwrap_or(bounds);
 
-                        cx.paint_stacking_context(None, |cx| {
+                        cx.paint_stacking_context(None, None, |cx| {
                             cx.scene.push_quad(Quad {
                                 bounds: overlay_region,
                                 background: Some(overlay_color(cx)),