Mobile: address code-review findings

Fixes from a review of the mobile-UI branch.

- Guard the inspector size clamps so min<=max (f32::clamp panics otherwise) and set a
  window min_inner_size; prevents a crash / degenerate layout at small sizes. Same
  small-size guards for the piano-roll landscape toggle bar and the intent grid.
- Landscape 2->1 edge reveal: snap to fullscreen at frac>=0.5 (the preview's phase
  boundary) instead of COLLAPSE_HI, so releasing mid-drag no longer jumps backward.
  Both edges.
- Gate the double-tap-drag marquee to mobile so it can't hijack a desktop Brush/Draw
  drag.
- Timeline mobile long-press: request a repaint while counting down so it fires even
  when the finger is held perfectly still.
- Landscape folded top bar: lay the filename+search+overflow cluster out within a
  reserved center span (gutters for the pane's own label/buttons) and elide the
  filename, so it can't overlap or overflow the header.
- Node editor connect/disconnect: assert + document the top-level-graph assumption.
- Palette search field focuses only when unfocused (no per-frame focus stomp); reset
  the inspector tap-anchor on orientation change; remove stale main.rs.backup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Skyler Lehmkuhl 2026-07-03 01:55:40 -04:00
parent 9646eef487
commit bb6b6fa9e3
10 changed files with 75 additions and 1592 deletions

View File

@ -211,6 +211,7 @@ fn main() -> eframe::Result {
let mut viewport_builder = egui::ViewportBuilder::default()
.with_inner_size(initial_size)
.with_min_inner_size([360.0, 300.0]) // keep layouts (esp. the mobile shell) above degenerate sizes
.with_title("Lightningbeam Editor")
.with_app_id("lightningbeam-editor"); // Set app_id for Wayland

File diff suppressed because it is too large Load Diff

View File

@ -81,8 +81,9 @@ pub fn render(app: &mut EditorApp, ctx: &egui::Context) {
)
};
let col_w = (grid_rect.width() - (cols as f32 - 1.0) * gap) / cols as f32;
let card_h = (grid_rect.height() - (rows as f32 - 1.0) * gap) / rows as f32;
// Floor at 0 so a very small window can't produce negative/inverted card rects.
let col_w = ((grid_rect.width() - (cols as f32 - 1.0) * gap) / cols as f32).max(0.0);
let card_h = ((grid_rect.height() - (rows as f32 - 1.0) * gap) / rows as f32).max(0.0);
for (i, intent) in intents(&pal).iter().enumerate() {
let col = (i % cols) as f32;
let row = (i / cols) as f32;

View File

@ -197,6 +197,8 @@ pub struct MobileState {
pub inspector_frac: f32,
/// Inspector side-panel width as a fraction of the region (landscape).
pub inspector_width_frac: f32,
/// Last frame's orientation (landscape?), to reset orientation-specific cached state on rotation.
pub was_landscape: bool,
/// Whether the inspector sheet is currently shown. Gated to appear on pointer *release* (a tap),
/// not on press, so press+drag interactions aren't interrupted by the sheet popping up.
pub inspector_visible: bool,
@ -235,6 +237,7 @@ impl Default for MobileState {
show_instruments: false,
inspector_frac: 0.45,
inspector_width_frac: 0.4,
was_landscape: false,
inspector_visible: false,
inspector_anchor_y: 0.0,
inspector_dismissed: false,
@ -268,6 +271,13 @@ pub fn render_mobile_shell(
state.window_count = 2;
state.window_top = state.window_top.min(STACK.len().saturating_sub(2));
}
// On an orientation change, the cached inspector tap-anchor is in the old layout's coordinate
// space; drop it to a non-covering default so the portrait reflow doesn't mis-decide until the
// next tap re-anchors.
if landscape != state.was_landscape {
state.was_landscape = landscape;
state.inspector_anchor_y = available_rect.top();
}
// Background (device color; bands paint over most of it).
ui.painter().rect_filled(available_rect, 0.0, pal.bg);
@ -317,11 +327,17 @@ pub fn render_mobile_shell(
let mut inspector_shown = state.inspector_visible;
// Inspector geometry: a bottom sheet in portrait, a right-side vertical column in landscape.
// Clamp defensively: on a very small region the desired minimum can exceed the available space,
// and `f32::clamp` panics if `min > max`, so keep `min <= max`.
let inspector_rect = if landscape {
let w = (region.width() * state.inspector_width_frac).clamp(180.0, region.width() - 140.0);
let lo = 180.0_f32.min(region.width());
let hi = (region.width() - 140.0).max(lo);
let w = (region.width() * state.inspector_width_frac).clamp(lo, hi);
egui::Rect::from_min_max(egui::pos2(region.right() - w, region.top()), region.max)
} else {
let h = (region.height() * state.inspector_frac).clamp(120.0, region.height() - 60.0);
let lo = 120.0_f32.min(region.height());
let hi = (region.height() - 60.0).max(lo);
let h = (region.height() * state.inspector_frac).clamp(lo, hi);
egui::Rect::from_min_max(egui::pos2(region.left(), region.bottom() - h), region.max)
};

View File

@ -704,13 +704,14 @@ fn commit_drag(d: StackDrag, state: &mut MobileState, content_area: egui::Rect,
}
}
Handle::BottomEdge if state.window_count == 2 && max == 2 => {
// Snap the two-phase 2→1 reveal: near the top → revealed pane fullscreen; a middling drag
// → slid down to the next 2-pane split; barely moved → stay put.
// Snap the two-phase 2→1 reveal to match the preview's phases: past the midpoint (phase 2,
// collapsing) → revealed pane fullscreen; a middling drag (phase 1, sliding) → the next
// 2-pane split; barely moved → stay put.
let top = state.window_top;
let frac = (-d.offset / h).clamp(0.0, 1.0);
let even2 = even_arr(2);
if top + 2 < N {
if frac >= COLLAPSE_HI {
if frac >= 0.5 {
let t = ((frac - 0.5) / 0.5).clamp(0.0, 1.0);
begin_anim(state, top + 1, 2, even2, top + 2, 1, even_arr(1), t, now);
} else if frac > COLLAPSE_LO {
@ -718,7 +719,7 @@ fn commit_drag(d: StackDrag, state: &mut MobileState, content_area: egui::Rect,
begin_anim(state, top, 2, even2, top + 1, 2, even2, t, now);
}
// else: barely moved → stay on the current [top, top+1] split.
} else if frac >= COLLAPSE_HI {
} else if frac >= 0.5 {
begin_anim(state, top, 2, even2, top + 1, 1, even_arr(1), frac, now);
}
}
@ -728,7 +729,7 @@ fn commit_drag(d: StackDrag, state: &mut MobileState, content_area: egui::Rect,
let frac = (d.offset / h).clamp(0.0, 1.0);
let even2 = even_arr(2);
if top > 0 {
if frac >= COLLAPSE_HI {
if frac >= 0.5 {
let t = ((frac - 0.5) / 0.5).clamp(0.0, 1.0);
begin_anim(state, top - 1, 2, even2, top - 1, 1, even_arr(1), t, now);
} else if frac > COLLAPSE_LO {
@ -736,7 +737,7 @@ fn commit_drag(d: StackDrag, state: &mut MobileState, content_area: egui::Rect,
begin_anim(state, top, 2, even2, top - 1, 2, even2, t, now);
}
// else: barely moved → stay on the current split.
} else if frac >= COLLAPSE_HI {
} else if frac >= 0.5 {
begin_anim(state, top, 2, even2, top, 1, even_arr(1), frac, now);
}
}

View File

@ -118,15 +118,31 @@ pub fn render_inline(
.and_then(|p| p.file_name())
.map(|s| s.to_string_lossy().to_string())
.unwrap_or_else(|| "Lightningbeam".to_string());
let galley = ui
.painter()
.layout_no_wrap(name, egui::FontId::proportional(14.0), pal.text);
let name_w = galley.rect.width();
let name_h = galley.rect.height();
let btn = header.height().min(BTN);
let gap = 6.0;
// Reserve gutters so the cluster never overlaps the pane header's own grip/label (left) or its
// fullscreen / node-toggle buttons (right).
let left_gutter = 150.0;
let right_gutter = 2.0 * BTN + 16.0;
let span_left = header.left() + left_gutter;
let span_right = (header.right() - right_gutter).max(span_left);
let span_w = span_right - span_left;
// Filename elided to fit whatever's left after the two buttons.
let max_name_w = (span_w - gap - 2.0 * btn).max(0.0);
let galley = {
let mut job =
egui::text::LayoutJob::simple_singleline(name, egui::FontId::proportional(14.0), pal.text);
job.wrap.max_width = max_name_w;
job.wrap.max_rows = 1;
job.wrap.break_anywhere = true;
ui.painter().layout_job(job)
};
let name_w = galley.rect.width();
let name_h = galley.rect.height();
let cluster_w = name_w + gap + 2.0 * btn;
let x0 = (header.center().x - cluster_w / 2.0).max(header.left() + 8.0);
// Center within the reserved span, then clamp so the right edge stays inside it.
let x0 = (span_left + (span_w - cluster_w) / 2.0).clamp(span_left, (span_right - cluster_w).max(span_left));
let cy = header.center().y;
let bt = header.top() + (header.height() - btn) / 2.0;
@ -223,7 +239,11 @@ fn render_palette(ui: &mut egui::Ui, full: egui::Rect, rc: &mut RenderContext, s
.hint_text("Search commands…")
.desired_width(inner.width()),
);
// Focus the field only when it isn't already focused — re-requesting every frame would stomp
// focus and prevent anything else in the panel from taking it.
if !te.has_focus() {
te.request_focus();
}
// Filtered list.
let q = state.palette_query.to_lowercase();

View File

@ -607,6 +607,9 @@ impl NodeGraphPane {
/// Cable an output port → input port: update the frontend graph + dispatch `Connect`.
fn mobile_connect(&mut self, out_node: NodeId, out_port: usize, in_node: NodeId, in_port: usize) {
// Mobile has no subgraph navigation yet, so cables always target the track-level graph. If
// subgraph nav is added, route through `va_context()` / `graph_connect_in_template` like desktop.
debug_assert!(self.subgraph_stack.is_empty(), "mobile node connect assumes the top-level graph");
let Some(track_id) = self.track_id else { return };
let output_id = self.state.graph.nodes.get(out_node).and_then(|n| n.outputs.get(out_port)).map(|(_, id)| *id);
let input_id = self.state.graph.nodes.get(in_node).and_then(|n| n.inputs.get(in_port)).map(|(_, id)| *id);
@ -625,8 +628,10 @@ impl NodeGraphPane {
}
}
/// Remove a cable: update the frontend graph + dispatch `Disconnect`.
/// Remove a cable: update the frontend graph + dispatch `Disconnect`. Like `mobile_connect`, this
/// assumes the track-level graph (mobile has no subgraph navigation).
fn mobile_disconnect(&mut self, out_node: NodeId, out_port: usize, in_node: NodeId, in_port: usize) {
debug_assert!(self.subgraph_stack.is_empty(), "mobile node disconnect assumes the top-level graph");
let Some(track_id) = self.track_id else { return };
let output_id = self.state.graph.nodes.get(out_node).and_then(|n| n.outputs.get(out_port)).map(|(_, id)| *id);
let input_id = self.state.graph.nodes.get(in_node).and_then(|n| n.inputs.get(in_port)).map(|(_, id)| *id);

View File

@ -392,7 +392,9 @@ impl PianoRollPane {
// the conventional piano roll. (Portrait uses the keyboard-primary "Synthesia" view below.)
let mut rect = rect;
if shared.is_mobile && !shared.is_portrait {
let bar = Rect::from_min_max(rect.min, pos2(rect.right(), rect.min.y + 30.0));
// Clamp the bar height so a very short pane can't invert the remaining content rect.
let bar_h = 30.0_f32.min(rect.height());
let bar = Rect::from_min_max(rect.min, pos2(rect.right(), rect.min.y + bar_h));
rect = Rect::from_min_max(pos2(rect.left(), bar.bottom()), rect.max);
self.render_keys_notes_toggle(ui, bar, shared);
if self.landscape_keys {

View File

@ -11407,12 +11407,12 @@ impl StagePane {
}
}
// P5 gesture: double-tap-drag on empty space → transient marquee, regardless of the active
// tool. The second tap arms it; a drag then drives ToolState::MarqueeSelecting (resolved by
// the release handler above). While armed we suppress the normal tool dispatch so a
// P5 gesture (mobile): double-tap-drag on empty space → transient marquee, regardless of the
// active tool. The second tap arms it; a drag then drives ToolState::MarqueeSelecting (resolved
// by the release handler above). While armed we suppress the normal tool dispatch so a
// brush/etc. doesn't paint during the gesture. A double-tap without a drag falls through
// (Select tool handles zoom-to-fit).
if !is_replaying && !alt_held {
// (Select tool handles zoom-to-fit). Mobile-only so it can't hijack a desktop Brush/Draw drag.
if shared.is_mobile && !is_replaying && !alt_held {
use vello::kurbo::Point;
let point = Point::new(world_pos.x as f64, world_pos.y as f64);
let press_pos = mouse_canvas_pos.to_pos2();

View File

@ -5891,6 +5891,11 @@ impl PaneRenderer for TimelinePane {
} else if now - t0 >= 0.4 {
self.lp_time = None;
long_press = Some(self.lp_pos);
} else {
// Still counting down — request a repaint so the threshold is re-evaluated even if
// the finger is perfectly still and egui would otherwise go idle (no input events).
let remaining = (0.4 - (now - t0)).max(0.0);
ui.ctx().request_repaint_after(std::time::Duration::from_secs_f64(remaining));
}
}
}