From 50ef13780bdd44ea564ad52337ac06c7990fa2ae Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Wed, 12 Aug 2026 23:18:15 +0200 Subject: [PATCH] refactor: encode required invariants --- src/backend/wayland/clipboard/completion.rs | 40 +++--- src/backend/wayland/portal_task.rs | 9 +- src/backend/wayland/session/persistence.rs | 5 +- .../wayland/state/core/output/transition.rs | 7 +- src/backend/wayland/state/onboarding.rs | 15 ++- src/backend/wayland/state/pdf_export.rs | 19 +-- .../wayland/state/toolbar/inline/render.rs | 7 +- src/backend/wayland/toolbar/view/top/build.rs | 65 +++++----- src/capture/manager.rs | 9 +- src/config/document/merge.rs | 5 +- src/config/field_metadata.rs | 114 ++++++++++-------- src/config/tests/document.rs | 7 ++ src/config/tests/validate.rs | 12 ++ src/config/types/boards.rs | 20 ++- src/config/validate/boards.rs | 8 +- src/daemon/protocol_v2/command/client.rs | 5 +- src/input/state/core/base/toast_queue.rs | 47 ++++---- src/ocr/controller.rs | 9 +- src/runtime_ui_state/pipeline/integration.rs | 10 +- src/runtime_ui_state/pipeline/staging.rs | 14 ++- src/runtime_ui_state/seeds.rs | 10 +- src/session/snapshot/save/payload.rs | 38 ++---- src/session/snapshot/save/recovery.rs | 30 ++--- src/toolbar_gtk/bridge.rs | 5 +- src/toolbar_gtk/view/capture_suppression.rs | 8 +- src/toolbar_gtk/view/top_bar.rs | 22 ++++ src/toolbar_gtk/view/top_bar/controls.rs | 65 +++++----- src/toolbar_gtk/view/top_bar/popovers.rs | 69 ++++++----- src/toolbar_gtk/view/top_bar/strip.rs | 6 +- src/toolbar_gtk/view/top_bar/style_pill.rs | 33 +++-- src/ui/toolbar/model/event_policy.rs | 65 +++++++--- src/ui/toolbar/model/style_pill/control.rs | 39 +++++- src/ui/toolbar/model/top_spec/control.rs | 8 ++ .../toolbar/model/top_spec/tests/behavior.rs | 33 +++++ 34 files changed, 495 insertions(+), 363 deletions(-) diff --git a/src/backend/wayland/clipboard/completion.rs b/src/backend/wayland/clipboard/completion.rs index 2ac820c4..d5caf6ac 100644 --- a/src/backend/wayland/clipboard/completion.rs +++ b/src/backend/wayland/clipboard/completion.rs @@ -213,29 +213,25 @@ where } pub(in crate::backend::wayland) fn poll(&mut self) -> ClipboardPoll { - let Some(active) = self.active.as_ref() else { + let Some(active) = self.active.take() else { return ClipboardPoll::Idle; }; let active_id = active.id; match active.receiver.try_recv() { - Err(TryRecvError::Empty) => ClipboardPoll::Pending { id: active_id }, - Err(TryRecvError::Disconnected) => { - let active = self.active.take().expect("active receiver checked above"); - ClipboardPoll::Disconnected { - id: active.id, - context: active.context, - } - } - Ok(ProducerMessage::Ready { id, outcome }) if id == active_id => { - let active = self.active.take().expect("active receiver checked above"); - ClipboardPoll::Ready { - id, - context: active.context, - outcome, - } + Err(TryRecvError::Empty) => { + self.active = Some(active); + ClipboardPoll::Pending { id: active_id } } + Err(TryRecvError::Disconnected) => ClipboardPoll::Disconnected { + id: active.id, + context: active.context, + }, + Ok(ProducerMessage::Ready { id, outcome }) if id == active_id => ClipboardPoll::Ready { + id, + context: active.context, + outcome, + }, Ok(ProducerMessage::Failed { id, reason }) if id == active_id => { - let active = self.active.take().expect("active receiver checked above"); ClipboardPoll::ProducerFailed { id, context: active.context, @@ -244,7 +240,6 @@ where } Ok(ProducerMessage::Ready { id, .. } | ProducerMessage::Failed { id, .. }) => { self.healthy = false; - let active = self.active.take().expect("active receiver checked above"); ClipboardPoll::ProducerFailed { id: active.id, context: active.context, @@ -290,12 +285,11 @@ impl ClipboardProducerExitGuard { } fn publish(mut self, message: ProducerMessage) { - let result = self + let sender = self .sender - .as_ref() - .expect("producer sender retained until publication") - .try_send(message); - self.sender.take(); + .take() + .expect("clipboard producer still holds its sender until publish"); + let result = sender.try_send(message); self.terminal_published = true; match result { Ok(()) | Err(TrySendError::Disconnected(_)) => {} diff --git a/src/backend/wayland/portal_task.rs b/src/backend/wayland/portal_task.rs index ab75436a..956d1284 100644 --- a/src/backend/wayland/portal_task.rs +++ b/src/backend/wayland/portal_task.rs @@ -150,12 +150,11 @@ impl PortalTaskExitGuard { } fn publish(mut self, message: PortalMessage) { - let result = self + let sender = self .sender - .as_ref() - .expect("portal sender retained until terminal publication") - .try_send(message); - self.sender.take(); + .take() + .expect("portal task still holds its sender until publish"); + let result = sender.try_send(message); self.terminal_published = true; match result { Ok(()) | Err(TrySendError::Disconnected(_)) => {} diff --git a/src/backend/wayland/session/persistence.rs b/src/backend/wayland/session/persistence.rs index 9cfeeb35..631c7e2b 100644 --- a/src/backend/wayland/session/persistence.rs +++ b/src/backend/wayland/session/persistence.rs @@ -395,7 +395,10 @@ impl PersistenceController { shutdown_error = Some(err); } self.request_tx.take(); - let join = self.worker.take().expect("worker presence checked").join(); + let Some(worker) = self.worker.take() else { + return shutdown_error.map_or(Ok(()), Err); + }; + let join = worker.join(); self.active_id = None; if join.is_err() { self.healthy = false; diff --git a/src/backend/wayland/state/core/output/transition.rs b/src/backend/wayland/state/core/output/transition.rs index c87beda9..a7ee7a1e 100644 --- a/src/backend/wayland/state/core/output/transition.rs +++ b/src/backend/wayland/state/core/output/transition.rs @@ -123,10 +123,9 @@ impl WaylandState { return Ok(true); } - let pending = self - .session - .take_pending_output_transition() - .expect("pending transition checked above"); + let Some(pending) = self.session.take_pending_output_transition() else { + return Ok(false); + }; if let Err(err) = self.run_output_transition( pending.staged_options.clone(), pending.physical_output_identity.clone(), diff --git a/src/backend/wayland/state/onboarding.rs b/src/backend/wayland/state/onboarding.rs index 7e245c38..d21e8959 100644 --- a/src/backend/wayland/state/onboarding.rs +++ b/src/backend/wayland/state/onboarding.rs @@ -246,6 +246,7 @@ impl WaylandState { let mut changed = false; let mut hint_kind: Option<&'static str> = None; + let mut status_bar_hint_entry: Option<&str> = None; { let state = self.onboarding.state_mut(); if !state.first_run_completed { @@ -286,7 +287,7 @@ impl WaylandState { // status bar is the most important of the three — the board picker's // on-screen entry point is easy to miss — so it comes first and at // the earliest threshold among the new surfaces. - } else if status_bar_entry.is_some() + } else if let Some(entry) = status_bar_entry && state.sessions_seen >= 3 && !state.hint_status_bar_shown && state.hint_status_bar_count < DEFERRED_HINT_REPEAT_MAX @@ -295,6 +296,7 @@ impl WaylandState { state.hint_status_bar_count = state.hint_status_bar_count.saturating_add(1); changed = true; hint_kind = Some("status_bar"); + status_bar_hint_entry = Some(entry); } else if canvas_hint_relevant && state.sessions_seen >= 5 && !state.hint_canvas_popover_shown @@ -329,10 +331,13 @@ impl WaylandState { "Press {} to search actions.", self.shortcut_label(Action::ToggleCommandPalette, "Command Palette") ), - "status_bar" => format!( - "Click the {} segment in the status bar to switch boards and pages.", - status_bar_entry.expect("status-bar hint requires a visible picker entry") - ), + "status_bar" => { + let entry = status_bar_hint_entry + .expect("status_bar hint is only queued when the picker is on screen"); + format!( + "Click the {entry} segment in the status bar to switch boards and pages." + ) + } "canvas_popover" => { "Open \u{201c}Canvas\u{2026}\u{201d} from the \u{2026} overflow for boards, \ pages, zoom, and advanced controls." diff --git a/src/backend/wayland/state/pdf_export.rs b/src/backend/wayland/state/pdf_export.rs index eb8b4e76..3b0afb2c 100644 --- a/src/backend/wayland/state/pdf_export.rs +++ b/src/backend/wayland/state/pdf_export.rs @@ -260,15 +260,16 @@ fn frame_content_bounds(frame: &Frame) -> Option { found = true; } - found.then(|| { - CanvasExportRect::new( - min_x as f64, - min_y as f64, - (max_x - min_x).max(1) as f64, - (max_y - min_y).max(1) as f64, - ) - .expect("positive bounds") - }) + found + .then(|| { + CanvasExportRect::new( + min_x as f64, + min_y as f64, + (max_x - min_x).max(1) as f64, + (max_y - min_y).max(1) as f64, + ) + }) + .flatten() } #[cfg(test)] diff --git a/src/backend/wayland/state/toolbar/inline/render.rs b/src/backend/wayland/state/toolbar/inline/render.rs index 40157329..8c3d31a7 100644 --- a/src/backend/wayland/state/toolbar/inline/render.rs +++ b/src/backend/wayland/state/toolbar/inline/render.rs @@ -58,16 +58,17 @@ impl WaylandState { hit.rect.2 *= ui_scale; hit.rect.3 *= ui_scale; } - self.data.inline_top_rect = Some(( + let top_rect = ( top_offset.0, top_offset.1, top_size.0 as f64, top_size.1 as f64, - )); + ); + self.data.inline_top_rect = Some(top_rect); crate::backend::wayland::toolbar::hit::clip_hit_regions_to_bounds( &mut self.data.inline_top_hits, 0, - self.data.inline_top_rect.expect("top rect was just set"), + top_rect, ); } } diff --git a/src/backend/wayland/toolbar/view/top/build.rs b/src/backend/wayland/toolbar/view/top/build.rs index 9510bcc8..151e196f 100644 --- a/src/backend/wayland/toolbar/view/top/build.rs +++ b/src/backend/wayland/toolbar/view/top/build.rs @@ -424,7 +424,7 @@ fn build_top_minimized_tab( (0.0, 0.0, width, height), WidgetKind::IconButton { glyph: IconFn(toolbar_icons::top_toolbar_icon_painter( - control.icon(snapshot).expect("restore icon"), + model::TopToolbarIcon::Restore, )), icon_size: (height * 0.75).min(18.0), style: ButtonStyle::plain(), @@ -456,7 +456,7 @@ fn build_top_micro_chip( (0.0, 0.0, width, height), WidgetKind::MicroChip { glyph: IconFn(toolbar_icons::top_toolbar_icon_painter( - control.icon(snapshot).expect("micro chip tool icon"), + model::TopToolbarIcon::Tool(model::semantic_icon_for_tool(snapshot.active_tool)), )), ring_color: ( snapshot.color.r, @@ -633,9 +633,10 @@ fn push_style_pill( ), selected: control.active(snapshot), }, - control - .event(snapshot) - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + Some(Interaction::click( + control.click_event(snapshot), + control.tooltip(snapshot), + )), )); x += TOP_CHIP_SIZE + gap; } @@ -648,9 +649,10 @@ fn push_style_pill( color: (entry.color.r, entry.color.g, entry.color.b, entry.color.a), selected: control.active(snapshot), }, - control - .event(snapshot) - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + Some(Interaction::click( + control.click_event(snapshot), + control.tooltip(snapshot), + )), )); let is_last = index + 1 == swatch_count; x += TOP_SWATCH_SIZE + if is_last { gap } else { TOP_SWATCH_GAP }; @@ -658,7 +660,8 @@ fn push_style_pill( model::StylePillControl::ThicknessSlider | model::StylePillControl::OpacitySlider | model::StylePillControl::FontSizeSlider => { - let (slider_spec, value) = control.slider(snapshot).expect("slider control"); + let (slider_spec, value) = control.slider_value(snapshot); + let event = control.click_event(snapshot); let kind = match control { model::StylePillControl::ThicknessSlider => HitKind::DragSetThickness { min: slider_spec.min, @@ -683,7 +686,7 @@ fn push_style_pill( t: slider_spec.t_from_value(value), }, Some(Interaction { - event: control.event(snapshot).expect("slider event"), + event, kind, tooltip: None, }), @@ -701,7 +704,7 @@ fn push_style_pill( row_h, ), WidgetKind::Label(LabelSpec::new( - control.value_text(snapshot).expect("opacity readout"), + control.required_value_text(snapshot), TOP_LABEL_FONT_SIZE, true, )), @@ -722,15 +725,16 @@ fn push_style_pill( ), WidgetKind::TextButton { label: LabelSpec::new( - control.value_text(snapshot).expect("numeral text"), + control.required_value_text(snapshot), TOP_LABEL_FONT_SIZE, true, ), style: ButtonStyle::plain(), }, - control - .event(snapshot) - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + Some(Interaction::click( + control.click_event(snapshot), + control.tooltip(snapshot), + )), )); x += ToolbarLayoutSpec::TOP_STYLE_VALUE_W + gap; } @@ -748,9 +752,10 @@ fn push_style_pill( checked: control.active(snapshot), label: LabelSpec::new(control.label(snapshot), MINI_LABEL_FONT_SIZE, false), }, - control - .event(snapshot) - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + Some(Interaction::click( + control.click_event(snapshot), + control.tooltip(snapshot), + )), )); x += w + gap; } @@ -767,9 +772,10 @@ fn push_style_pill( label: LabelSpec::new(control.label(snapshot), TOP_LABEL_FONT_SIZE, true), style: ButtonStyle::plain(), }, - control - .event(snapshot) - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + Some(Interaction::click( + control.click_event(snapshot), + control.tooltip(snapshot), + )), )); x += ToolbarLayoutSpec::TOP_STYLE_RESET_W + gap; } @@ -785,7 +791,7 @@ fn push_style_pill( ), WidgetKind::TextButton { label: LabelSpec::new( - control.value_text(snapshot).unwrap_or_default(), + control.required_value_text(snapshot), TOP_LABEL_FONT_SIZE, true, ), @@ -795,16 +801,15 @@ fn push_style_pill( ButtonStyle::disabled() }, }, - (enabled) - .then(|| control.event(snapshot)) - .flatten() - .map(|event| Interaction::click(event, control.tooltip(snapshot))), + (enabled).then(|| { + Interaction::click(control.click_event(snapshot), control.tooltip(snapshot)) + }), )); x += ToolbarLayoutSpec::TOP_STYLE_SEL_VALUE_W + gap; } model::StylePillControl::SelectionStepper(_) => { let enabled = control.enabled(snapshot); - let steps = control.steps(snapshot).expect("stepper halves"); + let steps = control.required_steps(snapshot); let step_w = ToolbarLayoutSpec::TOP_STYLE_STEP_W; let value_w = ToolbarLayoutSpec::TOP_STYLE_SEL_VALUE_W; let step_style = if enabled { @@ -827,7 +832,7 @@ fn push_style_pill( format!("{id}.value"), (x + step_w, center(row_h), value_w, row_h), WidgetKind::Label(LabelSpec::new( - control.value_text(snapshot).unwrap_or_default(), + control.required_value_text(snapshot), TOP_LABEL_FONT_SIZE, true, )), @@ -847,7 +852,7 @@ fn push_style_pill( } model::StylePillControl::FontFamilySegment | model::StylePillControl::EraserModeSegment => { - let segments = control.segments(snapshot).expect("segment halves"); + let segments = control.required_segments(snapshot); // A clear gap before the segment so Sans│Mono never crowd the // preceding numeral ("72pt") to its left (M7-C3). x += ToolbarLayoutSpec::TOP_STYLE_SEGMENT_LEAD; @@ -1075,7 +1080,7 @@ fn control_button_node_with_tooltip( }; WidgetKind::IconButton { glyph: IconFn(toolbar_icons::top_toolbar_icon_painter( - control.icon(snapshot).expect("button icon"), + control.glyph(snapshot), )), icon_size, style, diff --git a/src/capture/manager.rs b/src/capture/manager.rs index 92250e09..ecdb73aa 100644 --- a/src/capture/manager.rs +++ b/src/capture/manager.rs @@ -471,11 +471,10 @@ impl CaptureWorkerExitGuard { } fn publish(&self, completion: CaptureCompletion) -> bool { - let result = self - .completion_tx - .as_ref() - .expect("capture completion sender retained by worker") - .try_send(completion); + let Some(tx) = self.completion_tx.as_ref() else { + return false; + }; + let result = tx.try_send(completion); match result { Ok(()) => { (self.notifier)(); diff --git a/src/config/document/merge.rs b/src/config/document/merge.rs index 9fd127c1..e3fb2465 100644 --- a/src/config/document/merge.rs +++ b/src/config/document/merge.rs @@ -425,10 +425,7 @@ fn merge_value(raw: &mut Value, previous: Option<&Value>, updated: &Value, path: while raw.len() > updated.len() { raw.remove(raw.len() - 1); } - for index in 0..updated.len() { - let updated_value = updated - .get(index) - .expect("array index is bounded by updated length"); + for (index, updated_value) in updated.iter().enumerate() { if let Some(raw_value) = raw.get_mut(index) { merge_value( raw_value, diff --git a/src/config/field_metadata.rs b/src/config/field_metadata.rs index d7b17dbd..79ab2f74 100644 --- a/src/config/field_metadata.rs +++ b/src/config/field_metadata.rs @@ -83,61 +83,71 @@ pub struct PerformanceFieldMetadata { pub constraint: ScalarConstraint, } -pub const PERFORMANCE_FIELD_METADATA: &[PerformanceFieldMetadata] = &[ - PerformanceFieldMetadata { - id: PerformanceFieldId::BufferCount, - path: "performance.buffer_count", - group: PerformanceFieldGroup::Rendering, - label: "Buffer count (2-4)", - help: "2 uses less memory; 3 is recommended; 4 adds another queued buffer.", - search_terms: &["rendering", "buffer", "double triple quad buffering"], - constraint: ScalarConstraint::UnsignedChoice(PERFORMANCE_BUFFER_COUNTS), - }, - PerformanceFieldMetadata { - id: PerformanceFieldId::EnableVsync, - path: "performance.enable_vsync", - group: PerformanceFieldGroup::Rendering, - label: "Enable VSync", - help: "Synchronizes rendering with display refresh to prevent tearing, with some input latency.", - search_terms: &["rendering", "vsync", "tearing", "display refresh"], - constraint: ScalarConstraint::Boolean, - }, - PerformanceFieldMetadata { - id: PerformanceFieldId::MaxFpsNoVsync, - path: "performance.max_fps_no_vsync", - group: PerformanceFieldGroup::Rendering, - label: "Max FPS (VSync off)", - help: "Caps frame rate when VSync is off. Default 120; try 144 or 240 on high-refresh displays; 0 means unlimited.", - search_terms: &["rendering", "fps", "frame rate", "vsync off", "unlimited"], - constraint: ScalarConstraint::Unsigned { - min: 0, - max: u32::MAX, - }, +const PERFORMANCE_BUFFER_COUNT: PerformanceFieldMetadata = PerformanceFieldMetadata { + id: PerformanceFieldId::BufferCount, + path: "performance.buffer_count", + group: PerformanceFieldGroup::Rendering, + label: "Buffer count (2-4)", + help: "2 uses less memory; 3 is recommended; 4 adds another queued buffer.", + search_terms: &["rendering", "buffer", "double triple quad buffering"], + constraint: ScalarConstraint::UnsignedChoice(PERFORMANCE_BUFFER_COUNTS), +}; + +const PERFORMANCE_ENABLE_VSYNC: PerformanceFieldMetadata = PerformanceFieldMetadata { + id: PerformanceFieldId::EnableVsync, + path: "performance.enable_vsync", + group: PerformanceFieldGroup::Rendering, + label: "Enable VSync", + help: "Synchronizes rendering with display refresh to prevent tearing, with some input latency.", + search_terms: &["rendering", "vsync", "tearing", "display refresh"], + constraint: ScalarConstraint::Boolean, +}; + +const PERFORMANCE_MAX_FPS_NO_VSYNC: PerformanceFieldMetadata = PerformanceFieldMetadata { + id: PerformanceFieldId::MaxFpsNoVsync, + path: "performance.max_fps_no_vsync", + group: PerformanceFieldGroup::Rendering, + label: "Max FPS (VSync off)", + help: "Caps frame rate when VSync is off. Default 120; try 144 or 240 on high-refresh displays; 0 means unlimited.", + search_terms: &["rendering", "fps", "frame rate", "vsync off", "unlimited"], + constraint: ScalarConstraint::Unsigned { + min: 0, + max: u32::MAX, }, - PerformanceFieldMetadata { - id: PerformanceFieldId::UiAnimationFps, - path: "performance.ui_animation_fps", - group: PerformanceFieldGroup::Animations, - label: "UI Animation FPS", - help: "Controls UI effect ticks without changing input responsiveness; 30-60 is recommended, 0 means unlimited.", - search_terms: &[ - "animation", - "ui", - "fps", - "effects", - "toasts", - "click highlights", - ], - constraint: ScalarConstraint::Unsigned { - min: 0, - max: PERFORMANCE_UI_ANIMATION_FPS_MAX, - }, +}; + +const PERFORMANCE_UI_ANIMATION_FPS: PerformanceFieldMetadata = PerformanceFieldMetadata { + id: PerformanceFieldId::UiAnimationFps, + path: "performance.ui_animation_fps", + group: PerformanceFieldGroup::Animations, + label: "UI Animation FPS", + help: "Controls UI effect ticks without changing input responsiveness; 30-60 is recommended, 0 means unlimited.", + search_terms: &[ + "animation", + "ui", + "fps", + "effects", + "toasts", + "click highlights", + ], + constraint: ScalarConstraint::Unsigned { + min: 0, + max: PERFORMANCE_UI_ANIMATION_FPS_MAX, }, +}; + +pub const PERFORMANCE_FIELD_METADATA: &[PerformanceFieldMetadata] = &[ + PERFORMANCE_BUFFER_COUNT, + PERFORMANCE_ENABLE_VSYNC, + PERFORMANCE_MAX_FPS_NO_VSYNC, + PERFORMANCE_UI_ANIMATION_FPS, ]; pub fn performance_field_metadata(id: PerformanceFieldId) -> &'static PerformanceFieldMetadata { - PERFORMANCE_FIELD_METADATA - .iter() - .find(|metadata| metadata.id == id) - .expect("every PerformanceFieldId must have metadata") + match id { + PerformanceFieldId::BufferCount => &PERFORMANCE_BUFFER_COUNT, + PerformanceFieldId::EnableVsync => &PERFORMANCE_ENABLE_VSYNC, + PerformanceFieldId::MaxFpsNoVsync => &PERFORMANCE_MAX_FPS_NO_VSYNC, + PerformanceFieldId::UiAnimationFps => &PERFORMANCE_UI_ANIMATION_FPS, + } } diff --git a/src/config/tests/document.rs b/src/config/tests/document.rs index 4cb559fc..6cbe7b8f 100644 --- a/src/config/tests/document.rs +++ b/src/config/tests/document.rs @@ -1872,6 +1872,13 @@ fn performance_metadata_is_unique_and_matches_example_and_docs() { ); } assert_eq!(ids.len(), PerformanceFieldId::ALL.len()); + for id in PerformanceFieldId::ALL { + assert_eq!( + performance_field_metadata(id).id, + id, + "lookup is by id, not table index" + ); + } } #[test] diff --git a/src/config/tests/validate.rs b/src/config/tests/validate.rs index bfb2f174..5c9d8e97 100644 --- a/src/config/tests/validate.rs +++ b/src/config/tests/validate.rs @@ -85,6 +85,18 @@ fn drawing_polygon_sides_validation_keeps_supported_bounds() { assert_eq!(config.drawing.polygon_sides, 12); } +#[test] +fn default_overlay_item_is_the_transparent_board() { + let overlay = BoardsConfig::default_overlay_item(); + assert_eq!(overlay.id, "transparent"); + assert_eq!(overlay.name, "Overlay"); + assert!(overlay.background.is_transparent()); + let first = &BoardsConfig::default_items()[0]; + assert_eq!(first.id, overlay.id); + assert_eq!(first.name, overlay.name); + assert!(first.background.is_transparent()); +} + #[test] fn validate_boards_uses_boundary_id_normalization() { let mut config = Config { diff --git a/src/config/types/boards.rs b/src/config/types/boards.rs index 0a372191..0d6c284f 100644 --- a/src/config/types/boards.rs +++ b/src/config/types/boards.rs @@ -61,8 +61,8 @@ impl BoardsConfig { default_board_items() } - pub fn from_legacy(legacy: &BoardConfig) -> Self { - let mut items = vec![BoardItemConfig { + pub fn default_overlay_item() -> BoardItemConfig { + BoardItemConfig { id: "transparent".to_string(), name: "Overlay".to_string(), background: BoardBackgroundConfig::Transparent("transparent".to_string()), @@ -70,7 +70,11 @@ impl BoardsConfig { auto_adjust_pen: false, persist: true, pinned: false, - }]; + } + } + + pub fn from_legacy(legacy: &BoardConfig) -> Self { + let mut items = vec![BoardsConfig::default_overlay_item()]; if legacy.enabled { items.push(BoardItemConfig { @@ -220,15 +224,7 @@ fn default_board_pinned() -> bool { fn default_board_items() -> Vec { vec![ - BoardItemConfig { - id: "transparent".to_string(), - name: "Overlay".to_string(), - background: BoardBackgroundConfig::Transparent("transparent".to_string()), - default_pen_color: None, - auto_adjust_pen: false, - persist: true, - pinned: false, - }, + BoardsConfig::default_overlay_item(), BoardItemConfig { id: "whiteboard".to_string(), name: "Whiteboard".to_string(), diff --git a/src/config/validate/boards.rs b/src/config/validate/boards.rs index 03006801..46f33e27 100644 --- a/src/config/validate/boards.rs +++ b/src/config/validate/boards.rs @@ -69,13 +69,7 @@ impl Config { boards.items.insert(0, item); } else { warn!("No transparent board defined; adding default overlay board"); - boards.items.insert( - 0, - BoardsConfig::default_items() - .into_iter() - .find(|item| item.background.is_transparent()) - .expect("default items include transparent board"), - ); + boards.items.insert(0, BoardsConfig::default_overlay_item()); } } diff --git a/src/daemon/protocol_v2/command/client.rs b/src/daemon/protocol_v2/command/client.rs index 4bdc09d4..6bbaab60 100644 --- a/src/daemon/protocol_v2/command/client.rs +++ b/src/daemon/protocol_v2/command/client.rs @@ -230,8 +230,9 @@ fn cancel_open(control: &mut CommandControl) -> Result { } bump_revision(control)?; control.decision = CommandDecision::Canceled; - control.response = Some(CommandResponse::Canceled); - let digest = response_digest(control.response.as_ref().unwrap())?; + let response = CommandResponse::Canceled; + let digest = response_digest(&response)?; + control.response = Some(response); control.caller_disposition = CallerDisposition::Acknowledged { response_digest: digest, }; diff --git a/src/input/state/core/base/toast_queue.rs b/src/input/state/core/base/toast_queue.rs index cbf71b20..38264fc4 100644 --- a/src/input/state/core/base/toast_queue.rs +++ b/src/input/state/core/base/toast_queue.rs @@ -194,11 +194,8 @@ impl ToastQueue { // a critical failure. Re-run the normal preemption rule after the // in-place update instead of leaving that critical update hidden // behind a lower-priority active toast. - if active - .as_ref() - .is_some_and(|current| priority > current.priority) - { - self.requeue_preempted(active.take().expect("checked above"), now); + if let Some(current) = active.take_if(|current| priority > current.priority) { + self.requeue_preempted(current, now); self.activate_next(active, now); debug_assert!(active.as_ref().is_some_and(|current| current.key == key)); return ToastPushOutcome::Displayed; @@ -211,30 +208,28 @@ impl ToastQueue { return ToastPushOutcome::HintYielded; } - match active { - None => { - self.enqueue_fifo(priority, key, toast); - self.activate_next(active, now); - if active.as_ref().is_some_and(|current| current.key == key) { - ToastPushOutcome::Displayed - } else { - ToastPushOutcome::Queued - } - } - Some(current) if priority > current.priority => { - // Preempt: the active toast yields and is re-queued while - // fresh (hints are dropped outright). - self.requeue_preempted(active.take().expect("checked above"), now); - self.enqueue_fifo(priority, key, toast); - self.activate_next(active, now); - debug_assert!(active.as_ref().is_some_and(|current| current.key == key)); + if active.is_none() { + self.enqueue_fifo(priority, key, toast); + self.activate_next(active, now); + return if active.as_ref().is_some_and(|current| current.key == key) { ToastPushOutcome::Displayed - } - Some(_) => { - self.enqueue_fifo(priority, key, toast); + } else { ToastPushOutcome::Queued - } + }; + } + + if let Some(current) = active.take_if(|current| priority > current.priority) { + // Preempt: the active toast yields and is re-queued while + // fresh (hints are dropped outright). + self.requeue_preempted(current, now); + self.enqueue_fifo(priority, key, toast); + self.activate_next(active, now); + debug_assert!(active.as_ref().is_some_and(|current| current.key == key)); + return ToastPushOutcome::Displayed; } + + self.enqueue_fifo(priority, key, toast); + ToastPushOutcome::Queued } /// Advance timers: expire the active toast and promote the next pending diff --git a/src/ocr/controller.rs b/src/ocr/controller.rs index c548a938..c5c72368 100644 --- a/src/ocr/controller.rs +++ b/src/ocr/controller.rs @@ -242,12 +242,11 @@ impl WorkerExitGuard { } fn publish(mut self, message: WorkerMessage) { - let result = self + let sender = self .sender - .as_ref() - .expect("OCR worker sender retained until publication") - .try_send(message); - self.sender.take(); + .take() + .expect("OCR worker still holds its sender until publish"); + let result = sender.try_send(message); self.terminal_published = true; match result { Ok(()) | Err(TrySendError::Disconnected(_)) => {} diff --git a/src/runtime_ui_state/pipeline/integration.rs b/src/runtime_ui_state/pipeline/integration.rs index c4d77664..35df798f 100644 --- a/src/runtime_ui_state/pipeline/integration.rs +++ b/src/runtime_ui_state/pipeline/integration.rs @@ -131,14 +131,8 @@ impl PersistencePipeline { &mut self, revision: AcceptedStateRevision, ) -> Option { - if !self.receipts.get(&revision).is_some_and(Option::is_some) { - return None; - } - let outcome = self - .receipts - .remove(&revision) - .and_then(|outcome| outcome) - .expect("terminal receipt was checked above"); + let outcome = self.receipts.get_mut(&revision).and_then(Option::take)?; + self.receipts.remove(&revision); if revision > self.settled_through { self.consumed_terminal_receipts.insert(revision); } diff --git a/src/runtime_ui_state/pipeline/staging.rs b/src/runtime_ui_state/pipeline/staging.rs index 2e091f05..0d58e17e 100644 --- a/src/runtime_ui_state/pipeline/staging.rs +++ b/src/runtime_ui_state/pipeline/staging.rs @@ -100,12 +100,14 @@ impl PersistencePipeline { pub(crate) fn hold_trailing_replacements(&mut self) -> Vec { let mut held = Vec::new(); - while matches!(self.pending.back(), Some(PendingStage::Replace(_))) { - let PendingStage::Replace(stage) = self.pending.pop_back().expect("tail checked") - else { - unreachable!(); - }; - held.push(stage.into()); + while let Some(stage) = self.pending.pop_back() { + match stage { + PendingStage::Replace(stage) => held.push(stage.into()), + other => { + self.pending.push_back(other); + break; + } + } } held.reverse(); held diff --git a/src/runtime_ui_state/seeds.rs b/src/runtime_ui_state/seeds.rs index e9c47886..bc26629e 100644 --- a/src/runtime_ui_state/seeds.rs +++ b/src/runtime_ui_state/seeds.rs @@ -211,15 +211,15 @@ impl InteractionSeedRegistry { ) -> Result { let state = self .state(target) - .filter(|state| state.normalized_value.is_some()) + .ok_or_else(|| SeedRegistryError::MissingTarget(target.clone()))?; + let normalized_seed = state + .normalized_value + .clone() .ok_or_else(|| SeedRegistryError::MissingTarget(target.clone()))?; Ok(SeedGuard { target: target.clone(), generation: state.generation, - normalized_seed: state - .normalized_value - .clone() - .expect("current seed checked above"), + normalized_seed, }) } diff --git a/src/session/snapshot/save/payload.rs b/src/session/snapshot/save/payload.rs index 3b7f57e8..d64389f3 100644 --- a/src/session/snapshot/save/payload.rs +++ b/src/session/snapshot/save/payload.rs @@ -50,10 +50,6 @@ impl PayloadCandidate { None } } - - pub(super) fn fits_expanded_limit(&self, max_expanded_size: u64) -> bool { - self.expanded_limit_exceeded(max_expanded_size).is_none() - } } pub(super) struct PreparedPayload { @@ -97,18 +93,15 @@ pub(super) fn payload_within_limit( let full_started = Instant::now(); let full_payload = payload_candidate(snapshot, options, last_modified)?; log_payload_candidate("full", &full_payload, full_started.elapsed()); - if full_payload.fits_limit(options, max_expanded_size) { + let Some(full_limit) = full_payload.limit_exceeded(options, max_expanded_size) else { return Ok(PreparedPayload::write( full_payload, SaveSnapshotOutcome::Full, )); - } + }; let full_raw_size = full_payload.raw_size; let full_final_size = full_payload.final_size(); - let full_limit = full_payload - .limit_exceeded(options, max_expanded_size) - .expect("full payload should exceed a save/load limit"); let visible_only = snapshot_without_history(snapshot); if visible_only.is_empty() && visible_only.tool_state.is_none() { warn!( @@ -127,10 +120,7 @@ pub(super) fn payload_within_limit( let visible_started = Instant::now(); let visible_payload = payload_candidate(&visible_only, options, last_modified)?; log_payload_candidate("visible-only", &visible_payload, visible_started.elapsed()); - if !visible_payload.fits_limit(options, max_expanded_size) { - let visible_limit = visible_payload - .limit_exceeded(options, max_expanded_size) - .expect("visible payload should exceed a save/load limit"); + if let Some(visible_limit) = visible_payload.limit_exceeded(options, max_expanded_size) { return Err(SavePayloadTooLarge { limit: visible_limit, written_size: visible_payload.final_size(), @@ -155,7 +145,16 @@ pub(super) fn payload_within_limit( depth_one_started.elapsed(), ); - if depth_one_payload.fits_limit(options, max_expanded_size) { + if let Some(depth_one_limit) = depth_one_payload.limit_exceeded(options, max_expanded_size) + { + warn!( + "Even one persisted history entry cannot be saved safely ({}; {} bytes written from {} raw bytes, compression={}); skipping history-depth scan and saving visible data only", + depth_one_limit.description(), + depth_one_payload.final_size(), + depth_one_payload.raw_size, + depth_one_payload.compressed + ); + } else { let fitting_history = if history_depth == 1 || visible_near_limit { if visible_near_limit && history_depth > 1 { warn!( @@ -194,17 +193,6 @@ pub(super) fn payload_within_limit( SaveSnapshotOutcome::TrimmedHistory { depth }, )); } - } else { - let depth_one_limit = depth_one_payload - .limit_exceeded(options, max_expanded_size) - .expect("depth-one payload should exceed a save/load limit"); - warn!( - "Even one persisted history entry cannot be saved safely ({}; {} bytes written from {} raw bytes, compression={}); skipping history-depth scan and saving visible data only", - depth_one_limit.description(), - depth_one_payload.final_size(), - depth_one_payload.raw_size, - depth_one_payload.compressed - ); } } diff --git a/src/session/snapshot/save/recovery.rs b/src/session/snapshot/save/recovery.rs index ef2e9a8a..f4dfefbb 100644 --- a/src/session/snapshot/save/recovery.rs +++ b/src/session/snapshot/save/recovery.rs @@ -65,13 +65,9 @@ fn recovery_payload( let full_started = Instant::now(); let full_payload = payload_candidate(snapshot, options, last_modified)?; log_payload_candidate("recovery full", &full_payload, full_started.elapsed()); - if full_payload.fits_expanded_limit(max_expanded_size) { + let Some(full_limit) = full_payload.expanded_limit_exceeded(max_expanded_size) else { return Ok(Some((full_payload, SaveSnapshotOutcome::Full))); - } - - let full_limit = full_payload - .expanded_limit_exceeded(max_expanded_size) - .expect("full recovery payload should exceed expanded limit"); + }; warn!( "Full session recovery payload cannot be saved safely ({}; {} bytes written from {} raw bytes, compression={}); trying visible data without undo/redo history", full_limit.description(), @@ -91,20 +87,16 @@ fn recovery_payload( &visible_payload, visible_started.elapsed(), ); - if visible_payload.fits_expanded_limit(max_expanded_size) { - return Ok(Some((visible_payload, SaveSnapshotOutcome::VisibleOnly))); + if let Some(visible_limit) = visible_payload.expanded_limit_exceeded(max_expanded_size) { + return Err(anyhow!( + "Session recovery data cannot be saved safely ({}; {} bytes written from {} raw bytes, compression={}); skipping recovery", + visible_limit.description(), + visible_payload.final_size(), + visible_payload.raw_size, + visible_payload.compressed + )); } - - let visible_limit = visible_payload - .expanded_limit_exceeded(max_expanded_size) - .expect("visible recovery payload should exceed expanded limit"); - Err(anyhow!( - "Session recovery data cannot be saved safely ({}; {} bytes written from {} raw bytes, compression={}); skipping recovery", - visible_limit.description(), - visible_payload.final_size(), - visible_payload.raw_size, - visible_payload.compressed - )) + Ok(Some((visible_payload, SaveSnapshotOutcome::VisibleOnly))) } pub(super) fn remove_session_file_after_clear_marker(session_path: &Path) { diff --git a/src/toolbar_gtk/bridge.rs b/src/toolbar_gtk/bridge.rs index 2fb05b2d..a9eb4d26 100644 --- a/src/toolbar_gtk/bridge.rs +++ b/src/toolbar_gtk/bridge.rs @@ -430,7 +430,10 @@ fn finish_thread_within( thread.take(); return ThreadShutdownOutcome::TimedOut; } - match thread.take().expect("thread checked above").join() { + let handle = thread + .take() + .expect("finished GTK thread handle is still owned here"); + match handle.join() { Ok(()) => ThreadShutdownOutcome::Joined, Err(_) => ThreadShutdownOutcome::Panicked, } diff --git a/src/toolbar_gtk/view/capture_suppression.rs b/src/toolbar_gtk/view/capture_suppression.rs index 1b190327..12bdf0b5 100644 --- a/src/toolbar_gtk/view/capture_suppression.rs +++ b/src/toolbar_gtk/view/capture_suppression.rs @@ -425,9 +425,11 @@ async fn wait_for_surface_presentation( gtk4::glib::timeout_future(CAPTURE_PAINT_POLL_INTERVAL).await; } - let frame_counter = rendered_counter - .get() - .expect("dedicated transparent render counter recorded"); + let Some(frame_counter) = rendered_counter.get() else { + return Err(format!( + "GTK capture suppression generation {generation} lost the dedicated transparent render from {name}" + )); + }; log::info!( "capture.preflight id={generation} component=gtk surface={name} phase=rendered frame_counter={frame_counter} sequence={ordinal}/{target_count} elapsed_ms={}", started.elapsed().as_millis() diff --git a/src/toolbar_gtk/view/top_bar.rs b/src/toolbar_gtk/view/top_bar.rs index 65bcadf2..897d6ca9 100644 --- a/src/toolbar_gtk/view/top_bar.rs +++ b/src/toolbar_gtk/view/top_bar.rs @@ -93,6 +93,28 @@ const END_MARGIN: (f64, f64) = (12.0, 0.0); use super::{CaptureProofTarget, CaptureSurfaceContent, Updater}; +fn control_face_button( + control: model::TopToolbarControl, + snapshot: &ToolbarSnapshot, + button_size: (f64, f64), + icon_size: f64, + use_icons: bool, + tooltip: &str, + label: &str, +) -> gtk4::Button { + if use_icons { + icon_button( + top_toolbar_icon_painter(control.glyph(snapshot)), + button_size, + icon_size, + tooltip, + ) + .button + } else { + text_button(label, button_size, tooltip) + } +} + /// Snapshot inputs the shapes-popover grid renders from: active tool, /// override, fill flag, polygon sides. type ShapesContentKey = (Tool, Option, bool, u8); diff --git a/src/toolbar_gtk/view/top_bar/controls.rs b/src/toolbar_gtk/view/top_bar/controls.rs index 77bfdd6d..ed1e7270 100644 --- a/src/toolbar_gtk/view/top_bar/controls.rs +++ b/src/toolbar_gtk/view/top_bar/controls.rs @@ -66,17 +66,15 @@ impl TopBar { assert_eq!(control, model::TopToolbarControl::ShapePicker); let tooltip = control.tooltip(snapshot); let label = control.label(snapshot); - let button = if use_icons { - icon_button( - top_toolbar_icon_painter(control.icon(snapshot).expect("shape-picker icon")), - button_size, - icon_size, - &tooltip, - ) - .button - } else { - text_button(&label, button_size, &tooltip) - }; + let button = control_face_button( + control, + snapshot, + button_size, + icon_size, + use_icons, + &tooltip, + &label, + ); set_control_widget_id(&button, control); let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); @@ -173,17 +171,15 @@ impl TopBar { ) -> gtk4::Button { let tooltip = control.tooltip(snapshot); let label = control.label(snapshot); - let button = if use_icons { - icon_button( - top_toolbar_icon_painter(control.icon(snapshot).expect("action icon")), - button_size, - icon_size, - &tooltip, - ) - .button - } else { - text_button(&label, button_size, &tooltip) - }; + let button = control_face_button( + control, + snapshot, + button_size, + icon_size, + use_icons, + &tooltip, + &label, + ); set_control_widget_id(&button, control); let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); @@ -331,10 +327,12 @@ impl TopBar { button.add_css_class("chrome"); let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); - let icon = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("pin icon")), - size * 0.62, - ); + let glyph = if snapshot.top_pinned { + model::TopToolbarIcon::Pin + } else { + model::TopToolbarIcon::Unpin + }; + let icon = IconWidget::new(top_toolbar_icon_painter(glyph), size * 0.62); button.set_child(Some(&icon.area)); let sender = self.feedback.clone(); let pinned = Rc::new(Cell::new(snapshot.top_pinned)); @@ -348,9 +346,12 @@ impl TopBar { let handle = button.clone(); self.updaters.borrow_mut().push(Box::new(move |snapshot| { pinned.set(snapshot.top_pinned); - icon.set_painter(top_toolbar_icon_painter( - control.icon(snapshot).expect("pin icon"), - )); + let glyph = if snapshot.top_pinned { + model::TopToolbarIcon::Pin + } else { + model::TopToolbarIcon::Unpin + }; + icon.set_painter(top_toolbar_icon_painter(glyph)); if snapshot.top_pinned { handle.add_css_class("pinned"); } else { @@ -379,7 +380,7 @@ impl TopBar { button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); button.set_tooltip_text(Some(&control.tooltip(snapshot))); let icon = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("overflow icon")), + top_toolbar_icon_painter(model::TopToolbarIcon::Overflow), icon_size, ); button.set_child(Some(&icon.area)); @@ -484,7 +485,7 @@ impl TopBar { button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); button.set_tooltip_text(Some(&control.tooltip(snapshot))); let icon = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("about icon")), + top_toolbar_icon_painter(model::TopToolbarIcon::About), size * 0.6, ); button.set_child(Some(&icon.area)); @@ -542,7 +543,7 @@ impl TopBar { button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); button.set_tooltip_text(Some(&control.tooltip(snapshot))); let icon = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("minimize icon")), + top_toolbar_icon_painter(model::TopToolbarIcon::Minimize), size * 0.6, ); button.set_child(Some(&icon.area)); diff --git a/src/toolbar_gtk/view/top_bar/popovers.rs b/src/toolbar_gtk/view/top_bar/popovers.rs index 37b1088d..b42a83d9 100644 --- a/src/toolbar_gtk/view/top_bar/popovers.rs +++ b/src/toolbar_gtk/view/top_bar/popovers.rs @@ -21,6 +21,19 @@ pub(super) fn set_popover_capture_transparent( set_popover_input_enabled(popover, input_enabled); } +fn paired_popover_surface<'a>( + popover: Option<&'a gtk4::Popover>, + surface: Option<&'a CaptureSurfaceContent>, +) -> Option<(&'a gtk4::Popover, &'a CaptureSurfaceContent)> { + if popover.is_none() && surface.is_none() { + return None; + } + Some(( + popover.expect("popover capture surface is paired at construction"), + surface.expect("capture surface is paired with its popover"), + )) +} + fn set_popover_input_enabled(popover: >k4::Popover, enabled: bool) { popover.set_can_target(enabled); if let Some(surface) = popover.surface() { @@ -84,7 +97,8 @@ impl TopBar { self.settings_capture_surface.as_ref(), ), ] { - let (Some(popover), Some(capture_surface)) = (popover, capture_surface) else { + let Some((popover, capture_surface)) = paired_popover_surface(popover, capture_surface) + else { continue; }; if transparent && !popover.is_visible() { @@ -138,8 +152,7 @@ impl TopBar { ] .into_iter() .filter_map(|(name, popover, capture_surface)| { - let popover = popover?; - let capture_surface = capture_surface?; + let (popover, capture_surface) = paired_popover_surface(popover, capture_surface)?; (popover.is_visible() && popover.is_mapped()) .then(|| CaptureProofTarget::new_withdrawable(name, popover, capture_surface)) }) @@ -176,7 +189,7 @@ impl TopBar { if self.shapes_content_key.get() != Some(content_key) { self.shapes_capture_surface .as_ref() - .expect("shapes popover capture surface") + .expect("shapes popover capture surface is paired") .set_content(&self.build_shapes_popover_content( snapshot, button_size, @@ -213,7 +226,7 @@ impl TopBar { let spec = model::TopToolbarSpec::build(snapshot, plan); self.overflow_capture_surface .as_ref() - .expect("overflow popover capture surface") + .expect("overflow popover capture surface is paired") .set_content(&self.build_overflow_popover_content( snapshot, &spec, @@ -250,7 +263,7 @@ impl TopBar { let (content, updaters) = self.build_canvas_popover_content(snapshot, scale); self.canvas_capture_surface .as_ref() - .expect("canvas popover capture surface") + .expect("canvas popover capture surface is paired") .set_content(&content); // Delay-slider values ride these persistent updaters (the // content key omits them), so a delay drag never rebuilds @@ -277,7 +290,7 @@ impl TopBar { if self.session_content_key.borrow().as_ref() != Some(&content_key) { self.session_capture_surface .as_ref() - .expect("session popover capture surface") + .expect("session popover capture surface is paired") .set_content(&self.build_session_popover_content(snapshot, scale)); *self.session_content_key.borrow_mut() = Some(content_key); } @@ -300,7 +313,7 @@ impl TopBar { let (content, updaters) = self.build_settings_popover_content(snapshot, scale); self.settings_capture_surface .as_ref() - .expect("settings popover capture surface") + .expect("settings popover capture surface is paired") .set_content(&content); self.settings_updaters = updaters; *self.settings_content_key.borrow_mut() = Some(content_key); @@ -440,17 +453,15 @@ impl TopBar { let control = model::TopToolbarControl::Tool(tool); let tooltip = control.tooltip(snapshot); let label = control.label(snapshot); - let button = if use_icons { - icon_button( - top_toolbar_icon_painter(control.icon(snapshot).expect("tool icon")), - button_size, - icon_size, - &tooltip, - ) - .button - } else { - text_button(&label, button_size, &tooltip) - }; + let button = control_face_button( + control, + snapshot, + button_size, + icon_size, + use_icons, + &tooltip, + &label, + ); set_prefixed_control_widget_id(&button, "top.picker.", control); let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); @@ -542,17 +553,15 @@ impl TopBar { for control in spec.overflow().iter().copied() { let tooltip = control.overflow_tooltip(snapshot); let label = control.label(snapshot); - let button = if use_icons { - icon_button( - top_toolbar_icon_painter(control.icon(snapshot).expect("overflow icon")), - button_size, - icon_size, - &tooltip, - ) - .button - } else { - text_button(&label, button_size, &tooltip) - }; + let button = control_face_button( + control, + snapshot, + button_size, + icon_size, + use_icons, + &tooltip, + &label, + ); set_prefixed_control_widget_id(&button, "top.overflow.", control); let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); diff --git a/src/toolbar_gtk/view/top_bar/strip.rs b/src/toolbar_gtk/view/top_bar/strip.rs index 1924e0c6..80f98fb8 100644 --- a/src/toolbar_gtk/view/top_bar/strip.rs +++ b/src/toolbar_gtk/view/top_bar/strip.rs @@ -41,7 +41,7 @@ impl TopBar { restore.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); restore.set_tooltip_text(Some(&control.tooltip(snapshot))); let icon = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("restore icon")), + top_toolbar_icon_painter(model::TopToolbarIcon::Restore), (MINIMIZED_SIZE.1 * 0.75 * scale).min(18.0 * scale), ); restore.set_child(Some(&icon.area)); @@ -80,7 +80,7 @@ impl TopBar { chip.set_tooltip_text(Some(&control.tooltip(snapshot))); let painter = Rc::new(Cell::new(crate::toolbar_icons::top_toolbar_icon_painter( - control.icon(snapshot).expect("micro chip tool icon"), + model::TopToolbarIcon::Tool(model::semantic_icon_for_tool(snapshot.active_tool)), ))); let ring: Rc> = Rc::new(Cell::new(( ( @@ -283,7 +283,7 @@ impl TopBar { model::TopToolbarNode::Control(control) => match control { model::TopToolbarControl::DragHandle => { let grip = IconWidget::new( - top_toolbar_icon_painter(control.icon(snapshot).expect("drag icon")), + top_toolbar_icon_painter(model::TopToolbarIcon::Drag), sz(HANDLE_SIZE), ); set_control_widget_id(&grip.area, control); diff --git a/src/toolbar_gtk/view/top_bar/style_pill.rs b/src/toolbar_gtk/view/top_bar/style_pill.rs index 2d5bbdaf..9149edc2 100644 --- a/src/toolbar_gtk/view/top_bar/style_pill.rs +++ b/src/toolbar_gtk/view/top_bar/style_pill.rs @@ -80,7 +80,7 @@ impl TopBar { chip.button .update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); let sender = self.feedback.clone(); - let event = control.event(snapshot).expect("color chip event"); + let event = control.click_event(snapshot); chip.button.connect_clicked(move |_| { send_event(&sender, event.clone()); }); @@ -104,7 +104,7 @@ impl TopBar { .button .update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); let sender = self.feedback.clone(); - let event = control.event(snapshot).expect("swatch event"); + let event = control.click_event(snapshot); swatch.button.connect_clicked(move |_| { send_event(&sender, event.clone()); }); @@ -123,7 +123,7 @@ impl TopBar { model::StylePillControl::ThicknessSlider | model::StylePillControl::OpacitySlider | model::StylePillControl::FontSizeSlider => { - let (slider_spec, value) = control.slider(snapshot).expect("slider control"); + let (slider_spec, value) = control.slider_value(snapshot); let format = match control { model::StylePillControl::ThicknessSlider => format_px as fn(f64) -> String, model::StylePillControl::OpacitySlider => format_percent, @@ -174,12 +174,12 @@ impl TopBar { // precise-entry popup (the shared event path, like the // color chip's overlay picker popup). let button = pill_button( - &control.value_text(snapshot).expect("numeral text"), + &control.required_value_text(snapshot), sz(STYLE_VALUE_W), sz(STYLE_ROW_H), ); let sender = self.feedback.clone(); - let event = control.event(snapshot).expect("numeral event"); + let event = control.click_event(snapshot); button.connect_clicked(move |_| { send_event(&sender, event.clone()); }); @@ -191,7 +191,7 @@ impl TopBar { button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); append_gap(&pill, button.upcast_ref(), gap); self.updaters.borrow_mut().push(Box::new(move |snapshot| { - button.set_label(&control.value_text(snapshot).expect("numeral text")); + button.set_label(&control.required_value_text(snapshot)); })); } model::StylePillControl::FillToggle | model::StylePillControl::AutoNumberToggle => { @@ -238,7 +238,7 @@ impl TopBar { button.set_tooltip_text(Some(&tooltip)); } let sender = self.feedback.clone(); - let event = control.event(snapshot).expect("reset event"); + let event = control.click_event(snapshot); button.connect_clicked(move |_| { send_event(&sender, event.clone()); }); @@ -253,7 +253,7 @@ impl TopBar { } model::StylePillControl::SelectionCycle(_) => { let button = pill_button( - &control.value_text(snapshot).unwrap_or_default(), + &control.required_value_text(snapshot), sz(STYLE_SEL_VALUE_W), sz(STYLE_ROW_H), ); @@ -265,13 +265,13 @@ impl TopBar { let accessible_label = control.label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); let sender = self.feedback.clone(); - let event = control.event(snapshot).expect("selection cycle event"); + let event = control.click_event(snapshot); button.connect_clicked(move |_| { send_event(&sender, event.clone()); }); append_gap(&pill, button.upcast_ref(), gap); self.updaters.borrow_mut().push(Box::new(move |snapshot| { - button.set_label(&control.value_text(snapshot).unwrap_or_default()); + button.set_label(&control.required_value_text(snapshot)); button.set_sensitive(control.enabled(snapshot)); button.set_tooltip_text(control.tooltip(snapshot).as_deref()); })); @@ -280,15 +280,14 @@ impl TopBar { let row = gtk4::Box::new(gtk4::Orientation::Horizontal, px(2.0)); set_semantic_widget_id(&row, control.id().as_ref()); row.set_valign(gtk4::Align::Center); - let steps = control.steps(snapshot).expect("stepper halves"); + let steps = control.required_steps(snapshot); let mut handles: Vec = Vec::new(); let minus = pill_button(steps[0].label, sz(STYLE_STEP_W), sz(STYLE_ROW_H)); set_semantic_widget_id(&minus, steps[0].id); minus.set_tooltip_text(Some(&steps[0].tooltip)); row.append(&minus); handles.push(minus.clone()); - let value = - gtk4::Label::new(Some(&control.value_text(snapshot).unwrap_or_default())); + let value = gtk4::Label::new(Some(&control.required_value_text(snapshot))); set_semantic_widget_id(&value, &format!("{}.value", control.id())); value.set_width_request(px(STYLE_SEL_VALUE_W)); row.append(&value); @@ -307,7 +306,7 @@ impl TopBar { } append_gap(&pill, row.upcast_ref(), gap); self.updaters.borrow_mut().push(Box::new(move |snapshot| { - value.set_label(&control.value_text(snapshot).unwrap_or_default()); + value.set_label(&control.required_value_text(snapshot)); let enabled = control.enabled(snapshot); for button in &handles { button.set_sensitive(enabled); @@ -322,7 +321,7 @@ impl TopBar { // A clear gap before the segment so Sans│Mono never crowd // the preceding numeral ("72pt") to its left (M7-C3). row.set_margin_start(px(STYLE_SEGMENT_LEAD)); - let segments = control.segments(snapshot).expect("segment halves"); + let segments = control.required_segments(snapshot); let mut handles: Vec<(gtk4::Button, &'static str)> = Vec::new(); for segment in &segments { let button = pill_button(segment.label, -1.0, sz(STYLE_TAB_H)); @@ -342,9 +341,7 @@ impl TopBar { } append_gap(&pill, row.upcast_ref(), gap); self.updaters.borrow_mut().push(Box::new(move |snapshot| { - let Some(segments) = control.segments(snapshot) else { - return; - }; + let segments = control.required_segments(snapshot); for (button, id) in &handles { let active = segments .iter() diff --git a/src/ui/toolbar/model/event_policy.rs b/src/ui/toolbar/model/event_policy.rs index 4e71debe..2c561ae3 100644 --- a/src/ui/toolbar/model/event_policy.rs +++ b/src/ui/toolbar/model/event_policy.rs @@ -414,10 +414,19 @@ fn persistence_for_event(event: &ToolbarEvent) -> ToolbarPersistence { } // A section row in the customization list is that section's // visibility, not an individual item override. - ToolbarEvent::SetToolbarItemHidden(id, _) if section_flag_for_item(*id).is_some() => { - ToolbarPersistence::RuntimeUi(Runtime::NamedSection( - section_flag_for_item(*id).expect("guarded above"), - )) + ToolbarEvent::SetToolbarItemHidden(id, hidden) => { + if let Some(flag) = section_flag_for_item(*id) { + ToolbarPersistence::RuntimeUi(Runtime::NamedSection(flag)) + } else { + ToolbarPersistence::RuntimeUi(Runtime::ItemVisibility { + id: *id, + setting: if *hidden { + ToolbarItemVisibilitySetting::Hidden + } else { + ToolbarItemVisibilitySetting::Default + }, + }) + } } ToolbarEvent::ToggleStatusBar(_) => ToolbarPersistence::RuntimeUi(Runtime::StatusBar), ToolbarEvent::ToggleStatusBoardBadge(_) => { @@ -455,18 +464,6 @@ fn persistence_for_event(event: &ToolbarEvent) -> ToolbarPersistence { ToolbarEvent::SetStatusBarItemVisible(item, _) => { ToolbarPersistence::RuntimeUi(Runtime::StatusBarItem(*item)) } - // Section rows were routed to `NamedSection` above; every other item - // keeps its own visibility override. - ToolbarEvent::SetToolbarItemHidden(id, hidden) => { - ToolbarPersistence::RuntimeUi(Runtime::ItemVisibility { - id: *id, - setting: if *hidden { - ToolbarItemVisibilitySetting::Hidden - } else { - ToolbarItemVisibilitySetting::Default - }, - }) - } ToolbarEvent::MoveToolbarItem { group, .. } | ToolbarEvent::StartToolbarItemDrag { group, .. } | ToolbarEvent::ResetToolbarItemOrder(group) => { @@ -630,6 +627,42 @@ mod tests { None ); } + + #[test] + fn hiding_a_section_row_persists_as_section_visibility() { + let id = ToolbarSectionFlag::Actions.item_id(); + assert_eq!( + persistence_for_event(&ToolbarEvent::SetToolbarItemHidden(id, true)), + ToolbarPersistence::RuntimeUi(ToolbarRuntimeUiPersistenceTarget::NamedSection( + ToolbarSectionFlag::Actions + )), + ); + assert_eq!( + persistence_for_event(&ToolbarEvent::SetToolbarItemHidden(id, false)), + ToolbarPersistence::RuntimeUi(ToolbarRuntimeUiPersistenceTarget::NamedSection( + ToolbarSectionFlag::Actions + )), + ); + } + + #[test] + fn hiding_a_tool_item_persists_as_item_visibility() { + let id = "top.tool.pen".parse::().expect("item id"); + assert_eq!( + persistence_for_event(&ToolbarEvent::SetToolbarItemHidden(id, true)), + ToolbarPersistence::RuntimeUi(ToolbarRuntimeUiPersistenceTarget::ItemVisibility { + id, + setting: ToolbarItemVisibilitySetting::Hidden, + }), + ); + assert_eq!( + persistence_for_event(&ToolbarEvent::SetToolbarItemHidden(id, false)), + ToolbarPersistence::RuntimeUi(ToolbarRuntimeUiPersistenceTarget::ItemVisibility { + id, + setting: ToolbarItemVisibilitySetting::Default, + }), + ); + } } #[cfg(test)] diff --git a/src/ui/toolbar/model/style_pill/control.rs b/src/ui/toolbar/model/style_pill/control.rs index e8347951..7bc33022 100644 --- a/src/ui/toolbar/model/style_pill/control.rs +++ b/src/ui/toolbar/model/style_pill/control.rs @@ -84,6 +84,13 @@ impl StylePillControl { }) } + /// Primary click for controls whose event lives on the control itself. + /// Segmented controls and steppers keep events on their halves. + pub(crate) fn click_event(self, snapshot: &ToolbarSnapshot) -> ToolbarEvent { + self.event(snapshot) + .expect("this style-pill control has a primary click event") + } + pub(crate) fn enabled(self, snapshot: &ToolbarSnapshot) -> bool { match self { // Locked/mixed-locked entries surface as disabled controls, @@ -119,6 +126,13 @@ impl StylePillControl { } } + /// Slider range plus current value. Callers that already matched a slider + /// variant use this instead of skipping the control. + pub(crate) fn slider_value(self, snapshot: &ToolbarSnapshot) -> (ToolbarSliderSpec, f64) { + self.slider(snapshot) + .expect("this style-pill control is a slider") + } + /// Live readout for sliders and their numeral buttons. The unit follows /// the tool context: px for thickness/size targets (the snapshot /// already routes eraser/marker sizes through `thickness`), pt for @@ -139,6 +153,13 @@ impl StylePillControl { } } + /// Live readout for a value-bearing control already matched by the + /// renderer. Selection entries in the spec always have a snapshot row. + pub(crate) fn required_value_text(self, snapshot: &ToolbarSnapshot) -> String { + self.value_text(snapshot) + .expect("this style-pill control has a live value") + } + pub(crate) fn label(self, snapshot: &ToolbarSnapshot) -> Cow<'static, str> { match self { Self::ColorChip => Cow::Borrowed("Color picker"), @@ -152,10 +173,8 @@ impl StylePillControl { } Self::OpacitySlider => Cow::Borrowed("Marker opacity"), Self::FontSizeSlider => Cow::Borrowed("Text size"), - Self::ThicknessValue | Self::FontSizeValue => Cow::Owned( - self.value_text(snapshot) - .expect("numeral controls have a value text"), - ), + Self::ThicknessValue => Cow::Owned(format!("{:.0}px", snapshot.thickness)), + Self::FontSizeValue => Cow::Owned(format!("{:.0}pt", snapshot.font_size)), Self::FillToggle => Cow::Borrowed(action_short_label(Action::ToggleFill)), Self::AutoNumberToggle => Cow::Borrowed("Auto-number"), Self::CounterReset(_) => Cow::Borrowed("Reset"), @@ -305,4 +324,16 @@ impl StylePillControl { }, ]) } + + /// Segment halves for a control already matched as segmented. + pub(crate) fn required_segments(self, snapshot: &ToolbarSnapshot) -> [StylePillSegment; 2] { + self.segments(snapshot) + .expect("this style-pill control is segmented") + } + + /// −/+ halves for a selection stepper already present in the spec. + pub(crate) fn required_steps(self, snapshot: &ToolbarSnapshot) -> [StylePillStep; 2] { + self.steps(snapshot) + .expect("this style-pill stepper has minus/plus halves") + } } diff --git a/src/ui/toolbar/model/top_spec/control.rs b/src/ui/toolbar/model/top_spec/control.rs index 6a088e6e..276ac11d 100644 --- a/src/ui/toolbar/model/top_spec/control.rs +++ b/src/ui/toolbar/model/top_spec/control.rs @@ -294,6 +294,14 @@ impl TopToolbarControl { }) } + /// Glyph for controls that always have one. Empty presets and the + /// highlight ring stay on [`Self::icon`]. + pub(crate) fn glyph(self, snapshot: &ToolbarSnapshot) -> TopToolbarIcon { + self.icon(snapshot).expect( + "this toolbar control always has a glyph; empty presets and the highlight ring use icon()", + ) + } + pub(crate) fn label(self, snapshot: &ToolbarSnapshot) -> Cow<'static, str> { match self { Self::Restore => Cow::Borrowed("Show toolbar"), diff --git a/src/ui/toolbar/model/top_spec/tests/behavior.rs b/src/ui/toolbar/model/top_spec/tests/behavior.rs index 80afce8b..c0363adc 100644 --- a/src/ui/toolbar/model/top_spec/tests/behavior.rs +++ b/src/ui/toolbar/model/top_spec/tests/behavior.rs @@ -202,6 +202,39 @@ fn layout_cycle_control_maps_each_mode_to_its_next_event_and_current_icon() { assert_eq!(control.accessible_label(&snapshot), "Cycle toolbar layout"); } +#[test] +fn required_chrome_and_tool_controls_have_a_glyph() { + let snapshot = snapshot(); + for control in [ + TopToolbarControl::Restore, + TopToolbarControl::DragHandle, + TopToolbarControl::Pin, + TopToolbarControl::Overflow, + TopToolbarControl::About, + TopToolbarControl::Minimize, + TopToolbarControl::LayoutMode, + TopToolbarControl::Undo, + TopToolbarControl::Redo, + TopToolbarControl::ShapePicker, + ] { + assert_eq!( + Some(control.glyph(&snapshot)), + control.icon(&snapshot), + "{control:?} glyph matches the optional icon" + ); + } + let mut unpinned = snapshot.clone(); + unpinned.top_pinned = false; + assert_eq!( + TopToolbarControl::Pin.glyph(&unpinned), + TopToolbarIcon::Unpin + ); + let mut pinned = snapshot.clone(); + pinned.top_pinned = true; + assert_eq!(TopToolbarControl::Pin.glyph(&pinned), TopToolbarIcon::Pin); + assert_eq!(TopToolbarControl::HighlightRing.icon(&snapshot), None); +} + /// The tooltip names the current mode and where the click lands, for all /// three presets. #[test]