Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/combobox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ where
dismiss_handler,
cx,
))
.with_priority(1),
.with_priority(crate::layer::LAYER_OVERLAY),
)
})
}
Expand Down
27 changes: 27 additions & 0 deletions crates/moon-ui-components/src/layer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//! Named deferred-draw priority bands for MoonUI overlay surfaces.
//!
//! GPUI deferred draws are sorted into one global ascending order by `priority` and painted in
//! that order (`moon-gpui` `window.rs`, `deferred_draw_traversal_order`). This file is the only
//! place these bands are numbered. A higher number paints later and therefore on top.
//!
//! The band is absolute, not relative to the hosting surface: a child overlay inside a
//! higher-band parent must opt into a band above that parent or it is painted underneath.
//! `MoonSelect::in_popover()` (`moon/select.rs:378`) is the existing opt-in; the managed
//! tooltip avoids the problem by sitting above every band unconditionally.

/// Longbridge default overlay band: plain popovers, select menus, comboboxes, context menus,
/// native-menu fallback, and dropdown submenus.
pub(crate) const LAYER_OVERLAY: usize = 1;

/// Moon popover and Moon dropdown surfaces.
pub(crate) const LAYER_MOON_POPOVER: usize = 30_000;

/// A menu opened from inside a Moon popover (`MoonSelect::in_popover`).
pub(crate) const LAYER_MOON_POPOVER_MENU: usize = 31_000;

/// The managed tooltip overlay. The tooltip is the topmost transient surface and must outrank
/// every other deferred band.
pub(crate) const LAYER_TOOLTIP: usize = 100_000;

#[cfg(test)]
mod tests;
20 changes: 20 additions & 0 deletions crates/moon-ui-components/src/layer/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! Band-ordering tests for the deferred-draw layer constants live here and are authored by the
//! breakage prover.

use super::{LAYER_MOON_POPOVER, LAYER_MOON_POPOVER_MENU, LAYER_OVERLAY, LAYER_TOOLTIP};

/// `layer.rs` is the only place these bands are numbered. Collapsing two bands together or
/// renumbering `LAYER_TOOLTIP` under `LAYER_MOON_POPOVER_MENU` lets a Moon popover or its
/// in-popover select menu repaint over a tooltip again, reproducing MoonUI #628.
#[test]
fn tooltip_outranks_every_other_deferred_band() {
assert!(LAYER_TOOLTIP > LAYER_MOON_POPOVER_MENU);
assert!(LAYER_MOON_POPOVER_MENU > LAYER_MOON_POPOVER);
assert!(LAYER_MOON_POPOVER > LAYER_OVERLAY);

// Independently derived from the frozen literal at
// crates/moon-ui-components/src/time/date_picker.rs:512 (`.with_priority(2)`), a
// component-mirror donor file this ordering must never regress below.
let date_picker_calendar_priority: usize = 2;
assert!(LAYER_TOOLTIP > date_picker_calendar_priority);
}
1 change: 1 addition & 0 deletions crates/moon-ui-components/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod icon;
mod index_path;
#[cfg(any(feature = "inspector", debug_assertions))]
mod inspector;
mod layer;
pub mod moon;
mod root;
mod styled;
Expand Down
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/menu/context_menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl<E: ParentElement + Styled + IntoElement + 'static> Element for ContextMenu<
),
),
)
.with_priority(1)
.with_priority(crate::layer::LAYER_OVERLAY)
.into_any(),
);
}
Expand Down
7 changes: 6 additions & 1 deletion crates/moon-ui-components/src/moon/dropdown/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,12 @@ impl MoonPopupMenu {
}
.into_any_element(),
})
.with_priority(1),
// Deliberately the base band even though the parent dropdown sits at
// LAYER_MOON_POPOVER (moon/dropdown/trigger.rs:654, set unconditionally),
// so an overlapping submenu paints under its parent. Pre-existing, not
// fixed here; do not "correct" it to LAYER_MOON_POPOVER without checking
// the paint-order consequences.
.with_priority(crate::layer::LAYER_OVERLAY),
);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/moon/dropdown/trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ impl RenderOnce for MoonDropdown {
let mut popover = CorePopover::new(ElementId::from(self.id.clone()))
.appearance(false)
.anchor(Anchor::TopLeft)
.deferred_priority(30_000)
.deferred_priority(crate::layer::LAYER_MOON_POPOVER)
.open(open)
.trigger_any(trigger)
.content(move |_, window, cx| {
Expand Down
4 changes: 1 addition & 3 deletions crates/moon-ui-components/src/moon/popover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ use super::{
tokens::{MoonPalette, MoonRect, rgba_from},
};

/// Shared layer boundary for popovers and controls opening menus above them.
pub(super) const MOON_POPOVER_PRIORITY: usize = 30_000;
const POPOVER_PADDING: f32 = 6.0;
const POPOVER_BORDER: f32 = 1.0;

Expand Down Expand Up @@ -368,7 +366,7 @@ impl RenderOnce for MoonPopover {
let mut popover = CorePopover::new(ElementId::from(self.id.clone()))
.anchor(anchor_for(self.placement))
.appearance(false)
.deferred_priority(MOON_POPOVER_PRIORITY)
.deferred_priority(crate::layer::LAYER_MOON_POPOVER)
.overlay_closable(self.overlay_closable)
.open(open)
.trigger_any(trigger)
Expand Down
217 changes: 217 additions & 0 deletions crates/moon-ui-components/src/moon/popover/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,220 @@ fn intrinsic_popover_shrink_wraps_its_rendered_child(cx: &mut gpui::TestAppConte
assert_eq!(child.size.width, px(73.0));
assert_eq!(popup.size.width, child.size.width + px(chrome));
}

// ---- Scene-order proof: managed tooltip must outrank its hosting Moon popover ----

use crate::ElementExt as _;
use crate::Root;
use crate::moon::{MoonButton, MoonThemeConfig};
use crate::tooltip::{TooltipContent, TooltipOverlay};
use gpui::{
AnyWindowHandle, AppContext as _, AtlasKey, AtlasTile, Bounds, DevicePixels,
HeadlessAppContext, NoopTextSystem, ParentElement as _, Pixels, PlatformAtlas,
PlatformHeadlessRenderer, Quad, Scene, Size, size,
};
use std::{
borrow::Cow,
cell::{Cell, RefCell},
rc::Rc,
sync::Arc,
time::Duration,
};

/// Fresh copy of moon/select/tests.rs's module-private recorder pair: exporting it would mean
/// rewriting a test file that already carries a passing test, out of scope for a proof dispatch.
struct HostedTooltipSceneRecorder(Rc<RefCell<Vec<Quad>>>);
impl PlatformHeadlessRenderer for HostedTooltipSceneRecorder {
fn render_scene_to_image(
&mut self,
scene: &Scene,
size: Size<DevicePixels>,
) -> anyhow::Result<image::RgbaImage> {
self.render_scene(scene, size)?;
anyhow::bail!("scene recorder does not rasterize images")
}
fn render_scene(&mut self, scene: &Scene, _size: Size<DevicePixels>) -> anyhow::Result<()> {
self.0.replace(scene.quads.clone());
Ok(())
}
fn sprite_atlas(&self) -> Arc<dyn PlatformAtlas> {
Arc::new(HostedTooltipQuadOnlyAtlas)
}
}

struct HostedTooltipQuadOnlyAtlas;
impl PlatformAtlas for HostedTooltipQuadOnlyAtlas {
fn get_or_insert_with<'a>(
&self,
_key: &AtlasKey,
_build: &mut dyn FnMut() -> anyhow::Result<Option<(Size<DevicePixels>, Cow<'a, [u8]>)>>,
) -> anyhow::Result<Option<AtlasTile>> {
Ok(None)
}
fn remove(&self, _key: &AtlasKey) {}
}

const HOSTED_TOOLTIP_PROBE_W: f32 = 96.0;
const HOSTED_TOOLTIP_PROBE_H: f32 = 28.0;

/// Fixed-size tooltip body so its scene quad cannot be mistaken for the popover chrome.
struct HostedTooltipProbeBody;
impl Render for HostedTooltipProbeBody {
fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
div()
.w(px(HOSTED_TOOLTIP_PROBE_W))
.h(px(HOSTED_TOOLTIP_PROBE_H))
.bg(gpui::black())
}
}

/// Always-open Moon popover hosting a button positioned well inside the popover box on every side.
struct HostedTooltipHarness {
button_bounds: Rc<Cell<Bounds<Pixels>>>,
}
impl Render for HostedTooltipHarness {
fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
let button_bounds = self.button_bounds.clone();
MoonPopover::new("hosted-tooltip")
.open(true)
.width(400.0)
.trigger(div().w(px(20.0)).h(px(20.0)))
.content(
div().relative().w(px(360.0)).h(px(240.0)).child(
div()
.id("hosted-tooltip:button")
.absolute()
.left(px(140.0))
.top(px(100.0))
.w(px(80.0))
.h(px(32.0))
.on_prepaint(move |bounds, _, _| button_bounds.set(bounds))
.child(MoonButton::new("hosted-button").tooltip("Hosted tooltip")),
),
)
}
}

/// Pumps N draw frames so prepaint capture and animation state settle deterministically.
fn hosted_tooltip_pump(cx: &mut HeadlessAppContext, window: AnyWindowHandle, frames: usize) {
for _ in 0..frames {
cx.update_window(window, |_, window, cx| {
window.refresh();
window.draw(cx).clear();
})
.expect("headless frame must draw");
cx.run_until_parked();
}
}

/// tooltip.rs TooltipOverlay::render .with_priority(LAYER_TOOLTIP) must keep the managed tooltip
/// above its hosting Moon popover (LAYER_TOOLTIP == 100_000 vs LAYER_MOON_POPOVER == 30_000).
/// Lowering it back to a literal, or pointing it at LAYER_OVERLAY, reintroduces MoonUI #628.
/// layer::tests pins the constants RELATION; this pins that the overlay actually USES it.
#[test]
fn popover_hosted_tooltip_paints_above_its_popover() {
for (theme, config) in [
("dark", MoonThemeConfig::moon_terminal()),
("light", MoonThemeConfig::moon_light()),
] {
let quads = Rc::new(RefCell::new(Vec::new()));
let recorded = quads.clone();
let mut cx = HeadlessAppContext::with_platform(
Arc::new(NoopTextSystem::new()),
Arc::new(()),
move || Some(Box::new(HostedTooltipSceneRecorder(recorded.clone()))),
);
cx.update(crate::init);
cx.update(|cx| MoonTheme::install_config(config.clone(), cx));

let button_bounds = Rc::new(Cell::new(Bounds::default()));
let window = cx
.open_window(size(px(800.0), px(600.0)), {
let button_bounds = button_bounds.clone();
move |window, cx| {
let view = cx.new(|_| HostedTooltipHarness { button_bounds });
cx.new(|cx| Root::new(view, window, cx))
}
})
.expect("headless window must open");

hosted_tooltip_pump(&mut cx, window.into(), 4);
let trigger_bounds = button_bounds.get();

cx.update_window(window.into(), |_, window, cx| {
let overlay =
Root::tooltip_overlay(window, cx).expect("Root must own a tooltip overlay");
overlay.update(cx, |overlay: &mut TooltipOverlay, cx| {
let content = TooltipContent {
build: Rc::new(|_, cx| cx.new(|_| HostedTooltipProbeBody).into()),
trigger_bounds,
};
overlay.request_show(content, window, cx);
});
})
.expect("headless window must accept the direct request_show drive");

// Cold-start timer (tooltip.rs:409-434) never fires under run_until_parked alone
// (dispatcher.rs:76-78); advance the simulated clock by hand.
cx.advance_clock(Duration::from_millis(500));
let scale = cx
.update_window(window.into(), |_, window, _| window.scale_factor())
.expect("headless window must retain its scale");
hosted_tooltip_pump(&mut cx, window.into(), 8);
// The recorder only fires on an actual render pass; a plain draw() never reaches it.
let _ = cx.capture_screenshot(window.into());

let quads = quads.borrow();
let popovers: Vec<_> = quads
.iter()
.filter(|q| !q.background.is_transparent() && q.bounds.size.width.0 == 400.0 * scale)
.collect();
let tooltips: Vec<_> = quads
.iter()
.filter(|q| {
!q.background.is_transparent()
&& q.bounds.size.width.0 == HOSTED_TOOLTIP_PROBE_W * scale
&& q.bounds.size.height.0 == HOSTED_TOOLTIP_PROBE_H * scale
})
.collect();

assert_eq!(
popovers.len(),
1,
"{theme}: unique hosting popover: {quads:?}"
);
// Must exist BEFORE any order comparison -- a "not found" path that skips the compare
// could never redden under the named mutation.
assert_eq!(
tooltips.len(),
1,
"{theme}: tooltip quad must exist: {quads:?}"
);
let popover = popovers[0];
let tooltip = tooltips[0];
let target = tooltip.bounds.center();

assert!(
popover.bounds.contains(&target),
"{theme}: tooltip must render inside its popover"
);
assert!(
popover.content_mask.bounds.contains(&target),
"{theme}: popover must not be clipped at the probe point"
);
assert!(
tooltip.content_mask.bounds.contains(&target),
"{theme}: tooltip must not be clipped at the probe point"
);
assert_ne!(
tooltip.order, popover.order,
"{theme}: overlapping surfaces must have distinct draw orders"
);
assert!(
tooltip.order > popover.order,
"{theme}: tooltip must paint above its hosting popover; popover={}, tooltip={}",
popover.order,
tooltip.order
);
}
}
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/moon/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ where
.with_size(size_for(trigger_size, menu_size));

if self.in_popover {
select = select.menu_priority(super::popover::MOON_POPOVER_PRIORITY + 1);
select = select.menu_priority(crate::layer::LAYER_MOON_POPOVER_MENU);
}

if let Some(trigger_variant) = self.trigger_variant {
Expand Down
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/native_menu/fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl Render for FallbackMenuOverlay {
.snap_to_window_with_margin(px(8.))
.child(active.menu.clone()),
)
.with_priority(1),
.with_priority(crate::layer::LAYER_OVERLAY),
);
}
root
Expand Down
4 changes: 2 additions & 2 deletions crates/moon-ui-components/src/popover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Popover {
appearance: true,
overlay_closable: true,
close_on_content_click: false,
deferred_priority: 1,
deferred_priority: crate::layer::LAYER_OVERLAY,
default_open: false,
open: None,
on_open_change: None,
Expand Down Expand Up @@ -153,7 +153,7 @@ impl Popover {
self
}

/// Override deferred overlay priority.
/// Override deferred overlay priority. The default is `LAYER_OVERLAY`.
pub fn deferred_priority(mut self, priority: usize) -> Self {
self.deferred_priority = priority;
self
Expand Down
2 changes: 1 addition & 1 deletion crates/moon-ui-components/src/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl Default for SelectOptions {
title_prefix: None,
menu_width: Length::Auto,
menu_max_h: rems(20.).into(),
menu_priority: 1,
menu_priority: crate::layer::LAYER_OVERLAY,
disabled: false,
appearance: true,
search_placeholder: None,
Expand Down
Loading
Loading