Skip to content

Render unsized plots at the default figure size in notebook sessions #1389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seeM wants to merge 6 commits into from
Closed

Render unsized plots at the default figure size in notebook sessions #1389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3cee9cc
Render unsized plots at the default figure size in notebook sessions
seeM Jul 31, 2026
bd04828
defer plot size completion to the render sites
seeM Aug 3, 2026
2493912
document that output_width_px is currently unused
seeM Aug 3, 2026
c1dfc67
refresh stored intrinsic size on jupyter plot updates
seeM Aug 3, 2026
e4a4d1b
extract plot sizing into a PlotSizing type in plots/sizing
seeM Aug 3, 2026
84dc1cc
Polish plot sizing documentation
seeM Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/amalthea/src/wire/execute_request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ pub struct ExecuteRequestPositron {
#[serde(rename = "fig-height")]
pub fig_height: Option<f64>,

/// Output area width in pixels
/// Width of the frontend output area in pixels.
///
/// Retained for protocol compatibility but not used for plot sizing. Plots
/// without explicit dimensions use the default figure size.
pub output_width_px: Option<f64>,

/// Device pixel ratio of the output area
Expand Down
21 changes: 7 additions & 14 deletions crates/ark/src/console/console_graphics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,25 @@

use std::rc::Rc;

use amalthea::comm::plot_comm::IntrinsicSize;
use amalthea::comm::plot_comm::PlotRenderSettings;
use amalthea::wire::execute_request::CodeLocation;
use amalthea::wire::execute_request::ExecuteRequestPositron;

use crate::console::Console;

impl Console {
/// Push execution context to the graphics device when an execute request starts.
/// Pushes execution context to the graphics device when an execute request starts.
///
/// Stores the execution_id, code, code_location, and optional sizing overrides
/// so they can be captured when new plots are created during execution.
/// The context attributes newly created plots to the execution and captures its
/// requested figure dimensions and device pixel ratio.
pub(super) fn graphics_on_execute_request(
&self,
execution_id: String,
code: String,
code_location: Option<CodeLocation>,
render_settings: Option<PlotRenderSettings>,
intrinsic_size: Option<IntrinsicSize>,
positron: Option<&ExecuteRequestPositron>,
) {
self.device_context().set_execution_context(
execution_id,
code,
code_location,
render_settings,
intrinsic_size,
);
self.device_context()
.set_execution_context(execution_id, code, code_location, positron);
}

/// Process pending graphics changes after an execute request completes.
Expand Down
12 changes: 3 additions & 9 deletions crates/ark/src/console/console_repl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,20 +1522,14 @@ impl Console {
reply_tx,
});

// Push execution context to graphics device for plot attribution
// and optional sizing overrides from Quarto.
// Make plot attribution and request-specific sizing available to the
// graphics device.
let code_location = exec_req.code_location().log_err().flatten();
let (render_settings, intrinsic_size) = exec_req
.positron
.as_ref()
.map(graphics_device::compute_plot_overrides)
.unwrap_or((None, None));
self.graphics_on_execute_request(
originator.header.msg_id.clone(),
exec_req.code.clone(),
code_location,
render_settings,
intrinsic_size,
exec_req.positron.as_ref(),
);

input
Expand Down
1 change: 1 addition & 0 deletions crates/ark/src/plots.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
//

pub mod graphics_device;
pub mod sizing;
211 changes: 21 additions & 190 deletions crates/ark/src/plots/graphics_device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use amalthea::comm::plot_comm::PlotRenderFormat;
use amalthea::comm::plot_comm::PlotRenderSettings;
use amalthea::comm::plot_comm::PlotResult;
use amalthea::comm::plot_comm::PlotSize;
use amalthea::comm::plot_comm::PlotUnit;
use amalthea::comm::plot_comm::UpdateParams;
use amalthea::socket::comm::CommOutgoingTx;
use amalthea::socket::iopub::IOPubMessage;
Expand Down Expand Up @@ -58,7 +57,9 @@ use crate::comm_handler::CommHandlerContext;
use crate::console::Console;
use crate::console::SessionMode;
use crate::modules::ARK_ENVS;
use crate::r_task;
use crate::plots::sizing::IntrinsicSizeExt;
use crate::plots::sizing::PlotSizing;
use crate::plots::sizing::DEFAULT_DPI;

pub const PLOT_COMM_NAME: &str = "positron.plot";

Expand Down Expand Up @@ -92,11 +93,8 @@ struct ExecutionContext {
execution_id: String,
code: String,
code_location: Option<CodeLocation>,
/// Render settings override from the execute request (e.g. Quarto sizing metadata).
/// When `Some`, used instead of `DeviceContext::prerender_settings` for pre-rendering.
render_settings: Option<PlotRenderSettings>,
/// Intrinsic size from the execute request (e.g. Quarto's fig-width/fig-height in inches).
intrinsic_size: Option<IntrinsicSize>,
/// Unresolved figure dimensions and pixel ratio from the execute request.
sizing: PlotSizing,
}

/// Per-plot context captured at creation time.
Expand Down Expand Up @@ -237,15 +235,13 @@ impl DeviceContext {
execution_id: String,
code: String,
code_location: Option<CodeLocation>,
render_settings: Option<PlotRenderSettings>,
intrinsic_size: Option<IntrinsicSize>,
positron: Option<&ExecuteRequestPositron>,
) {
*self.execution_context.borrow_mut() = Some(ExecutionContext {
execution_id,
code,
code_location,
render_settings,
intrinsic_size,
sizing: PlotSizing::from_request(positron),
});
}

Expand Down Expand Up @@ -631,10 +627,10 @@ impl DeviceContext {
let ctx = self.capture_execution_context();
self.store_plot_context(id, &ctx);

// Use render settings from the execute request if available, otherwise fall back
// to the default prerender settings.
// Prefer explicit figure dimensions; otherwise use the frontend's prerender settings.
let settings = ctx
.render_settings
.sizing
.requested_render_settings()
.unwrap_or_else(|| self.prerender_settings.get());

let open_data = match self.render_plot(id, &settings) {
Expand Down Expand Up @@ -722,7 +718,7 @@ impl DeviceContext {
code: ctx.code.clone(),
origin,
},
intrinsic_size: ctx.intrinsic_size.clone(),
intrinsic_size: ctx.sizing.intrinsic_size(),
});
}

Expand Down Expand Up @@ -789,6 +785,12 @@ impl DeviceContext {
return;
});

// A plot update inherits the current execution's figure dimensions, matching
// knitr's treatment of plots modified by a later chunk.
if let Some(plot_ctx) = self.plot_contexts.borrow_mut().get_mut(id) {
plot_ctx.intrinsic_size = ctx.sizing.intrinsic_size();
}

let transient = TransientValue {
display_id: id.to_string(),
data: None,
Expand All @@ -814,28 +816,7 @@ impl DeviceContext {
id: &PlotId,
ctx: &ExecutionContext,
) -> Result<(serde_json::Value, serde_json::Value), anyhow::Error> {
let base = ctx.render_settings.unwrap_or(PlotRenderSettings {
size: PlotSize {
width: 800,
height: 600,
},
pixel_ratio: 1.0,
format: PlotRenderFormat::Png,
});

let width = r_option_positive_f64("ark.plot.width")
.map(|w| (w * DEFAULT_DPI).round() as i64)
.unwrap_or(base.size.width);
let height = r_option_positive_f64("ark.plot.height")
.map(|h| (h * DEFAULT_DPI).round() as i64)
.unwrap_or(base.size.height);
let pixel_ratio = r_option_positive_f64("ark.plot.pixel_ratio").unwrap_or(base.pixel_ratio);

let settings = PlotRenderSettings {
size: PlotSize { width, height },
pixel_ratio,
format: base.format,
};
let settings = ctx.sizing.resolved_render_settings();

let data = unwrap!(self.render_plot(id, &settings), Err(error) => {
return Err(anyhow!("Failed to render plot with id {id} due to: {error}."));
Expand Down Expand Up @@ -980,140 +961,6 @@ impl From<&PlotId> for RObject {
}
}

/// Default DPI for converting inches to pixels.
/// Matches R's default: 96 on macOS, 72 on Linux/Windows.
/// See `default_resolution_in_pixels_per_inch()` in graphics.R.
const DEFAULT_DPI: f64 = if cfg!(target_os = "macos") {
96.0
} else {
72.0
};

/// Default aspect ratio (width:height) used when only output_width_px is provided.
const DEFAULT_ASPECT_RATIO: f64 = 4.0 / 3.0;

/// Default figure size in inches, matching Quarto's base/HTML format
/// defaults (`fig-width: 7`, `fig-height: 5`). Other formats differ (e.g.
/// pdf is 5.5 x 3.5), but Positron doesn't know the target format.
const DEFAULT_FIG_WIDTH: f64 = 7.0;
const DEFAULT_FIG_HEIGHT: f64 = 5.0;

trait IntrinsicSizeExt {
/// Convert an intrinsic size to a logical-pixel-based `PlotSize`.
///
/// Returns dimensions in CSS/logical pixels. The R rendering layer handles
/// physical pixel scaling via the separate `pixel_ratio` parameter.
fn to_plot_size(&self) -> PlotSize;
}

impl IntrinsicSizeExt for IntrinsicSize {
fn to_plot_size(&self) -> PlotSize {
match self.unit {
PlotUnit::Inches => PlotSize {
width: (self.width * DEFAULT_DPI).round() as i64,
height: (self.height * DEFAULT_DPI).round() as i64,
},
PlotUnit::Pixels => PlotSize {
width: self.width.round() as i64,
height: self.height.round() as i64,
},
}
}
}

trait FromExecuteRequest: Sized {
fn from_execute_request(req: &ExecuteRequestPositron) -> Option<Self>;
}

impl FromExecuteRequest for PlotRenderSettings {
/// Create render settings from an execute request's Positron metadata.
///
/// If `fig_width` and/or `fig_height` is set (Quarto), returns settings
/// with size in logical pixels (inches * DPI). Either dimension may be set
/// alone; the missing one falls back to the Quarto default.
///
/// Otherwise if `output_width_px` is set, returns settings at that width
/// with a 4:3 aspect ratio.
///
/// Sizes are in CSS/logical pixels. The R rendering layer handles physical
/// pixel scaling via the separate `pixel_ratio` parameter.
fn from_execute_request(req: &ExecuteRequestPositron) -> Option<Self> {
let pixel_ratio = req.output_pixel_ratio.unwrap_or(1.0);

if let Some((width, height)) = requested_fig_size(req) {
return Some(Self {
size: PlotSize {
width: (width * DEFAULT_DPI).round() as i64,
height: (height * DEFAULT_DPI).round() as i64,
},
pixel_ratio,
format: PlotRenderFormat::Png,
});
}

if let Some(width_px) = req.output_width_px {
if width_px > 0.0 {
return Some(Self {
size: PlotSize {
width: width_px.round() as i64,
height: (width_px / DEFAULT_ASPECT_RATIO).round() as i64,
},
pixel_ratio,
format: PlotRenderFormat::Png,
});
}
}

None
}
}

impl FromExecuteRequest for IntrinsicSize {
/// Create an intrinsic size from an execute request's Positron metadata.
///
/// Only returns `Some` when `fig_width` and/or `fig_height` is set
/// (i.e. Quarto sizing), providing the intrinsic size in inches.
fn from_execute_request(req: &ExecuteRequestPositron) -> Option<Self> {
let (width, height) = requested_fig_size(req)?;

Some(Self {
width,
height,
unit: PlotUnit::Inches,
source: String::from("Quarto"),
})
}
}

/// The figure size requested via `fig-width`/`fig-height`, in inches.
///
/// Either option may be set alone, matching `quarto render`; the missing (or
/// non-positive) dimension falls back to the Quarto default. Returns `None`
/// when neither dimension is set.
fn requested_fig_size(req: &ExecuteRequestPositron) -> Option<(f64, f64)> {
let width = req.fig_width.filter(|width| *width > 0.0);
let height = req.fig_height.filter(|height| *height > 0.0);

if width.is_none() && height.is_none() {
return None;
}

Some((
width.unwrap_or(DEFAULT_FIG_WIDTH),
height.unwrap_or(DEFAULT_FIG_HEIGHT),
))
}

/// Compute render settings and intrinsic size from execute request metadata.
pub(crate) fn compute_plot_overrides(
req: &ExecuteRequestPositron,
) -> (Option<PlotRenderSettings>, Option<IntrinsicSize>) {
(
PlotRenderSettings::from_execute_request(req),
IntrinsicSize::from_execute_request(req),
)
}

/// Activation callback
///
/// Only used for logging
Expand Down Expand Up @@ -1369,21 +1216,6 @@ unsafe extern "C-unwind" fn ps_graphics_default_dpi() -> anyhow::Result<SEXP> {
Ok(RObject::from(DEFAULT_DPI as i32).sexp)
}

/// Read a positive `f64` from an R option. Returns `None` if the option is
/// unset, not numeric, or not positive.
fn r_option_positive_f64(name: &str) -> Option<f64> {
let value = r_task(|| {
RFunction::from("getOption")
.param("x", name)
.call()?
.to::<f64>()
});
match value {
Ok(v) if v > 0.0 => Some(v),
_ => None,
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -1400,8 +1232,9 @@ mod tests {
assert_eq!(ctx.execution_id, "");
assert_eq!(ctx.code, "");
assert!(ctx.code_location.is_none());
assert!(ctx.render_settings.is_none());
assert!(ctx.intrinsic_size.is_none());
assert!(ctx.sizing.fig_width.is_none());
assert!(ctx.sizing.fig_height.is_none());
assert!(ctx.sizing.pixel_ratio.is_none());
}

#[test]
Expand All @@ -1412,7 +1245,6 @@ mod tests {
String::from("plot(1:10)"),
None,
None,
None,
);

let ctx = dc.capture_execution_context();
Expand All @@ -1428,7 +1260,6 @@ mod tests {
String::from("plot(1:10)"),
None,
None,
None,
);
dc.clear_execution_context();

Expand Down
Loading
Loading