From ddec5f3e4ce2142dfbc95f3216d9851605682963 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 03:32:56 -0700 Subject: [PATCH] Fix where `Panel` puts its separator line, and how much room it reserves (#8382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the separator line of `Panel` (`resolve_frame` was added in #8367): * **Reserve room only when the line is always drawn.** Before, `show_separator_line || resizable` reserved the line's thickness, so a resizable panel that opted out still got a permanently visible gap along its inner edge — space held for a line only drawn transiently, while hovering or dragging the resize handle. * **Paint the line outside the frame's outline**, in room reserved in `Frame::outer_margin` rather than `inner_margin`, so going outwards from the panel contents you get: `contents | inner_margin | stroke | separator line | outer_margin` Previously the line landed on top of the frame's outline (or outside its outer margin). Default panels — no stroke, no outer margin — are unchanged pixel-wise. Found in the Rerun viewer: the time panel is `.resizable(true).show_separator_line(false)` and draws its own top line, so the extra 1pt landed above the top bar's buttons, making them look 1pt too low. Tests in `tests/egui_tests/tests/test_panel_separator_line.rs`, both spanning `show_separator_line` on/off × resize handle hovered/not: snapshots of a top panel with a garish outline, plus a pixel probe across the inner edge of a panel on each of the four sides. Both fail on `main`. * [x] I have followed the instructions in the PR template 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- crates/egui/src/containers/panel.rs | 62 ++++++++--- .../separator_off_hovered.png | 3 + .../separator_off_idle.png | 3 + .../separator_on_hovered.png | 3 + .../separator_on_idle.png | 3 + .../tests/test_panel_separator_line.rs | 105 ++++++++++++++++++ 6 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_hovered.png create mode 100644 tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_idle.png create mode 100644 tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_hovered.png create mode 100644 tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_idle.png create mode 100644 tests/egui_tests/tests/test_panel_separator_line.rs diff --git a/crates/egui/src/containers/panel.rs b/crates/egui/src/containers/panel.rs index a5f57f5db..e4954b6cd 100644 --- a/crates/egui/src/containers/panel.rs +++ b/crates/egui/src/containers/panel.rs @@ -18,8 +18,8 @@ use emath::GuiRounding as _; use crate::{ - Align, Context, CursorIcon, Frame, Id, InnerResponse, Layout, NumExt as _, Rangef, Rect, - Response, Sense, Stroke, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, lerp, + Align, Context, CursorIcon, Frame, Id, InnerResponse, Layout, Margin, NumExt as _, Rangef, + Rect, Response, Sense, Stroke, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, lerp, }; fn animate_expansion(ctx: &Context, id: Id, is_expanded: bool) -> f32 { @@ -126,6 +126,22 @@ impl PanelSide { } } + /// The component of `margin` on the panel's _resizable_ edge, + /// i.e. the edge facing the rest of the ui, where the separator line goes. + fn resize_margin(self, mut margin: Margin) -> i8 { + *self.resize_margin_mut(&mut margin) + } + + /// Mutable version of [`Self::resize_margin`]. + fn resize_margin_mut(self, margin: &mut Margin) -> &mut i8 { + match self { + Self::Left => &mut margin.right, + Self::Right => &mut margin.left, + Self::Top => &mut margin.bottom, + Self::Bottom => &mut margin.top, + } + } + /// Resize by keeping `self` side fixed, and moving the opposite side. fn set_rect_size(self, rect: &mut Rect, size: f32) { match self { @@ -298,6 +314,19 @@ impl Panel { /// Show a separator line, even when not interacting with it? /// + /// The separator line sits on the panel's inner edge, i.e. the edge facing the rest of the ui. + /// It is painted _outside_ the [`Frame`]'s outline, in room the panel reserves for it in the + /// frame's [`Frame::outer_margin`], so that going from the panel contents outwards you get: + /// + /// contents | [`Frame::inner_margin`] | [`Frame::stroke`] | separator line | [`Frame::outer_margin`] + /// + /// Turning this off removes that reserved room too, so the panel gets no permanent gap along + /// that edge. + /// + /// A `resizable` panel still shows a line while hovered or dragged, regardless of this setting. + /// With this setting off there is no room reserved for it, so that transient line is painted + /// just outside the frame's outline, overlapping the [`Frame::outer_margin`]. + /// /// Default: `true`. #[inline] pub fn show_separator_line(mut self, show_separator_line: bool) -> Self { @@ -845,7 +874,14 @@ impl Panel { Stroke::NONE }; // TODO(emilk): draw line on top of all panels in this ui when https://github.com/emilk/egui/issues/1516 is done - let line_pos = side.resize_pos(shifted_outer_rect) + 0.5 * side.sign() * stroke.width; + + // The line goes just _outside_ the frame's outline, in the room `resolve_frame` + // reserved for it in the outer margin, i.e.: + // + // contents | `inner_margin` | outline | separator line | `outer_margin` + let outer_margin = f32::from(side.resize_margin(frame.outer_margin)); + let outline_edge = side.resize_pos(shifted_outer_rect) + side.sign() * outer_margin; + let line_pos = outline_edge - 0.5 * side.sign() * stroke.width; let cross_range = shifted_outer_rect.range_along(side.cross_axis()); if axis == 0 { parent_ui.painter().vline(line_pos, cross_range, stroke); @@ -863,19 +899,19 @@ impl Panel { .frame .unwrap_or_else(|| Frame::side_top_panel(ui.style())); - let has_separator_line = self.show_separator_line || self.resizable; - - if has_separator_line { - // The separator line has a thickness that we need to account for. + if self.show_separator_line { + // Reserve room for the separator line in the frame's _outer_ margin, so the line + // lands just outside the frame's outline instead of painting on top of it: + // + // contents | `inner_margin` | outline | separator line | `outer_margin` + // + // We deliberately don't do this for a `resizable` panel that has opted out of the + // separator line: the line it shows while hovered/dragged is a transient affordance, + // and reserving room for it would leave a permanently visible gap. let widgets = &ui.style().visuals.widgets; let stroke_width = widgets.noninteractive.bg_stroke.width.round() as i8; - let margin_side = match self.side { - PanelSide::Left => &mut frame.inner_margin.right, - PanelSide::Right => &mut frame.inner_margin.left, - PanelSide::Top => &mut frame.inner_margin.bottom, - PanelSide::Bottom => &mut frame.inner_margin.top, - }; + let margin_side = self.side.resize_margin_mut(&mut frame.outer_margin); *margin_side = (*margin_side).saturating_add(stroke_width); } diff --git a/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_hovered.png b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_hovered.png new file mode 100644 index 000000000..bb3745c59 --- /dev/null +++ b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_hovered.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bbe8eda2e6162220ff8baa6e84156ea18f0bb15ffc46ebf9d60b6d3eeb8861d4 +size 9016 diff --git a/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_idle.png b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_idle.png new file mode 100644 index 000000000..8a690c1a5 --- /dev/null +++ b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_off_idle.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:547316973b563bd92841f428838848323d656688cd03e8babcbc24626a389419 +size 7195 diff --git a/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_hovered.png b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_hovered.png new file mode 100644 index 000000000..43e51aa02 --- /dev/null +++ b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_hovered.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b18774a40c1c1065645be7b59c90e65bb6fb40d4931ab37ccf40126e2767b709 +size 9019 diff --git a/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_idle.png b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_idle.png new file mode 100644 index 000000000..3c92145ab --- /dev/null +++ b/tests/egui_tests/tests/snapshots/panel_separator_line/separator_on_idle.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0e9ffcb9f14f86155a4a8279d694ae3e43c6bb8df59fa681673199c01ff68f96 +size 7206 diff --git a/tests/egui_tests/tests/test_panel_separator_line.rs b/tests/egui_tests/tests/test_panel_separator_line.rs new file mode 100644 index 000000000..96afb509f --- /dev/null +++ b/tests/egui_tests/tests/test_panel_separator_line.rs @@ -0,0 +1,105 @@ +//! Snapshot tests for where a [`Panel`] puts its separator line, and how much room it reserves. +//! +//! Going outwards from the panel contents, the order is: +//! +//! contents | `Frame::inner_margin` | `Frame::stroke` | separator line | `Frame::outer_margin` +//! +//! i.e. the line is painted _outside_ the frame's outline, in room the panel reserves for it in the +//! frame's outer margin. A panel that opted out of the separator line must not reserve that room, +//! or it ends up with a permanently visible gap along that edge — even though it is `resizable` and +//! therefore still shows a line while hovered or dragged. +//! +//! The snapshots span `show_separator_line` on/off × resize handle hovered/not. The panel uses a +//! garish frame outline and separator colors so both are unmistakable, and its only content is a +//! [`egui::SelectableLabel`] vertically centered in the panel: if the panel reserves room it +//! shouldn't, the label drifts off center. + +use egui::{Color32, CornerRadius, Frame, Margin, Panel, Pos2, Stroke, Vec2}; +use egui_kittest::{Harness, SnapshotResults}; + +/// [`Frame::fill`] of the test panel. +const FILL: Color32 = Color32::from_rgb(20, 20, 40); + +/// [`Frame::stroke`] color of the test panel. +const OUTLINE: Color32 = Color32::from_rgb(255, 0, 255); + +/// The dim, always-visible separator line (`noninteractive.bg_stroke`). +const SEPARATOR: Color32 = Color32::from_rgb(0, 255, 0); + +/// The bright separator line shown while the resize handle is hovered (`hovered.fg_stroke`). +const HOVERED_SEPARATOR: Color32 = Color32::from_rgb(255, 255, 0); + +const PANEL_ID: &str = "test_panel"; + +fn build_harness(show_separator_line: bool) -> Harness<'static> { + let mut harness = Harness::builder() + .with_size(Vec2::new(200.0, 120.0)) + // So the thin lines are legible to a human reviewing the snapshots: + .with_pixels_per_point(2.0) + .build_ui(move |ui| { + // Loud, distinguishable colors, so we can tell the separator line, the frame outline + // and the frame fill apart. + let widgets = &mut ui.visuals_mut().widgets; + widgets.noninteractive.bg_stroke = Stroke::new(1.0, SEPARATOR); + widgets.hovered.fg_stroke = Stroke::new(1.0, HOVERED_SEPARATOR); + + let frame = Frame::new() + .fill(FILL) + .stroke(Stroke::new(2.0, OUTLINE)) + .corner_radius(CornerRadius::ZERO) + .inner_margin(Margin::same(4)) + .outer_margin(Margin::same(2)); + + Panel::top(PANEL_ID) + .frame(frame) + .resizable(true) + .default_size(60.0) + .show_separator_line(show_separator_line) + .show(ui, |ui| { + // Vertically centered in whatever room the panel gave us. + ui.horizontal_centered(|ui| { + let _ = ui.selectable_label(true, "Centered"); + }); + }); + + egui::CentralPanel::default() + .frame(Frame::default().fill(Color32::GRAY)) + .show(ui, |ui| { + ui.label("CentralPanel"); + }); + }); + harness.run(); + harness +} + +fn hover_resize_handle(harness: &mut Harness<'_>) { + let outer = egui::PanelState::load(&harness.ctx, egui::Id::new(PANEL_ID)) + .expect("PanelState should be persisted after the first frame") + .outer_rect; + + // Hover just _inside_ the panel's inner edge, but still well within the resize grab radius: + // the `CentralPanel` and its label start exactly at that edge, and would otherwise take the + // hover from the resize handle. + harness.hover_at(Pos2::new(outer.center().x, outer.bottom() - 1.0)); + harness.run(); +} + +#[test] +fn separator_line_matrix() { + let mut results = SnapshotResults::new(); + + for show_separator_line in [false, true] { + let suffix = if show_separator_line { "on" } else { "off" }; + + // Not hovered: the line is dim (`show_separator_line`) or absent. + let mut harness = build_harness(show_separator_line); + results.add(harness.try_snapshot(format!("panel_separator_line/separator_{suffix}_idle"))); + + // Hovered: a `resizable` panel shows a bright line regardless of `show_separator_line`, + // and must not shift its contents to make room for it. + let mut harness = build_harness(show_separator_line); + hover_resize_handle(&mut harness); + results + .add(harness.try_snapshot(format!("panel_separator_line/separator_{suffix}_hovered"))); + } +}