Add pinning row height (#83)

sandlbn · web-flow · commit de69972933ba · 2026-07-04T23:15:52.000-07:00
* Add pinning row height

* Add text wrapping

* Reformat
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "phosphor"
-version = "0.4.3"
+version = "0.4.4"
 edition = "2021"
 description = "Phosphor — a SID player for USBSID-Pico / Ultimate 64"
 
diff --git a/src/main.rs b/src/main.rs
@@ -80,6 +80,93 @@ use std::time::{Duration, Instant};
 
 use std::sync::{Arc, Mutex};
 
+// ─────────────────────────────────────────────────────────────────────────────
+//  Opt-in per-frame timing profiler
+// ─────────────────────────────────────────────────────────────────────────────
+//
+// Enable with `PHOSPHOR_PROFILE_UPDATE=1`. When set, each `update()` and
+// `view()` call is timed via an RAII guard; every 30 samples (roughly one
+// second at the 30 Hz tick rate) we print the per-frame averages to
+// stderr:
+//
+//   [perf] update=0.31ms/frame  view=3.14ms/frame
+//
+// Zero cost when the env var is unset (single `OnceLock` bool check per
+// frame + no accumulator writes).
+
+use std::sync::atomic::{AtomicU64, Ordering as AtomicOrdering};
+use std::sync::OnceLock;
+
+static PROFILE_ENABLED: OnceLock<bool> = OnceLock::new();
+static UPDATE_ACCUM_US: AtomicU64 = AtomicU64::new(0);
+static VIEW_ACCUM_US: AtomicU64 = AtomicU64::new(0);
+static SAMPLE_COUNT: AtomicU64 = AtomicU64::new(0);
+
+fn profile_enabled() -> bool {
+    *PROFILE_ENABLED.get_or_init(|| std::env::var("PHOSPHOR_PROFILE_UPDATE").is_ok())
+}
+
+enum ProfilerKind {
+    Update,
+    View,
+}
+
+struct ProfilerGuard {
+    kind: ProfilerKind,
+    start: Instant,
+}
+
+impl ProfilerGuard {
+    #[inline]
+    fn update() -> Option<Self> {
+        if profile_enabled() {
+            Some(Self {
+                kind: ProfilerKind::Update,
+                start: Instant::now(),
+            })
+        } else {
+            None
+        }
+    }
+
+    #[inline]
+    fn view() -> Option<Self> {
+        if profile_enabled() {
+            Some(Self {
+                kind: ProfilerKind::View,
+                start: Instant::now(),
+            })
+        } else {
+            None
+        }
+    }
+}
+
+impl Drop for ProfilerGuard {
+    fn drop(&mut self) {
+        let us = self.start.elapsed().as_micros() as u64;
+        match self.kind {
+            ProfilerKind::Update => {
+                UPDATE_ACCUM_US.fetch_add(us, AtomicOrdering::Relaxed);
+            }
+            ProfilerKind::View => {
+                VIEW_ACCUM_US.fetch_add(us, AtomicOrdering::Relaxed);
+                let n = SAMPLE_COUNT.fetch_add(1, AtomicOrdering::Relaxed) + 1;
+                if n >= 30 {
+                    let u = UPDATE_ACCUM_US.swap(0, AtomicOrdering::Relaxed);
+                    let v = VIEW_ACCUM_US.swap(0, AtomicOrdering::Relaxed);
+                    SAMPLE_COUNT.store(0, AtomicOrdering::Relaxed);
+                    eprintln!(
+                        "[perf] update={:.2}ms/frame  view={:.2}ms/frame  (last {n} frames)",
+                        u as f64 / (n as f64) / 1000.0,
+                        v as f64 / (n as f64) / 1000.0,
+                    );
+                }
+            }
+        }
+    }
+}
+
 use crossbeam_channel::{self, Receiver, Sender};
 use iced::widget::{column, container, mouse_area, rule, Space};
 use iced::{event, time, Color, Element, Length, Subscription, Task, Theme};
@@ -698,6 +785,7 @@ impl App {
     }
 
     fn update(&mut self, message: Message) -> Task<Message> {
+        let _perf = ProfilerGuard::update();
         match message {
             // ── Any interaction dismisses the context menu ────────────────
             // (handled per-message below where needed; explicit dismiss too)
@@ -711,7 +799,7 @@ impl App {
                 // is stored and reused for x as well.
                 //
                 // Logical row centre y =
-                //   playlist_viewport_y + display_pos*ROW_HEIGHT - scroll_offset_y + ROW_HEIGHT/2
+                //   playlist_viewport_y + display_pos*row_height() - scroll_offset_y + row_height()/2
                 //
                 // pixel_ratio = raw_cursor_y / logical_row_y
                 // We round to the nearest common value (1.0, 1.5, 2.0, 3.0) to avoid
@@ -724,9 +812,9 @@ impl App {
                     .unwrap_or(0) as f32;
 
                 // Logical y of the centre of the clicked row.
-                let logical_row_y = self.playlist_viewport_y + display_pos * ui::ROW_HEIGHT
+                let logical_row_y = self.playlist_viewport_y + display_pos * ui::row_height()
                     - self.playlist_scroll_offset_y
-                    + ui::ROW_HEIGHT * 0.5;
+                    + ui::row_height() * 0.5;
 
                 // Calibrate ratio when we have valid logical geometry.
                 if logical_row_y > 0.0 && y > 0.0 {
@@ -739,10 +827,10 @@ impl App {
 
                 let ratio = self.pixel_ratio;
                 let menu_x = (x / ratio).max(0.0);
-                let menu_y = (self.playlist_viewport_y + display_pos * ui::ROW_HEIGHT
+                let menu_y = (self.playlist_viewport_y + display_pos * ui::row_height()
                     - self.playlist_scroll_offset_y
-                    + ui::ROW_HEIGHT)
-                    .max(0.0);
+                    + ui::row_height())
+                .max(0.0);
 
                 eprintln!("[phosphor] ShowContextMenu idx={idx} raw=({x:.1},{y:.1}) ratio={ratio:.2} menu=({menu_x:.1},{menu_y:.1})");
 
@@ -1258,7 +1346,7 @@ impl App {
                 let total = self.filtered_indices.len();
                 // Update virtual scroll offset immediately so the newly selected
                 // row is included in the render window before the snap_to fires.
-                self.playlist_scroll_offset_y = next as f32 * ui::ROW_HEIGHT;
+                self.playlist_scroll_offset_y = next as f32 * ui::row_height();
                 if total > 1 {
                     return iced::widget::operation::snap_to(
                         ui::playlist_scrollable_id(),
@@ -1283,7 +1371,7 @@ impl App {
                 self.selected = Some(self.filtered_indices[prev]);
                 let total = self.filtered_indices.len();
                 // Same immediate update for SelectPrev.
-                self.playlist_scroll_offset_y = prev as f32 * ui::ROW_HEIGHT;
+                self.playlist_scroll_offset_y = prev as f32 * ui::row_height();
                 if total > 1 {
                     return iced::widget::operation::snap_to(
                         ui::playlist_scrollable_id(),
@@ -2867,6 +2955,7 @@ impl App {
     }
 
     fn view(&self) -> Element<'_, Message> {
+        let _perf = ProfilerGuard::view();
         // ── Mini player mode ─────────────────────────────────────────────────
         if self.mini_mode {
             let current_duration = self.playlist.current_entry().and_then(|e| e.duration_secs);
diff --git a/src/ui/mod.rs b/src/ui/mod.rs
@@ -1092,14 +1092,25 @@ pub fn search_bar<'a>(
 // ─────────────────────────────────────────────────────────────────────────────
 
 /// Height of a single playlist row in logical pixels.
-/// Must match the actual rendered height: 4px top pad + 13px text + 4px bottom
-/// pad + 1px rule = 22px.  We add a small buffer (26px) so a partially visible
-/// row at either edge is always included.
-pub const ROW_HEIGHT: f32 = 26.0;
+/// Row height that scales with the user's chosen base font size. At the
+/// default 12 pt base (scale = 1.0) this returns 26.0, matching the
+/// historical constant. At a 16 pt base (scale ≈ 1.33) it returns ~35,
+/// keeping the virtual-scroll math in sync with the actually-rendered
+/// text so rows don't shake or clip into their neighbours.
+///
+/// Derived as: 13 pt text intrinsic height (~18 px) + 4+4 px top/bottom
+/// padding on the inner row + a small line-height slack — comes out to
+/// exactly 26 px at scale 1.0.
+pub fn row_height() -> f32 {
+    26.0 * font::scale()
+}
 
 /// Number of extra rows to render above and below the visible window.
-/// Acts as a scroll lookahead so rows don't pop in mid-scroll.
-const OVERSCAN: usize = 8;
+/// Acts as a scroll lookahead so rows don't pop in mid-scroll. Kept
+/// deliberately small — every extra row is a fresh widget tree we
+/// rebuild every frame. At smaller font sizes more rows fit into the
+/// viewport, so this constant compounds the per-frame widget count.
+const OVERSCAN: usize = 3;
 
 /// Build the scrollable playlist table with sortable column headers.
 /// `filtered_indices` maps visible row position → actual `playlist.entries` index.
@@ -1217,12 +1228,15 @@ pub fn playlist_view<'a>(
 
         // ── Virtual window calculation ────────────────────────────────────
         // Compute which rows are visible, with overscan on both sides.
-        let first_visible = ((scroll_offset_y / ROW_HEIGHT) as usize).saturating_sub(OVERSCAN);
-        let rows_in_view = (viewport_height / ROW_HEIGHT).ceil() as usize + 1;
+        // Snapshot the row height once so every step of the math uses the
+        // same value even if the font scale changes mid-render.
+        let rh = row_height();
+        let first_visible = ((scroll_offset_y / rh) as usize).saturating_sub(OVERSCAN);
+        let rows_in_view = (viewport_height / rh).ceil() as usize + 1;
         let last_visible = (first_visible + rows_in_view + OVERSCAN * 2).min(total_rows);
 
         // Top spacer — replaces all rows above the render window
-        let top_space = first_visible as f32 * ROW_HEIGHT;
+        let top_space = first_visible as f32 * rh;
         if top_space > 0.0 {
             rows = rows.push(Space::new().height(Length::Fixed(top_space)));
         }
@@ -1251,7 +1265,7 @@ pub fn playlist_view<'a>(
 
         // Bottom spacer — replaces all rows below the render window
         let bottom_rows = total_rows.saturating_sub(last_visible);
-        let bottom_space = bottom_rows as f32 * ROW_HEIGHT;
+        let bottom_space = bottom_rows as f32 * rh;
         if bottom_space > 0.0 {
             rows = rows.push(Space::new().height(Length::Fixed(bottom_space)));
         }
@@ -1642,6 +1656,11 @@ fn playlist_entry_row<'a>(
             .padding(Padding::from([0, 4])),
     )
     .width(Length::Fill)
+    // Pin to the same value the virtual scroller uses so 1-px intrinsic
+    // font-metric differences between rows can't accumulate into visible
+    // shaking, and font-size changes stay consistent.
+    .height(Length::Fixed(row_height()))
+    .clip(true)
     .style(move |_theme: &Theme| container::Style {
         background: bg,
         ..Default::default()
@@ -1675,54 +1694,63 @@ fn playlist_row_content<'a>(
     };
     let indicator = if is_current { "▶ " } else { "  " };
 
+    // Column cell wrapper: forces the text to a single line AND clips
+    // any horizontal overflow so long titles / authors can't bleed
+    // into the neighbouring column. `Length::Fill` for height lets the
+    // container inherit the fixed row height instead of doing its own
+    // intrinsic layout pass every frame.
+    let nowrap_cell = |content: Element<'a, Message>, width: Length| -> Element<'a, Message> {
+        container(content)
+            .width(width)
+            .height(Length::Fill)
+            .clip(true)
+            .into()
+    };
+    let nowrap_text = |s: String, col: Color, width: Length| -> Element<'a, Message> {
+        nowrap_cell(
+            text(s)
+                .size(font::sized(size))
+                .color(col)
+                .wrapping(iced::widget::text::Wrapping::None)
+                .into(),
+            width,
+        )
+    };
+
     let title_cell: Element<'a, Message> = if has_wds {
-        row![
-            text(title).size(font::sized(size)).color(color),
-            text(" (Karaoke)")
-                .size(font::sized(10.0))
-                .color(Color::from_rgb(0.30, 0.75, 0.45)),
-        ]
-        .spacing(4)
-        .width(Length::FillPortion(4))
-        .into()
+        nowrap_cell(
+            row![
+                text(title)
+                    .size(font::sized(size))
+                    .color(color)
+                    .wrapping(iced::widget::text::Wrapping::None),
+                text(" (Karaoke)")
+                    .size(font::sized(10.0))
+                    .color(Color::from_rgb(0.30, 0.75, 0.45))
+                    .wrapping(iced::widget::text::Wrapping::None),
+            ]
+            .spacing(4)
+            .into(),
+            Length::FillPortion(4),
+        )
     } else {
-        text(title)
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::FillPortion(4))
-            .into()
+        nowrap_text(title, color, Length::FillPortion(4))
     };
 
     row![
-        text(format!("{indicator}{num:>3}"))
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::Fixed(50.0)),
+        nowrap_text(format!("{indicator}{num:>3}"), color, Length::Fixed(50.0)),
         title_cell,
-        text(author)
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::FillPortion(3)),
-        text(released)
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::FillPortion(2)),
-        text(time)
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::Fixed(55.0)),
-        text(sid_type)
-            .size(font::sized(size))
-            .color(type_color)
-            .width(Length::Fixed(42.0)),
-        text(sids)
-            .size(font::sized(size))
-            .color(color)
-            .width(Length::Fixed(45.0)),
+        nowrap_text(author, color, Length::FillPortion(3)),
+        nowrap_text(released, color, Length::FillPortion(2)),
+        nowrap_text(time, color, Length::Fixed(55.0)),
+        nowrap_text(sid_type, type_color, Length::Fixed(42.0)),
+        nowrap_text(sids, color, Length::Fixed(45.0)),
     ]
     .spacing(8)
     .align_y(Alignment::Center)
     .padding(Padding::from([4, 4]))
+    // Fill the fixed-height parent so intrinsic row height never wins.
+    .height(Length::Fill)
     .into()
 }
 
