From 1f805378d454dac48b5c214029d254055fdf961b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 05:35:20 -0400 Subject: [PATCH 01/29] feat(window): shared craft-window crate for consistent window decorations (#217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(window): shared crate deciding who draws the window frame Add craft-window, which decides whether the desktop or the app draws the window frame (titlebar, borders, window buttons) and applies it to an egui ViewportBuilder. This fixes the drift where some Craft apps drew their own titlebar on KDE while others kept KWin's. The rule: macOS, Windows and X11 keep the desktop frame; a Wayland compositor with zxdg_decoration_manager_v1 (KDE, Sway, Hyprland, COSMIC) keeps its frame; one without it (GNOME) leaves the frame to the app. _TITLEBAR=app|system|desktop overrides. When the app draws the frame, the decision carries the window-button Layout from GNOME's button-layout so the app's header bar matches the desktop's order. Decision logic only; drawing the buttons stays in each app. Extracted and generalized from pdfcraft's decorations.rs. forbid(unsafe_code); default build has no egui dependency (egui feature adds decorate()). * style(window): rustfmt the Wayland probe and Dispatch impl cargo fmt --check failed CI: collapse the thread-spawn builder chain and the Dispatch::event signature onto the lines rustfmt wants (verified with rustfmt 1.98 against the workspace rustfmt.toml). * fix(window): address review — WAYLAND_SOCKET, bounded gsettings, checkout creds - Skip the Wayland probe when WAYLAND_SOCKET is set: connect_to_env would take ownership of the inherited fd and unset the variable, closing the descriptor winit needs. Keep the desktop frame instead. - Run the gsettings button-layout lookup behind the same 500 ms bounded worker as the Wayland probe, so a stalled gsettings can't block startup. Factored the worker into a shared bounded() helper. - window.yml: set persist-credentials: false on checkout so the token isn't left in .git/config while cargo runs dependency build scripts. - README: document the socket guard and the bounded gsettings lookup. Verified with rustfmt 1.98 and clippy -D warnings (both feature sets). * fix(window): reap the gsettings child on timeout; document probe tradeoffs Follow-up to review: the button-layout lookup now spawns gsettings with piped output, reads it on a worker, and kills+reaps the child whether it finishes or times out — so a stalled gsettings leaves neither a blocked process nor a parked worker (it is a one-shot startup call). Also document, on the Wayland probe, that an inherited WAYLAND_SOCKET (rare; normal launches get WAYLAND_DISPLAY) deliberately falls back to the desktop frame to protect winit's connection — the same frame an app that doesn't probe would get, a safe default rather than a regression. Verified: rustfmt 1.98 clean, clippy -D warnings clean (default + egui), tests pass. --- .github/workflows/window.yml | 36 +++ Cargo.lock | 8 + Cargo.toml | 1 + crates/window/Cargo.toml | 22 ++ crates/window/README.md | 70 ++++++ crates/window/src/lib.rs | 439 +++++++++++++++++++++++++++++++++++ 6 files changed, 576 insertions(+) create mode 100644 .github/workflows/window.yml create mode 100644 crates/window/Cargo.toml create mode 100644 crates/window/README.md create mode 100644 crates/window/src/lib.rs diff --git a/.github/workflows/window.yml b/.github/workflows/window.yml new file mode 100644 index 00000000..61407086 --- /dev/null +++ b/.github/workflows/window.yml @@ -0,0 +1,36 @@ +name: Window decorations + +on: + workflow_dispatch: + push: + branches: [main] + paths: ['crates/window/**', 'Cargo.toml', 'Cargo.lock', '.github/workflows/window.yml'] + pull_request: + paths: ['crates/window/**', 'Cargo.toml', 'Cargo.lock', '.github/workflows/window.yml'] + +permissions: + contents: read + +jobs: + window: + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + with: + # The token isn't needed after checkout; cargo runs build scripts and proc macros from + # dependencies, so don't leave it in .git/config for that third-party code to read. + persist-credentials: false + - uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + - run: cargo fmt --check + # Default (no egui) and the egui feature both build and lint cleanly. + - run: cargo clippy -p craft-window --all-targets --locked -- -D warnings + - run: cargo clippy -p craft-window --all-targets --locked --features egui -- -D warnings + - run: cargo test -p craft-window --locked -- --nocapture + - run: cargo test -p craft-window --locked --features egui -- --nocapture diff --git a/Cargo.lock b/Cargo.lock index 4d9de428..4c7cda04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -732,6 +732,14 @@ dependencies = [ "serde_json", ] +[[package]] +name = "craft-window" +version = "0.1.0" +dependencies = [ + "egui", + "wayland-client", +] + [[package]] name = "craft-winprint-gdi" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index ca53f61a..319c1138 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ craft-print = { path = "crates/print" } craft-quit = { path = "crates/quit" } craft-settings = { path = "crates/settings" } craft-ui = { path = "crates/ui" } +craft-window = { path = "crates/window" } egui = { version = "0.36", default-features = false } rayon = "1" serde = { version = "1", features = ["derive"] } diff --git a/crates/window/Cargo.toml b/crates/window/Cargo.toml new file mode 100644 index 00000000..54db833d --- /dev/null +++ b/crates/window/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "craft-window" +description = "Decide who draws the window frame on Linux (the compositor or the app) and apply it, so every Craft app gets the same native titlebar and borders per desktop." +version.workspace = true +edition.workspace = true +license.workspace = true +rust-version.workspace = true +repository.workspace = true + +[features] +# `apply`/`decorations`: decide decorations and set them on an `egui::ViewportBuilder`. +egui = ["dep:egui"] + +[dependencies] +egui = { workspace = true, optional = true } + +# Only Linux and the BSDs probe a Wayland compositor; everywhere else the frame is the OS's. +[target.'cfg(any(target_os = "linux", target_os = "freebsd", target_os = "openbsd", target_os = "netbsd"))'.dependencies] +wayland-client = "0.31" + +[lints] +workspace = true diff --git a/crates/window/README.md b/crates/window/README.md new file mode 100644 index 00000000..75b4c681 --- /dev/null +++ b/crates/window/README.md @@ -0,0 +1,70 @@ +craft-window +============ + +Decide who draws the window frame — the desktop or the app — so every Craft app shows the same +titlebar and borders on a given desktop. + +## Why + +egui apps either keep the desktop's window frame (titlebar, borders, window buttons) or turn it off +and draw their own. Across the suite this had drifted: + +- some apps always drew their own frame on Linux (so KDE's titlebar was replaced by the app's), +- some always kept the desktop's, +- some probed the compositor and only drew their own on GNOME. + +So two Craft apps open side by side on KDE could wear different titlebars. This crate makes the +decision in one place, matching each desktop. + +## The rule + +| Platform | Frame | +| --- | --- | +| macOS, Windows | desktop (the OS) | +| Linux/BSD on X11 | desktop (the window manager) | +| Linux/BSD on Wayland **with** `zxdg_decoration_manager_v1` (KDE, Sway, Hyprland, COSMIC) | desktop (the compositor) | +| Linux/BSD on Wayland **without** it (GNOME's Mutter) | the app | + +When the app draws the frame, the decision carries the window-button `Layout` the desktop names +(GNOME's `button-layout`), so the app's header bar puts minimize/maximize/close where that desktop +puts them. This crate decides *whether* and *where*; drawing the buttons stays in each app, whose +title bars differ. + +### Override + +`_TITLEBAR` forces the choice. With the default prefix `CRAFT`: `CRAFT_TITLEBAR=app` forces +the app frame, `=system` (or `=desktop`) forces the desktop frame. Apps pass their own prefix +(`Prefix::new("PDFCRAFT")` → `PDFCRAFT_TITLEBAR`) so each keeps its own variable name. + +## Use + +```rust,ignore +use craft_window::{Choice, Prefix, decorate}; + +// Once at startup, before the window is created: +let choice = Choice::probe(Prefix::new("PHOTOCRAFT")); +let viewport = decorate(egui::ViewportBuilder::default().with_title("PhotoCraft"), choice.frame()); + +// If the app draws the frame, lay the header-bar buttons out by the desktop's order: +if let Some(layout) = choice.frame().layout() { + // layout.left / layout.right -> draw Minimize/Maximize/Close there +} +``` + +An app that offers a "System title bar" preference checks `choice.overridden()` and its own setting, +and can override the probe by setting the frame to `Frame::Desktop`. + +The `egui` feature adds [`decorate`]. Without it, the crate is pure decision logic (`Choice`, +`Frame`, `Layout`) with no egui dependency. + +## Notes + +- The Wayland probe opens a short-lived connection and reads the global registry once, on its own + thread with a 500 ms timeout; on failure or timeout it keeps the desktop's frame. Call it once at + startup. +- It never touches the connection winit inherits: when `WAYLAND_SOCKET` is set (a compositor handed + the app an open descriptor), the probe skips connecting and keeps the desktop frame, so it can't + consume the socket winit needs. +- The GNOME `button-layout` lookup (`gsettings`) runs behind the same bounded worker, so a stalled + `gsettings` can't block startup either. +- `#![forbid(unsafe_code)]`, and the never-crash lints the workspace requires. diff --git a/crates/window/src/lib.rs b/crates/window/src/lib.rs new file mode 100644 index 00000000..42f936c5 --- /dev/null +++ b/crates/window/src/lib.rs @@ -0,0 +1,439 @@ +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +#![forbid(unsafe_code)] + +//! Decide who draws the window frame, so every Craft app shows the same titlebar and borders on a +//! given desktop. +//! +//! egui apps either let the desktop draw the window frame (the title bar, borders and window +//! buttons) or turn it off and draw their own. Across the suite this decision had drifted: some +//! apps always drew their own frame on Linux, some always kept the desktop's, and some probed the +//! compositor — so on KDE, say, one app wore KWin's title bar while another wore its own. This +//! crate makes the decision one place. +//! +//! The rule, by platform: +//! +//! - **macOS and Windows:** the OS always draws the frame ([`Frame::Desktop`]). (An app that insets +//! its content under the traffic lights, or draws its own caption buttons on Windows, still asks +//! the OS for a decorated window and overdraws it; that is the app's choice and not this crate's.) +//! - **Linux and the BSDs, X11:** the window manager draws the frame ([`Frame::Desktop`]). +//! - **Linux and the BSDs, Wayland:** a compositor that offers server-side decorations +//! (`zxdg_decoration_manager_v1` — KDE, Sway, Hyprland, COSMIC) draws the frame +//! ([`Frame::Desktop`]). One that does not (GNOME's Mutter) leaves the frame to the app +//! ([`Frame::App`]): without this, winit draws an imitation frame that matches no desktop. +//! +//! An environment variable overrides the choice, for users whose compositor the probe can't read +//! or who simply prefer the other frame. With the default prefix `CRAFT`, `CRAFT_TITLEBAR=app` +//! forces [`Frame::App`] and `=system` (or `=desktop`) forces [`Frame::Desktop`]; apps may pass +//! their own prefix (`PDFCRAFT`, `FIGCRAFT`, …) so each keeps its own variable name. +//! +//! When the app draws the frame, [`Frame::App`] carries the [`Layout`] of window buttons the +//! desktop names (GNOME's `button-layout`), so the app's header bar puts minimize, maximize and +//! close where that desktop puts them. This crate decides *whether* and *where*; drawing the +//! buttons stays with each app (their title bars differ). +//! +//! # With egui +//! +//! The `egui` feature adds [`decorate`], which applies the decision to an +//! [`egui::ViewportBuilder`] before the window is created: +//! +//! ```ignore +//! let viewport = egui::ViewportBuilder::default().with_title("PhotoCraft"); +//! // The app may still prefer its own frame when the user asked for it in preferences: +//! let choice = craft_window::Choice::probe(craft_window::Prefix::default()); +//! let viewport = craft_window::decorate(viewport, choice.frame()); +//! ``` +//! +//! The probe opens a short-lived Wayland connection and reads the global registry once; it never +//! touches the connection winit inherits. Call it once at startup. + +/// The environment-variable prefix an app overrides the frame with: `_TITLEBAR`. +/// +/// `Prefix::default()` is `CRAFT`, so `CRAFT_TITLEBAR`. An app that wants its own name passes +/// [`Prefix::new`] (`Prefix::new("PDFCRAFT")` reads `PDFCRAFT_TITLEBAR`). The name is used as +/// given; it should be ASCII letters, digits and underscores, as environment names are. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Prefix(&'static str); + +impl Default for Prefix { + fn default() -> Self { + Self("CRAFT") + } +} + +impl Prefix { + /// An app-specific prefix, so the override variable is `_TITLEBAR`. + pub const fn new(prefix: &'static str) -> Self { + Self(prefix) + } + + /// The environment variable this prefix reads, e.g. `CRAFT_TITLEBAR`. + pub fn var_name(self) -> String { + format!("{}_TITLEBAR", self.0) + } +} + +/// Who draws the window frame. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Frame { + /// The desktop draws it: the OS on macOS and Windows, the window manager on X11, the + /// compositor on a Wayland session with server-side decorations. The app leaves decorations on. + Desktop, + /// The app draws it (GNOME's Wayland session, or an override). The app turns decorations off + /// and draws a header bar with the window buttons in this [`Layout`]. + App(Layout), +} + +impl Frame { + /// Whether the app should keep the OS/compositor decorations on (`true` for [`Frame::Desktop`]). + /// This is the value to pass to `egui::ViewportBuilder::with_decorations`. + pub fn decorations(&self) -> bool { + matches!(self, Frame::Desktop) + } + + /// The window-button layout when the app draws the frame, else `None`. + pub fn layout(&self) -> Option<&Layout> { + match self { + Frame::App(layout) => Some(layout), + Frame::Desktop => None, + } + } +} + +/// How the frame was decided, so an app can log it or offer a "System title bar" preference that +/// knows the current state. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Choice { + frame: Frame, + /// `Some(true)` when `_TITLEBAR` forced the app frame, `Some(false)` when it forced the + /// desktop frame, `None` when the platform decided. + overridden: Option, +} + +impl Choice { + /// Probe the running platform and desktop and decide who draws the frame. + /// + /// `prefix` names the override variable (`_TITLEBAR`). On macOS, Windows and X11 this + /// does no I/O; on a Wayland session it opens a short-lived connection to read the registry. + pub fn probe(prefix: Prefix) -> Self { + match override_frame(&prefix.var_name()) { + Some(true) => Self { frame: Frame::App(Layout::from_desktop()), overridden: Some(true) }, + Some(false) => Self { frame: Frame::Desktop, overridden: Some(false) }, + None if app_draws_frame() => Self { frame: Frame::App(Layout::from_desktop()), overridden: None }, + None => Self { frame: Frame::Desktop, overridden: None }, + } + } + + /// The decided frame. + pub fn frame(&self) -> &Frame { + &self.frame + } + + /// The decided frame, consumed. + pub fn into_frame(self) -> Frame { + self.frame + } + + /// Whether `_TITLEBAR` set the frame: `Some(true)` it forced the app's, `Some(false)` + /// the desktop's, `None` the platform decided. + pub fn overridden(&self) -> Option { + self.overridden + } +} + +/// Read ``: `app` → force the app frame, `system`/`desktop` → force the desktop frame, +/// anything else (or unset) → let the platform decide. +fn override_frame(var: &str) -> Option { + parse_override(std::env::var(var).ok().as_deref()) +} + +/// The override a `_TITLEBAR` value asks for: `Some(true)` the app frame, `Some(false)` the +/// desktop frame, `None` to let the platform decide. Pure, so it is tested without the environment. +fn parse_override(value: Option<&str>) -> Option { + match value.map(str::trim) { + Some("app") => Some(true), + Some("system" | "desktop") => Some(false), + _ => None, + } +} + +/// A window button. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Button { + Minimize, + Maximize, + Close, +} + +/// Which window buttons go on which end of the app-drawn header bar, outermost last. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Layout { + pub left: Vec