diff --git a/src/app.rs b/src/app.rs index 2618b10..82f50e1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,6 +3,7 @@ use cosmic::iced::{Color, Font, Limits, Rectangle, Subscription, window}; use cosmic::iced::alignment::{Horizontal, Vertical}; use cosmic::iced::platform_specific::shell::wayland::commands::popup::{destroy_popup, get_popup}; +use cosmic::iced::widget::text::LineHeight; use cosmic::widget::{self, autosize, container, rectangle_tracker::{ RectangleTracker, RectangleUpdate, rectangle_tracker_subscription, }}; @@ -25,6 +26,44 @@ const NM_DEBOUNCE_SECS: u64 = 5; /// Delay after NM reports connected before we fetch, to let DNS/routing settle. const NM_SETTLE_DELAY_SECS: u64 = 2; +// --------------------------------------------------------------------------- +// Popup geometry +// +// The popup surface must have a size that is *constant* across every state it +// can render (loading placeholder, loaded forecast, error message), and both +// `toggle_popup` and `view_window` must agree on that size exactly. +// +// Violating either invariant previously killed the applet: the compositor was +// told the surface could be up to 3206px wide (`toggle_popup` measured the +// forecast in *bytes* via `str::len`, and the box-drawing characters are 3 +// bytes each) while `view_window` laid out 1106px (measuring in *chars*). The +// popup then also resized when the async fetch replaced "Loading…" with the +// much wider table. Either way the surface attached a buffer before acking the +// initial `configure`, and the compositor terminated us with: +// +// xdg_surface#83: error 3: must ack the initial configure before attaching buffer +// +// Everything below is therefore derived from fixed constants and never from the +// text currently being displayed. +// --------------------------------------------------------------------------- + +/// Column count of the wttr.in `?A&T` forecast table. This layout is a +/// fixed-width grid, so it does not vary with the reported conditions. +const FORECAST_COLUMNS: f32 = 125.0; +/// Line count allotted to the forecast report. The report itself is 38 lines; +/// the surplus is headroom so a slightly taller report cannot force a resize. +const FORECAST_LINES: f32 = 40.0; +/// Font size used for the popup's monospace report. +const MONO_FONT_SIZE: f32 = 14.0; +/// Line height used for the popup's monospace report. Set explicitly rather +/// than left to the theme so the popup's height is fully determined by these +/// constants instead of by font metrics we would have to guess at. +const MONO_LINE_HEIGHT: f32 = 18.0; +/// Approximate advance width of one monospace character at `MONO_FONT_SIZE`. +const MONO_CHAR_WIDTH: f32 = 8.4; +/// Padding between the popup's border and its text. +const POPUP_PADDING: f32 = 12.0; + /// D-Bus proxy for monitoring `NetworkManager` connectivity changes. #[zbus::proxy( interface = "org.freedesktop.NetworkManager", @@ -193,25 +232,25 @@ impl cosmic::Application for AppModel { "Loading…".to_string() }; - // Calculate the width needed for the monospace content. - // The default monospace font is ~8.4px per character at 14px font size, - // plus padding for container (12*2) and popup chrome (~32). - #[allow(clippy::cast_precision_loss)] - let max_line_len = display_text.lines() - .map(|line| line.chars().count()) - .max() - .unwrap_or(40) as f32; - let estimated_char_width = 8.4; - let content_width = max_line_len * estimated_char_width + 56.0; - let popup_width = content_width.max(400.0); + // Pad every state out to a full-height report so that the popup's + // measured size does not change when the fetch resolves. + let display_text = pad_to_forecast_height(&display_text); + let (content_width, content_height) = popup_content_size(); + let (surface_width, surface_height) = popup_surface_size(); + + // Fixing both axes means the content measures identically no matter + // which of the three states above is being rendered. let content = container( widget::text(display_text) .font(Font::MONOSPACE) + .size(MONO_FONT_SIZE) + .line_height(LineHeight::Absolute(MONO_LINE_HEIGHT.into())) .wrapping(cosmic::iced::widget::text::Wrapping::None) .width(cosmic::iced::Length::Fixed(content_width)) + .height(cosmic::iced::Length::Fixed(content_height)), ) - .padding(12); + .padding(POPUP_PADDING); // Build our own popup container instead of using // self.core.applet.popup_container() which hardcodes max_width(360). @@ -248,11 +287,12 @@ impl cosmic::Application for AppModel { cosmic::widget::Id::new("weather-popup-autosize"), ) .limits( + // Must match the positioner size limits set in `toggle_popup`. Limits::NONE .min_width(1.0) .min_height(1.0) - .max_width(popup_width) - .max_height(1080.0), + .max_width(surface_width) + .max_height(surface_height), ) .into() } @@ -460,26 +500,16 @@ impl AppModel { popup_settings.positioner.size = None; - // Compute popup width from the weather content. - // ~8.4px per monospace character at default font size, - // plus padding for container (12*2) and popup chrome (~32). - #[allow(clippy::cast_precision_loss)] - let max_line_len = self.full_weather.lines() - .map(str::len) - .max() - .unwrap_or(0) as f32; - // Use content-based width if available, otherwise a - // reasonable default for the wttr.in 4-column table. - let popup_width = if max_line_len > 0.0 { - (max_line_len * 8.4 + 56.0).max(400.0) - } else { - 1100.0 - }; + // Report the same constant geometry that `view_window` lays out. This + // previously measured `self.full_weather` in bytes via `str::len()`, + // which triple-counts the table's 3-byte box-drawing characters and so + // disagreed with the renderer by ~2100px. + let (surface_width, surface_height) = popup_surface_size(); popup_settings.positioner.size_limits = Limits::NONE .min_width(1.0) .min_height(1.0) - .max_width(popup_width) - .max_height(1080.0); + .max_width(surface_width) + .max_height(surface_height); if self.full_weather.is_empty() { let units = self.units; @@ -595,3 +625,46 @@ fn units_suffix(units: Option) -> String { None => String::new(), } } + +/// Size of the popup's text area, in logical pixels. +/// +/// Deliberately takes no arguments: the popup is always sized for a full +/// forecast table regardless of what is currently loaded. See the "Popup +/// geometry" comment at the top of this module for why this must be constant. +fn popup_content_size() -> (f32, f32) { + ( + // One column of slack so a slightly wider glyph advance than + // `MONO_CHAR_WIDTH` cannot clip the table's right-hand border. + (FORECAST_COLUMNS + 1.0) * MONO_CHAR_WIDTH, + FORECAST_LINES * MONO_LINE_HEIGHT, + ) +} + +/// Outer size of the popup surface, including `POPUP_PADDING` on all sides. +/// +/// This is the single value both `toggle_popup` (which reports it to the +/// compositor as the positioner's size limits) and `view_window` (which uses it +/// as the autosize limits) must agree on. +fn popup_surface_size() -> (f32, f32) { + let (width, height) = popup_content_size(); + ( + width + POPUP_PADDING * 2.0, + height + POPUP_PADDING * 2.0, + ) +} + +/// Pad `text` to exactly `FORECAST_LINES` lines. +/// +/// This keeps the "Loading…" placeholder and any error message occupying the +/// same vertical space as a fully loaded forecast, so the popup does not resize +/// when the in-flight request resolves. Longer text is left intact; the fixed +/// height of the content container clips it rather than growing the surface. +fn pad_to_forecast_height(text: &str) -> String { + #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] + let target = FORECAST_LINES as usize; + let mut padded = text.to_string(); + for _ in text.lines().count()..target { + padded.push('\n'); + } + padded +}