From f2dce493e71fc7c409ec1205f7f84dd40596af05 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 14:55:04 -0700 Subject: [PATCH 1/4] perf(legend): reuse the setup measurement on the initial draw Every legend was measured twice. draw_legend() measures once during setup so par(oma) can be sized before the plot is drawn, and tinylegend() measured again inside recordGraphics() because the device may have been resized. On the initial draw nothing has changed between the two, so the second pass re-derived a result it already had, at roughly the cost of drawing the legend itself. measure_fake_legend() now records the device it measured on, and tinylegend() re-measures only when dev.size() no longer matches. Resize correctness is preserved by construction rather than by trying to detect a replay: a resized device fails the comparison and measures again. Multi-legend is unaffected, since each draw_legend() call builds its own legend_env and so carries its own stamp. Measuring passes per plot drop from 2 to 1 (6 to 4 for multi-legend). Plots that draw a legend are ~5% faster overall and ~8% for the lighter ones (points_small_by 5.58ms -> 5.13ms); plots without a legend are untouched. Full suite passes, 729/729, with NOT_CRAN=true. Rendering was compared against main across 8 configurations replayed through a sequence of device sizes (shrink, grow, back to the original, then the same size again), covering outer and inner legends, facets, gradient, multi-legend, ljust = "center" and a theme: byte-identical in all 40 comparisons. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- R/legend.R | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/R/legend.R b/R/legend.R index 0dff41d12..47b5ad7a5 100644 --- a/R/legend.R +++ b/R/legend.R @@ -258,8 +258,14 @@ tinylegend = function(legend_env) { legend_env$args[["text.width"]] = NULL } - # Re-measure legend dimensions (device size may have changed on resize) - legend_env$dims = measure_fake_legend(legend_env) + # Re-measure legend dimensions, but only when the device has actually + # changed. draw_legend() already measured on this device during setup, so on + # the initial draw the result is still current and a measuring pass costs + # about as much as drawing the legend itself. A resize replays through here + # with a different dev.size(), which forces the re-measure. + if (is.null(legend_env$dims) || !identical(legend_env$dims_dev, dev.size())) { + legend_env$dims = measure_fake_legend(legend_env) + } # Calculate and apply soma (outer margin adjustment based on legend size) # When soma_target is set (multi-legend), use it directly so all legends @@ -389,6 +395,10 @@ measure_fake_legend = function(legend_env) { } } + # Record the device this measurement was taken on, so callers can tell + # whether a cached result is still valid (see tinylegend()). + legend_env$dims_dev = dev.size() + do.call("legend", fklgnd.args) } From e139faad0fc98588f7fbaed43d78627106f8e9f9 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 18:46:50 -0700 Subject: [PATCH 2/4] fix(legend): import grDevices::dev.size The device-size guard added in the previous commit calls dev.size() without declaring it, which R CMD check flags as an undefined global. Add it to the existing grDevices importFrom tag in legend.R and regenerate NAMESPACE. R CMD check is clean again (Status: OK). Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- NAMESPACE | 1 + R/legend.R | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NAMESPACE b/NAMESPACE index 1be75dec9..60690e2a9 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -68,6 +68,7 @@ importFrom(grDevices, dev.list, dev.new, dev.off, + dev.size, extendrange, gray.colors, hcl, diff --git a/R/legend.R b/R/legend.R index 47b5ad7a5..38578f353 100644 --- a/R/legend.R +++ b/R/legend.R @@ -939,7 +939,7 @@ build_legend_env = function( #' with a legend in the margin. #' #' @importFrom graphics grconvertX grconvertY rasterImage strheight strwidth xinch -#' @importFrom grDevices as.raster recordGraphics +#' @importFrom grDevices as.raster dev.size recordGraphics #' @importFrom utils modifyList #' #' @examples From 728500c5e1fd1aa861ad81fc5f5d926fcec0d6af Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 18:47:52 -0700 Subject: [PATCH 3/4] news --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 702579885..57e45bbc4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -313,7 +313,7 @@ not 'at'`. The free-facet code path listed the eligible types by name, so ### Internals -- Performance improvemnts. (#723 @grantmcdermott) +- Performance improvemnts. (#723, #724 @grantmcdermott) ## v0.7.0 From 021127e2bc5cef7dc19f313f3042dbafc29ad06d Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Wed, 9 Sep 2026 19:09:13 -0700 Subject: [PATCH 4/4] fix(legend): key the measurement cache on device identity, not just size dev.size() alone does not identify a device. Replaying a recorded plot onto a different backend that happens to have the same dimensions (text metrics differ by backend) would reuse the source device's measurement, leaving outer margins and gradient swatches mis-sized. Include dev.cur(), which is a named integer and so carries the backend as well as the device number. Demonstrated by recording on pdf and replaying onto svglite at a matching 8x8: main's replay matches a native draw on the destination, the previous key's did not. With the device in the key it matches again. Note this is a narrow hole rather than a common one: the everyday export path, dev.copy2pdf(), re-measures under the old key anyway. It needs an explicitly same-sized replay onto another backend. The guard costs one dev.cur() call and does not move the benchmarks, so it is cheap insurance. Also fixes the changelog typo flagged in review. Spotted in review by Copilot on #724. Claude-Session: https://claude.ai/code/session_01AjZRHA9ZvJkDPMFdDXUcUs --- NEWS.md | 2 +- R/legend.R | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index 57e45bbc4..7f61fbc37 100644 --- a/NEWS.md +++ b/NEWS.md @@ -313,7 +313,7 @@ not 'at'`. The free-facet code path listed the eligible types by name, so ### Internals -- Performance improvemnts. (#723, #724 @grantmcdermott) +- Performance improvements. (#723, #724 @grantmcdermott) ## v0.7.0 diff --git a/R/legend.R b/R/legend.R index 38578f353..9abc71f39 100644 --- a/R/legend.R +++ b/R/legend.R @@ -261,9 +261,9 @@ tinylegend = function(legend_env) { # Re-measure legend dimensions, but only when the device has actually # changed. draw_legend() already measured on this device during setup, so on # the initial draw the result is still current and a measuring pass costs - # about as much as drawing the legend itself. A resize replays through here - # with a different dev.size(), which forces the re-measure. - if (is.null(legend_env$dims) || !identical(legend_env$dims_dev, dev.size())) { + # about as much as drawing the legend itself. A resize, or a replay onto a + # different device, fails the key comparison and forces the re-measure. + if (is.null(legend_env$dims) || !identical(legend_env$dims_dev, legend_dev_key())) { legend_env$dims = measure_fake_legend(legend_env) } @@ -368,6 +368,17 @@ tinylegend = function(legend_env) { # Measure legend dimensions using a fake (non-plotted) legend +# Identity of the device a legend measurement belongs to. Size alone is not +# enough: replaying or copying a display list onto a same-sized device with a +# different backend (dev.copy(), dev.print(), an IDE's plot export) yields +# different text metrics, so the device itself has to be part of the key. +# dev.cur() is a named integer, so this captures the backend as well as the +# device number. +legend_dev_key = function() { + list(dev = dev.cur(), size = dev.size()) +} + + measure_fake_legend = function(legend_env) { fklgnd.args = modifyList( legend_env$args, @@ -395,9 +406,9 @@ measure_fake_legend = function(legend_env) { } } - # Record the device this measurement was taken on, so callers can tell + # Record which device this measurement was taken on, so callers can tell # whether a cached result is still valid (see tinylegend()). - legend_env$dims_dev = dev.size() + legend_env$dims_dev = legend_dev_key() do.call("legend", fklgnd.args) }