From 825a8f0927c17baa14b67c8d5ea1d212e2294350 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 18 Dec 2023 19:15:54 +0200 Subject: [PATCH 1/4] Initial fix of the z-index Co-Authored-By: Antonio Scandurra Co-Authored-By: Nathan Sobo --- crates/gpui2/src/elements/div.rs | 12 +- crates/gpui2/src/scene.rs | 165 ++++++++++++++------ crates/gpui2/src/window.rs | 75 +++++++-- crates/ui2/src/components/list/list_item.rs | 4 +- crates/workspace2/src/toolbar.rs | 2 + 5 files changed, 187 insertions(+), 71 deletions(-) diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index 0d477fc62d76c1c24371ad811f6e8143b05f627c..6e9c95e19f4268e929a09d8eae671ebf7831fcff 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -1447,9 +1447,15 @@ impl Interactivity { cx.on_action(action_type, listener) } - f(style, scroll_offset.unwrap_or_default(), cx) - }, - ); + 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 let Some(group) = self.group.as_ref() { GroupBounds::pop(group, cx); diff --git a/crates/gpui2/src/scene.rs b/crates/gpui2/src/scene.rs index 68c068dfe98473872024097d3ca1745866fc66eb..15a4a37d5b1b6d67e0d6be53e75a6e2a772e1d1c 100644 --- a/crates/gpui2/src/scene.rs +++ b/crates/gpui2/src/scene.rs @@ -857,55 +857,116 @@ impl Bounds { } } -// #[cfg(test)] -// mod tests { -// use crate::{point, size}; - -// use super::*; -// use smallvec::smallvec; - -// #[test] -// fn test_scene() { -// let mut scene = SceneBuilder::new(); -// assert_eq!(scene.layers_by_order.len(), 0); - -// scene.insert(&smallvec![1].into(), quad()); -// scene.insert(&smallvec![2].into(), shadow()); -// scene.insert(&smallvec![3].into(), quad()); - -// let mut batches_count = 0; -// for _ in scene.build().batches() { -// batches_count += 1; -// } -// assert_eq!(batches_count, 3); -// } - -// fn quad() -> Quad { -// Quad { -// order: 0, -// bounds: Bounds { -// origin: point(ScaledPixels(0.), ScaledPixels(0.)), -// size: size(ScaledPixels(100.), ScaledPixels(100.)), -// }, -// content_mask: Default::default(), -// background: Default::default(), -// border_color: Default::default(), -// corner_radii: Default::default(), -// border_widths: Default::default(), -// } -// } - -// fn shadow() -> Shadow { -// Shadow { -// order: Default::default(), -// bounds: Bounds { -// origin: point(ScaledPixels(0.), ScaledPixels(0.)), -// size: size(ScaledPixels(100.), ScaledPixels(100.)), -// }, -// corner_radii: Default::default(), -// content_mask: Default::default(), -// color: Default::default(), -// blur_radius: Default::default(), -// } -// } -// } +#[cfg(test)] +mod tests { + use super::*; + use crate::{point, px, size, Size}; + use smallvec::smallvec; + + // todo!() + // #[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 f7ebddd0fea2b725e45d798b1a75e1d76ed3b5f7..5d06cfe1a540a2e931e6295504f3235f7e6ba5fd 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -39,24 +39,36 @@ use std::{ Arc, }, }; -use util::ResultExt; +use util::{post_inc, ResultExt}; const ACTIVE_DRAG_Z_INDEX: u8 = 1; /// A global stacking order, which is created by stacking successive z-index values. /// Each z-index will always be interpreted in the context of its parent z-index. -#[derive(Deref, DerefMut, Clone, Debug, Ord, PartialOrd, PartialEq, Eq)] +#[derive(Deref, DerefMut, Clone, Ord, PartialOrd, PartialEq, Eq, Default)] pub struct StackingOrder { #[deref] #[deref_mut] - z_indices: SmallVec<[u8; 64]>, + context_stack: SmallVec<[StackingContext; 64]>, } -impl Default for StackingOrder { - fn default() -> Self { - StackingOrder { - z_indices: SmallVec::new(), +#[derive(Clone, Ord, PartialOrd, PartialEq, Eq)] +pub struct StackingContext { + // TODO kb use u16 and/or try to push the `id` above into the stacking order + z_index: u8, + id: u16, +} + +impl std::fmt::Debug for StackingOrder { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut stacks = self.context_stack.iter().peekable(); + while let Some(z_index) = stacks.next() { + write!(f, "{}.{}", z_index.z_index, z_index.id)?; + if stacks.peek().is_some() { + write!(f, "->")?; + } } + Ok(()) } } @@ -284,6 +296,7 @@ pub(crate) struct Frame { pub(crate) scene_builder: SceneBuilder, pub(crate) depth_map: Vec<(StackingOrder, Bounds)>, pub(crate) z_index_stack: StackingOrder, + pub(crate) stacking_context_id_stack: Vec, content_mask_stack: Vec>, element_offset_stack: Vec>, } @@ -297,6 +310,7 @@ impl Frame { dispatch_tree, scene_builder: SceneBuilder::default(), z_index_stack: StackingOrder::default(), + stacking_context_id_stack: vec![0], depth_map: Default::default(), content_mask_stack: Vec::new(), element_offset_stack: Vec::new(), @@ -308,6 +322,8 @@ impl Frame { self.mouse_listeners.values_mut().for_each(Vec::clear); self.dispatch_tree.clear(); self.depth_map.clear(); + self.stacking_context_id_stack.clear(); + self.stacking_context_id_stack.push(0); } fn focus_path(&self) -> SmallVec<[FocusId; 8]> { @@ -931,8 +947,20 @@ impl<'a> WindowContext<'a> { /// 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 { - self.window.next_frame.z_index_stack.push(z_index); + let id = post_inc( + self.window + .next_frame + .stacking_context_id_stack + .last_mut() + .unwrap(), + ); + self.window.next_frame.stacking_context_id_stack.push(0); + self.window + .next_frame + .z_index_stack + .push(StackingContext { z_index, id }); let result = f(self); + self.window.next_frame.stacking_context_id_stack.pop(); self.window.next_frame.z_index_stack.pop(); result } @@ -2046,6 +2074,30 @@ pub trait BorrowWindow: BorrowMut + BorrowMut { result } + /// 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 id = post_inc( + self.window_mut() + .next_frame + .stacking_context_id_stack + .last_mut() + .unwrap(), + ); + self.window_mut() + .next_frame + .stacking_context_id_stack + .push(0); + self.window_mut() + .next_frame + .z_index_stack + .push(StackingContext { z_index, id }); + let result = f(self); + self.window_mut().next_frame.stacking_context_id_stack.pop(); + self.window_mut().next_frame.z_index_stack.pop(); + result + } + /// Update the global element offset relative to the current offset. This is used to implement /// scrolling. fn with_element_offset( @@ -2269,13 +2321,6 @@ impl<'a, V: 'static> ViewContext<'a, V> { &mut self.window_cx } - pub fn with_z_index(&mut self, z_index: u8, f: impl FnOnce(&mut Self) -> R) -> R { - self.window.next_frame.z_index_stack.push(z_index); - let result = f(self); - self.window.next_frame.z_index_stack.pop(); - result - } - pub fn on_next_frame(&mut self, f: impl FnOnce(&mut V, &mut ViewContext) + 'static) where V: 'static, diff --git a/crates/ui2/src/components/list/list_item.rs b/crates/ui2/src/components/list/list_item.rs index bdb5c2b854f2cfb7a04cbd70334d81a4daab98c4..a6f99350534c7767baf73f1758c4b7c2e6c5ace4 100644 --- a/crates/ui2/src/components/list/list_item.rs +++ b/crates/ui2/src/components/list/list_item.rs @@ -129,6 +129,7 @@ impl RenderOnce for ListItem { fn render(self, cx: &mut WindowContext) -> Self::Rendered { h_stack() .id(self.id) + .bg(gpui::green()) .w_full() .relative() // When an item is inset draw the indent spacing outside of the item @@ -171,7 +172,8 @@ impl RenderOnce for ListItem { }) }) .when_some(self.on_click, |this, on_click| { - this.cursor_pointer().on_click(on_click) + this.cursor_copy() + .on_click(move |event, cx| on_click(dbg!(event), cx)) }) .when_some(self.on_secondary_mouse_down, |this, on_mouse_down| { this.on_mouse_down(MouseButton::Right, move |event, cx| { diff --git a/crates/workspace2/src/toolbar.rs b/crates/workspace2/src/toolbar.rs index cd25582f36cbd397688a500e85c0e405865dae63..f83879a64ed5536f1602571c00e59dd1a18bc611 100644 --- a/crates/workspace2/src/toolbar.rs +++ b/crates/workspace2/src/toolbar.rs @@ -105,6 +105,8 @@ impl Render for Toolbar { v_stack() .p_1() .gap_2() + // todo!() use a proper constant here (ask Marshall & Nate) + .z_index(80) .border_b() .border_color(cx.theme().colors().border_variant) .bg(cx.theme().colors().toolbar_background) From 8f1c5375efa1ee8829f18755391da59d3b1cbaf8 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Dec 2023 12:30:57 +0200 Subject: [PATCH 2/4] Fix more z-index and rendering issues Co-Authored-By: Antonio Scandurra --- crates/collab_ui2/src/collab_titlebar_item.rs | 1 + crates/ui2/src/components/context_menu.rs | 36 ++++++++++++++----- crates/ui2/src/components/list/list_item.rs | 5 ++- crates/ui2/src/components/tab_bar.rs | 1 + 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/crates/collab_ui2/src/collab_titlebar_item.rs b/crates/collab_ui2/src/collab_titlebar_item.rs index 023b77ab8f340a8f46e306aa0578c4037090cdd0..914e5c7bb64b08c9dc45337df8af4a78b9255aeb 100644 --- a/crates/collab_ui2/src/collab_titlebar_item.rs +++ b/crates/collab_ui2/src/collab_titlebar_item.rs @@ -68,6 +68,7 @@ impl Render for CollabTitlebarItem { h_stack() .id("titlebar") + .z_index(160) .justify_between() .w_full() .h(rems(1.75)) diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 8fce15d1c69d0fcf97f8ac1757874be4884950b6..84c66f3534e05da1f55b3dc7630aa6871bee8313 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, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView, - IntoElement, Render, Subscription, View, VisualContext, + px, Action, AnyElement, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, + FocusableView, IntoElement, Render, Subscription, View, VisualContext, }; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev}; use std::{rc::Rc, time::Duration}; @@ -18,6 +18,9 @@ pub enum ContextMenuItem { handler: Rc, action: Option>, }, + CustomEntry { + entry_render: Box AnyElement>, + }, } pub struct ContextMenu { @@ -83,6 +86,16 @@ 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(), @@ -230,9 +243,9 @@ impl Render for ContextMenu { el }) .flex_none() - .child( - List::new().children(self.items.iter().enumerate().map( - |(ix, item)| match item { + .child(List::new().children(self.items.iter_mut().enumerate().map( + |(ix, item)| { + match item { ContextMenuItem::Separator => ListSeparator.into_any_element(), ContextMenuItem::Header(header) => { ListSubHeader::new(header.clone()).into_any_element() @@ -255,7 +268,7 @@ impl Render for ContextMenu { Label::new(label.clone()).into_any_element() }; - ListItem::new(label.clone()) + ListItem::new(ix) .inset(true) .selected(Some(ix) == self.selected_index) .on_click(move |_, cx| handler(cx)) @@ -271,9 +284,14 @@ 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 a6f99350534c7767baf73f1758c4b7c2e6c5ace4..e8689030c36220894c1de3c0ea258fbb7f4e8246 100644 --- a/crates/ui2/src/components/list/list_item.rs +++ b/crates/ui2/src/components/list/list_item.rs @@ -129,7 +129,6 @@ impl RenderOnce for ListItem { fn render(self, cx: &mut WindowContext) -> Self::Rendered { h_stack() .id(self.id) - .bg(gpui::green()) .w_full() .relative() // When an item is inset draw the indent spacing outside of the item @@ -172,8 +171,8 @@ impl RenderOnce for ListItem { }) }) .when_some(self.on_click, |this, on_click| { - this.cursor_copy() - .on_click(move |event, cx| on_click(dbg!(event), cx)) + this.cursor_pointer() + .on_click(move |event, cx| on_click(event, cx)) }) .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 7cff2f51bd80f4dc38df6b9e2423d1627024ea33..b10ffd1c630a3b0099190235328bbc43f8564854 100644 --- a/crates/ui2/src/components/tab_bar.rs +++ b/crates/ui2/src/components/tab_bar.rs @@ -96,6 +96,7 @@ impl RenderOnce for TabBar { div() .id(self.id) + .z_index(120) .group("tab_bar") .flex() .flex_none() From f6d31917c15d1328991ccae317964190b309f1ff Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Dec 2023 17:05:39 +0200 Subject: [PATCH 3/4] Optimize stack id in-memory layout --- crates/gpui2/src/scene.rs | 226 ++++++++++++++++++------------------- crates/gpui2/src/window.rs | 66 ++++------- 2 files changed, 136 insertions(+), 156 deletions(-) diff --git a/crates/gpui2/src/scene.rs b/crates/gpui2/src/scene.rs index 15a4a37d5b1b6d67e0d6be53e75a6e2a772e1d1c..517632ed79d0f21d880502c0a92385aa01692ecf 100644 --- a/crates/gpui2/src/scene.rs +++ b/crates/gpui2/src/scene.rs @@ -857,116 +857,116 @@ impl Bounds { } } -#[cfg(test)] -mod tests { - use super::*; - use crate::{point, px, size, Size}; - use smallvec::smallvec; - - // todo!() - // #[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(), - }, - } - } -} +// 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 5d06cfe1a540a2e931e6295504f3235f7e6ba5fd..ddda9f030267a504f44cf1518b6f5ba865f0e3d2 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -39,7 +39,7 @@ use std::{ Arc, }, }; -use util::{post_inc, ResultExt}; +use util::ResultExt; const ACTIVE_DRAG_Z_INDEX: u8 = 1; @@ -49,25 +49,21 @@ const ACTIVE_DRAG_Z_INDEX: u8 = 1; pub struct StackingOrder { #[deref] #[deref_mut] - context_stack: SmallVec<[StackingContext; 64]>, -} - -#[derive(Clone, Ord, PartialOrd, PartialEq, Eq)] -pub struct StackingContext { - // TODO kb use u16 and/or try to push the `id` above into the stacking order - z_index: u8, - id: u16, + context_stack: SmallVec<[u8; 64]>, + id: u32, } impl std::fmt::Debug for StackingOrder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut stacks = self.context_stack.iter().peekable(); + write!(f, "[({}): ", self.id)?; while let Some(z_index) = stacks.next() { - write!(f, "{}.{}", z_index.z_index, z_index.id)?; + write!(f, "{z_index}")?; if stacks.peek().is_some() { write!(f, "->")?; } } + write!(f, "]")?; Ok(()) } } @@ -296,7 +292,7 @@ pub(crate) struct Frame { pub(crate) scene_builder: SceneBuilder, pub(crate) depth_map: Vec<(StackingOrder, Bounds)>, pub(crate) z_index_stack: StackingOrder, - pub(crate) stacking_context_id_stack: Vec, + pub(crate) next_stacking_order_id: u32, content_mask_stack: Vec>, element_offset_stack: Vec>, } @@ -310,7 +306,7 @@ impl Frame { dispatch_tree, scene_builder: SceneBuilder::default(), z_index_stack: StackingOrder::default(), - stacking_context_id_stack: vec![0], + next_stacking_order_id: 0, depth_map: Default::default(), content_mask_stack: Vec::new(), element_offset_stack: Vec::new(), @@ -322,8 +318,7 @@ impl Frame { self.mouse_listeners.values_mut().for_each(Vec::clear); self.dispatch_tree.clear(); self.depth_map.clear(); - self.stacking_context_id_stack.clear(); - self.stacking_context_id_stack.push(0); + self.next_stacking_order_id = 0; } fn focus_path(&self) -> SmallVec<[FocusId; 8]> { @@ -947,20 +942,14 @@ impl<'a> WindowContext<'a> { /// 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 id = post_inc( - self.window - .next_frame - .stacking_context_id_stack - .last_mut() - .unwrap(), - ); - self.window.next_frame.stacking_context_id_stack.push(0); - self.window - .next_frame - .z_index_stack - .push(StackingContext { z_index, id }); + 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.stacking_context_id_stack.pop(); + self.window.next_frame.next_stacking_order_id = new_next_stacking_order_id; self.window.next_frame.z_index_stack.pop(); result } @@ -2077,23 +2066,14 @@ 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 id = post_inc( - self.window_mut() - .next_frame - .stacking_context_id_stack - .last_mut() - .unwrap(), - ); - self.window_mut() - .next_frame - .stacking_context_id_stack - .push(0); - self.window_mut() - .next_frame - .z_index_stack - .push(StackingContext { z_index, id }); + 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; + 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.stacking_context_id_stack.pop(); + self.window_mut().next_frame.next_stacking_order_id = new_next_stacking_order_id; self.window_mut().next_frame.z_index_stack.pop(); result } From cf12d62fc5283ba0c0c2f8c482ef9a2cc20e704a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 19 Dec 2023 23:36:32 +0200 Subject: [PATCH 4/4] 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 914e5c7bb64b08c9dc45337df8af4a78b9255aeb..17ee5087bb0e4ec28771e8d4a1cef7616f790a6e 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 6e9c95e19f4268e929a09d8eae671ebf7831fcff..57fd3d6a8af274d305d37dbc4d2d62baba8ccbd3 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 4f81f604c8d3799923b9fb76742019cb1724c0b5..ab7b8d914094b677b647123074883611665d263b 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 517632ed79d0f21d880502c0a92385aa01692ecf..811b2b6e30897f4fe23913e879608c02a26b98bd 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 ddda9f030267a504f44cf1518b6f5ba865f0e3d2..1b2669b40c5cc4d95018ab6ec4a827d551b79dac 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 e9335a4220312d33eeb1fcab4670661d3f037ba3..07910896f31320d108e45490e3aab70c06a76fb1 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 84c66f3534e05da1f55b3dc7630aa6871bee8313..8fce15d1c69d0fcf97f8ac1757874be4884950b6 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 e8689030c36220894c1de3c0ea258fbb7f4e8246..bdb5c2b854f2cfb7a04cbd70334d81a4daab98c4 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 b10ffd1c630a3b0099190235328bbc43f8564854..d2e6e9518bdae89bd31506f1c1d8fad4660881fa 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 f83879a64ed5536f1602571c00e59dd1a18bc611..7436232b04e556ebe8ed648363908471512d1585 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)