mirror of
https://github.com/emilk/egui.git
synced 2026-08-29 04:40:03 -04:00
Fix where Panel puts its separator line, and how much room it reserves (#8382)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -18,8 +18,8 @@
|
|||||||
use emath::GuiRounding as _;
|
use emath::GuiRounding as _;
|
||||||
|
|
||||||
use crate::{
|
use crate::{
|
||||||
Align, Context, CursorIcon, Frame, Id, InnerResponse, Layout, NumExt as _, Rangef, Rect,
|
Align, Context, CursorIcon, Frame, Id, InnerResponse, Layout, Margin, NumExt as _, Rangef,
|
||||||
Response, Sense, Stroke, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, lerp,
|
Rect, Response, Sense, Stroke, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, lerp,
|
||||||
};
|
};
|
||||||
|
|
||||||
fn animate_expansion(ctx: &Context, id: Id, is_expanded: bool) -> f32 {
|
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.
|
/// Resize by keeping `self` side fixed, and moving the opposite side.
|
||||||
fn set_rect_size(self, rect: &mut Rect, size: f32) {
|
fn set_rect_size(self, rect: &mut Rect, size: f32) {
|
||||||
match self {
|
match self {
|
||||||
@@ -298,6 +314,19 @@ impl Panel {
|
|||||||
|
|
||||||
/// Show a separator line, even when not interacting with it?
|
/// 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`.
|
/// Default: `true`.
|
||||||
#[inline]
|
#[inline]
|
||||||
pub fn show_separator_line(mut self, show_separator_line: bool) -> Self {
|
pub fn show_separator_line(mut self, show_separator_line: bool) -> Self {
|
||||||
@@ -845,7 +874,14 @@ impl Panel {
|
|||||||
Stroke::NONE
|
Stroke::NONE
|
||||||
};
|
};
|
||||||
// TODO(emilk): draw line on top of all panels in this ui when https://github.com/emilk/egui/issues/1516 is done
|
// 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());
|
let cross_range = shifted_outer_rect.range_along(side.cross_axis());
|
||||||
if axis == 0 {
|
if axis == 0 {
|
||||||
parent_ui.painter().vline(line_pos, cross_range, stroke);
|
parent_ui.painter().vline(line_pos, cross_range, stroke);
|
||||||
@@ -863,19 +899,19 @@ impl Panel {
|
|||||||
.frame
|
.frame
|
||||||
.unwrap_or_else(|| Frame::side_top_panel(ui.style()));
|
.unwrap_or_else(|| Frame::side_top_panel(ui.style()));
|
||||||
|
|
||||||
let has_separator_line = self.show_separator_line || self.resizable;
|
if self.show_separator_line {
|
||||||
|
// Reserve room for the separator line in the frame's _outer_ margin, so the line
|
||||||
if has_separator_line {
|
// lands just outside the frame's outline instead of painting on top of it:
|
||||||
// The separator line has a thickness that we need to account for.
|
//
|
||||||
|
// 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 widgets = &ui.style().visuals.widgets;
|
||||||
let stroke_width = widgets.noninteractive.bg_stroke.width.round() as i8;
|
let stroke_width = widgets.noninteractive.bg_stroke.width.round() as i8;
|
||||||
|
|
||||||
let margin_side = match self.side {
|
let margin_side = self.side.resize_margin_mut(&mut frame.outer_margin);
|
||||||
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,
|
|
||||||
};
|
|
||||||
*margin_side = (*margin_side).saturating_add(stroke_width);
|
*margin_side = (*margin_side).saturating_add(stroke_width);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,3 @@
|
|||||||
|
version https://git-lfs.github.com/spec/v1
|
||||||
|
oid sha256:bbe8eda2e6162220ff8baa6e84156ea18f0bb15ffc46ebf9d60b6d3eeb8861d4
|
||||||
|
size 9016
|
||||||
@@ -0,0 +1,3 @@
|
|||||||
|
version https://git-lfs.github.com/spec/v1
|
||||||
|
oid sha256:547316973b563bd92841f428838848323d656688cd03e8babcbc24626a389419
|
||||||
|
size 7195
|
||||||
@@ -0,0 +1,3 @@
|
|||||||
|
version https://git-lfs.github.com/spec/v1
|
||||||
|
oid sha256:b18774a40c1c1065645be7b59c90e65bb6fb40d4931ab37ccf40126e2767b709
|
||||||
|
size 9019
|
||||||
@@ -0,0 +1,3 @@
|
|||||||
|
version https://git-lfs.github.com/spec/v1
|
||||||
|
oid sha256:0e9ffcb9f14f86155a4a8279d694ae3e43c6bb8df59fa681673199c01ff68f96
|
||||||
|
size 7206
|
||||||
105
tests/egui_tests/tests/test_panel_separator_line.rs
Normal file
105
tests/egui_tests/tests/test_panel_separator_line.rs
Normal file
@@ -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")));
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user