From cf12d62fc5283ba0c0c2f8c482ef9a2cc20e704a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Dec 2023 23:36:32 +0200 Subject: [PATCH] Tidy up z-index handling Co-Authored-By: Antonio Scandurra --- crates/collab_ui2/src/collab_titlebar_item.rs | 2 +- crates/gpui2/src/elements/div.rs | 17 +-- crates/gpui2/src/elements/img.rs | 2 +- crates/gpui2/src/scene.rs | 114 ------------------ crates/gpui2/src/window.rs | 24 +--- crates/terminal_view2/src/terminal_element.rs | 10 +- crates/ui2/src/components/context_menu.rs | 36 ++---- crates/ui2/src/components/list/list_item.rs | 3 +- crates/ui2/src/components/tab_bar.rs | 2 +- crates/workspace2/src/toolbar.rs | 3 +- 10 files changed, 31 insertions(+), 182 deletions(-) diff --git a/crates/collab_ui2/src/collab_titlebar_item.rs b/crates/collab_ui2/src/collab_titlebar_item.rs index 914e5c7bb6..17ee5087bb 100644 --- a/crates/collab_ui2/src/collab_titlebar_item.rs +++ b/crates/collab_ui2/src/collab_titlebar_item.rs @@ -68,7 +68,7 @@ impl Render for CollabTitlebarItem { h_stack() .id("titlebar") - .z_index(160) + .z_index(160) // todo!("z-index") .justify_between() .w_full() .h(rems(1.75)) diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index 6e9c95e19f..57fd3d6a8a 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -1448,14 +1448,15 @@ impl Interactivity { } cx.with_z_index(style.z_index.unwrap_or(0), |cx| { - if style.background.as_ref().is_some_and(|fill| { - fill.color().is_some_and(|color| !color.is_transparent()) - }) { - cx.add_opaque_layer(bounds) - }f(style, scroll_offset.unwrap_or_default(), cx) - }) - }, - ); + if style.background.as_ref().is_some_and(|fill| { + fill.color().is_some_and(|color| !color.is_transparent()) + }) { + cx.add_opaque_layer(bounds) + } + f(style, scroll_offset.unwrap_or_default(), cx) + }) + }, + ); if let Some(group) = self.group.as_ref() { GroupBounds::pop(group, cx); diff --git a/crates/gpui2/src/elements/img.rs b/crates/gpui2/src/elements/img.rs index 4f81f604c8..ab7b8d9140 100644 --- a/crates/gpui2/src/elements/img.rs +++ b/crates/gpui2/src/elements/img.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use crate::{ - point, size, Bounds, DevicePixels, Element, ImageData, InteractiveElement, + point, size, BorrowWindow, Bounds, DevicePixels, Element, ImageData, InteractiveElement, InteractiveElementState, Interactivity, IntoElement, LayoutId, Pixels, SharedString, Size, StyleRefinement, Styled, WindowContext, }; diff --git a/crates/gpui2/src/scene.rs b/crates/gpui2/src/scene.rs index 517632ed79..811b2b6e30 100644 --- a/crates/gpui2/src/scene.rs +++ b/crates/gpui2/src/scene.rs @@ -856,117 +856,3 @@ impl Bounds { .expect("Polygon should not be empty") } } - -// todo!() -// #[cfg(test)] -// mod tests { -// use super::*; -// use crate::{point, px, size, Size}; -// use smallvec::smallvec; - -// #[test] -// fn test_scene() { -// let mut scene = SceneBuilder::default(); -// assert_eq!(scene.layers_by_order.len(), 0); - -// // div with z_index(1) -// // glyph with z_index(1) -// // div with z_index(1) -// // glyph with z_index(1) - -// scene.insert( -// &smallvec![1].into(), -// quad( -// point(px(0.), px(0.)), -// size(px(100.), px(100.)), -// crate::black(), -// ), -// ); -// scene.insert( -// &smallvec![1, 1].into(), -// sprite( -// point(px(0.), px(0.)), -// size(px(10.), px(10.)), -// crate::white(), -// ), -// ); -// scene.insert( -// &smallvec![1].into(), -// quad( -// point(px(10.), px(10.)), -// size(px(20.), px(20.)), -// crate::green(), -// ), -// ); -// scene.insert( -// &smallvec![1, 1].into(), -// sprite(point(px(15.), px(15.)), size(px(5.), px(5.)), crate::blue()), -// ); - -// assert!(!scene.layers_by_order.is_empty()); - -// for batch in scene.build().batches() { -// println!("new batch"); -// match batch { -// PrimitiveBatch::Quads(quads) => { -// for quad in quads { -// if quad.background == crate::black() { -// println!(" black quad"); -// } else if quad.background == crate::green() { -// println!(" green quad"); -// } else { -// todo!(" ((( bad quad"); -// } -// } -// } -// PrimitiveBatch::MonochromeSprites { sprites, .. } => { -// for sprite in sprites { -// if sprite.color == crate::white() { -// println!(" white sprite"); -// } else if sprite.color == crate::blue() { -// println!(" blue sprite"); -// } else { -// todo!(" ((( bad sprite") -// } -// } -// } -// _ => todo!(), -// } -// } -// } - -// fn quad(origin: Point, size: Size, background: Hsla) -> Quad { -// Quad { -// order: 0, -// bounds: Bounds { origin, size }.scale(1.), -// background, -// content_mask: ContentMask { -// bounds: Bounds { origin, size }, -// } -// .scale(1.), -// border_color: Default::default(), -// corner_radii: Default::default(), -// border_widths: Default::default(), -// } -// } - -// fn sprite(origin: Point, size: Size, color: Hsla) -> MonochromeSprite { -// MonochromeSprite { -// order: 0, -// bounds: Bounds { origin, size }.scale(1.), -// content_mask: ContentMask { -// bounds: Bounds { origin, size }, -// } -// .scale(1.), -// color, -// tile: AtlasTile { -// texture_id: AtlasTextureId { -// index: 0, -// kind: crate::AtlasTextureKind::Monochrome, -// }, -// tile_id: crate::TileId(0), -// bounds: Default::default(), -// }, -// } -// } -// } diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index ddda9f0302..1b2669b40c 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -39,7 +39,7 @@ use std::{ Arc, }, }; -use util::ResultExt; +use util::{post_inc, ResultExt}; const ACTIVE_DRAG_Z_INDEX: u8 = 1; @@ -939,21 +939,6 @@ impl<'a> WindowContext<'a> { self.window.requested_cursor_style = Some(style) } - /// Called during painting to invoke the given closure in a new stacking context. The given - /// z-index is interpreted relative to the previous call to `stack`. - pub fn with_z_index(&mut self, z_index: u8, f: impl FnOnce(&mut Self) -> R) -> R { - let new_stacking_order_id = self.window.next_frame.next_stacking_order_id; - let new_next_stacking_order_id = new_stacking_order_id + 1; - - self.window.next_frame.next_stacking_order_id = 0; - self.window.next_frame.z_index_stack.id = new_stacking_order_id; - self.window.next_frame.z_index_stack.push(z_index); - let result = f(self); - self.window.next_frame.next_stacking_order_id = new_next_stacking_order_id; - self.window.next_frame.z_index_stack.pop(); - result - } - /// Called during painting to track which z-index is on top at each pixel position pub fn add_opaque_layer(&mut self, bounds: Bounds) { let stacking_order = self.window.next_frame.z_index_stack.clone(); @@ -2066,14 +2051,11 @@ pub trait BorrowWindow: BorrowMut + BorrowMut { /// Called during painting to invoke the given closure in a new stacking context. The given /// z-index is interpreted relative to the previous call to `stack`. fn with_z_index(&mut self, z_index: u8, f: impl FnOnce(&mut Self) -> R) -> R { - let new_stacking_order_id = self.window_mut().next_frame.next_stacking_order_id; - let new_next_stacking_order_id = new_stacking_order_id + 1; - - self.window_mut().next_frame.next_stacking_order_id = 0; + let new_stacking_order_id = + post_inc(&mut self.window_mut().next_frame.next_stacking_order_id); self.window_mut().next_frame.z_index_stack.id = new_stacking_order_id; self.window_mut().next_frame.z_index_stack.push(z_index); let result = f(self); - self.window_mut().next_frame.next_stacking_order_id = new_next_stacking_order_id; self.window_mut().next_frame.z_index_stack.pop(); result } diff --git a/crates/terminal_view2/src/terminal_element.rs b/crates/terminal_view2/src/terminal_element.rs index e9335a4220..07910896f3 100644 --- a/crates/terminal_view2/src/terminal_element.rs +++ b/crates/terminal_view2/src/terminal_element.rs @@ -1,11 +1,11 @@ use editor::{Cursor, HighlightedRange, HighlightedRangeLine}; use gpui::{ black, div, fill, point, px, red, relative, AnyElement, AsyncWindowContext, AvailableSpace, - Bounds, DispatchPhase, Element, ElementId, ExternalPaths, FocusHandle, Font, FontStyle, - FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState, Interactivity, - IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, Pixels, - PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement, Styled, - TextRun, TextStyle, TextSystem, UnderlineStyle, WhiteSpace, WindowContext, + BorrowWindow, Bounds, DispatchPhase, Element, ElementId, ExternalPaths, FocusHandle, Font, + FontStyle, FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState, + Interactivity, IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, + Pixels, PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement, + Styled, TextRun, TextStyle, TextSystem, UnderlineStyle, WhiteSpace, WindowContext, }; use itertools::Itertools; use language::CursorShape; diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 84c66f3534..8fce15d1c6 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -3,8 +3,8 @@ use crate::{ ListSeparator, ListSubHeader, }; use gpui::{ - px, Action, AnyElement, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, - FocusableView, IntoElement, Render, Subscription, View, VisualContext, + px, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, + IntoElement, Render, Subscription, View, VisualContext, }; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; use std::{rc::Rc, time::Duration}; @@ -18,9 +18,6 @@ pub enum ContextMenuItem { handler: Rc, action: Option>, }, - CustomEntry { - entry_render: Box AnyElement>, - }, } pub struct ContextMenu { @@ -86,16 +83,6 @@ impl ContextMenu { self } - pub fn custom_entry( - mut self, - entry_render: impl Fn(&mut WindowContext) -> AnyElement + 'static, - ) -> Self { - self.items.push(ContextMenuItem::CustomEntry { - entry_render: Box::new(entry_render), - }); - self - } - pub fn action(mut self, label: impl Into, action: Box) -> Self { self.items.push(ContextMenuItem::Entry { label: label.into(), @@ -243,9 +230,9 @@ impl Render for ContextMenu { el }) .flex_none() - .child(List::new().children(self.items.iter_mut().enumerate().map( - |(ix, item)| { - match item { + .child( + List::new().children(self.items.iter().enumerate().map( + |(ix, item)| match item { ContextMenuItem::Separator => ListSeparator.into_any_element(), ContextMenuItem::Header(header) => { ListSubHeader::new(header.clone()).into_any_element() @@ -268,7 +255,7 @@ impl Render for ContextMenu { Label::new(label.clone()).into_any_element() }; - ListItem::new(ix) + ListItem::new(label.clone()) .inset(true) .selected(Some(ix) == self.selected_index) .on_click(move |_, cx| handler(cx)) @@ -284,14 +271,9 @@ impl Render for ContextMenu { ) .into_any_element() } - ContextMenuItem::CustomEntry { entry_render } => ListItem::new(ix) - .inset(true) - .selected(Some(ix) == self.selected_index) - .child(entry_render(cx)) - .into_any_element(), - } - }, - ))), + }, + )), + ), ) } } diff --git a/crates/ui2/src/components/list/list_item.rs b/crates/ui2/src/components/list/list_item.rs index e8689030c3..bdb5c2b854 100644 --- a/crates/ui2/src/components/list/list_item.rs +++ b/crates/ui2/src/components/list/list_item.rs @@ -171,8 +171,7 @@ impl RenderOnce for ListItem { }) }) .when_some(self.on_click, |this, on_click| { - this.cursor_pointer() - .on_click(move |event, cx| on_click(event, cx)) + this.cursor_pointer().on_click(on_click) }) .when_some(self.on_secondary_mouse_down, |this, on_mouse_down| { this.on_mouse_down(MouseButton::Right, move |event, cx| { diff --git a/crates/ui2/src/components/tab_bar.rs b/crates/ui2/src/components/tab_bar.rs index b10ffd1c63..d2e6e9518b 100644 --- a/crates/ui2/src/components/tab_bar.rs +++ b/crates/ui2/src/components/tab_bar.rs @@ -96,7 +96,7 @@ impl RenderOnce for TabBar { div() .id(self.id) - .z_index(120) + .z_index(120) // todo!("z-index") .group("tab_bar") .flex() .flex_none() diff --git a/crates/workspace2/src/toolbar.rs b/crates/workspace2/src/toolbar.rs index f83879a64e..7436232b04 100644 --- a/crates/workspace2/src/toolbar.rs +++ b/crates/workspace2/src/toolbar.rs @@ -105,8 +105,7 @@ impl Render for Toolbar { v_stack() .p_1() .gap_2() - // todo!() use a proper constant here (ask Marshall & Nate) - .z_index(80) + .z_index(80) // todo!("z-index") .border_b() .border_color(cx.theme().colors().border_variant) .bg(cx.theme().colors().toolbar_background)