From 295731d3369b33a6a40cf0d3fdd2fe0a86f9dd3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 6 Oct 2026 22:23:58 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P20=20Debug:=20stale=20select?= =?UTF-8?q?ions,=20frames=20and=20focus=20paths=20mark=20none;=20the=20tim?= =?UTF-8?q?eline's=20wheel=20keeps=20a=20drawable=20range?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/debug/flame.rs | 38 ++++++++++----- src/debug/hex.rs | 38 +++++++++++---- src/debug/stack.rs | 10 ++-- src/debug/tests.rs | 109 ++++++++++++++++++++++++++++++++++++++++++ src/debug/timeline.rs | 14 ++++-- tasks/panics.md | 3 +- 6 files changed, 184 insertions(+), 28 deletions(-) diff --git a/src/debug/flame.rs b/src/debug/flame.rs index 317a343..d57e09b 100644 --- a/src/debug/flame.rs +++ b/src/debug/flame.rs @@ -37,11 +37,17 @@ pub struct Placed { pub samples: u64, } -/// The frames under `focus`, a path of call indices, each spanning its share of the samples; `focus` fills the width and its callers stack above it, full width. +/// The frames under `focus`, a path of call indices kept as far as the profile holds it, each spanning its share of the samples; `focus` fills the width and its callers stack above it, full width. pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec { let mut out = Vec::new(); let mut frame = root; + let mut kept = focus.len(); for (depth, ix) in focus.iter().enumerate() { + let Some(call) = frame.calls.get(*ix) else { + log::error!("flame graph: focus {focus:?} leaves the profile at depth {depth}"); + kept = depth; + break; + }; out.push(Placed { path: focus[..depth].to_vec(), name: frame.name.clone(), @@ -50,7 +56,7 @@ pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec { end: 1.0, samples: frame.total(), }); - frame = &frame.calls[*ix]; + frame = call; } fn under( frame: &ProfileFrame, @@ -78,7 +84,7 @@ pub fn place(root: &ProfileFrame, focus: &[usize]) -> Vec { at += width; } } - under(frame, focus.to_vec(), focus.len(), 0.0, 1.0, &mut out); + under(frame, focus[..kept].to_vec(), kept, 0.0, 1.0, &mut out); out } @@ -247,11 +253,17 @@ impl RenderOnce for Flamegraph { move |bounds, _, window, cx| { let pixels = text_size.to_pixels(window.rem_size()); let face = gpui::font(font.clone()); - let advance = window - .text_system() - .advance(window.text_system().resolve_font(&face), pixels, 'm') - .expect("the code font has an m") - .width; + let resolved = window.text_system().resolve_font(&face); + let advance = + match window.text_system().advance(resolved, pixels, 'm') { + Ok(advance) => advance.width, + Err(error) => { + log::error!( + "flame graph: no advance for its font: {error:#}" + ); + pixels * 0.6 + } + }; for frame in paint_placed.iter() { let x0 = bounds.origin.x + bounds.size.width * frame.start; let width = bounds.size.width * (frame.end - frame.start); @@ -287,7 +299,7 @@ impl RenderOnce for Flamegraph { underline: None, strikethrough: None, }; - window + let painted = window .text_system() .shape_line(label.into(), pixels, &[run], None) .paint( @@ -301,8 +313,12 @@ impl RenderOnce for Flamegraph { None, window, cx, - ) - .expect("a flame label paints"); + ); + if let Err(error) = painted { + log::error!( + "flame graph: a label failed to paint: {error:#}" + ); + } } } }, diff --git a/src/debug/hex.rs b/src/debug/hex.rs index a9e68c7..f387f99 100644 --- a/src/debug/hex.rs +++ b/src/debug/hex.rs @@ -62,18 +62,26 @@ impl HexViewer { } } - /// The address of the first byte. + /// The address of the first byte; the last must fit an address too. pub fn base(mut self, base: u64) -> Self { + let last = self.bytes.len().saturating_sub(1) as u64; + assert!( + base.checked_add(last).is_some(), + "a dump at {base:#X} runs past the last address" + ); self.base = base; self } pub fn selected(mut self, bytes: Range) -> Self { - assert!( - bytes.end <= self.bytes.len(), - "a selection lies inside the bytes" - ); - self.selected = Some(bytes); + let inside = bytes.start < bytes.end && bytes.end <= self.bytes.len(); + if !inside { + log::error!( + "hex viewer: no bytes {bytes:?} in {}; none selected", + self.bytes.len() + ); + } + self.selected = inside.then_some(bytes); self } @@ -90,9 +98,12 @@ impl RenderOnce for HexViewer { let theme = cx.theme(); let colors = theme.colors.clone(); let rows = self.bytes.len().div_ceil(WIDTH); - let digits = format!("{:X}", self.base + self.bytes.len() as u64) - .len() - .max(8); + let digits = format!( + "{:X}", + self.base + self.bytes.len().saturating_sub(1) as u64 + ) + .len() + .max(8); let reading = self .selected .as_ref() @@ -198,6 +209,15 @@ impl RenderOnce for HexViewer { mod tests { use super::*; + #[test] + fn a_selection_off_the_bytes_selects_none() { + let viewer = || HexViewer::new("hex", vec![1, 2, 3]); + let (inside, at_end, past) = (0..2, 3..3, 2..9); + assert_eq!(viewer().selected(inside.clone()).selected, Some(inside)); + assert_eq!(viewer().selected(at_end).selected, None); + assert_eq!(viewer().selected(past).selected, None); + } + #[test] fn bytes_read_as_numbers_and_text() { let bytes = [0x2A, 0x01, 0x00, 0x00, 0xFF]; diff --git a/src/debug/stack.rs b/src/debug/stack.rs index 8bcdab2..5817451 100644 --- a/src/debug/stack.rs +++ b/src/debug/stack.rs @@ -71,10 +71,12 @@ impl CallStack { } pub fn current(mut self, frame: usize) -> Self { - assert!( - frame < self.frames.len(), - "the current frame is on the stack" - ); + if frame >= self.frames.len() { + log::error!( + "call stack: no frame {frame} of {}; none marked", + self.frames.len() + ); + } self.current = frame; self } diff --git a/src/debug/tests.rs b/src/debug/tests.rs index a02d04e..b0b4678 100644 --- a/src/debug/tests.rs +++ b/src/debug/tests.rs @@ -138,3 +138,112 @@ fn tab_reaches_each_span_and_enter_picks_it(cx: &mut TestAppContext) { "the second Tab stands on the second span" ); } + +/// A stack whose current frame left with a shorter stack. +struct Unwound; + +impl Render for Unwound { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let frame = super::StackFrame { + function: "main".into(), + path: "src/main.rs".into(), + line: 3, + library: false, + }; + div() + .w(px(400.0)) + .child(super::CallStack::new("stack", [frame]).current(4)) + } +} + +#[gpui::test] +fn a_current_frame_past_the_stack_marks_none(cx: &mut TestAppContext) { + cx.update(Theme::init); + let (_, cx) = cx.add_window_view(|_, _| Unwound); + settle(cx); +} + +/// A flame graph focused on a call its new profile lacks; a dump ending at the last address. +struct Edges(bool); + +impl Render for Edges { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let root = super::ProfileFrame { + name: "root".into(), + own: 1, + calls: vec![], + }; + div().w(px(400.0)).h(px(100.0)).child(match self.0 { + true => super::Flamegraph::new("flame", root) + .focus(vec![0]) + .into_any_element(), + false => super::HexViewer::new("hex", vec![0x42]) + .base(u64::MAX) + .into_any_element(), + }) + } +} + +#[gpui::test] +fn a_lost_focus_and_the_last_address_draw(cx: &mut TestAppContext) { + cx.update(Theme::init); + for flame in [true, false] { + let (_, cx) = cx.add_window_view(move |_, _| Edges(flame)); + settle(cx); + } +} + +/// A timeline whose owner keeps each range it reports. +struct Zooming((f64, f64)); + +impl Render for Zooming { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let owner = cx.entity(); + div().w(px(400.0)).child( + TimelineProfiler::new("timeline", ["Main"], vec![], self.0).on_range( + move |range, _, cx| { + owner.update(cx, |owner, cx| { + owner.0 = range; + cx.notify(); + }) + }, + ), + ) + } +} + +#[gpui::test] +fn a_zoom_past_the_finest_range_stays_put(cx: &mut TestAppContext) { + cx.update(Theme::init); + let start = (1.0, 1.0 + f64::EPSILON); + let (owner, cx) = cx.add_window_view(move |_, _| Zooming(start)); + settle(cx); + for _ in 0..2 { + cx.simulate_event(gpui::ScrollWheelEvent { + position: point(px(250.0), px(30.0)), + delta: gpui::ScrollDelta::Pixels(point(px(0.0), px(400.0))), + modifiers: Modifiers::none(), + touch_phase: gpui::TouchPhase::Moved, + }); + settle(cx); + } + let range = owner.read_with(cx, |owner, _| owner.0); + assert!(range.1 > range.0, "{range:?}"); +} + +#[gpui::test] +fn a_wheel_too_large_for_a_zoom_stays_put(cx: &mut TestAppContext) { + cx.update(Theme::init); + let (owner, cx) = cx.add_window_view(|_, _| Zooming((0.0, 25.0))); + settle(cx); + for pixels in [300_000.0, -300_000.0] { + cx.simulate_event(gpui::ScrollWheelEvent { + position: point(px(250.0), px(30.0)), + delta: gpui::ScrollDelta::Pixels(point(px(0.0), px(pixels))), + modifiers: Modifiers::none(), + touch_phase: gpui::TouchPhase::Moved, + }); + settle(cx); + } + assert_eq!(owner.read_with(cx, |owner, _| owner.0), (0.0, 25.0)); +} diff --git a/src/debug/timeline.rs b/src/debug/timeline.rs index 847d804..829e8bd 100644 --- a/src/debug/timeline.rs +++ b/src/debug/timeline.rs @@ -187,13 +187,21 @@ impl RenderOnce for TimelineProfiler { let (x, y) = (f64::from(f32::from(delta.x)), f64::from(f32::from(delta.y))); cx.stop_propagation(); let width = f64::from(f32::from(bounds.size.width)).max(1.0); - if x.abs() > y.abs() { + let next = if x.abs() > y.abs() { let shift = -x / width * length; - on_range((from + shift, to + shift), window, cx); + Some((from + shift, to + shift)) } else { let pointer = f64::from(f32::from(event.position.x - bounds.origin.x)) / width; - on_range(zoomed((from, to), pointer, (-y / ZOOM).exp()), window, cx); + let factor = (-y / ZOOM).exp(); + (factor > 0.0 && factor.is_finite()) + .then(|| zoomed((from, to), pointer, factor)) + }; + let fits = |next: &(f64, f64)| { + next.0.is_finite() && next.1.is_finite() && next.1 > next.0 + }; + if let Some(next) = next.filter(fits) { + on_range(next, window, cx); } }) .children(self.tracks.iter().enumerate().map(|(track, name)| { diff --git a/tasks/panics.md b/tasks/panics.md index 9d90ccf..c0f3bd9 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -34,7 +34,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P17 Code Editor — `src/editor` - [x] P18 Terminal — `src/terminal` - [x] P19 Git — `src/git` -- [ ] P20 Debug — `src/debug` +- [x] P20 Debug — `src/debug` - [ ] P21 Documents — `src/documents` - [ ] P22 Collaboration — `src/collab` - [ ] P23 AI Chat — `src/chat` @@ -85,3 +85,4 @@ One line per item: what changed, what stayed and why, review rounds. - P17: the editor keeps every owner offset on whole characters of its text (checked on intake, carried through edits by `editor::anchors`, cleared when the whole text changes); stale selections and edit batches are dropped; a painted row whose line the text lost takes no click, drag or IME query; the sticky header reads this frame; a glob too big to compile matches nothing; a missing font advance logs. Kept: owner contracts and construction. Review: 5 rounds, passed on the fifth. - P18: row paints and the font advance log gpui faults; a pty side caps at what ConPTY and alacritty's pixel sizes hold; a default shell past the list keeps the one before. Kept: constant regexes, captured matches, keyed rows, non-empty shells. Review: 2 rounds. - P19: a blame that lags its code draws every line, bare where no owner fits; `git::resolve` returns `Option` for a conflict gone from the text. Kept: diff groups, merge regions, similar's pinned resolutions. Review: 2 rounds. +- P20: a hex selection off the bytes and a stack frame past the stack select none; a flame focus keeps what the profile holds; the timeline's wheel reports only finite, positive ranges from finite factors; a dump's last byte must have an address; flame paints log. Kept: disassembly patterns, `zoomed`'s contract. Review: 3 rounds.