From 153cddf5ea8d633b8d2cd920ed03353e82e7f967 Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Fri, 9 Oct 2026 11:42:44 +0200 Subject: [PATCH 1/2] Allow output info to view --- ggsql-cli/src/main.rs | 89 ++++++++++++++++++++++++++++----- src/writer/hephaestus/mod.rs | 2 +- src/writer/hephaestus/window.rs | 29 +++++++++-- src/writer/mod.rs | 2 +- 4 files changed, 103 insertions(+), 19 deletions(-) diff --git a/ggsql-cli/src/main.rs b/ggsql-cli/src/main.rs index cf3c76354..2fd0e5997 100644 --- a/ggsql-cli/src/main.rs +++ b/ggsql-cli/src/main.rs @@ -85,12 +85,14 @@ pub struct RenderArgs { pub verbose: bool, } -/// The flags `view` takes: where the data comes from and how the window looks. +/// The flags `view` takes: where the data comes from, how the window looks, +/// and — optionally — where the plot goes once the window closes. /// -/// Deliberately not [`RenderArgs`]: there is no `--writer` to choose and no -/// `--output` to write, and `-D` carries the viewer's own settings rather than -/// a writer's. The reader flags are the same ones, so they come from the same -/// [`ReaderArgs`]. +/// Shares the reader flags with [`RenderArgs`] through [`ReaderArgs`], and its +/// `--writer`/`--output` mean the same thing they do there, but `-D` is the +/// viewer's own: the window's settings (`width`, `height`, `background`, +/// `title`) are taken from it, and any other key is forwarded to the writer +/// for the after-close render. #[derive(Args)] pub struct ViewArgs { #[command(flatten)] @@ -104,11 +106,27 @@ pub struct ViewArgs { value_name = "KEY=VALUE[;...]", long_help = "Settings for the viewer window, as `key=value`. Repeatable, and one flag \ may carry several settings separated by `;` (quote it, as most shells read \ - `;` themselves): `-D 'width=1280;title=My plot'`.\n\nSettings:\n \ - width, height, background, title" + `;` themselves): `-D 'width=1280;title=My plot'`.\n\nViewer settings:\n \ + width, height, background, title\n\nAny other key (e.g. `dpi`) is forwarded \ + to the writer when `--output` or `--writer` is given. The render's width and \ + height always come from the window's size when it closed." )] pub viewer_options: Vec, + /// Output format for a render after the window closes + /// + /// With `--output` or on its own (writing to stdout), renders the plot + /// once the window closes, sized to the window as the user left it. + #[arg(short, long, long_help = writers::writer_help())] + pub writer: Option, + + /// Render the plot to this file after the window closes + /// + /// The render uses the window's dimensions at close, so the file matches + /// what was last on screen. + #[arg(short, long)] + pub output: Option, + /// Show verbose output (execution details, statistics) #[arg(short, long)] pub verbose: bool, @@ -193,7 +211,9 @@ pub enum Commands { /// Show a ggsql query's plot in a window /// /// Blocks until the window is closed. Resizing the window re-lays-out the - /// plot rather than stretching it. + /// plot rather than stretching it. With `--output` (and optionally + /// `--writer`), the plot is rendered to file when the window closes, at + /// the window's final dimensions, which are also logged to the terminal. /// /// Requires the `window` feature and a working GPU adapter. View { @@ -486,9 +506,33 @@ fn render_spec(spec: Spec, args: &RenderArgs, writer: &WriterSpec) { fn cmd_view(query: String, args: &ViewArgs) { #[cfg(feature = "window")] { - use ggsql::writer::PlotViewer; + use ggsql::writer::{PlotViewer, VIEWER_OPTIONS}; + + // `-D` serves two masters: the viewer's own keys configure the window, + // everything else is meant for the writer of the after-close render. + let mut viewer_pairs: Vec = Vec::new(); + let mut writer_pairs: Vec = Vec::new(); + for raw in &args.viewer_options { + for setting in raw.split(';').map(str::trim).filter(|s| !s.is_empty()) { + let key = setting.split('=').next().unwrap_or("").trim(); + if VIEWER_OPTIONS.contains(&key) { + viewer_pairs.push(setting.to_string()); + } else { + writer_pairs.push(setting.to_string()); + } + } + } + + let render_requested = args.writer.is_some() || args.output.is_some(); + if !writer_pairs.is_empty() && !render_requested { + eprintln!( + "Settings like '{}' are writer settings; pass --output or --writer to use them", + writer_pairs[0] + ); + std::process::exit(1); + } - let options = WriterOptions::parse(args.viewer_options.clone()).unwrap_or_else(|e| { + let options = WriterOptions::parse(viewer_pairs).unwrap_or_else(|e| { eprintln!("{}", e); std::process::exit(1); }); @@ -531,10 +575,31 @@ fn cmd_view(query: String, args: &ViewArgs) { eprintln!("Close the window to exit."); } - // Blocks on the main thread until the window closes. - if let Err(e) = viewer.show(&spec) { + // Blocks on the main thread until the window closes, returning the + // size the user left the window at. + let (width, height) = viewer.show(&spec).unwrap_or_else(|e| { eprintln!("{}", e); std::process::exit(1); + }); + eprintln!("Window closed at {}x{}", width, height); + + if render_requested { + // The window's closing size wins over any width/height the writer + // would default to: the file should match what was on screen. + writer_pairs.push(format!("width={width}")); + writer_pairs.push(format!("height={height}")); + let render_args = RenderArgs { + source: ReaderArgs { + reader: args.source.reader.clone(), + cache: args.source.cache.clone(), + }, + writer: args.writer.clone(), + writer_options: writer_pairs, + output: args.output.clone(), + verbose: args.verbose, + }; + let writer = render_args.writer(); + render_spec(spec, &render_args, &writer); } } #[cfg(not(feature = "window"))] diff --git a/src/writer/hephaestus/mod.rs b/src/writer/hephaestus/mod.rs index 32720e656..fa5f4b73f 100644 --- a/src/writer/hephaestus/mod.rs +++ b/src/writer/hephaestus/mod.rs @@ -84,7 +84,7 @@ pub use tiff::{TiffCompression, TiffWriter}; #[cfg(feature = "webp")] pub use webp::WebpWriter; #[cfg(feature = "window")] -pub use window::PlotViewer; +pub use window::{PlotViewer, VIEWER_OPTIONS}; // Re-exported so a caller can name the setting without depending on the // renderer crate; the variants are the format's own vocabulary. diff --git a/src/writer/hephaestus/window.rs b/src/writer/hephaestus/window.rs index d2a56d62b..3f4d30f92 100644 --- a/src/writer/hephaestus/window.rs +++ b/src/writer/hephaestus/window.rs @@ -4,6 +4,9 @@ //! composition every writer builds, and because `ggsql-cli` uses only public //! `ggsql::*` API and has no renderer dependency of its own. +use std::cell::Cell; +use std::rc::Rc; + use hephaestus::plot::PlotComposition; use hephaestus::window::{self, Event, EventCtx, Frame, WindowApp, WindowConfig}; @@ -16,7 +19,7 @@ use crate::{GgsqlError, Result}; /// /// Notably not `units` or `dpi`: a window's size is logical pixels and its /// resolution belongs to the display it opens on. -const VIEWER_OPTIONS: &[&str] = &["width", "height", "background", "title"]; +pub const VIEWER_OPTIONS: &[&str] = &["width", "height", "background", "title"]; /// Default window size, matching the renderer's own. const DEFAULT_WIDTH: u32 = 800; @@ -113,21 +116,34 @@ impl PlotViewer { /// Show the plot and **block until the window closes.** /// - /// Must be called from the main thread, as the platform event loops require. + /// Returns the window's size in logical pixels as it was when it closed, + /// so a caller rendering the same plot to a file can match what the user + /// last saw. Must be called from the main thread, as the platform event + /// loops require. /// /// # Errors /// /// Returns `GgsqlError::WriterError` if the plot cannot be composed, if no /// GPU adapter can drive a window, or if the event loop fails. - pub fn show(&self, spec: &Spec) -> Result<()> { + pub fn show(&self, spec: &Spec) -> Result<(u32, u32)> { let view = super::compose::prepare(spec.plot(), spec.data())?; let config = WindowConfig::new(self.title.clone()) .size(self.width, self.height) .background(self.background); - window::run(config, SpecApp { view }) - .map_err(|e| GgsqlError::WriterError(format!("the plot viewer failed: {e}"))) + // The last size the plot was drawn at, tracked across resizes so it + // survives `window::run` consuming the app. + let closing_size = Rc::new(Cell::new((self.width, self.height))); + let app = SpecApp { + view, + closing_size: Rc::clone(&closing_size), + }; + + window::run(config, app) + .map_err(|e| GgsqlError::WriterError(format!("the plot viewer failed: {e}")))?; + + Ok(closing_size.get()) } } @@ -145,6 +161,7 @@ impl Default for PlotViewer { /// One composition, redrawn at whatever size the window currently is. struct SpecApp { view: PlotComposition, + closing_size: Rc>, } impl WindowApp for SpecApp { @@ -152,6 +169,8 @@ impl WindowApp for SpecApp { // The frame reports its own size and dpi, which is what makes a resize // a re-layout rather than a rescale. let (scene, size, dpi) = frame.parts(); + self.closing_size + .set((size.width.round() as u32, size.height.round() as u32)); self.view.render(scene, size, dpi); } diff --git a/src/writer/mod.rs b/src/writer/mod.rs index 02d235a2b..2fe619a30 100644 --- a/src/writer/mod.rs +++ b/src/writer/mod.rs @@ -87,7 +87,7 @@ pub use hephaestus::SvgWriter; // Not a writer — it produces no output — but it needs the same composition, so // it lives beside them. See its own docs for why it is not a `Writer` impl. #[cfg(feature = "window")] -pub use hephaestus::PlotViewer; +pub use hephaestus::{PlotViewer, VIEWER_OPTIONS}; #[cfg(feature = "png")] pub use hephaestus::{PngCompression, PngWriter}; #[cfg(feature = "tiff")] From b60f77783f1a5866cb6d1609458dc1f1f4fa2ddc Mon Sep 17 00:00:00 2001 From: Thomas Lin Pedersen Date: Fri, 9 Oct 2026 11:58:14 +0200 Subject: [PATCH 2/2] Properly wire up dpi --- ggsql-cli/src/main.rs | 34 ++++++++++++++++++-------- src/writer/hephaestus/mod.rs | 2 +- src/writer/hephaestus/window.rs | 42 +++++++++++++++++++++++---------- src/writer/mod.rs | 2 +- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/ggsql-cli/src/main.rs b/ggsql-cli/src/main.rs index 2fd0e5997..9397f6a11 100644 --- a/ggsql-cli/src/main.rs +++ b/ggsql-cli/src/main.rs @@ -107,9 +107,10 @@ pub struct ViewArgs { long_help = "Settings for the viewer window, as `key=value`. Repeatable, and one flag \ may carry several settings separated by `;` (quote it, as most shells read \ `;` themselves): `-D 'width=1280;title=My plot'`.\n\nViewer settings:\n \ - width, height, background, title\n\nAny other key (e.g. `dpi`) is forwarded \ - to the writer when `--output` or `--writer` is given. The render's width and \ - height always come from the window's size when it closed." + width, height, background, title\n\nAny other key (e.g. `compression`) is \ + forwarded to the writer when `--output` or `--writer` is given, except `dpi` \ + and `units`, which are refused: the render's size and resolution always come \ + from the window when it closed, so what you see is what you get." )] pub viewer_options: Vec, @@ -510,11 +511,20 @@ fn cmd_view(query: String, args: &ViewArgs) { // `-D` serves two masters: the viewer's own keys configure the window, // everything else is meant for the writer of the after-close render. + // `dpi` and `units` belong to neither: the render must match what was + // on screen, and both would reinterpret the window's size. let mut viewer_pairs: Vec = Vec::new(); let mut writer_pairs: Vec = Vec::new(); for raw in &args.viewer_options { for setting in raw.split(';').map(str::trim).filter(|s| !s.is_empty()) { let key = setting.split('=').next().unwrap_or("").trim(); + if matches!(key, "dpi" | "units") { + eprintln!( + "view takes no '{key}' setting: the render uses the window's own size and \ + resolution, so what you see is what you get" + ); + std::process::exit(1); + } if VIEWER_OPTIONS.contains(&key) { viewer_pairs.push(setting.to_string()); } else { @@ -576,18 +586,22 @@ fn cmd_view(query: String, args: &ViewArgs) { } // Blocks on the main thread until the window closes, returning the - // size the user left the window at. - let (width, height) = viewer.show(&spec).unwrap_or_else(|e| { + // frame (size and dpi) the user left the window at. + let frame = viewer.show(&spec).unwrap_or_else(|e| { eprintln!("{}", e); std::process::exit(1); }); - eprintln!("Window closed at {}x{}", width, height); + eprintln!( + "Window closed at {}x{} (dpi: {})", + frame.width, frame.height, frame.dpi + ); if render_requested { - // The window's closing size wins over any width/height the writer - // would default to: the file should match what was on screen. - writer_pairs.push(format!("width={width}")); - writer_pairs.push(format!("height={height}")); + // The window's closing frame wins over the writer's defaults: the + // file should match what was on screen, chrome included. + writer_pairs.push(format!("width={}", frame.width)); + writer_pairs.push(format!("height={}", frame.height)); + writer_pairs.push(format!("dpi={}", frame.dpi)); let render_args = RenderArgs { source: ReaderArgs { reader: args.source.reader.clone(), diff --git a/src/writer/hephaestus/mod.rs b/src/writer/hephaestus/mod.rs index fa5f4b73f..05a517add 100644 --- a/src/writer/hephaestus/mod.rs +++ b/src/writer/hephaestus/mod.rs @@ -84,7 +84,7 @@ pub use tiff::{TiffCompression, TiffWriter}; #[cfg(feature = "webp")] pub use webp::WebpWriter; #[cfg(feature = "window")] -pub use window::{PlotViewer, VIEWER_OPTIONS}; +pub use window::{ClosingFrame, PlotViewer, VIEWER_OPTIONS}; // Re-exported so a caller can name the setting without depending on the // renderer crate; the variants are the format's own vocabulary. diff --git a/src/writer/hephaestus/window.rs b/src/writer/hephaestus/window.rs index 3f4d30f92..69d4370b4 100644 --- a/src/writer/hephaestus/window.rs +++ b/src/writer/hephaestus/window.rs @@ -116,34 +116,38 @@ impl PlotViewer { /// Show the plot and **block until the window closes.** /// - /// Returns the window's size in logical pixels as it was when it closed, - /// so a caller rendering the same plot to a file can match what the user - /// last saw. Must be called from the main thread, as the platform event - /// loops require. + /// Returns the frame the plot was last drawn in — logical size and dpi — + /// as it was when the window closed, so a caller rendering the same plot + /// to a file can match what the user last saw. Must be called from the + /// main thread, as the platform event loops require. /// /// # Errors /// /// Returns `GgsqlError::WriterError` if the plot cannot be composed, if no /// GPU adapter can drive a window, or if the event loop fails. - pub fn show(&self, spec: &Spec) -> Result<(u32, u32)> { + pub fn show(&self, spec: &Spec) -> Result { let view = super::compose::prepare(spec.plot(), spec.data())?; let config = WindowConfig::new(self.title.clone()) .size(self.width, self.height) .background(self.background); - // The last size the plot was drawn at, tracked across resizes so it + // The last frame the plot was drawn at, tracked across resizes so it // survives `window::run` consuming the app. - let closing_size = Rc::new(Cell::new((self.width, self.height))); + let closing = Rc::new(Cell::new(ClosingFrame { + width: self.width, + height: self.height, + dpi: 96.0, + })); let app = SpecApp { view, - closing_size: Rc::clone(&closing_size), + closing: Rc::clone(&closing), }; window::run(config, app) .map_err(|e| GgsqlError::WriterError(format!("the plot viewer failed: {e}")))?; - Ok(closing_size.get()) + Ok(closing.get()) } } @@ -158,10 +162,21 @@ impl Default for PlotViewer { } } +/// The frame the plot was last drawn in when the window closed. +/// +/// `dpi` is the display's (`96 × scale factor` — 192 on a Retina display), +/// which a file render needs to reproduce the window's text and line sizes. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct ClosingFrame { + pub width: u32, + pub height: u32, + pub dpi: f64, +} + /// One composition, redrawn at whatever size the window currently is. struct SpecApp { view: PlotComposition, - closing_size: Rc>, + closing: Rc>, } impl WindowApp for SpecApp { @@ -169,8 +184,11 @@ impl WindowApp for SpecApp { // The frame reports its own size and dpi, which is what makes a resize // a re-layout rather than a rescale. let (scene, size, dpi) = frame.parts(); - self.closing_size - .set((size.width.round() as u32, size.height.round() as u32)); + self.closing.set(ClosingFrame { + width: size.width.round() as u32, + height: size.height.round() as u32, + dpi, + }); self.view.render(scene, size, dpi); } diff --git a/src/writer/mod.rs b/src/writer/mod.rs index 2fe619a30..e4d882a9b 100644 --- a/src/writer/mod.rs +++ b/src/writer/mod.rs @@ -87,7 +87,7 @@ pub use hephaestus::SvgWriter; // Not a writer — it produces no output — but it needs the same composition, so // it lives beside them. See its own docs for why it is not a `Writer` impl. #[cfg(feature = "window")] -pub use hephaestus::{PlotViewer, VIEWER_OPTIONS}; +pub use hephaestus::{ClosingFrame, PlotViewer, VIEWER_OPTIONS}; #[cfg(feature = "png")] pub use hephaestus::{PngCompression, PngWriter}; #[cfg(feature = "tiff")]